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

Dynamic EnvFrom existing K8s Secrets #45

Merged
merged 4 commits into from
Jul 4, 2023

Conversation

LiamStorkey
Copy link
Contributor

@LiamStorkey LiamStorkey commented Jun 29, 2023

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

#44

Describe the solution you've provided

Added a new value that creates a new envFrom secretRef from a pre-existing k8s secret (managed by the user in their own desired way - like external secrets operator). This allows users to create environment variables from secure strings without needing to have them in the plain text of the values files

Describe alternatives you've considered

I did not want to add to the helm chart the ability to create the secrets from an external value source (like AWS SSM) because that would force users to have specific tools already deployed into their clusters. Let them create the secret and manage it in their own way.

Additional context
The secret would be a key-value pair and would create the environment variables in the same way the current configMapRef does.

If the variable is enabled in the values file, this will create the
environment variables from the key value pairs in the k8s secret
dynamically. The k8s secret must already exist and be named <k8s release
name>-secret-environment-variables.

✅ Closes: launchdarkly#44
Tests secretEnvironmentVariables variable
Changed the vlaue to be a secret reference now and if it exists the
chart will act on it
@keelerm84
Copy link
Member

I was hoping I would have time this week to review this, but unfortunately I haven't had the chance to get back to this.

I will try to review, merge, and release this early next week.

Thank you for your contribution, and again, my apologies for the delay in resolution.

@LiamStorkey
Copy link
Contributor Author

That is completely fine @keelerm84
Thanks for letting me know!

@keelerm84
Copy link
Member

@LiamStorkey I'm looking at this this morning and I had a question. On the issue, you mentioned:

Currently, the helm chart supports envfrom the configMapRef but this does not support secure string secrets (like sdk keys) as it would require them to be in plain text in the values files.

The existing secrets configuration doesn't require the plaintext value of the secret to be in your value file. It requires configuration for the environment name, the secret name and key; however, that doesn't expose the actual secret VALUE.

Does that negate the need for this change?

@LiamStorkey
Copy link
Contributor Author

@keelerm84 no - Another key feature of this change is how it is dynamic in nature. The method you have mentioned requires the values file for this chart to know the secret's Key in order to get its value. This change will dynamically grab all the key-value pairs in the secret, whether it's 1 or 15.

@keelerm84 keelerm84 changed the base branch from main to contrib July 4, 2023 14:58
@keelerm84 keelerm84 merged commit ae145e8 into launchdarkly:contrib Jul 4, 2023
@keelerm84
Copy link
Member

Thank you for the clarification, and again for the contribution. This change is available in v2.3.0.

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.

3 participants