[DND-487] feat: support global.imagePullSecrets across Opik chart#6968
Open
jms200 wants to merge 3 commits into
Open
[DND-487] feat: support global.imagePullSecrets across Opik chart#6968jms200 wants to merge 3 commits into
jms200 wants to merge 3 commits into
Conversation
Adds an `opik.imagePullSecrets` helper that resolves the imagePullSecrets
list with parent-chart precedence:
1. .Values.global.imagePullSecrets (propagated from comet-ml-helm-chart;
when set, used exclusively)
2. local chart field (.Values.imagePullSecrets for app pods,
.Values.clickhouse.imagePullSecrets for CH)
Pairs with comet-ml-helm-chart PR #466.
Wired into:
- templates/deployment.yaml (all Opik app pods, served by the
component range loop)
- templates/clickhouseinstallation.yaml (CHI pod template — already at
spec.templates.podTemplates[].spec)
- templates/clickhouse-backup-cronjob.yaml (CH backup CronJob)
Drive-by fix: clickhouse-backup-cronjob had `imagePullSecrets:` nested
inside `containers[0]:` (column 12), which is not a valid PodSpec
location — Kubernetes silently dropped it. The helper places it at
PodSpec level (column 10) which is correct.
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Re-runs helm-docs after the values.yaml update so the rendered README table picks up the new global.imagePullSecrets field description. Also sweeps prior README drift (version badge bump from 2.0.48 to current 2.0.56 from upstream Chart.yaml bumps).
github-actions
Bot
added
the
documentation
Contributor
Author
|
Part of a 3-PR set for DND-487 — must merge in this order:
|
andrescrz
previously approved these changes
Jun 10, 2026
Member
|
@copilot resolve the merge conflicts in this pull request |
…-image-pull-secrets # Conflicts: # deployment/helm_chart/opik/README.md
andrescrz
approved these changes
Jun 10, 2026
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.
Details
Adds an
opik.imagePullSecretshelper that resolves the imagePullSecrets list with parent-chart precedence:.Values.global.imagePullSecrets— propagated from the parentcomet-ml-helm-chartglobal:block. When set, used exclusively..Values.imagePullSecretsfor app pods,.Values.clickhouse.imagePullSecretsfor ClickHouse pods. Used when (1) is unset.Pairs with comet-ml-helm-chart #466.
Wired into:
templates/deployment.yaml(all Opik app pods, served by the component range loop)templates/clickhouseinstallation.yaml(CHI pod template — already atspec.templates.podTemplates[].spec)templates/clickhouse-backup-cronjob.yaml(CH backup CronJob)Drive-by fix:
templates/clickhouse-backup-cronjob.yamlhadimagePullSecrets:nested insidecontainers[0]:(indented at column 12), which is not a valid PodSpec location — the Kubernetes API server silently dropped it. The helper now places it at the PodSpec level (column 10), which is correct. Anyone relying onclickhouse.imagePullSecretsfor the backup job today was silently broken; this fixes that.For default customers (no
global.imagePullSecretsset), render output is identical tomainexcept for the CronJob fix above.Change checklist
.Values.global.imagePullSecretswith strict precedence over chart-local fieldsopik.imagePullSecretsadded totemplates/_helpers.tplimagePullSecretsfrom container-level (silently dropped) to PodSpec-levelvalues.yamldocuments the newglobal.imagePullSecretsfieldREADME.mdregenerated viahelm-docsmainresolvedIssues
AI-WATERMARK
AI-WATERMARK: yes
deployment/helm_chart/opik/README.mdTesting
Render-tested locally with
helm templateacross three value scenarios:imagePullSecrets+clickhouse.imagePullSecrets)opik-app-secretopik-ch-secretopik-ch-secret(now at PodSpec level)global.imagePullSecretssetcustomer-global(wins)customer-global(wins)customer-global(wins)helm lintpasses locally. CIlint-helm-chartpasses on this PR.Documentation
values.yaml:global.imagePullSecretsannotated with helm-docs# --syntax so it renders in the chart README tableREADME.md: regenerated viahelm-docsto surface the new field; merge conflict withmainresolved by taking main's updated version badge and preserving theglobal.imagePullSecretsvalues table entry