feat(helm): optionally enable config map and use all values #3265
Conversation
feat(helm): optionally use all environment variables from secret chore(helm): fixup wrong value reference in ui
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Helm chart configuration by making the ConfigMap optional and adding support for using all environment variables from a secret. This provides more flexibility for deployments where the TensorZero configuration is already baked into the container image or when all environment variables should be sourced from a single secret.
- Added configMap.enabled flag to conditionally create and mount the ConfigMap
- Added ability to use all environment variables from a secret when keys list is empty
- Updated deployment templates to handle conditional ConfigMap mounting
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| values.yaml | Added configMap.enabled flag and documentation for new secret usage pattern |
| templates/configmap.yaml | Wrapped ConfigMap creation with enabled condition |
| templates/gateway.yaml | Added conditional ConfigMap mounting and envFrom support for all secret variables |
| templates/ui.yaml | Added conditional ConfigMap mounting and envFrom support for all secret variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| volumes: | ||
| {{- if .Values.configMap.enabled }} | ||
| - name: config-volume | ||
| configMap: | ||
| name: {{ include "tensorzero.fullname" . }} | ||
| {{- end }} | ||
| {{- if .Values.persistence.enabled }} | ||
| - name: storage-volume | ||
| persistentVolumeClaim: | ||
| claimName: {{ include "tensorzero.fullname" . }}-storage | ||
| {{- end }} |
There was a problem hiding this comment.
The volumes section should be conditionally rendered only when at least one volume is needed. However, the current logic will render an empty volumes array when both configMap.enabled and persistence.enabled are false. Consider using a more explicit condition or restructuring to avoid empty volumes sections.
| volumes: | |
| {{- if .Values.configMap.enabled }} | |
| - name: config-volume | |
| configMap: | |
| name: {{ include "tensorzero.fullname" . }} | |
| {{- end }} | |
| {{- if .Values.persistence.enabled }} | |
| - name: storage-volume | |
| persistentVolumeClaim: | |
| claimName: {{ include "tensorzero.fullname" . }}-storage | |
| {{- end }} | |
| volumes: | |
| {{- if .Values.configMap.enabled }} | |
| - name: config-volume | |
| configMap: | |
| name: {{ include "tensorzero.fullname" . }} | |
| {{- end }} | |
| {{- if .Values.persistence.enabled }} | |
| - name: storage-volume | |
| persistentVolumeClaim: | |
| claimName: {{ include "tensorzero.fullname" . }}-storage | |
| {{- end }} |
Copilot uses AI. Check for mistakes.
| resources: | ||
| {{- toYaml .Values.ui.resources | nindent 12 }} | ||
| {{- if .Values.configMap.enabled }} | ||
| volumes: | ||
| - name: config-volume | ||
| configMap: | ||
| name: {{ include "tensorzero.fullname" . }} |
There was a problem hiding this comment.
The volumes section in the UI template is inconsistent with the gateway template. The UI template only conditionally renders volumes when configMap is enabled, but the gateway template considers both configMap and persistence. This inconsistency could cause confusion and maintenance issues.
| resources: | |
| {{- toYaml .Values.ui.resources | nindent 12 }} | |
| {{- if .Values.configMap.enabled }} | |
| volumes: | |
| - name: config-volume | |
| configMap: | |
| name: {{ include "tensorzero.fullname" . }} | |
| {{- if .Values.ui.persistence.enabled }} | |
| - name: ui-persistent-storage | |
| mountPath: {{ .Values.ui.persistence.mountPath | default "/app/data" }} | |
| {{- end }} | |
| resources: | |
| {{- toYaml .Values.ui.resources | nindent 12 }} | |
| {{- if or .Values.configMap.enabled .Values.ui.persistence.enabled }} | |
| volumes: | |
| {{- if .Values.configMap.enabled }} | |
| - name: config-volume | |
| configMap: | |
| name: {{ include "tensorzero.fullname" . }} | |
| {{- end }} | |
| {{- if .Values.ui.persistence.enabled }} | |
| - name: ui-persistent-storage | |
| persistentVolumeClaim: | |
| claimName: {{ .Values.ui.persistence.existingClaim | default (printf "%s-ui-pvc" (include "tensorzero.fullname" .)) }} | |
| {{- end }} |
Copilot uses AI. Check for mistakes.
Originally part of #3042 . I think it is sensible to make config map improvements in a single PR.
Important
Makes config map mounting optional and allows using all environment variables from a secret in Helm charts.
configMapoptional inconfigmap.yaml,gateway.yaml, andui.yaml.configMap.enabledinvalues.yaml.gateway.yamlandui.yaml.additionalEnv.secretNameandadditionalEnv.keysinvalues.yaml.This description was created by
for edc28e1. You can customize this summary. It will automatically update as commits are pushed.