Skip to content

[chart/v1-2x-test] Add securityContexts.disableDefaults flag for OpenShift SCC compatibility (#70238) - #72108

Open
Aaryan123456679 wants to merge 1 commit into
apache:chart/v1-2x-testfrom
Aaryan123456679:backport-3ac3851-chart/v1-2x-test
Open

[chart/v1-2x-test] Add securityContexts.disableDefaults flag for OpenShift SCC compatibility (#70238)#72108
Aaryan123456679 wants to merge 1 commit into
apache:chart/v1-2x-testfrom
Aaryan123456679:backport-3ac3851-chart/v1-2x-test

Conversation

@Aaryan123456679

Copy link
Copy Markdown
Contributor

Manual backport of #70238 to chart/v1-2x-test (automated backport failed with cherry-pick conflicts).

securityContexts.disableDefaults lets the chart skip its default runAsUser/fsGroup (pod) and runAsUser (container) values when securityContexts.pod/securityContexts.containers (or per-component overrides) are left empty, so platforms like OpenShift can assign these via SCC instead of the chart hard-coding them.

Conflict resolution notes

chart/v1-2x-test has diverged from main in ways unrelated to this PR, so the automatic cherry-pick (cherry_picker 3ac3851 chart/v1-2x-test) failed and this was resolved manually:

  • _helpers.yaml: localPodSecurityContext / localContainerSecurityContext needed the dual-arg (list <node> .) form from main to see the global disableDefaults flag, while keeping this branch's legacy <node>.securityContext (singular) fallback that main had already dropped — losing it broke TestSecurityContext::test_check_local_uid and three other pre-existing tests until it was restored.
  • worker-deployment.yaml and friends: adapted main's .Values.workers.celery.* values to this branch's flat .Values.workers.* (the Celery/Kubernetes worker-set split under workers.celery hasn't been backported here).
  • chart/tests/helm_tests/security/test_security_context.py no longer exists on this branch (its content lives at helm-tests/tests/helm_tests/security/test_security_context.py instead); the new test_disable_defaults_* coverage was added there, with the same workers.celeryworkers adaptation.

Test plan

  • helm lint chart/ passes
  • breeze testing helm-tests --test-type security — 431 passed
  • breeze testing helm-tests --test-type airflow_aux — 1143 passed
  • breeze testing helm-tests --test-type other — 433 passed
  • prek run --stage pre-commit and prek run --stage manual against chart/v1-2x-test — all passed (including kubeconform, Lint chart/values.schema.json)

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Sonnet 5)

Generated-by: Claude Code (Sonnet 5) following the guidelines

…Shift SCC compatibility (apache#70238)

* Allow runAsUser and fsGroup to be explicitly null in pod securityContexts

OpenShift assigns runAsUser/fsGroup automatically via SCC and rejects
pods that hard-code them, so users need to null these out in the
rendered pod securityContext. The vendored PodSecurityContext schema
required these fields to be integers, so setting them to null failed
Helm's values.schema.json validation before the templates ever
rendered (the templates already pass through explicit nulls
correctly).

Introduces a new AirflowPodSecurityContext schema definition (not
io.k8s.*-prefixed, so it survives vendor_k8s_json_schema.py
regeneration) that allows null for runAsUser/runAsGroup/fsGroup, and
points all 16 securityContexts.pod $refs at it instead of the vendored
definition.

closes: apache#41630

* Add newsfragment for Helm chart securityContexts fix

* Rename newsfragment to match this PR's number

Named after the GitHub issue instead of the actual PR number, which
the check-newsfragment-pr-number CI check expects.

* Replace null-based OpenShift SCC workaround with a securityContexts.disableDefaults flag

Explicit null in securityContexts.pod/containers was fragile and made every
call site guess whether to render a key at all. A dedicated flag is more
explicit about intent and lets the chart omit the default runAsUser/fsGroup
(and related) values entirely on platforms like OpenShift where the cluster's
SCC assigns them automatically and rejects hard-coded values.

* Update production guide for the disableDefaults securityContexts flag

The OpenShift SCC guidance still described the earlier null-based
workaround, which no longer omits the rendered securityContext keys.

* Fix docs spellcheck failure from unescaped securityContexts.* in schema description

securityContexts.pod / securityContexts.containers appeared as plain text in
the disableDefaults description, which the docs build renders as prose rather
than code, tripping the spellchecker on the camelCase identifier.

* Deprecate implicit securityContexts.disableDefaults default and unify helper argument order

Address review feedback: mark the disableDefaults false default as
deprecated (surfaced via NOTES.txt, values.yaml, values.schema.json and
the newsfragment) since it will flip to true in a future release, and
align airflowPodSecurityContextsIds's argument order with the rest of
the security-context helpers in the chart. Also adds coverage for
component-level securityContexts overrides taking priority over
disableDefaults, both for Deployments and for the pod-template-file
path used by KubernetesExecutor/KubernetesPodOperator.

* Extend disableDefaults component-override test coverage to all chart components

The disableDefaults escape hatch touches every component's security
context, but the regression test only exercised scheduler and
workers.celery. Parametrize it across all 13 affected components so a
future template change can't silently break the override behavior for
components that were previously untested, and add databaseCleanup
coverage that no other test in this file provided either.

* Tidy disableDefaults wording in values.yaml and values.schema.json

The parenthetical note doesn't need a trailing period, and the
values.yaml comment was missing backticks around the referenced keys
that every other comment in the file uses.

* Address review feedback on disableDefaults test coverage

Split the mixed CronJob/Deployment assertions in
test_disable_defaults_pod_and_container into separate tests instead of
branching on list position, replace per-doc Python loops and
init-container name comprehensions with jmespath expressions to match
the rest of the file, and drop docstrings that only restated what the
test name already says.

* Update chart/values.schema.json

Co-authored-by: Przemysław Mirowski <17602603+Miretpl@users.noreply.github.com>

* Update chart/values.yaml

Co-authored-by: Przemysław Mirowski <17602603+Miretpl@users.noreply.github.com>

* Fixing PR comments

---------
(cherry picked from commit 3ac3851)

Co-authored-by: Aaryan Mahajan <aaryanmhjn@gmail.com>
Co-authored-by: Przemysław Mirowski <17602603+Miretpl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant