Skip to content

feat: add valueFrom field handling in envVars for FeatureFlagSource and InProcessConfiguration#802

Open
KyryloKarpenko wants to merge 1 commit intoopen-feature:mainfrom
KyryloKarpenko:main
Open

feat: add valueFrom field handling in envVars for FeatureFlagSource and InProcessConfiguration#802
KyryloKarpenko wants to merge 1 commit intoopen-feature:mainfrom
KyryloKarpenko:main

Conversation

@KyryloKarpenko
Copy link

This PR

Allow to configure envVars using valueFrom field.
Verified that the Flagd sidecar is created with the configured source (secrets or config map).

Related Issues

-

Notes

According to the CRD documentation for FeatureFlagSource and InProcessConfiguration, it is possible to configure the source for the environment variable's value in the valueFrom field. In reality, the envVars could be configured using only the value field.

Follow-up Tasks

-

How to test

Works with end-to-end.yaml

apiVersion: core.openfeature.dev/v1beta1
kind: FeatureFlagSource
metadata:
  name: end-to-end
  namespace: open-feature-demo
spec:
  sources:
    - source: azblob://my-bucket/test.json
      provider: azblob
  envVarPrefix: ""
  envVars:
    - name: AZURE_STORAGE_ACCOUNT
      valueFrom:
        secretKeyRef:
          name: my-secret
          key: account_name
    - name: AZURE_STORAGE_SAS_TOKEN
      valueFrom:
        secretKeyRef:
          name: my-secret
          key: sas_token

@gemini-code-assist
Copy link

Summary of Changes

Hello @KyryloKarpenko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the flexibility and security of environment variable management within FeatureFlagSource and InProcessConfiguration resources. By introducing support for Kubernetes' valueFrom mechanism, it allows users to dynamically inject environment variables from various sources, such as secrets and config maps, into the Flagd sidecar or in-process configurations. This change addresses a current limitation where only direct value assignments were possible, making the operator more robust and compliant with standard Kubernetes practices for sensitive data handling.

Highlights

  • Enhanced Environment Variable Configuration: The FeatureFlagSource and InProcessConfiguration resources now support the valueFrom field for envVars, allowing environment variables to be sourced dynamically from Kubernetes objects like secrets, config maps, field references, and resource field references.
  • Improved Security and Flexibility: This change enables more secure handling of sensitive information (e.g., credentials) by allowing them to be referenced from Kubernetes secrets rather than being hardcoded, aligning with best practices for Kubernetes deployments.
  • Comprehensive Testing and Documentation: New unit tests have been added to validate the correct parsing and application of valueFrom configurations, and the FeatureFlagSource documentation has been updated with an example demonstrating its usage with secretKeyRef.
Changelog
  • api/core/v1beta1/featureflagsource_types.go
    • Updated the ToEnvVars method to conditionally set Value or ValueFrom for environment variables based on which field is provided.
  • api/core/v1beta1/featureflagsource_types_test.go
    • Added new test cases to Test_FLagSourceConfiguration_ToEnvVars to verify valueFrom functionality using configMapKeyRef, fieldRef, resourceFieldRef, and secretKeyRef.
  • api/core/v1beta1/inprocessconfiguration_types.go
    • Modified the ToEnvVars method to handle valueFrom for environment variables in InProcessConfiguration, similar to FeatureFlagSource.
  • api/core/v1beta1/inprocessconfiguration_types_test.go
    • Introduced new test cases in Test_InProcessConfiguration_ToEnvVars to validate valueFrom for various Kubernetes sources like configMapKeyRef and secretKeyRef.
  • docs/feature_flag_source.md
    • Added an example demonstrating how to use valueFrom with secretKeyRef for envVars in FeatureFlagSource documentation.
Activity
  • No human activity has occurred on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for valueFrom in envVars for FeatureFlagSource and InProcessConfiguration resources. This allows sourcing environment variables from secrets and configmaps, which is a great improvement for managing sensitive data. The implementation is correct and includes necessary test and documentation updates. My feedback focuses on improving code conciseness and consistency in the new logic for handling envVars.

Comment on lines +224 to +234
newEnvVar := corev1.EnvVar{
Name: fc.decorateEnvVarName(envVar.Name),
}

if envVar.Value != "" {
newEnvVar.Value = envVar.Value
} else if envVar.ValueFrom != nil {
newEnvVar.ValueFrom = envVar.ValueFrom
}

envs = append(envs, newEnvVar)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for handling value and valueFrom is correct, but it can be simplified for better readability and to be more idiomatic. Also, the indentation seems to be using spaces instead of tabs, which is inconsistent with the rest of the file.

This suggested refactoring is more concise and still correctly prioritizes value over valueFrom.

		newEnvVar := envVar
		newEnvVar.Name = fc.decorateEnvVarName(envVar.Name)
		// Per K8s API, Value and ValueFrom are mutually exclusive.
		// If Value is set, we must clear ValueFrom.
		if newEnvVar.Value != "" {
			newEnvVar.ValueFrom = nil
		}
		envs = append(envs, newEnvVar)

Comment on lines +149 to +159
newEnvVar := corev1.EnvVar{
Name: common.EnvVarKey(fc.EnvVarPrefix, envVar.Name),
}

if envVar.Value != "" {
newEnvVar.Value = envVar.Value
} else if envVar.ValueFrom != nil {
newEnvVar.ValueFrom = envVar.ValueFrom
}

envs = append(envs, newEnvVar)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for handling value and valueFrom is correct, but it can be simplified for better readability and to be more idiomatic. Also, the indentation seems to be using spaces instead of tabs, which is inconsistent with the rest of the file.

This suggested refactoring is more concise and still correctly prioritizes value over valueFrom.

		newEnvVar := envVar
		newEnvVar.Name = common.EnvVarKey(fc.EnvVarPrefix, envVar.Name)
		// Per K8s API, Value and ValueFrom are mutually exclusive.
		// If Value is set, we must clear ValueFrom.
		if newEnvVar.Value != "" {
			newEnvVar.ValueFrom = nil
		}
		envs = append(envs, newEnvVar)

…nd InProcessConfiguration

Signed-off-by: KyryloKarpenko <karpenkokirillll55@gmail.com>
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

Successfully merging this pull request may close these issues.

1 participant