airflow icon indicating copy to clipboard operation
airflow copied to clipboard

fix(chart): dag processor and api server scc validation

Open brancof opened this issue 7 months ago • 4 comments

While testing Airflow v3 on OpenShift 4.18 using Helm Chart 1.17.0, I encountered some issues.

Since Airflow 3.0.0, both the API Server and DAG Processor are enabled by default. Although this is expected behavior, the deployment fails due to OpenShift’s Security Context Constraints (SCC) restrictions.

To resolve this, the SCC role bindings need to be expanded to properly support all deployment scenarios from Airflow v2 to v3, covering both the API Server and DAG Processor components.

brancof avatar Jun 16 '25 22:06 brancof

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 (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst) Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits 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: [email protected] Slack: https://s.apache.org/airflow-slack

boring-cyborg[bot] avatar Jun 16 '25 22:06 boring-cyborg[bot]

Wasn't the dag processor part resolved in https://github.com/apache/airflow/pull/51080 ?

eladkal avatar Jun 16 '25 22:06 eladkal

Wasn't the dag processor part resolved in #51080 ?

Kinda @eladkal, it depended on whether the .Values.dagProcessor.enabled was set or not. By default, the .Values.dagProcessor.enabled is ~ or nil, so in the block - ref, it will never be templated as expected:

  {{- if .Values.dagProcessor.enabled }}
  - kind: ServiceAccount
    name: {{ include "dagProcessor.serviceAccountName" . }}
    namespace: "{{ .Release.Namespace }}"
  {{- end }}

To solve this we need to apply the same logic that’s used in the deployment and service account conditions for the DAG Processor - ref. I may have gone too far in trying to make it as DRY as possible, but this might prevent problems like this one in the future.


To test this, just set rbac.createSCCRoleBinding: true, and use all the default values.

brancof avatar Jun 16 '25 23:06 brancof

@brancof can you take a look at the failing tests?

eladkal avatar Jun 17 '25 06:06 eladkal

@eladkal I believe the /helm-tests/tests/helm_tests/security/test_scc_rolebinding.py was failing (test_create_scc and test_create_scc_worker_only) because there is no entry for the api-server resource.

Ref: https://github.com/apache/airflow/actions/runs/15693620873/job/44214695300

Since the changes add the api-server service account reference to the list of subjects (in the ClusterRoleBinding), and since by default in v3 the API server is enabled, the tests are failing.

Possible solution:

  • Add an "or" logic, i.e. it is release-name-airflow-webserver or release-name-airflow-api-server

I just don't know if more tests are required because of this (api-server enabled, or webserver enabled).

brancof avatar Jun 25 '25 23:06 brancof

All tests should pass now (at least locally they did). Can the PR be reviewed? @eladkal @dstandish @jedcunningham @hussein-awala

brancof avatar Jul 09 '25 22:07 brancof

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Sep 26 '25 00:09 github-actions[bot]