open-feature-operator icon indicating copy to clipboard operation
open-feature-operator copied to clipboard

how to leverage OFO for workloads external to k8s , say a web frontend or something running outside k8s

Open rjdp opened this issue 10 months ago • 13 comments

I am using OFO, and it works great for backend deployments that are in my cluster, how can i use it for my frontend applications or application outside of k8s

rjdp avatar Apr 11 '24 13:04 rjdp

Hey @rjdp . Great observation. It's entirely possible to stand up a single flagd that watches a number of CRDs and isn't injected into a workload pod, and then expose that via an ingress to the outside world so that you could call it with the flagd-web provider, or perhaps OFREP providers which are under active development.

This pattern could be supported by OFO APIs itself, but we currently don't have any such plans. Once question would be how far to go... would OFO just setup the centralized flagd? Or would is also expose it (maybe with the gateway API)? I think exposing it would be a tall order, but I'm not sure...

Interested in other takes on this, especially from some people who have tinkered in this area already, cc @Kavindu-Dodan @bacherfl @thisthat @beeme1mr

@thomaspoignant @lukas-reining might be slightly interesting considering OFREP developments.

toddbaert avatar Apr 17 '24 15:04 toddbaert

If I have to wear my operator hat, I would not like OFO exposing the service. IMHO OFO should only configure flagd to expose OFREP and a standard K8s service. Everything else is very hard to generalize, and I would suggest letting the end-user configure it.

thisthat avatar Apr 17 '24 17:04 thisthat

I also like the suggestion, I lately thought about this too after adding updating flagd to the new version OFREP in the playground and used it for my local version of the cloud-native demo. So I am totally for adding an option for this.

Regarding the Ingress or maybe even Gateway: Looking at e.g. the Jaeger operator I think it makes sense and is doable to even include optional ingress generation. By the Jaeger operator, services are created always and an ingress can be created optionally too.

The following is how it works with the Jaeger operator and I think it makes sense to do this for OFREP too.

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  namespace: open-feature-demo
  name: jaeger
spec:
  strategy: allinone
  allInOne:
      query:
        base-path: "/"
  ingress:
    enabled: true
    pathType: Prefix
    ingressClassName: nginx
    annotations:
      cert-manager.io/cluster-issuer: prod-issuer
      nginx.ingress.kubernetes.io/force-ssl-redirect: "false"
      kubernetes.io/ingress.class: nginx
    hosts:
      - jaeger.of-cloud-native.reining.dev
    tls:
      - hosts:
          - jaeger.of-cloud-native.reining.dev
        secretName: of-demo-jaeger-server-tls´

An inspiration could be the CRD of Jaeger: https://github.com/jaegertracing/jaeger-operator/blob/main/docs/api.md#jaegerspecingress

Currently I think Ingress is still more common, so also more important, but we could also directly add the options for the Gateway API.

lukas-reining avatar Apr 18 '24 15:04 lukas-reining

Any thoughts @toddbaert, @thisthat? I could really use that too. Also I can volunteer to build it if we go the route and no one has the capacity to build it. (but do not have to)

lukas-reining avatar Apr 28 '24 10:04 lukas-reining

Having an optional field in our CRD with type IngressSpec that, if not empty, is used to generate the Ingress is a good option IMHO 👍

thisthat avatar Apr 29 '24 09:04 thisthat

I think if Jaeger has a successful pattern it's not a bad idea.

toddbaert avatar Apr 29 '24 13:04 toddbaert

@toddbaert I can look into implementing this, so feel free to assign me if we would like to have this feature :)

bacherfl avatar Apr 30 '24 11:04 bacherfl

@bacherfl I will assign you then! :) Personally I think we should have Ingress like Jaeger and even a Gateway definition. What do you think?

lukas-reining avatar Apr 30 '24 21:04 lukas-reining

@toddbaert @lukas-reining Looking into this right now - one thing I would like to have your opinion on is whether we should either:

  • extend the FlagSource CRD we have right now to support the creation of a flagd deployment + ingress (i.e. similar to what is done for the flagd-proxy deployment here. Note that the configuration for the deployment created here is defined by the environment variables here, so this means that this will not be very flexible in terms of configuring the flagd deployment
  • introduce a new CRD dedicated to deploying flagd (i.e. like Jaeger does with its Jaeger CRD)

I think a new CRDS would be the cleaner solution as the FlagSource CRD already serves a number of different purposes (and with the deployment of the flagd-proxy more that it conceptually should imho), but also interested in what you guys think about this?

bacherfl avatar May 02 '24 11:05 bacherfl

I would support a separate CRD. The reasons as you already mentioned: clear separation of concerns, clear code, better testability.

Also with adding new parameters to the CRD, a refactoring which changes the structure of the FeatureFlagSource CRD should take place that the still keep the CRD structured and clear for the user which parameter belongs to which functionality -> this in the end adds additional complexity in the need of a new API version, which potentially leads to adding conversion webhooks, moving API HUB version etc.

Separate CRD is a clean a in my opinion the least complex option.

odubajDT avatar May 02 '24 13:05 odubajDT

I feel a bit conflicted. I can see arguments for both.

Logically, the FlagSource CRD defines sources for flags - it's both an aggregation of Flag definitions, and a configuration of a flagd. As a logical aggregation of flags, it doesn't make sense to put anything about an ingress here... but as a flagd configuration abstraction, it does.

I think I tend toward some new CRD for this purpose, which would be associated with a FlagSource?

toddbaert avatar May 02 '24 13:05 toddbaert

thanks for the feedback all - I will implement this with a new CRD then since we all seem to lean more towards this option

bacherfl avatar May 02 '24 16:05 bacherfl

I am also for a new CRD,basically for the same reasons as @odubajDT and @toddbaert. I guess concerns are well separated this way and one can mix and match flags with flagd instances. (Even though this might not be needed that often 😅)

lukas-reining avatar May 02 '24 19:05 lukas-reining

Alright, the PR should be ready to be reviewed now https://github.com/open-feature/open-feature-operator/pull/633 :)

bacherfl avatar May 13 '24 07:05 bacherfl

Thanks so much guys! absolutely delighted to see such quick action <3

rjdp avatar May 15 '24 17:05 rjdp