You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "henry3260 (via GitHub)" <gi...@apache.org> on 2026/01/17 05:51:02 UTC
[PR] Keep consistency with the previous PR [airflow]
henry3260 opened a new pull request, #60677:
URL: https://github.com/apache/airflow/pull/60677
Keep consistency with the previous PR like https://github.com/apache/airflow/pull/60238
##### 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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3765182464
Hi @henry3260, can I finish this section? I need it for #60420, which I would like to finish and properly tested today
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on code in PR #60677:
URL: https://github.com/apache/airflow/pull/60677#discussion_r2701342019
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -191,11 +193,13 @@ spec:
subPath: {{ .Values.logs.persistence.subPath }}
{{- end }}
{{- end }}
- {{- $kerberosInitContainerEnabled := or (.Values.workers.celery.kerberosInitContainer).enabled (.Values.workers.kerberosInitContainer).enabled }}
+ {{- $kerberosInitContainerEnabled := or (.Values.workers.celery.kerberosInitContainer).enabled (and (not (has (.Values.workers.celery.kerberosInitContainer).enabled (list true false))) (.Values.workers.kerberosInitContainer).enabled) }}
Review Comment:
```suggestion
{{- $kerberosInitContainerEnabled := or .Values.workers.celery.kerberosInitContainer.enabled (and (not (has .Values.workers.celery.kerberosInitContainer.enabled (list true false))) .Values.workers.kerberosInitContainer.enabled) }}
```
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -191,11 +193,13 @@ spec:
subPath: {{ .Values.logs.persistence.subPath }}
{{- end }}
{{- end }}
- {{- $kerberosInitContainerEnabled := or (.Values.workers.celery.kerberosInitContainer).enabled (.Values.workers.kerberosInitContainer).enabled }}
+ {{- $kerberosInitContainerEnabled := or (.Values.workers.celery.kerberosInitContainer).enabled (and (not (has (.Values.workers.celery.kerberosInitContainer).enabled (list true false))) (.Values.workers.kerberosInitContainer).enabled) }}
{{- $kerberosInitContainerResources := (.Values.workers.celery.kerberosInitContainer).resources | default (.Values.workers.kerberosInitContainer).resources | default dict }}
{{- if and (semverCompare ">=2.8.0" .Values.airflowVersion) $kerberosInitContainerEnabled }}
- name: kerberos-init
image: {{ template "airflow_image" . }}
+ securityContext: {{- toYaml .Values.workers.kerberosInitContainer.securityContexts.container | nindent 12 }}
+ lifecycle: {{- toYaml .Values.workers.kerberosInitContainer.containerLifecycleHooks | nindent 12 }}
Review Comment:
We will need to add additional test cases for these options
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3764370484
> Okay! So we should update the tests by explicitly setting workers.celery.kerberosInitContainer to nil if we want to verify workers.kerberosInitContainer, assuming I understand correctly.
Depends where. If you are testing the `enabled` flag behaviour - yes (not always, of course :D), if it is required to test something else, then no
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3764195113
> As I was here, I've made a quick review of the current state to not be late like last time :D
Many thanks for your kind guidance!!!
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3764223350
> @henry3260, the change in chart behaviour is by intent (there was a discussion in #59730), so currently, we want to make sure that the behaviour will be the same for every `workers.celery/kubernetes` work and will be properly communicated.
>
> Behaviour same as in already moved configs means:
>
> 1. If there is a default value specified, the user will need to manually unset the proper field in `celery/kubernetes` sections (for this, we also want to have a dedicated newsfragment file to make sure that this will be more visible to users in release notes),
> 2. If there is no default value specified, the behaviour remains unchanged.
>
> For simple examples for the above points, you can take a look at (also in terms of what is changed, how and where):
>
> 1. [Add workers.celery.replicas field #59730](https://github.com/apache/airflow/pull/59730)
> 2. [Add workers.celery.revisionHistoryLimit field #60056](https://github.com/apache/airflow/pull/60056)
>
> If you have further questions, feel free to ask!
Okay! So we should update the tests by explicitly setting workers.celery.kerberosInitContainer to nil if we want to verify workers.kerberosInitContainer, assuming I understand correctly.
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3930514192
No worries. Just letting you know that we can get back to this after 1.19.0 was released, so take your time
--
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 missing support for: securityContexts and containerLifecycleHooks [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3941460273
> No worries. Just letting you know that we can get back to this after 1.19.0 was released, so take your time
Hi @Miretpl , feel free to share your thoughts when you have time :)
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3763656932
@Miretpl To ensure the fallback logic works properly with the standard has check, I believe we should remove the default enabled: false in values.yaml (leaving it unset/nil). Otherwise, the default value overrides the old configuration in tests. WDYT?"
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3929071784
> Hi @henry3260, getting back to this, we would need to implement the missing support, which I've mentioned somewhere above + test cases for them. If you have any questions, feel free to ask!
Sorry for late update! I will complete this pr soon.
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3765429613
> Hi @henry3260, can I finish this section? I need it for #60420, which I would like to finish and properly tested today
Do you need my help to add additional test cases for securityContext and lifecycle?
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3910869015
Hi @henry3260, getting back to this, we would need to implement the missing support, which I've mentioned somewhere above + test cases for them. If you have any questions, feel free to ask!
--
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 missing support for: securityContexts and containerLifecycleHooks [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on code in PR #60677:
URL: https://github.com/apache/airflow/pull/60677#discussion_r2838336511
##########
chart/files/pod-template-file.kubernetes-helm-yaml:
##########
@@ -24,6 +24,8 @@
{{- $securityContext := include "airflowPodSecurityContext" (list .Values.workers.kubernetes .Values.workers .Values) }}
{{- $containerSecurityContextKerberosSidecar := include "containerSecurityContext" (list .Values.workers.kerberosSidecar .Values) }}
{{- $containerLifecycleHooksKerberosSidecar := or .Values.workers.kerberosSidecar.containerLifecycleHooks .Values.containerLifecycleHooks }}
+{{- $containerSecurityContextKerberosInitContainer := .Values.workers.kubernetes.kerberosInitContainer.securityContexts.container | default .Values.workers.kerberosInitContainer.securityContexts.container }}
+{{- $containerLifecycleHooksKerberosInitContainer := .Values.workers.kubernetes.kerberosInitContainer.containerLifecycleHooks | default .Values.workers.kerberosInitContainer.containerLifecycleHooks }}
Review Comment:
```suggestion
{{- $containerLifecycleHooksKerberosInitContainer := or .Values.workers.kubernetes.kerberosInitContainer.containerLifecycleHooks .Values.workers.kerberosInitContainer.containerLifecycleHooks .Values.containerLifecycleHooks }}
```
for fallback logic consistency with the rest of the chart + adding `.Values.containerLifecycleHooks` for same fallback as for `worker-kerberos` container
##########
helm-tests/tests/helm_tests/airflow_core/test_worker.py:
##########
@@ -1050,6 +1050,120 @@ def test_kerberos_init_container_resources(self, workers_values):
},
}
+ @pytest.mark.parametrize(
+ ("workers_values", "expected"),
+ [
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ }
+ },
+ {"runAsUser": 1000},
+ ),
+ (
+ {
+ "celery": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ }
+ },
+ {"runAsUser": 2000},
+ ),
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ },
+ "celery": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ },
+ },
+ {"runAsUser": 2000},
+ ),
+ ],
+ )
+ def test_kerberos_init_container_security_context(self, workers_values, expected):
Review Comment:
It is not intuitive, but all securityContext-related test cases are in `security/test_security_context.py` file (despite pod-template tests). Could we move this test case there?
##########
chart/files/pod-template-file.kubernetes-helm-yaml:
##########
@@ -24,6 +24,8 @@
{{- $securityContext := include "airflowPodSecurityContext" (list .Values.workers.kubernetes .Values.workers .Values) }}
{{- $containerSecurityContextKerberosSidecar := include "containerSecurityContext" (list .Values.workers.kerberosSidecar .Values) }}
{{- $containerLifecycleHooksKerberosSidecar := or .Values.workers.kerberosSidecar.containerLifecycleHooks .Values.containerLifecycleHooks }}
+{{- $containerSecurityContextKerberosInitContainer := .Values.workers.kubernetes.kerberosInitContainer.securityContexts.container | default .Values.workers.kerberosInitContainer.securityContexts.container }}
Review Comment:
```suggestion
{{- $containerSecurityContextKerberosInitContainer := include "containerSecurityContext" (list .Values.workers.kubernetes.kerberosInitContainer .Values.workers.kerberosInitContainer .Values) }}
```
In general, we use functions for securityContext evaluation, so change for keeping chart consistency + `.Values` as the end for having same fallback as for `worker-kerberos` container, which is the same as this init container
##########
chart/templates/workers/worker-deployment.yaml:
##########
@@ -48,6 +48,8 @@
{{- $containerSecurityContextWaitForMigrations := include "containerSecurityContext" (list .Values.workers.waitForMigrations .Values) }}
{{- $containerSecurityContextLogGroomerSidecar := include "containerSecurityContext" (list .Values.workers.logGroomerSidecar .Values) }}
{{- $containerSecurityContextKerberosSidecar := include "containerSecurityContext" (list .Values.workers.kerberosSidecar .Values) }}
+{{- $containerSecurityContextKerberosInitContainer := include "containerSecurityContext" (list .Values.workers.kerberosInitContainer .Values) }}
+{{- $containerLifecycleHooksKerberosInitContainer := .Values.workers.kerberosInitContainer.containerLifecycleHooks }}
Review Comment:
```suggestion
{{- $containerLifecycleHooksKerberosInitContainer := .Values.workers.kerberosInitContainer.containerLifecycleHooks .Values.containerLifecycleHooks }}
```
for the same fallback logic as it for `worker-kerberos` container
##########
helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py:
##########
@@ -1384,3 +1384,115 @@ def test_kerberos_init_container_resources(self, workers_values):
"memory": "4Mi",
},
}
+
+ @pytest.mark.parametrize(
+ ("workers_values", "expected"),
+ [
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ }
+ },
+ {"runAsUser": 1000},
+ ),
+ (
+ {
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ }
+ },
+ {"runAsUser": 2000},
+ ),
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ },
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ },
+ },
+ {"runAsUser": 2000},
+ ),
+ ],
+ )
+ def test_kerberos_init_container_security_context(self, workers_values, expected):
Review Comment:
We could set `expected` to always be the same value, which would simplify our test case
##########
helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py:
##########
@@ -1384,3 +1384,115 @@ def test_kerberos_init_container_resources(self, workers_values):
"memory": "4Mi",
},
}
+
+ @pytest.mark.parametrize(
+ ("workers_values", "expected"),
+ [
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ }
+ },
+ {"runAsUser": 1000},
+ ),
+ (
+ {
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ }
+ },
+ {"runAsUser": 2000},
+ ),
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ },
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ },
+ },
+ {"runAsUser": 2000},
+ ),
+ ],
+ )
+ def test_kerberos_init_container_security_context(self, workers_values, expected):
+ docs = render_chart(
+ values={
+ "workers": workers_values,
+ },
+ show_only=["templates/pod-template-file.yaml"],
+ chart_dir=self.temp_chart_dir,
+ )
+
+ assert (
+ jmespath.search("spec.initContainers[?name=='kerberos-init'] | [0].securityContext", docs[0])
+ == expected
+ )
+
+ @pytest.mark.parametrize(
+ ("workers_values", "expected"),
+ [
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "containerLifecycleHooks": {"postStart": {"exec": {"command": ["echo", "base"]}}},
+ }
+ },
+ {"postStart": {"exec": {"command": ["echo", "base"]}}},
+ ),
+ (
+ {
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "containerLifecycleHooks": {
+ "postStart": {"exec": {"command": ["echo", "kubernetes"]}}
+ },
+ }
+ }
+ },
+ {"postStart": {"exec": {"command": ["echo", "kubernetes"]}}},
+ ),
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "containerLifecycleHooks": {"postStart": {"exec": {"command": ["echo", "base"]}}},
Review Comment:
```suggestion
"containerLifecycleHooks": {"preStop": {"exec": {"command": ["echo", "base"]}}},
```
`preStop` because we want to be sure that the overwrite and fallback logic works fully correct (there will be no `preStop` and `postStart` defined at the same time looking at the new test conf)
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3765496543
In my PR, I will leave `securityContexts` and `lifecycle` as they are (as I believe that not using those fields anywhere is a different issue that current implementation in terms of separation and worker sets). I noticed that the `kerberosInitContainer` is also in `pod-template-file`, which changes the logic a little, and I'm not sure now in which direction we should go with it. If you want to take a look - it is here #60751
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3764187600
@henry3260, the change in chart behaviour is by intent (there was a discussion in #59730), so currently, we want to make sure that the behaviour will be the same for every `workers.celery/kubernetes` work and will be properly communicated.
Behaviour same as in already moved configs means:
1. If there is a default value specified, the user will need to manually unset the proper field in `celery/kubernetes` sections (for this, we also want to have a dedicated newsfragment file to make sure that this will be more visible to users in release notes),
2. If there is no default value specified, the behaviour remains unchanged.
For simple examples for the above points, you can take a look at (also in terms of what is changed, how and where):
1. #59730
2. #60056
If you have further questions, feel free to ask!
--
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 missing support for: securityContexts and containerLifecycleHooks [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl merged PR #60677:
URL: https://github.com/apache/airflow/pull/60677
--
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 missing support for: securityContexts and containerLifecycleHooks [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on code in PR #60677:
URL: https://github.com/apache/airflow/pull/60677#discussion_r2844892807
##########
helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py:
##########
@@ -1384,3 +1384,115 @@ def test_kerberos_init_container_resources(self, workers_values):
"memory": "4Mi",
},
}
+
+ @pytest.mark.parametrize(
+ ("workers_values", "expected"),
+ [
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ }
+ },
+ {"runAsUser": 1000},
+ ),
+ (
+ {
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ }
+ },
+ {"runAsUser": 2000},
+ ),
+ (
+ {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 1000}},
+ },
+ "kubernetes": {
+ "kerberosInitContainer": {
+ "enabled": True,
+ "securityContexts": {"container": {"runAsUser": 2000}},
+ }
+ },
+ },
+ {"runAsUser": 2000},
+ ),
+ ],
+ )
+ def test_kerberos_init_container_security_context(self, workers_values, expected):
Review Comment:
> Contributor
Good catch! applied
--
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 missing support for: securityContexts and containerLifecycleHooks [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3956337893
@Miretpl @jscheffl Thanks!
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3765184278
> Hi @henry3260, can I finish this section? I need it for #60420, which I would like to finish and properly tested today
Feel free
--
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] Keep consistency with the previous PR [airflow]
Posted by "henry3260 (via GitHub)" <gi...@apache.org>.
henry3260 commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3765559931
> I think that we came to conclustion in the mentioned PR, so for this one, you would need only add missing support for:
>
> 1. `securityContexts`
> 2. `containerLifecycleHooks`
>
> under `kerberosInitContainer` section of `workers`, `workers.kubernetes` and `workers.celery`.
>
> + of course test cases for each. Will that be good?
looks good!
--
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] Keep consistency with the previous PR [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #60677:
URL: https://github.com/apache/airflow/pull/60677#issuecomment-3765549223
I think that we came to conclustion in the mentioned PR, so for this one, you would need only add missing support for:
1. `securityContexts`
2. `containerLifecycleHooks`
under `kerberosInitContainer` section of `workers`, `workers.kubernetes` and `workers.celery`.
\+ of course test cases for each. Will that be good?
--
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