Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move assignment of FF Configs from annotations to the FlagSourceConfiguration CR #355

Open
heckelmann opened this issue Feb 16, 2023 · 6 comments

Comments

@heckelmann
Copy link

With the introduction of the additional flagd configuration Custom Resource (PR #344) opens up the possibility to entirely get rid of the openfeature.dev/flagsourceconfiguration annotation by extending the introduced CR with some additional fields. This would help with the separation of concerns.

apiVersion: core.openfeature.dev/v1alpha3
kind: FlagSourceConfiguration
metadata:
  name: end-to-end-test-fs-config
spec:
  selector:
    labelSelector:
      matchLabels:
        app: my-workload
  syncProviders:
  - source: end-to-end-test-filepath2
    provider: kubernetes
  • As a platform operator this enables me to add for instance a cluster/environment-wide FF configuration too all deployments without changing the annotations for each deployment
  • As a developer, I don't need to take care of the FF configuration deployment
@toddbaert
Copy link
Member

@heckelmann in your mind, would this co-exist with the annotation? Or would we remove that entirely?

@heckelmann
Copy link
Author

@heckelmann in your mind, would this co-exist with the annotation? Or would we remove that entirely?

I don't have a strong opinion here but would prefer to have a single source of truth.

@toddbaert
Copy link
Member

@james-milligan suggested perhaps using the label to add the annotation internally, which I think is a highly interesting approach, since we could roll it out without too many code changes...

@heckelmann
Copy link
Author

heckelmann commented Feb 20, 2023

@james-milligan suggested perhaps using the label to add the annotation internally, which I think is a highly interesting approach, since we could roll it out without too many code changes...

Sounds reasonable. I like it.
So you mean annotations will be added to the deployment manifest? If so, we would need to make sure, that the deployment is not restarted by adding them.

@odubajDT
Copy link
Contributor

odubajDT commented Mar 4, 2023

@james-milligan suggested perhaps using the label to add the annotation internally, which I think is a highly interesting approach, since we could roll it out without too many code changes...

Sounds reasonable. I like it. So you mean annotations will be added to the deployment manifest? If so, we would need to make sure, that the deployment is not restarted by adding them.

As a user I may find it confusing when I apply a deployment and suddenly the deployment contains annotations, that i didn't add.

Theoretically this will (I guess) be done by the mutating webhook and with existence of the this information in the CR, I do not see a reason to have this information doubled. Also, we should always try to make the mutating webhook as lightweight as possible and do not add functionality we can have somewhere else.

If I may, I would like to implement this ticket when we have a clear vision how it should work :) I can also try to prepare a quick PoC so we see the comlexity of the required changes before taking the final decision.

@toddbaert
Copy link
Member

If I may, I would like to implement this ticket when we have a clear vision how it should work :) I can also try to prepare a quick PoC so we see the comlexity of the required changes before taking the final decision.

I totally support PoC if you have time for that. I would like to see the scope of the changes required to use labels here instead of the annotation.

I think my main concern is if we don't do this via the annotation, we have have more code paths doing the same thing, unless we get rid of the annotation, which is a significant breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants