feat(helm): add openai.secretRef support to querydoc sub-chart#1911
Open
TOMOFUMI-KONDO wants to merge 1 commit into
Open
feat(helm): add openai.secretRef support to querydoc sub-chart#1911TOMOFUMI-KONDO wants to merge 1 commit into
TOMOFUMI-KONDO wants to merge 1 commit into
Conversation
Mirrors the existing grafana-mcp pattern: when `openai.secretRef` is set, the chart skips rendering its own Secret and points the Deployment's `envFrom.secretRef.name` at the user-supplied Secret instead. Precedence rules: - `secretRef` set → no chart-owned Secret; Deployment references the external Secret by name - `apiKey` set, no `secretRef` → chart creates the Secret (existing behavior unchanged) - Neither set → no Secret, no secretRef in envFrom (existing behavior) - Both set → `secretRef` wins; no chart-owned Secret This allows operators to manage the OpenAI API key via External Secrets, sealed-secrets, or manual `kubectl create secret` without workarounds such as placeholder values and ArgoCD ignoreDifferences blocks. Added secret_test.yaml covering the four-row precedence table, and extended deployment_test.yaml with envFrom/checksum-annotation cases for each combination. Signed-off-by: TOMOFUMI-KONDO <ugax2kontomo0314@gmail.com>
TOMOFUMI-KONDO
requested review from
EItanya,
ilackarms,
iplay88keys,
jmhbh,
supreme-gg-gg and
yuval-k
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for referencing an existing Kubernetes Secret for the Querydoc OpenAI API key, avoiding chart-managed Secret creation when openai.secretRef is provided.
Changes:
- Documented
openai.secretRefin chart values (Querydoc + kagent umbrella values). - Updated Helm templates to (a) not render a Secret when
secretRefis set and (b) mount either chart-owned or external Secret into the Deployment. - Added helm-unittest coverage for Secret rendering and Deployment envFrom/annotations behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| helm/tools/querydoc/values.yaml | Documents openai.secretRef as an alternative to inline apiKey. |
| helm/tools/querydoc/templates/secret.yaml | Only renders chart-owned Secret when apiKey is set and no secretRef is provided. |
| helm/tools/querydoc/templates/deployment.yaml | References external Secret when secretRef is set; only sets checksum/secret for chart-owned Secret. |
| helm/tools/querydoc/tests/secret_test.yaml | Adds tests for Secret rendering precedence rules (secretRef wins). |
| helm/tools/querydoc/tests/deployment_test.yaml | Adds tests for envFrom + checksum/secret behavior with apiKey vs secretRef. |
| helm/kagent/values.yaml | Surfaces querydoc.openai.secretRef in the umbrella chart values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| templates: | ||
| - deployment.yaml | ||
| - configmap.yaml | ||
| - secret.yaml |
Contributor
Author
There was a problem hiding this comment.
All of the tests in this file have template: key, and in addition secret.yaml template target is required to test the behavior of include function in the deployment spec.
Comment on lines
+17
to
19
| {{- if and .Values.openai.apiKey (not .Values.openai.secretRef) }} | ||
| checksum/secret: {{ include (print $.Template.BasePath "/secret.yaml") . | sha256sum }} | ||
| {{- end }} |
Contributor
Author
There was a problem hiding this comment.
I think that it's outside the scope of this PR.
Comment on lines
+49
to
52
| {{- if or .Values.openai.apiKey .Values.openai.secretRef }} | ||
| - secretRef: | ||
| name: {{ include "querydoc.fullname" . }} | ||
| name: {{ .Values.openai.secretRef | default (include "querydoc.fullname" .) | quote }} | ||
| {{- end }} |
Contributor
Author
There was a problem hiding this comment.
I think that it's outside the scope of this PR.
Contributor
Author
|
@EItanya @ilackarms @yuval-k @supreme-gg-gg @iplay88keys @jmhbh |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Add
openai.secretRefto the querydoc sub-chart, mirroring the pattern already supported by thegrafana-mcpsub-chart. When set, the chart skips rendering its ownSecretand instead points the Deployment'senvFrom.secretRef.nameat the user-supplied Secret.Changes
Precedence logic for
openai.secretRefvsopenai.apiKeyapiKeysecretRefSecret(existing behavior unchanged)Secret; Deployment references user SecretsecretRefwins; no chartSecretSecret, nosecretRefinenvFrom(existing behavior)Template changes
templates/secret.yaml: guard changed from{{- if .Values.openai.apiKey }}to{{- if and .Values.openai.apiKey (not .Values.openai.secretRef) }}templates/deployment.yaml:envFromguard widened to{{- if or .Values.openai.apiKey .Values.openai.secretRef }}{{ .Values.openai.secretRef | default (include "querydoc.fullname" .) | quote }}checksum/secretannotation tightened to only appear when the chart owns the SecretValues
openai.secretRef: ""added (commented out) to bothhelm/tools/querydoc/values.yamland the parenthelm/kagent/values.yaml.Tests
tests/secret_test.yaml(new): covers the four-row precedence table for Secret creationtests/deployment_test.yaml: addedsecret.yamlto the rendered templates list (needed forinclude-based checksum), plus four new cases coveringenvFromandchecksum/secretannotation behavior for each combination