You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Miretpl (via GitHub)" <gi...@apache.org> on 2026/01/11 19:04:20 UTC

[PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts sections [airflow]

Miretpl opened a new pull request, #60396:
URL: https://github.com/apache/airflow/pull/60396

   <!--
   Thank you for contributing!
   
   Please provide above a brief description of the changes made in this pull request.
   Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
   
   Please make sure that your code changes are covered with tests.
   And in case of new features or big changes remember to adjust the documentation.
   
   Feel free to ping (in general) for the review if you do not see reaction for a few days
   (72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
   
   In case of an existing issue, reference it using one of the following:
   
   * closes: #ISSUE
   * related: #ISSUE
   -->
   related: https://github.com/apache/airflow/issues/28880
   
   This PR introduces two new fields: `workers.celery.securityContexts` and `workers.kubernetes.securityContexts`. Field `workers.securityContexts` is not deprecated as it was used by two worker types. Now it will be possible to use dedicated commands for both Celery and Kubernetes workers.
   
   Despite adding new sections, the following things were done:
   1. Update and separate tests in `security/test_security_context.py`
   2. Add missing overwrite tests for securityContext/securityContexts in `security/test_security_context.py` and `airflow_aux/test_pod_template_file.py`
   3. Rewrite the `airflowPodSecurityContext` and `containerSecurityContext` functions in `_helpers.yaml` to adjust them to reading nested `securityContexts` sections
   4. Remove unneeded if for `workers.celery.persistence.securityContexts` section
   
   NOT FINISHED, but it should be today
   
   ## Was generative AI tooling used to co-author this PR?
   
   <!--
   If generative AI tooling has been used in the process of authoring this PR, please
   change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
   -->
   
   - [ ] Yes (please specify the tool below)
   
   <!--
   Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   -->
   
   ---
   
   * Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
   * For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735691584

   Ahhh, I forgot to add it, sorry. I will add it in a couple of minutes
   
   Also, maybe at this time the network error in tests will resolve itself :D


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3739902726

   I will take a look at it in an hour or two


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735589339

   Adding a little more context to additional changes:
   1. Points 1 and 2 are done, because of point 3 - to make sure that the changes made to functions are not breaking the logic before
   2. Point 3 for simplification of future logic related to all securityContexts sections, which we will have in other, not yet done, sections in `workers.celery`/`workers.kubernetes`
   
   \+ I believe that this PR might be easier to review commit by commit 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735964988

   FYI: The S3 error is unrelated to your changes. I opened an issue to track this fix. Hopefully, that solves it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735672577

   Yes.
   
   The idea is, if some `workers` section is used in both places (`worker-deployment` and `pod-template-file`), we want to give an easy option to overwrite the values for one, but if someone wants to specify the same config for both, I think it should be possible (maybe with an exception for `resources` as having the same `resources` for celery workers and kubernetes executor pods would be a bad idea).
   
   Regarding the `newsfragment`, until now I only added them when the change was changing the behaviour of the chart, so e.g. in cases like:
   ```
   {{ workers.celery.example | default workers.example }}
   ```
   where `example` would be e.g. `1` by default, but if the default was unset or `null`, I didn't add the newsfragment (only the change in NOTES if the value was deprecated).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #60396:
URL: https://github.com/apache/airflow/pull/60396


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735627625

   This time you did not mark the (non celery/k8s) workers parameters as deprecated - is this by intend? Because now both can be specified? Also no newsfragment about this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3736963726

   Now a merge conflict from the other PR :-(


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3740292506

   Should be good now


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735786975

   I added the `Kubernetes` docSection to the newly added securityContexts. They should be rather there and not under the `Workers` section


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735681608

   > Yes.
   > 
   > The idea is, if some `workers` section is used in both places (`worker-deployment` and `pod-template-file`), we want to give an easy option to overwrite the values for one, but if someone wants to specify the same config for both, I think it should be possible (maybe with an exception for `resources` as having the same `resources` for celery workers and kubernetes executor pods would be a bad idea).
   
   Okay, this makes sense. Then also no paramter deprecated, also no newsfragment needed. That is fine.
   
   But can you please add a note to the new/old parameters describing the behavior of "one or the other" and the order of selection to both parameters please?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add workers.celery.securityContexts & workers.kubernetes.securityContexts [airflow]

Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60396:
URL: https://github.com/apache/airflow/pull/60396#issuecomment-3735749189

   Clarifying comments added - let me know if it's good now


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org