Skip to content

feat(argo-cd): support tpl in env var values#3842

Merged
jmeridth merged 2 commits intoargoproj:mainfrom
DrFaust92:feat/argo-cd-tpl-env
May 8, 2026
Merged

feat(argo-cd): support tpl in env var values#3842
jmeridth merged 2 commits intoargoproj:mainfrom
DrFaust92:feat/argo-cd-tpl-env

Conversation

@DrFaust92
Copy link
Copy Markdown
Contributor

@DrFaust92 DrFaust92 commented Apr 22, 2026

Checklist:

  • I have bumped the chart version according to versioning
  • I have updated the documentation according to documentation
  • I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • Any new values are backwards compatible and/or have sensible default.
  • I have signed off all my commits as required by DCO.
  • I have created a separate pull request for each chart according to pull requests
  • My build is green (troubleshooting builds).

Summary

Switch env var rendering in all component deployments/statefulsets from toYaml to tpl (toYaml .) $, enabling Helm template expressions inside env[].value fields.

This is a backwards-compatible change — plain string values continue to work as before.

Note: when using template expressions, reference root-level values with $.Values.x (not .Values.x) since the dot context inside the env list is the list element.

Example use case — auto-setting GOMEMLIMIT from the container memory limit in a parent chart/values overlay:

server:
  env:
    - name: GOMEMLIMIT
      value: "{{ printf \"%dMiB\" (mulf ((.Values.server.resources.limits.memory | trimSuffix \"Gi\" | float64) | mulf 1024) 0.85 | int) }}"

@mkilchhofer
Copy link
Copy Markdown
Member

Why you don't use resource field refs?

server:
  env:
    - name: GOMEMLIMIT
      valueFrom:
        resourceFieldRef:
          resource: limits.memory
          divisor: "1"

In my opinion this would be the exact use case for this:

$ kubectl  explain pod.spec.containers.env.valueFrom.resourceFieldRef
KIND:       Pod
VERSION:    v1

FIELD: resourceFieldRef <ResourceFieldSelector>


DESCRIPTION:
    Selects a resource of the container: only resources limits and requests
    (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu,
    requests.memory and requests.ephemeral-storage) are currently supported.
    ResourceFieldSelector represents container resources (cpu, memory) and their
    output format

FIELDS:
  containerName	<string>
    Container name: required for volumes, optional for env vars

  divisor	<Quantity>
    Specifies the output format of the exposed resources, defaults to "1"

  resource	<string> -required-
    Required: resource to select

@DrFaust92
Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion — I looked into resourceFieldRef but it's not viable for this use case.

The goal of setting GOMEMLIMIT is to give Go some headroom below the cgroup memory limit (typically ~90%) so GC kicks in before the kernel OOM-kills the pod. resourceFieldRef has no multiplier — only divisor, which does integer unit conversion. To get ~90% you'd want something like divisor: "1.1", but the API server rejects it:

spec.containers[0].env[0].valueFrom.resourceFieldRef.divisor: Invalid value:
only divisor's values 1, 1k, 1M, 1G, 1T, 1P, 1E, 1Ki, 1Mi, 1Gi, 1Ti, 1Pi, 1Ei
are supported with the memory resource

So resourceFieldRef can only express 100% of limits.memory, which defeats the purpose — at 100% Go thinks it has the entire cgroup budget for the heap, but real RSS includes stack, mmap, runtime overhead, etc., so you're back to OOM risk.

That leaves either hardcoding (drift-prone but explicit) or an entrypoint script that reads the cgroup limit and computes 90% — which is exactly the kind of thing chart-level templating with tpl would let users do cleanly. So this PR is still the right fix.

mkilchhofer
mkilchhofer previously approved these changes Apr 29, 2026
Copy link
Copy Markdown
Member

@mkilchhofer mkilchhofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DrFaust92 Perfect answer, thanks for these details! I now also found this issue over there:

Looks like you already made the right proposal for what is possible today.

mkilchhofer
mkilchhofer previously approved these changes May 1, 2026
Signed-off-by: Ilia <ilia.lazebnik@placer.ai>
@DrFaust92 DrFaust92 force-pushed the feat/argo-cd-tpl-env branch from 77338c6 to 64199d1 Compare May 5, 2026 21:47
@DrFaust92
Copy link
Copy Markdown
Contributor Author

DrFaust92 commented May 5, 2026

Hi @mkilchhofer — just rebased onto current main (resolved a trivial Chart.yaml version conflict, bumped to 9.5.12). The PR has been sitting through several Renovate-driven rebases now and the actual change is small and contained: just wrapping env var values with tpl in the argo-cd templates.

Would you be willing to merge this when you have a moment? Happy to address anything further if you'd like more changes — otherwise I'd love to stop chasing the rebase treadmill. Thanks again for the thoughtful review on the resourceFieldRef discussion.

jmeridth
jmeridth previously approved these changes May 6, 2026
Signed-off-by: Ilia Lazebnik <Ilia.lazebnik@gmail.com>
@jmeridth jmeridth merged commit 9376704 into argoproj:main May 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants