Add envFrom and remove lookup #260
Conversation
| # whether the chart `config` is updated or if one of the configuration | ||
| # secrets is updated. Helm template must be run with the | ||
| # --dry-run=server option to prevent a nil pointer. | ||
| checksum/config: {{ (coalesce (lookup "v1" "Secret" .Release.Namespace "trino-gateway-configuration").metadata (dict "resourceVersion" "0")).resourceVersion | sha256sum}} |
There was a problem hiding this comment.
Neither the PR description nor the commit message explain why we'd want to remove the checksum - was this wrong to do in the first place, is it not working as expected, or there are some edge cases we cannot address?
There was a problem hiding this comment.
I'll add this into the PR description.
In short, the we were taking the checksum of k8s Secrets that were created externally for passing sensitive configuration values. By including these in the checksum, running a helm update after a secret was modified would replace a pod.
This seemed like a good idea, but getting the secrets requires that Helm itself connect to a running cluster and get the secrets using the lookup function. We have received feedback that this is often not possible in release pipelines
| # - secretRef: | ||
| # name: password-secret | ||
| # ``` | ||
| envFrom: [] |
There was a problem hiding this comment.
I like how this simplifies things. You added a test values.yaml file in the other PR: https://github.com/trinodb/charts/pull/261/files#diff-3314f8997bf0267424c20c4ed15ab01144cba76dd4bd383d7511c4cdcd1bc60f, but the test.sh script here already runs Gateway tests. Please add a values.yaml file for it here too, can be a basic one, just to verify that the chart renders and installs correctly with these sections filled up.
Add
envFromto allow providing environment variables from Secrets, and remove usage of the Helmlookupfunction for merging Secrets into the configuration.This removes the custom checksum calculation as well. The checksum had included the optional configuration secrets, causing pod restarts whether configuration was updated through the chart or through a secret. Since this required retrieving Secrets from the target cluster through the Helm
lookupfunction, it caused the chart to be incompatible with release pipelines in which a manifest is generated in an environment that does not have access to the target cluster.