feat: add proxyVarsFromSecret value #196
Conversation
aslafy-z
force-pushed
the
feat/proxyVarsFromSecret
branch
3 times, most recently
from
77997af to
ad9329b
Compare
Signed-off-by: Zadkiel AHARONIAN <[email protected]>
aslafy-z
force-pushed
the
feat/proxyVarsFromSecret
branch
from
ad9329b to
a78c177
Compare
|
@tuunit What do you think of this proposal? |
| envFrom: | ||
| - secretRef: | ||
| name: {{ . }} | ||
| {{- end }} | ||
| env: | ||
| {{- if .Values.proxyVarsAsSecrets }} |
There was a problem hiding this comment.
proxyVarsAsSecrets and proxyVarsFromSecret should be mutually exclusive. Therefore I would like to see this check extended so that proxyVarsAsSecrets aren't rendered when proxyVarsFromSecret is set
There was a problem hiding this comment.
This PR was given to us as a "gift"; if we want to bring it in, we must introduce the changes we want. 🤷🏻♂️
Co-authored-by: Jan Larwig <[email protected]>
tuunit
force-pushed
the
feat/proxyVarsFromSecret
branch
from
06d61e5 to
b7bab50
Compare
tuunit
force-pushed
the
feat/proxyVarsFromSecret
branch
from
b7bab50 to
e2ae4b8
Compare
| @@ -237,7 +242,7 @@ spec: | |||
| {{- if .Values.extraEnv }} | |||
| {{ tpl (toYaml .Values.extraEnv) . | indent 8 }} | |||
| {{- end }} | |||
| {{- if .Values.envFrom }} | |||
| {{- if and (not .Values.envFromExistingSecret) .Values.envFrom }} | |||
| envFrom: | |||
There was a problem hiding this comment.
@pierluigilenoci I refactored the PR a little bit and gave the variable a better name but then I realised we already have an envFrom parameter that allows for using a secret as a source like so:
envFrom:
- secretRef:
name: my-secret
in the values.yaml without any changes to the chart.
There was a problem hiding this comment.
Therefore I don't see the benefit of this addition to the helm chart 🤔
There was a problem hiding this comment.
The original idea of PR has probably been lost over time.
However, you can find it in this comment:
#141 (comment)
There was a problem hiding this comment.
Since then it has been replaced with a more generic solution:
#201
There was a problem hiding this comment.
I would therefore close this PR.
Replaces #141