Chart: configurable enableServiceLinks

[johanjk]

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)

[jscheffl]

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.

[Miretpl]

Could you add test cases for this change?

I would change the default to true and introduce it to the 1.2x with a message that the default will be changed to false in the Chart 2.0. Without that, users can have a little harder time moving from 1.2x to 2.0 versions if someone is relying on the default behaviour.

[Miretpl]

Also, I think that newsfragment should be significant instead of feature as it is both a feature and a bug fix depending on the case.

[johanjk]

Could you add test cases for this change?

What kind of test are you after? e2e regression test?
Or schema validity? I have validate the schema using yamllint, kubeconform and kube-linter (yamllint+kubeconform is already part of the default test suit?).

[johanjk]

I opted to make the default null, to be fully backwards compatible, in case some users were using a policy that trigger on the non-existence of a value.

[Miretpl]

What kind of test are you after? e2e regression test?

Simple unit test for setting them with different options and testing the default behaviour. If someone wants to change, e.g. the default, it can be easier to catch if something will be changing within tests (we had a case in the past when it was useful). These could be added in the airflow_aux/test_airflow_common.py file

[potiuk]

@johanjk Converting to draft — this PR doesn't yet meet our Pull Request quality criteria.

• ❌ Merge conflicts. See docs.
• ❌ Other failing CI checks. See docs.
• ❌ Pre-commit / static checks. See docs.

See the linked criteria for how to fix each item, then mark the PR "Ready for review". This is not a rejection — just an invitation to bring the PR up to standard. No rush.

---

Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you.

[johanjk]

Added test case. Was briefly considering an explicit test for honoring the chart 2.0 default change:

default_false = (doc["metadata"]["labels"]["chart"] == "airflow-2.0.0")
expected = enable_service_links or not default_false

But didn't actually implement it. Would appreciate pointers if it is desirable.

Also added explicit testing for pod-template-file (Since I already overlooked it once)

[johanjk]

Rebased the commits on main to fix merge conflicts

[jscheffl]

For me looks fine now after the discussion... merging. Will merge and backport to chart 1.2x-line once CI is green

[github-actions]

Backport failed to create: chart/v1-2x-test. View the failure log Run details

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

You can attempt to backport this manually by running:

cherry_picker 56061a6 chart/v1-2x-test

This should apply the commit to the chart/v1-2x-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.