Skip to content

Chart: add serviceAccountTokenVolume to cleanup cron#67446

Merged
jscheffl merged 2 commits into
apache:mainfrom
johanjk:clean-token-volume
May 27, 2026
Merged

Chart: add serviceAccountTokenVolume to cleanup cron#67446
jscheffl merged 2 commits into
apache:mainfrom
johanjk:clean-token-volume

Conversation

@johanjk
Copy link
Copy Markdown
Contributor

@johanjk johanjk commented May 24, 2026

Chart support serviceAccountTokenVolume for cleanup job

When in an environment where policy dictate automountServiceAccountToken: false
the cleanup job require the same treatment as the scheduler, with a serviceAccountTokenVolume block.

Testing

Tested with

helm template chart | yq 'select(.metadata.name == "release-name-scheduler")'
helm template chart | yq 'select(.metadata.name == "release-name-cleanup")'

And values.yaml:

executor: "CeleryExecutor,KubernetesExecutor"

cleanup:
  enabled: true
  serviceAccount:
    automountServiceAccountToken: false
    serviceAccountTokenVolume:
      enabled: true

scheduler:
  serviceAccount:
    automountServiceAccountToken: false
    serviceAccountTokenVolume:
      enabled: true

as well as default values.yaml.

Current workaround

postRenders:
  - kustomize:
      patches:
        - target:
            version: v1
            kind: CronJob
            name: .*cleanup.*
          patch: |
            - op: add
              path: /spec/jobTemplate/spec/template/spec/volumes/-
              value:
                name: sa-token
                projected:
                  sources:
                    - serviceAccountToken:
                        path: token
                        expirationSeconds: 3600
                    - configMap:
                        name: kube-root-ca.crt
                        items:
                          - key: ca.crt
                            path: ca.crt
                    - downwardAPI:
                        items:
                          - path: namespace
                            fieldRef:
                              fieldPath: metadata.namespace
            - op: add
              path: /spec/jobTemplate/spec/template/spec/containers/0/volumeMounts/-
              value:
                name: sa-token
                mountPath: /var/run/secrets/kubernetes.io/serviceaccount
                readOnly: true

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented May 24, 2026

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

Comment thread chart/newsfragments/+b5a1f0bd.feature.rst Outdated
@johanjk johanjk force-pushed the clean-token-volume branch from f63ae4c to 10c1247 Compare May 25, 2026 13:45
Copy link
Copy Markdown
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

I am not 100% sure if this is a niece case to mount account token in this way. Code-wise it is looking good but I am not 100% confident it should be added to main line---- or if we rather should take both scheduler as well as cleanup consistently out into a Kustomize layer.

Opinions from other maintainers?

If merged, probably needs a back-port to 1-2x-line?

@jscheffl jscheffl added the backport-to-chart/v1-2x-test Automatic backport to chart 1.2x maintenance branch label May 25, 2026
@jscheffl jscheffl added this to the Airflow Helm Chart 1.22.0 milestone May 25, 2026
@Miretpl
Copy link
Copy Markdown
Contributor

Miretpl commented May 27, 2026

I am not 100% sure if this is a niece case to mount account token in this way.

Not sure either regarding how often it is used, but I agree with the docs which we have regarding that feature:

  1. https://airflow.apache.org/docs/helm-chart/stable/service-account-token-examples.html#overview
  2. https://airflow.apache.org/docs/helm-chart/stable/service-account-token-examples.html#high-security-environment

Code-wise it is looking good but I am not 100% confident it should be added to main line---- or if we rather should take both scheduler as well as cleanup consistently out into a Kustomize layer.

For now, I would add it to both. During discussions regarding 2.0, we agreed that we will keep some in the core chart and enhancing security will be possible via Kustomize, but as the Kustomize CI setup is in progress, I would, for now, go with the old way.

Opinions from other maintainers?

Not a maintainer, but these are my 2c 😄

If merged, probably needs a back-port to 1-2x-line?

Yup, I would add it there too, as it basically improves consistency within the chart, which we also discussed during 2.0.

@jscheffl
Copy link
Copy Markdown
Contributor

@Miretpl your 2ct are very welcome :-D

@jscheffl
Copy link
Copy Markdown
Contributor

Merging + back-porting with the aim to extract this in future into a Kustomize thing.

@jscheffl jscheffl merged commit 2ac00bb into apache:main May 27, 2026
193 of 194 checks passed
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg Bot commented May 27, 2026

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@github-actions
Copy link
Copy Markdown
Contributor

Backport successfully created: chart/v1-2x-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
chart/v1-2x-test PR Link

jscheffl pushed a commit that referenced this pull request May 27, 2026
…on (#67446) (#67617)

* [helm chart] add cleanup serviceAccountTokenVolume

* [helm chart] update doc for cleanup
(cherry picked from commit 2ac00bb)

Co-authored-by: johanjk <45788075+johanjk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:helm-chart Airflow Helm Chart backport-to-chart/v1-2x-test Automatic backport to chart 1.2x maintenance branch kind:documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants