cloudnative-pg / charts

CloudNativePG Helm Charts
Apache License 2.0
174 stars 82 forks source link

Add 'additionalEnvVars' to the chart #303

Closed Stevenpc3 closed 4 months ago

Stevenpc3 commented 4 months ago

Add ability to set additional ENV vars via additionalEnv value.

Array containing extra environment variables which can be templated. You can add this to an additional override file or example putting this in override.yaml

additionalEnv:
  - name: "{{ .Release.Namespace }}"
    value: "{{ .Release.Namespace }}"
  - name: MY_VAR
    value: mySpecialKey
  - name: 1234
    value: 5678
  - name: lottery
    value: 12345
  - name: bool
    value: false
  - name: empty
    value: ""

Then deploy with helm upgrade -i cnpg -n namespace-a charts/cnpg/ -f override.yaml

will generate (partial template below)

image

With these logs

image

Stevenpc3 commented 4 months ago

The cloudnative-pg.extraEnv doesn't looks like right to me. Does it work with multiple values at all?

Yes it does work with multiple values. If you further review my description you can see where I use two values and then pass them in and then highlight the results in the configmap which shows both values passed in.

itay-grudev commented 4 months ago

The cloudnative-pg.extraEnv doesn't looks like right to me. Does it work with multiple values at all?

Yes it does work with multiple values. If you further review my description you can see where I use two values and then pass them in and then highlight the results in the configmap which shows both values passed in.

I think the check whether the value is string only makes sense when you wrap your macro in a range - i.e.:

{{- range $e := .Values.additionalEnv }}
{{- include "cloudnative-pg.extraEnv" (dict "value" $e "context" $ ) | nindent 8 }}
{{- end }}

Otherwise .value will always be an array

itay-grudev commented 4 months ago

And then why don't you always do tpl. If there is no {{ sequence there wouldn't be anything to replace.

Stevenpc3 commented 4 months ago

The cloudnative-pg.extraEnv doesn't looks like right to me. Does it work with multiple values at all?

Yes it does work with multiple values. If you further review my description you can see where I use two values and then pass them in and then highlight the results in the configmap which shows both values passed in.

I think the check whether the value is string only makes sense when you wrap your macro in a range - i.e.:

{{- range $e := .Values.additionalEnv }}
{{- include "cloudnative-pg.extraEnv" (dict "value" $e "context" $ ) | nindent 8 }}
{{- end }}

Otherwise .value will always be an array

Ranging over means I need to add my own "-" before name for list conversion. Here is teh results of the template above and the override.yaml below. You can see how range changed the items from a list to individial line outputs.

image

Stevenpc3 commented 4 months ago

And then why don't you always do tpl. If there is no {{ sequence there wouldn't be anything to replace.

Yeah this is good. I made this change and it works well with how I testsed below.

image

Stevenpc3 commented 4 months ago

I simplified it and used the test cases of [] and the list above. It appears good and not sure I can simplify any further.

Stevenpc3 commented 4 months ago

there was a typo left in this PR. I have made a fix here https://github.com/cloudnative-pg/charts/pull/305