search

zabbix-community / helm-zabbix

Helm chart for Zabbix
https://artifacthub.io/packages/helm/zabbix-community/zabbix
Apache License 2.0
77 stars 45 forks source link

Add envfrom parameters #93

Closed Elbandi closed 1 month ago

Elbandi commented 1 month ago

What this PR does / why we need it:

Allow extra environment variables from secrets or configmaps.

(my usecase: added custom userparameters, which use some passwords. This password can set from secrets with env)

Special notes for your reviewer:

I hesitated the right variable name (envFrom vs extraEnvFrom). i checked some other helm charts, and more use envFrom names. like traefik, filebeat (with extraEnvs and extraVolumes).

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

aeciopires commented 1 month ago

Hey @Elbandi! Thank you for your contribution. But I think extraEnv can be refactored to add support for the same functionality as envFrom. This avoids confusion for users of helm charts.

What do you think? Could you make this change or can I make it?

The final solution is to change each template file:

before (example):

           {{- range $item := .Values.zabbixServer.extraEnv }}
            - name: {{ $item.name }}
              value: {{ $item.value | quote }}
            {{- end }}

after (example):

            {{- with .Values.zabbixServer.extraEnv }}
              {{- toYaml . | nindent 12 }}
            {{- end }}

The final values (example) can be:

  extraEnv:
    - name: "ZBX_EXAMPLE_MY_ENV_1"
      value: "true"
    - name: "ZBX_EXAMPLE_MY_ENV_2"
      value: "false"
    - name: "ZBX_EXAMPLE_MY_ENV_3"
      value: "100"
    - name: MY_USERNAME
      valueFrom:
        secretKeyRef:
          name: my-envs
          key: user
    - name: MY_PASSWORD
      valueFrom:
        secretKeyRef:
          name: my-envs
          key: password

Did you understand the suggestion?

aeciopires commented 1 month ago

Hi @Elbandi!

I launched the version 4.4.1. See:

Please sync this changes in your local branch

aeciopires commented 1 month ago

Hello @Elbandi!

I published a new chart version to support Zabbix 7.0 and added the improvements suggested in this PR, but I keep your credits related the idea and implementation.

More details: https://github.com/zabbix-community/helm-zabbix/pull/96

I will close this PR to avoid code conflicts

Thanks for your contribution.