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

Custom Environment Variable Support #182

Merged
merged 3 commits into from
Jan 29, 2024
Merged

Custom Environment Variable Support #182

merged 3 commits into from
Jan 29, 2024

Conversation

jillianwilson
Copy link
Contributor

@jillianwilson jillianwilson commented Jan 24, 2024

This MR aims to address #152 . Because there are many possible environment variables that users may want to add to their helm chart was decided to add the option to include custom environment variables to be added to the connect, injector, and operator containers.

This work can be tested using the following steps:

  1. Create a custom values file
  2. Add the following text:
connect:
  customEnvVars:
    - name: "CUSTOM_ENV_VAR1"
      value: "customvar2"
    - name: "CUSTOM_ENV_VAR2"
      value: "customvar2"
  1. Run the following command:
helm template -f newvalues.yaml connect ./connect
  1. Check that the new environment variables have been added to the connect container in the manifest.
    These steps should be followed for the injector and operator containers as well.

charts/connect/values.yaml Outdated Show resolved Hide resolved
charts/connect/values.yaml Outdated Show resolved Hide resolved
Copy link
Member

@edif2008 edif2008 left a comment

Choose a reason for hiding this comment

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

Functional review: ✅

I've tested the following cases and can confirm that all of them work as expected. 💪🏻

✅ Connect

helm template -f newvalues.yaml connect ./charts/connect > results.yaml

✅ Operator

helm template -f newvalues.yaml connect ./charts/connect --set connect.create=false --set operator.create=true > results.yaml

✅ Secrets Injector

helm template -f newvalues.yaml secrets-injector ./charts/secrets-injector > results.yaml

Code review: ✅

Code looks straight forward and on point.

Other notes

I've noticed the following warning when running the helm template commands:

$ helm template -f newvalues.yaml connect ./charts/connect > results.yaml
coalesce.go:286: warning: cannot overwrite table with non table for connect.connect.customEnvVars (map[])

I have a comment on the exact line that may be causing this, as well as a possible solution to it.

charts/connect/values.yaml Outdated Show resolved Hide resolved
@jillianwilson jillianwilson merged commit a8801d3 into main Jan 29, 2024
4 checks passed
@jillianwilson jillianwilson deleted the custom-env-vars branch January 29, 2024 13:11
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