kestra-io / helm-charts

Apache License 2.0
37 stars 26 forks source link

Allow the use of an existingSecrets for superadmin, license, operator basic auth etc #55

Open aresfkonrad opened 1 month ago

aresfkonrad commented 1 month ago

Feature description

As of now, the kestra operator does not allow the direct use of secrets for its configuration:

https://github.com/kestra-io/helm-charts/blob/0d3cff853315b91e8b5bbe46e14a14e66393cfce/charts/kestra/templates/operator.yaml#L249

          env:
            - name: QUARKUS_OPERATOR_SDK_CRD_APPLY
              value: "true"
            - name: QUARKUS_OPERATOR_SDK_CRD_VALIDATE
              value: "false"
            - name: KESTRA_API_URL
              value: http://{{ include "kestra.fullname" $service }}:8080
            {{- if .Values.operator.apiKey }}
            - name: KESTRA_API_API_KEY
              value: {{ .Values.operator.apiKey }}
            {{- end }}
            {{- if .Values.operator.basicAuth }}
            - name: KESTRA_API_BASIC_AUTH
              value: {{ .Values.operator.basicAuth }}
            {{- end }}

For other components, we could figure out a hacky way to use existingSecrets for the super admin account:

configuration:
  kestra:
              security:
                super-admin:
                  username: ${KESTRA_SUPERADMIN_USERNAME}
                  password: ${KESTRA_SUPERADMIN_PASSWORD}

for the kestra-ee license the following way:

configuration:
  kestra:
              ee:
                license:
                  id: "${LICENSE_ID}"
                  key: "${LICENSE_KEY}"

etc.

Unfortunately, this does not seem to work with the operator. We can set the variable only as a text based value - or leave it unset. We cannot redefine the environment variable on our own either, because we cannot provide any extraEnv-Style environment variable overrides.

A more elegant solution for all of these cases would be to allow the user to choose between a deployment using existingSecret and a deployment using plain strings. I found a relatively simple example here: https://stackoverflow.com/questions/74739625/can-we-define-env-variable-in-the-helm-charts-only-if-secrets-are-provisioned

{{- with .Values.existingSecret }}
    valueFrom:
      secretKeyRef:
        name: {{ . }}
        key: password
{{- else }}
    value: {{ .Values.cmpassword | required "either existingSecret or cmpassword is required" | quote }}
{{- end }}

The situation with Postgres and Minio is a little more complex since the secret is cross referenced between the kestra chart itself and the sub charts. This Feature request does not address the affected secrets on both of those dependencies.

We are looking forward to your opinions on that topic.

Thank you very much.

loicmathieu commented 1 month ago

I think a less intrusive solution would be to allow passing arbitrary env variables to the operator deployment, this is what is done for all other deployment and would allow you to set the API key or basic auth env variable to anything: other env variables, secret or config map, ...

aresfkonrad commented 1 month ago

@loicmathieu that should be fine as well.