Skip to content

[feature] Support in-process evaluator in OFO #542

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

Closed
toddbaert opened this issue Nov 14, 2023 · 9 comments · Fixed by #640
Closed

[feature] Support in-process evaluator in OFO #542

toddbaert opened this issue Nov 14, 2023 · 9 comments · Fixed by #640
Assignees
Labels
contribfest A good issue for Contribfest KubeCon EU '24 enhancement New feature or request

Comments

@toddbaert
Copy link
Member

toddbaert commented Nov 14, 2023

We now have JS, Java, and Go in-process flagd providers. These providers connect directly to a sync.proto-compliant gRPC server (such as flagd-proxy). OFO should be enhanced to support the use of these providers. This would require:

  • ability to specify this in APIs
  • OFO sets the corresponding env-var which configures the provider to use in-process evaluation (see flagd-injector), FLAGD_SOURCE_RESOLVER - the SA injection/patch should NOT take place.
    • host/port are configured similarly, through ENV_VARS
  • flagd is not deployed as a sidecar to the associated workload, since it's not needed
  • associated tests, including KUTTL tests
  • associated documentation

cc @odubajDT @thisthat @beeme1mr @Kavindu-Dodan

@thisthat thisthat added the enhancement New feature or request label Nov 15, 2023
@toddbaert toddbaert added the contribfest A good issue for Contribfest KubeCon EU '24 label Mar 11, 2024
@odubajDT odubajDT self-assigned this Apr 26, 2024
@odubajDT
Copy link
Contributor

After some discussions, it would be beneficial to do a refactoring of the existing FeatureFlagSource CRD structure to be able to distinguish which parameters are set for RPC or in-process evaluation. We agreed to draft a PoC with the potential solution (with bumping the API version to v1beta2) also containing the conversion webhooks, so users running v1beta1 versions won't need to adapt their resources in case they want to use only RPC evaluation.

@odubajDT
Copy link
Contributor

odubajDT commented May 2, 2024

Implemented a PoC supporting the in-process evaluation and additionally refactoring the actual API.

During the implementation, I got the feeling that it would be possible to move the in-process evaluation parameters into a new separate CRD to decrease the complexity of FeatureFlagSource CRD and therefore reducing the need to introduce conversion webhooks and new API version.

Also it can potentially bring the advantage of easier testability of the code as well as decreasing the complexity of the mutating webhook logic (especially the part handling the decision if the flagd sidecar injection should happen).

On the other side, it requires introducing a new standardized annotation, which will be used by deployments using OFO (like openfeature.dev/featureflagsource) for in-process evaluation

Open for any opinions!

@toddbaert @thisthat @bacherfl @Kavindu-Dodan @beeme1mr

@bacherfl
Copy link
Contributor

bacherfl commented May 6, 2024

In general I also have the impression that the current FeatureFlagSource is handling too many different responsibilities, so if we can extract something into a separate CRD i think that would be beneficial

@Kavindu-Dodan
Copy link
Contributor

@odubajDT what you are proposing is to keep FeatureFlagSource untouched and introduce a new CRD dedicated for in-process mode isn't it ?

I like the idea. We should also consider the ability to utilize flagd's grpc streaming capability 1 which was added recently (@toddbaert we could refine the issue on this).

Footnotes

@odubajDT
Copy link
Contributor

odubajDT commented May 7, 2024

@odubajDT what you are proposing is to keep FeatureFlagSource untouched and introduce a new CRD dedicated for in-process mode isn't it ?

Yes exactly!

I like the idea. We should also consider the ability to utilize flagd's grpc streaming capability 1 which was added recently (@toddbaert we could refine the issue on this).

Footnotes

1. * https://flagd.dev/reference/grpc-sync-service/
   
   [↩](#user-content-fnref-1-59ee0d967e0e1eb93c01c9f33d6dfa32)

@odubajDT
Copy link
Contributor

odubajDT commented May 7, 2024

Drafted a PoC to showcase how the possible solution with a separate CRD can look like. It shows that using a separate CRD makes the code way cleaner and reduces the complexity of the whole solution

@odubajDT
Copy link
Contributor

odubajDT commented May 8, 2024

If we decide to go with the new CRD, we should consider picking a new name. My suggestion would be: FeatureFlagInProcessConfiguration. Any other ideas?

@toddbaert @Kavindu-Dodan @beeme1mr @thisthat @bacherfl

@Kavindu-Dodan
Copy link
Contributor

@odubajDT FeatureFlagInProcessConfiguration sounds good (but please wait for feedback from others)

Besides, I assume the workflow here is you add FeatureFlagInProcessConfiguration CRD to your workload and the operator sets environment variables to the deployment. I think we need to check lang specific flagd in-proces provider capability to swithc processing mode (remote vs in-process) based on env configuration 🤔 (cc @toddbaert to be followed up if we don't do this already)

@thisthat
Copy link
Member

Regarding the naming, we would have the following then:

  • FeatureFlag
  • FeatureFlagSource
  • FeatureFlagInProcessConfiguration

I think it is consistent and explains the purpose clearly enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribfest A good issue for Contribfest KubeCon EU '24 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants