feat(helm): support tpl rendering in podAnnotations#28333
Conversation
|
|
Greptile SummaryThis PR pipes
Confidence Score: 5/5Safe to merge — the change is a focused one-liner that follows an established pattern already present in the same file and chart. The change is minimal and its behavior is well-understood: tpl passes plain strings unchanged, so existing users are unaffected. The templating caveat (literal {{ in annotation values now renders) is already documented in the PR description and identical to every other tpl call in this chart. The new unit test covers the primary use-case and the backward-compat path. No logic outside the Helm layer is touched. No files require special attention.
|
| Filename | Overview |
|---|---|
| deploy/charts/litellm-helm/templates/deployment.yaml | One-line change wraps podAnnotations serialization through tpl (toYaml .) $, consistent with existing extraInitContainers/extraContainers usage in the same file. |
| deploy/charts/litellm-helm/tests/deployment_tests.yaml | New test case verifies tpl rendering of two distinct {{ .Values.* }} expressions and confirms plain-string annotations pass through unchanged (backward-compat canary). |
Reviews (1): Last reviewed commit: "feat(helm): support tpl rendering in pod..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 litellm-agent: Auto-merge skipped — the staging branch Please rebase your branch onto |
Wrap toYaml with tpl (passing the root context $) in the deployment
template's podAnnotations block so users can reference Helm values and
templates (e.g. {{ include ... | sha256sum }}) inside pod annotations.
Primary use case: when users disable the chart's built-in ConfigMap
(proxyConfigMap.create=false) to provide their own, the built-in
checksum/config annotation is also disabled and ConfigMap changes no
longer trigger a rolling restart. With tpl support, users can
re-implement the checksum/config annotation themselves.
Matches the existing tpl pattern already used in this chart for
extraInitContainers, extraContainers (deployment.yaml:50,215) and the
migrations job (migrations-job.yaml:40,99). Direct precedent: commit
87d7e86 ("feat(helm): add tpl support to extraContainers and
extraInitContainers").
Adds a helm unittest case exercising root-Values access (proving the
root context is wired) and plain-string passthrough (backward-compat
canary). make test-unit-helm: 54/54 passing. helm lint: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7095589 to
2bfa475
Compare
PR overviewMedium: Pod annotations now execute Helm templatesThis PR changes Security review
Risk: 5/10 |
| {{- end }} | ||
| {{- with .Values.podAnnotations }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- tpl (toYaml .) $ | nindent 8 }} |
There was a problem hiding this comment.
Medium: Helm template execution in annotations
tpl evaluates annotation values as Helm templates, so a values author can set an annotation like {{ (lookup "v1" "Secret" "default" "some-secret").data.token }} and have Helm copy data readable by the release credentials into the rendered pod annotations. If this field is intended to be safe for less-trusted values input, keep it as plain YAML rendering or gate template evaluation behind a clearly separate opt-in value.
Closes #28332
Summary
Render
Values.podAnnotationsthroughtpl ... $indeploy/charts/litellm-helm/templates/deployment.yaml, so users can include template expressions (e.g. a sha256 of their custom ConfigMap) inside pod annotations.{{- with .Values.podAnnotations }} -{{- toYaml . | nindent 8 }} +{{- tpl (toYaml .) $ | nindent 8 }} {{- end }}Motivation
When users disable
proxyConfigMap.createto provide their own ConfigMap, the chart's built-inchecksum/configannotation is also disabled, so changes to a user-managed ConfigMap no longer roll the deployment. AllowingtplinpodAnnotationslets users re-implement that annotation themselves, e.g.:Consistency with existing chart code
This matches the
tpl (toYaml .) $pattern already used elsewhere in the same chart:deploy/charts/litellm-helm/templates/deployment.yaml:50—extraInitContainersdeploy/charts/litellm-helm/templates/deployment.yaml:215—extraContainersdeploy/charts/litellm-helm/templates/migrations-job.yaml:40,99— migrations jobDirect precedent: commit
87d7e86479("feat(helm): add tpl support to extraContainers and extraInitContainers", April 2026).Tests
Added one new
helm unittestcase todeploy/charts/litellm-helm/tests/deployment_tests.yaml:should support tpl in podAnnotations— setsproxyConfigMap.create: falseto mirror the real-world scenario, then asserts that a templated annotation referencing.Values.image.tagresolves correctly (proves$is wired), a second key referencing.Values.image.repositoryresolves correctly, and a plain-string annotation passes through unchanged (backward-compat canary).Run locally with
make test-unit-helm— all 54 tests pass.Also manually verified with
helm templatethat.Release.Nameand| sha256sumpipelines resolve correctly inside annotations.Backward compatibility
tplpasses plain strings (no{{ }}) through unchanged, so existing chart users see no behavior change. Users with a literal{{inside an annotation value would now see it rendered as a template — that caveat already applies to every othertplusage in this chart, and to the precedent PR above.Checklist
make test-unit-helmpasseshelm lint deploy/charts/litellm-helmpasses