Skip to content

Chart: configurable enableServiceLinks#67447

Open
johanjk wants to merge 2 commits into
apache:mainfrom
johanjk:service-links
Open

Chart: configurable enableServiceLinks#67447
johanjk wants to merge 2 commits into
apache:mainfrom
johanjk:service-links

Conversation

@johanjk
Copy link
Copy Markdown

@johanjk johanjk commented May 24, 2026

Chart support EnableServiceLinks

EnableServiceLinks is a source of subtle bugs, it introduces env vars that can conflict with user env vars etc.
It's also noisy, pollutes the env, and increase pod startup time.
Make it configurable in the chart, and default to false

It is open for debate if the default should stay true, as some users might technically rely on it existing.
Hence it can be seen as a breaking change. I am open to keeping the default true.

Testing

Tested with helm template chart for enableServiceLinks: true/false.

Current workaround

postRenders:
  - kustomize:
      patches:
        - target:
            version: v1
            kind: Deployment
          patch: |
            - op: add
              path: /spec/template/spec/enableServiceLinks
              value: false
        - target:
            version: v1
            kind: CronJob
          patch: |
            - op: add
              path: /spec/jobTemplate/spec/template/spec/enableServiceLinks
              value: false

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

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'd be OK with this change and also with the change of default. But I'd like other voices as well as this might be a change in behavior if you upgrade. So putting this into Chart 2.0 (only)?

In this case as it is a behavioral change, would request to add a short newsfragment.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants