[jaeger] Add extraSecretMounts to jaeger#747
[jaeger] Add extraSecretMounts to jaeger#747DevErenOzcan wants to merge 1 commit intojaegertracing:mainfrom
Conversation
…ing users to mount external secrets (e.g., via Vault CSI) seamlessly. Signed-off-by: Eren Özcan <eren.ozcan@wechiptech.com>
There was a problem hiding this comment.
Pull request overview
This PR adds an extraSecretMounts values hook to the Jaeger Deployment template so users can attach additional mounted content (notably Secrets Store CSI volumes) to the Jaeger pod without workarounds.
Changes:
- Added
jaeger.extraSecretMountstovalues.yaml(default empty list). - Rendered additional
volumeMountsand correspondingvolumesentries injaeger/templates/jaeger/jaeger-deploy.yamlbased onjaeger.extraSecretMounts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| charts/jaeger/values.yaml | Introduces the new jaeger.extraSecretMounts values key with an empty default. |
| charts/jaeger/templates/jaeger/jaeger-deploy.yaml | Wires jaeger.extraSecretMounts into the Jaeger Deployment’s volumeMounts and volumes (CSI-focused). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: {{ .name }} | ||
| csi: | ||
| {{- toYaml .csi | nindent 12 }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Because volumes: is always emitted but all items are conditional, the rendered manifest can end up with volumes: null when no volumes are configured. Other templates in this chart avoid this by conditionally emitting the entire volumes: block (e.g., templates/spark/spark-dependencies.yaml). Consider applying the same pattern here (wrap volumes: + entries in an if, or render volumes: []).
| # See https://www.jaegertracing.io/docs/cli/ | ||
| extraSecretMounts: [] |
There was a problem hiding this comment.
This new extraSecretMounts value is placed directly under the comment block for “command line arguments / CLI flags”, but it configures pod volume mounts rather than CLI args. Please move it to a more appropriate subsection (or add a short comment explaining its structure: name, mountPath, optional readOnly, and csi/secret fields) so users don’t miss or misinterpret it.
| - name: {{ .name }} | ||
| csi: | ||
| {{- toYaml .csi | nindent 12 }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
extraSecretMounts is used by other chart components (spark / es-* cronjobs) with a secretName-based schema that renders secret: volumes, but here it assumes a CSI-backed volume (csi:). Reusing the same values key for different structures across the chart is likely to confuse users. Consider supporting both secret: and csi: volume sources in one schema, or renaming this field to something CSI-specific to avoid an ambiguous API.
| - name: {{ .name }} | ||
| mountPath: {{ .mountPath }} | ||
| readOnly: {{ .readOnly }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Because volumeMounts: is always emitted but all items are conditional, the rendered manifest can end up with volumeMounts: null when no mounts are configured. Other templates in this chart avoid this by conditionally emitting the entire volumeMounts: block (e.g., templates/spark/spark-dependencies.yaml). Consider applying the same pattern here (wrap volumeMounts: + entries in an if, or render volumeMounts: []).
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You may re-open it if you need more time. |
Description
This PR introduces the
extraSecretMountscapability to the Jaeger deployment chart.Currently, users relying on external secret management tools like HashiCorp Vault via the Secrets Store CSI Driver face limitations because there is no native way to attach these external volumes directly to the Jaeger pods without complex workarounds (like creating auxiliary pause pods).
Similar to the implementation in
kube-prometheus-stack, this feature allows users to easily define secret mounts in theirvalues.yaml, and the template automatically handles both thevolumesandvolumeMountsconfigurations.Related Issues:
Changes Made
extraSecretMountsblock parsing tojaeger/templates/jaeger/jaeger-deploy.yaml(handles bothvolumeMountsandvolumes).extraSecretMounts: []list tojaeger/values.yamlto ensure backward compatibility and serve as documentation.How to Test
You can test this by adding the following block to your
values.yamland runninghelm template: