[chart/v1-2x-test] [Helm]Support tpl rendering in ServiceAccount annotations, metadataConnection, and config ConfigMap names (#64763)#65081
Merged
jscheffl merged 1 commit intochart/v1-2x-testfrom Apr 12, 2026
Conversation
…tations, metadataConnection, and config ConfigMap names (#64763) * Support tpl rendering in ServiceAccount annotations, metadataConnection, and config ConfigMap names The scheduler ServiceAccount already tpl-renders annotations via range/tpl, but all other ServiceAccounts (pgbouncer, webserver, workers, triggerer, dag-processor, api-server) use raw toYaml. This makes it impossible to use template expressions like {{ .Values.global.appName }}-sa@project.iam.gserviceaccount.com in annotation values for those components. Apply the same range/tpl pattern from the scheduler to all ServiceAccount templates for consistency. Also tpl-render data.metadataConnection.user and data.metadataConnection.db so wrapper charts can derive database credentials from template expressions instead of requiring static values. This affects the metadata connection secret and pgbouncer config. Also tpl-render webserverConfigConfigMapName and apiServerConfigConfigMapName so wrapper charts can use {{ .Release.Name }}-custom-config instead of hardcoding the release name. All changes are backward compatible: tpl on a plain string without template expressions returns the string unchanged. * Address review feedback: make tplDict generic, move tests to existing files - Remove nindent from tplDict helper, move indent to call sites - Move SA annotation tpl tests to test_annotations.py - Move metadataConnection tpl tests to test_metadata_connection_secret.py - Split Airflow 2/3 version tests into separate parametrized cases - Remove conditional logic from tests per review feedback - Fix result-backend nit (remove unnecessary parens) - Delete test_tpl_rendering.py (tests relocated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address round 2 review: split pgbouncer, use celery/k8s worker SA, fix versions - Separate pgbouncer into dedicated test (no conditional logic) - Use workers.celery.serviceAccount for Celery worker tpl test - Add workers.kubernetes.serviceAccount test for KubernetesExecutor - Use airflowVersion 2.11.0 (first supported 2.11) instead of 2.11.2 - Remove explicit airflowVersion from Airflow 3 tests (default is 3.x) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove kubernetes worker SA test — workers.kubernetes does not own SA template The worker SA template reads from workers.serviceAccount (merged from workers.celery), not workers.kubernetes.serviceAccount. Removed invalid test. All 82 tests in test_annotations.py and test_metadata_connection_secret.py pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix tplDict multi-annotation bug: use toYaml instead of printf The previous printf-based approach concatenated multiple annotations into a single YAML line due to whitespace trimming. Replace with dict-building + toYaml for correct YAML output with any number of entries. Also adds toString for non-string value safety. Added test_tpl_rendered_multiple_annotations to verify 3 annotations (2 templated + 1 plain) render correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add KubernetesExecutor worker SA tpl annotation test The worker-serviceaccount.yaml template serves both CeleryExecutor and KubernetesExecutor. Add test verifying tpl rendering works for the KubernetesExecutor path via workers.serviceAccount.annotations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- (cherry picked from commit 3a458de) Co-authored-by: shlomi tubul <33376277+shlomitubul@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
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.
The scheduler ServiceAccount already tpl-renders annotations via
range/tpl, but all other ServiceAccounts (pgbouncer, webserver, workers,
triggerer, dag-processor, api-server) use raw toYaml. This makes it
impossible to use template expressions like
{{ .Values.global.appName }}-sa@project.iam.gserviceaccount.com in
annotation values for those components.
Apply the same range/tpl pattern from the scheduler to all ServiceAccount
templates for consistency.
Also tpl-render data.metadataConnection.user and
data.metadataConnection.db so wrapper charts can derive database
credentials from template expressions instead of requiring static values.
This affects the metadata connection secret and pgbouncer config.
Also tpl-render webserverConfigConfigMapName and
apiServerConfigConfigMapName so wrapper charts can use
{{ .Release.Name }}-custom-config instead of hardcoding the release name.
All changes are backward compatible: tpl on a plain string without
template expressions returns the string unchanged.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
The worker SA template reads from workers.serviceAccount (merged from
workers.celery), not workers.kubernetes.serviceAccount. Removed invalid test.
All 82 tests in test_annotations.py and test_metadata_connection_secret.py pass.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
The previous printf-based approach concatenated multiple annotations
into a single YAML line due to whitespace trimming. Replace with
dict-building + toYaml for correct YAML output with any number of
entries. Also adds toString for non-string value safety.
Added test_tpl_rendered_multiple_annotations to verify 3 annotations
(2 templated + 1 plain) render correctly.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
The worker-serviceaccount.yaml template serves both CeleryExecutor and
KubernetesExecutor. Add test verifying tpl rendering works for the
KubernetesExecutor path via workers.serviceAccount.annotations.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
(cherry picked from commit 3a458de)
Co-authored-by: shlomi tubul 33376277+shlomitubul@users.noreply.github.com
Co-authored-by: Claude Opus 4.6 (1M context) noreply@anthropic.com