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

Added ability to use raw envrionment variables maps in deployment #651

Conversation

asaf400
Copy link
Contributor

@asaf400 asaf400 commented Dec 5, 2024

Hello, Internally we use external secret sources,
This PR adds the Ability for the environment variables to reference k8s objects that are per-defined under the pganalyze target namespace by cluster operators.

This solves an issue where a secret that does not conform to either existing methods:

  • extraEnv: simple key: value strings
  • configMap or secret: injection by using envFrom restricts the data fields conform to target environment variable names

By allowing helm operator to specify raw elements of the env spec of a container, it allows for additional options that are not currently covered.

I do not know how you do versioning for same appVersion patching of the chart-side only changes,
I see that most chart releases correspond to app releases, I ran helm-docs which is the only section in CONTRIBUTING.md that seems relevant for this type of PR

Edits: typo, versioning note

@lfittl lfittl requested a review from keiko713 February 19, 2025 00:40
Copy link
Contributor

@keiko713 keiko713 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This is looking good, it makes sense to add this 👍

Thanks for also updating the README using helm-docs. Looks like we haven't run this during the release (I think everyone is missing "prior to making a new release" part of "whenever you make changes to the Helm chart or prior to making a new release", including me) so the version bumped quite a lot - I'll make some update to CONTRIBUTING page so that we won't forget about bumping the version in README too (update: actually, I think it's not the end of the world if this part is stale - including this to the release is a bit of work, so I'll skip updating it for now).

I do not know how you do versioning for same appVersion patching of the chart-side only changes

We bump the version of the helm chart during the collector release (regardless of having a change in the helm chart part). This change will be released as a v0.65.1 (or v0.66.0) with the next release.

@@ -21,6 +21,15 @@ extraEnv: {}
# DB_ALL_NAMES: true (monitor all databases on the server)
# DB_USERNAME: your_monitoring_user

# -- Environment variables to be passed to the container
# Config settings can be defined in raw form, for use with externally maintained env value sources (configMapKeyRef, fieldRef, resourceFieldRef, secretKeyRef)
extraEnvRaw: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a memo, I can see that this pattern and the name extraEnvRaw is used in the other repos too, so this looks good 👍

(e.g. apache/superset@b35645c)

@keiko713 keiko713 merged commit e241078 into pganalyze:main Feb 25, 2025
3 checks passed
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.

2 participants