From 9c6877259de76fa20896d6307c9b2c581e8cc5de Mon Sep 17 00:00:00 2001 From: Miretpl Date: Sat, 17 Jan 2026 18:41:57 +0100 Subject: [PATCH 1/3] Revert "Separate workers service accounts (#52357)" This reverts commit 23d955d0e6be184417d0a0f2921357bc43f6d230. --- .../pod-template-file.kubernetes-helm-yaml | 4 - chart/templates/_helpers.yaml | 30 +-- .../rbac/pod-launcher-rolebinding.yaml | 10 - ...curity-context-constraint-rolebinding.yaml | 9 - .../workers/worker-celery-serviceaccount.yaml | 41 ---- .../templates/workers/worker-deployment.yaml | 4 - .../worker-kubernetes-serviceaccount.yaml | 41 ---- .../workers/worker-serviceaccount.yaml | 2 +- chart/values.schema.json | 69 ------- chart/values.yaml | 30 --- .../airflow_aux/test_annotations.py | 84 -------- .../airflow_aux/test_pod_template_file.py | 17 -- .../helm_tests/airflow_core/test_worker.py | 180 ++---------------- .../tests/helm_tests/security/test_rbac.py | 111 +---------- 14 files changed, 35 insertions(+), 597 deletions(-) delete mode 100644 chart/templates/workers/worker-celery-serviceaccount.yaml delete mode 100644 chart/templates/workers/worker-kubernetes-serviceaccount.yaml diff --git a/chart/files/pod-template-file.kubernetes-helm-yaml b/chart/files/pod-template-file.kubernetes-helm-yaml index 1f18ce9f47eb8..59a08f73003ac 100644 --- a/chart/files/pod-template-file.kubernetes-helm-yaml +++ b/chart/files/pod-template-file.kubernetes-helm-yaml @@ -230,11 +230,7 @@ spec: terminationGracePeriodSeconds: {{ .Values.workers.terminationGracePeriodSeconds }} tolerations: {{- toYaml $tolerations | nindent 4 }} topologySpreadConstraints: {{- toYaml $topologySpreadConstraints | nindent 4 }} - {{- if .Values.workers.useWorkerDedicatedServiceAccounts }} - serviceAccountName: {{ include "worker.kubernetes.serviceAccountName" . }} - {{- else }} serviceAccountName: {{ include "worker.serviceAccountName" . }} - {{- end }} volumes: {{- if .Values.dags.persistence.enabled }} - name: dags diff --git a/chart/templates/_helpers.yaml b/chart/templates/_helpers.yaml index 154e697b443ff..c791d0101e4f2 100644 --- a/chart/templates/_helpers.yaml +++ b/chart/templates/_helpers.yaml @@ -658,23 +658,13 @@ server_tls_key_file = /etc/pgbouncer/server.key {{- end }} {{- end }} -{{/* Helper for service account name generation */}} -{{- define "_serviceAccountNameGen" -}} - {{- if .sa.create }} - {{- default (printf "%s-%s" (include "airflow.serviceAccountName" .) (default .key .nameSuffix )) .sa.name | quote }} - {{- else }} - {{- default "default" .sa.name | quote }} - {{- end }} -{{- end }} - -{{/* Helper to generate service account name respecting .Values.$section.serviceAccount or .Values.$section.$subSection.serviceAccount flags */}} +{{/* Helper to generate service account name respecting .Values.$section.serviceAccount flags */}} {{- define "_serviceAccountName" -}} - {{- if .subKey }} - {{- $sa := get (get (get .Values .key) .subKey) "serviceAccount" -}} - {{- include "_serviceAccountNameGen" (merge (dict "sa" $sa "key" .key "nameSuffix" .nameSuffix) .) }} + {{- $sa := get (get .Values .key) "serviceAccount" }} + {{- if $sa.create }} + {{- default (printf "%s-%s" (include "airflow.serviceAccountName" .) (default .key .nameSuffix )) $sa.name | quote }} {{- else }} - {{- $sa := get (get .Values .key) "serviceAccount" }} - {{- include "_serviceAccountNameGen" (merge (dict "sa" $sa "key" .key "nameSuffix" .nameSuffix) .) }} + {{- default "default" $sa.name | quote }} {{- end }} {{- end }} @@ -724,16 +714,6 @@ server_tls_key_file = /etc/pgbouncer/server.key {{- include "_serviceAccountName" (merge (dict "key" "workers" "nameSuffix" "worker") .) -}} {{- end }} -{{/* Create the name of the worker celery service account to use */}} -{{- define "worker.celery.serviceAccountName" -}} - {{- include "_serviceAccountName" (merge (dict "key" "workers" "subKey" "celery" "nameSuffix" "worker-celery") .) -}} -{{- end }} - -{{/* Create the name of the worker kubernetes service account to use */}} -{{- define "worker.kubernetes.serviceAccountName" -}} - {{- include "_serviceAccountName" (merge (dict "key" "workers" "subKey" "kubernetes" "nameSuffix" "worker-kubernetes") .) -}} -{{- end }} - {{/* Create the name of the triggerer service account to use */}} {{- define "triggerer.serviceAccountName" -}} {{- include "_serviceAccountName" (merge (dict "key" "triggerer") .) -}} diff --git a/chart/templates/rbac/pod-launcher-rolebinding.yaml b/chart/templates/rbac/pod-launcher-rolebinding.yaml index ec84f0361910f..781e4fc082a26 100644 --- a/chart/templates/rbac/pod-launcher-rolebinding.yaml +++ b/chart/templates/rbac/pod-launcher-rolebinding.yaml @@ -68,24 +68,14 @@ subjects: {{- end }} {{- end }} {{- $workerAdded := false }} - {{- $workersDedicatedSA := .Values.workers.useWorkerDedicatedServiceAccounts -}} {{- range $executor := $executors }} {{- if and (has $executor $workerLaunchExecutors) (not $workerAdded) }} {{- $workerAdded = true }} - {{- if $workersDedicatedSA }} - - kind: ServiceAccount - name: {{ include "worker.celery.serviceAccountName" $ }} - namespace: "{{ $.Release.Namespace }}" - - kind: ServiceAccount - name: {{ include "worker.kubernetes.serviceAccountName" $ }} - namespace: "{{ $.Release.Namespace }}" - {{- else }} - kind: ServiceAccount name: {{ include "worker.serviceAccountName" $ }} namespace: "{{ $.Release.Namespace }}" {{- end }} {{- end }} - {{- end }} {{- if .Values.triggerer.enabled }} - kind: ServiceAccount name: {{ include "triggerer.serviceAccountName" . }} diff --git a/chart/templates/rbac/security-context-constraint-rolebinding.yaml b/chart/templates/rbac/security-context-constraint-rolebinding.yaml index 3e44c0c5556ce..185eb3a890e9c 100644 --- a/chart/templates/rbac/security-context-constraint-rolebinding.yaml +++ b/chart/templates/rbac/security-context-constraint-rolebinding.yaml @@ -55,19 +55,10 @@ subjects: name: {{ include "webserver.serviceAccountName" . }} namespace: "{{ .Release.Namespace }}" {{- if $hasWorkers }} - {{- if .Values.workers.useWorkerDedicatedServiceAccounts }} - - kind: ServiceAccount - name: {{ include "worker.celery.serviceAccountName" . }} - namespace: "{{ .Release.Namespace }}" - - kind: ServiceAccount - name: {{ include "worker.kubernetes.serviceAccountName" . }} - namespace: "{{ .Release.Namespace }}" - {{- else }} - kind: ServiceAccount name: {{ include "worker.serviceAccountName" . }} namespace: "{{ .Release.Namespace }}" {{- end }} - {{- end }} - kind: ServiceAccount name: {{ include "scheduler.serviceAccountName" . }} namespace: "{{ .Release.Namespace }}" diff --git a/chart/templates/workers/worker-celery-serviceaccount.yaml b/chart/templates/workers/worker-celery-serviceaccount.yaml deleted file mode 100644 index 087a6e35440f8..0000000000000 --- a/chart/templates/workers/worker-celery-serviceaccount.yaml +++ /dev/null @@ -1,41 +0,0 @@ -{{/* - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, - software distributed under the License is distributed on an - "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - KIND, either express or implied. See the License for the - specific language governing permissions and limitations - under the License. -*/}} - -####################################### -## Airflow Worker Celery ServiceAccount -####################################### -{{- if and .Values.workers.celery.serviceAccount.create .Values.workers.useWorkerDedicatedServiceAccounts (or (contains "CeleryExecutor" .Values.executor) (contains "CeleryKubernetesExecutor" .Values.executor)) }} -apiVersion: v1 -kind: ServiceAccount -automountServiceAccountToken: {{ .Values.workers.celery.serviceAccount.automountServiceAccountToken }} -metadata: - name: {{ include "worker.celery.serviceAccountName" . }} - labels: - tier: airflow - component: worker - release: {{ .Release.Name }} - chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" - heritage: {{ .Release.Service }} - {{- if or .Values.labels .Values.workers.labels }} - {{- mustMerge .Values.workers.labels .Values.labels | toYaml | nindent 4 }} - {{- end }} - {{- with .Values.workers.celery.serviceAccount.annotations }} - annotations: {{- toYaml . | nindent 4 }} - {{- end }} -{{- end }} diff --git a/chart/templates/workers/worker-deployment.yaml b/chart/templates/workers/worker-deployment.yaml index d7480e9efc14c..a349ff24eba4a 100644 --- a/chart/templates/workers/worker-deployment.yaml +++ b/chart/templates/workers/worker-deployment.yaml @@ -165,11 +165,7 @@ spec: {{- end }} terminationGracePeriodSeconds: {{ .Values.workers.terminationGracePeriodSeconds }} restartPolicy: Always - {{- if .Values.workers.useWorkerDedicatedServiceAccounts }} - serviceAccountName: {{ include "worker.celery.serviceAccountName" . }} - {{- else }} serviceAccountName: {{ include "worker.serviceAccountName" . }} - {{- end }} securityContext: {{ $securityContext | nindent 8 }} imagePullSecrets: {{ include "image_pull_secrets" . | nindent 8 }} initContainers: diff --git a/chart/templates/workers/worker-kubernetes-serviceaccount.yaml b/chart/templates/workers/worker-kubernetes-serviceaccount.yaml deleted file mode 100644 index cc924f8263eff..0000000000000 --- a/chart/templates/workers/worker-kubernetes-serviceaccount.yaml +++ /dev/null @@ -1,41 +0,0 @@ -{{/* - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, - software distributed under the License is distributed on an - "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - KIND, either express or implied. See the License for the - specific language governing permissions and limitations - under the License. -*/}} - -########################################### -## Airflow Worker Kubernetes ServiceAccount -########################################### -{{- if and .Values.workers.kubernetes.serviceAccount.create .Values.workers.useWorkerDedicatedServiceAccounts (or (contains "CeleryKubernetesExecutor" .Values.executor) (contains "KubernetesExecutor" .Values.executor) (contains "LocalKubernetesExecutor" .Values.executor)) }} -apiVersion: v1 -kind: ServiceAccount -automountServiceAccountToken: {{ .Values.workers.kubernetes.serviceAccount.automountServiceAccountToken }} -metadata: - name: {{ include "worker.kubernetes.serviceAccountName" . }} - labels: - tier: airflow - component: worker - release: {{ .Release.Name }} - chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" - heritage: {{ .Release.Service }} - {{- if or .Values.labels .Values.workers.labels }} - {{- mustMerge .Values.workers.labels .Values.labels | toYaml | nindent 4 }} - {{- end }} - {{- with .Values.workers.kubernetes.serviceAccount.annotations }} - annotations: {{- toYaml . | nindent 4 }} - {{- end }} -{{- end }} diff --git a/chart/templates/workers/worker-serviceaccount.yaml b/chart/templates/workers/worker-serviceaccount.yaml index dbe4757ac6ced..819845fa2eac4 100644 --- a/chart/templates/workers/worker-serviceaccount.yaml +++ b/chart/templates/workers/worker-serviceaccount.yaml @@ -20,7 +20,7 @@ ################################ ## Airflow Worker ServiceAccount ################################# -{{- if and .Values.workers.serviceAccount.create (not .Values.workers.useWorkerDedicatedServiceAccounts) (include "airflow.podLaunchingExecutor" .) }} +{{- if and .Values.workers.serviceAccount.create (include "airflow.podLaunchingExecutor" .) }} apiVersion: v1 kind: ServiceAccount automountServiceAccountToken: {{ .Values.workers.serviceAccount.automountServiceAccountToken }} diff --git a/chart/values.schema.json b/chart/values.schema.json index 70ddde7a43099..ea2e284503b3c 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2691,11 +2691,6 @@ } ] }, - "useWorkerDedicatedServiceAccounts": { - "description": "One common Service Account for all workers will be created if flag is set to false. If true, dedicated Service Accounts for every worker type will be created.", - "type": "boolean", - "default": false - }, "celery": { "description": "Airflow Celery Workers configuration.", "type": "object", @@ -2861,38 +2856,6 @@ } } }, - "serviceAccount": { - "description": "Create ServiceAccount.", - "type": "object", - "properties": { - "automountServiceAccountToken": { - "description": "Specifies if ServiceAccount's API credentials should be mounted onto Pods.", - "type": "boolean", - "default": true - }, - "create": { - "description": "Specifies whether a ServiceAccount should be created.", - "type": "boolean", - "default": true - }, - "name": { - "description": "The name of the ServiceAccount to use. If not set and create is true, a name is generated using the release name.", - "type": [ - "string", - "null" - ], - "default": null - }, - "annotations": { - "description": "Annotations to add to the Airflow Celery worker Kubernetes ServiceAccount.", - "type": "object", - "default": {}, - "additionalProperties": { - "type": "string" - } - } - } - }, "persistence": { "description": "Persistence configuration for Airflow Celery workers.", "type": "object", @@ -3095,38 +3058,6 @@ ] } } - }, - "serviceAccount": { - "description": "Create ServiceAccount.", - "type": "object", - "properties": { - "automountServiceAccountToken": { - "description": "Specifies if ServiceAccount's API credentials should be mounted onto Pods.", - "type": "boolean", - "default": true - }, - "create": { - "description": "Specifies whether a ServiceAccount should be created.", - "type": "boolean", - "default": true - }, - "name": { - "description": "The name of the ServiceAccount to use. If not set and create is true, a name is generated using the release name.", - "type": [ - "string", - "null" - ], - "default": null - }, - "annotations": { - "description": "Annotations to add to the worker Kubernetes ServiceAccount.", - "type": "object", - "default": {}, - "additionalProperties": { - "type": "string" - } - } - } } } } diff --git a/chart/values.yaml b/chart/values.yaml index 6a3c2f122f2a4..2f17a4aa50171 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -1043,10 +1043,6 @@ workers: # requests: # storage: "20Gi" - # One common Service Account for all workers will be created if flag is set to false. - # If true, dedicated Service Accounts for every worker type will be created. - useWorkerDedicatedServiceAccounts: false - celery: # Number of Airflow Celery workers replicas: 1 @@ -1120,19 +1116,6 @@ workers: pod: {} container: {} - # Create ServiceAccount for Airflow Celery workers - serviceAccount: - # default value is true - # ref: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ - automountServiceAccountToken: true - # Specifies whether a ServiceAccount should be created - create: true - # The name of the ServiceAccount to use. - # If not set and create is true, a name is generated using the release name - name: ~ - # Annotations to add to worker kubernetes service account. - annotations: {} - # Persistence volume configuration for Airflow Celery workers persistence: # Enable persistent volumes @@ -1191,19 +1174,6 @@ workers: pod: {} container: {} - # Create ServiceAccount for pods created with pod-template-file - serviceAccount: - # Auto mount service account token into the pod. Default value is true. - # ref: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ - automountServiceAccountToken: true - # Specifies whether a ServiceAccount should be created - create: true - # The name of the ServiceAccount to use. - # If not set and create is true, a name is generated using the release name. - name: ~ - # Annotations to add to worker kubernetes service account - annotations: {} - # Airflow scheduler settings scheduler: enabled: true diff --git a/helm-tests/tests/helm_tests/airflow_aux/test_annotations.py b/helm-tests/tests/helm_tests/airflow_aux/test_annotations.py index 85907f0e73a98..346f1aa752277 100644 --- a/helm-tests/tests/helm_tests/airflow_aux/test_annotations.py +++ b/helm-tests/tests/helm_tests/airflow_aux/test_annotations.py @@ -118,90 +118,6 @@ class TestServiceAccountAnnotations: "example": "worker", }, ), - ( - { - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "celery": { - "serviceAccount": { - "annotations": { - "example": "worker", - }, - }, - }, - }, - }, - "templates/workers/worker-celery-serviceaccount.yaml", - { - "example": "worker", - }, - ), - ( - { - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "serviceAccount": { - "annotations": { - "example": "missing", - }, - }, - "celery": { - "serviceAccount": { - "annotations": { - "example": "worker", - }, - }, - }, - }, - }, - "templates/workers/worker-celery-serviceaccount.yaml", - { - "example": "worker", - }, - ), - ( - { - "executor": "KubernetesExecutor", - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "kubernetes": { - "serviceAccount": { - "annotations": { - "example": "worker", - }, - }, - }, - }, - }, - "templates/workers/worker-kubernetes-serviceaccount.yaml", - { - "example": "worker", - }, - ), - ( - { - "executor": "KubernetesExecutor", - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "serviceAccount": { - "annotations": { - "example": "missing", - }, - }, - "kubernetes": { - "serviceAccount": { - "annotations": { - "example": "worker", - }, - }, - }, - }, - }, - "templates/workers/worker-kubernetes-serviceaccount.yaml", - { - "example": "worker", - }, - ), ( { "flower": { diff --git a/helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py b/helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py index 7bbcee8dc9f62..a6af8f2138b44 100644 --- a/helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py +++ b/helm-tests/tests/helm_tests/airflow_aux/test_pod_template_file.py @@ -1278,20 +1278,3 @@ def test_base_contains_kerberos_env(self, workers_values): scheduler_env = jmespath.search("spec.containers[0].env[*].name", docs[0]) assert set(["KRB5_CONFIG", "KRB5CCNAME"]).issubset(scheduler_env) - - def test_workers_kubernetes_service_account_custom_names_in_objects(self): - k8s_objects = render_chart( - "test-rbac", - values={ - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "kubernetes": {"serviceAccount": {"name": "TestWorkerKubernetes"}}, - }, - }, - show_only=[ - "templates/pod-template-file.yaml", - ], - chart_dir=self.temp_chart_dir, - ) - - assert jmespath.search("spec.serviceAccountName", k8s_objects[0]) == "TestWorkerKubernetes" diff --git a/helm-tests/tests/helm_tests/airflow_core/test_worker.py b/helm-tests/tests/helm_tests/airflow_core/test_worker.py index 8a7abae7ec4d3..2760cb1cfc4f3 100644 --- a/helm-tests/tests/helm_tests/airflow_core/test_worker.py +++ b/helm-tests/tests/helm_tests/airflow_core/test_worker.py @@ -1940,100 +1940,19 @@ def test_should_add_component_specific_labels(self): assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value" @pytest.mark.parametrize( - "executor", + ("executor", "creates_service_account"), [ - "CeleryKubernetesExecutor", - "CeleryExecutor,KubernetesExecutor", - "KubernetesExecutor", - "LocalKubernetesExecutor", + ("LocalExecutor", False), + ("CeleryExecutor", True), + ("CeleryKubernetesExecutor", True), + ("CeleryExecutor,KubernetesExecutor", True), + ("KubernetesExecutor", True), + ("LocalKubernetesExecutor", True), ], ) - @pytest.mark.parametrize( - ("workers_values", "obj"), - [ - ({"serviceAccount": {"create": True}}, "worker"), - ( - { - "useWorkerDedicatedServiceAccounts": True, - "kubernetes": {"serviceAccount": {"create": True}}, - }, - "worker-kubernetes", - ), - ], - ) - def test_should_create_worker_service_account_for_specific_kubernetes_executors( - self, executor, workers_values, obj - ): - docs = render_chart( - values={ - "executor": executor, - "workers": workers_values, - }, - show_only=[f"templates/workers/{obj}-serviceaccount.yaml"], - ) - - assert len(docs) == 1 - assert jmespath.search("kind", docs[0]) == "ServiceAccount" - - @pytest.mark.parametrize( - "executor", - [ - "CeleryExecutor", - "CeleryKubernetesExecutor", - "CeleryExecutor,KubernetesExecutor", - ], - ) - @pytest.mark.parametrize( - ("workers_values", "obj"), - [ - ({"serviceAccount": {"create": True}}, "worker"), - ( - { - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"create": True}}, - }, - "worker-celery", - ), - ], - ) - def test_should_create_worker_service_account_for_specific_celery_executors( - self, executor, workers_values, obj + def test_should_create_worker_service_account_for_specific_executors( + self, executor, creates_service_account ): - docs = render_chart( - values={ - "executor": executor, - "workers": workers_values, - }, - show_only=[f"templates/workers/{obj}-serviceaccount.yaml"], - ) - - assert len(docs) == 1 - assert jmespath.search("kind", docs[0]) == "ServiceAccount" - - def test_worker_service_account_creation_for_local_executor(self): - docs = render_chart( - values={ - "executor": "LocalExecutor", - "workers": { - "serviceAccount": {"create": True}, - }, - }, - show_only=["templates/workers/worker-serviceaccount.yaml"], - ) - - assert len(docs) == 0 - - @pytest.mark.parametrize( - "executor", - [ - "CeleryExecutor", - "CeleryKubernetesExecutor", - "CeleryExecutor,KubernetesExecutor", - "KubernetesExecutor", - "LocalKubernetesExecutor", - ], - ) - def test_worker_service_account_labels_per_executor(self, executor): docs = render_chart( values={ "executor": executor, @@ -2044,11 +1963,12 @@ def test_worker_service_account_labels_per_executor(self, executor): }, show_only=["templates/workers/worker-serviceaccount.yaml"], ) - - assert len(docs) == 1 - assert jmespath.search("kind", docs[0]) == "ServiceAccount" - assert "test_label" in jmespath.search("metadata.labels", docs[0]) - assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value" + if creates_service_account: + assert jmespath.search("kind", docs[0]) == "ServiceAccount" + assert "test_label" in jmespath.search("metadata.labels", docs[0]) + assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value" + else: + assert docs == [] def test_default_automount_service_account_token(self): docs = render_chart( @@ -2061,76 +1981,14 @@ def test_default_automount_service_account_token(self): ) assert jmespath.search("automountServiceAccountToken", docs[0]) is True - @pytest.mark.parametrize( - ("workers_values", "obj"), - [ - ( - {"useWorkerDedicatedServiceAccounts": True, "celery": {"serviceAccount": {"create": True}}}, - "worker-celery", - ), - ( - { - "useWorkerDedicatedServiceAccounts": True, - "kubernetes": {"serviceAccount": {"create": True}}, - }, - "worker-kubernetes", - ), - ], - ) - def test_dedicated_service_account_token_automount(self, workers_values, obj): + def test_overridden_automount_service_account_token(self): docs = render_chart( values={ - "executor": "CeleryExecutor,KubernetesExecutor", - "workers": workers_values, - }, - show_only=[f"templates/workers/{obj}-serviceaccount.yaml"], - ) - assert len(docs) == 1 - assert jmespath.search("automountServiceAccountToken", docs[0]) is True - - @pytest.mark.parametrize( - ("workers_values", "obj"), - [ - ({"serviceAccount": {"create": True, "automountServiceAccountToken": False}}, "worker"), - ( - { - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"create": True, "automountServiceAccountToken": False}}, - }, - "worker-celery", - ), - ( - { - "serviceAccount": {"create": True, "automountServiceAccountToken": True}, - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"create": True, "automountServiceAccountToken": False}}, - }, - "worker-celery", - ), - ( - { - "useWorkerDedicatedServiceAccounts": True, - "kubernetes": {"serviceAccount": {"create": True, "automountServiceAccountToken": False}}, - }, - "worker-kubernetes", - ), - ( - { - "serviceAccount": {"create": True, "automountServiceAccountToken": True}, - "useWorkerDedicatedServiceAccounts": True, - "kubernetes": {"serviceAccount": {"create": True, "automountServiceAccountToken": False}}, + "workers": { + "serviceAccount": {"create": True, "automountServiceAccountToken": False}, }, - "worker-kubernetes", - ), - ], - ) - def test_overridden_automount_service_account_token(self, workers_values, obj): - docs = render_chart( - values={ - "executor": "CeleryExecutor,KubernetesExecutor", - "workers": workers_values, }, - show_only=[f"templates/workers/{obj}-serviceaccount.yaml"], + show_only=["templates/workers/worker-serviceaccount.yaml"], ) assert jmespath.search("automountServiceAccountToken", docs[0]) is False diff --git a/helm-tests/tests/helm_tests/security/test_rbac.py b/helm-tests/tests/helm_tests/security/test_rbac.py index 4d23f09f7c278..ec299eba176af 100644 --- a/helm-tests/tests/helm_tests/security/test_rbac.py +++ b/helm-tests/tests/helm_tests/security/test_rbac.py @@ -67,6 +67,7 @@ SERVICE_ACCOUNT_NAME_TUPLES = [ ("ServiceAccount", "test-rbac-cleanup"), ("ServiceAccount", "test-rbac-scheduler"), + ("ServiceAccount", "test-rbac-worker"), ("ServiceAccount", "test-rbac-triggerer"), ("ServiceAccount", "test-rbac-pgbouncer"), ("ServiceAccount", "test-rbac-database-cleanup"), @@ -94,8 +95,6 @@ (CUSTOM_POSTGRESQL_NAME := "TestPostgresql"), ) CUSTOM_WEBSERVER_NAME = "TestWebserver" -CUSTOM_WORKER_CELERY_NAME = "TestWorkerCelery" -CUSTOM_WORKER_KUBERNETES_NAME = "TestWorkerKubernetes" parametrize_version = pytest.mark.parametrize("version", ["2.3.2", "2.4.0", "3.0.0", "default"]) @@ -111,7 +110,7 @@ def _get_values_with_version(self, values, version): def _is_airflow_3_or_above(self, version): return version == "default" or (parse_version(version) >= parse_version("3.0.0")) - def _get_object_tuples(self, version, sa: bool = True, dedicated_workers_sa: None | bool = None): + def _get_object_tuples(self, version, sa: bool = True): tuples = copy(DEPLOYMENT_NO_RBAC_NO_SA_KIND_NAME_TUPLES) if version in {"default", "3.0.0"}: tuples.append(("Service", "test-rbac-triggerer")) @@ -144,34 +143,15 @@ def _get_object_tuples(self, version, sa: bool = True, dedicated_workers_sa: Non if sa: tuples.append(("ServiceAccount", "test-rbac-webserver")) - if dedicated_workers_sa is not None: - if dedicated_workers_sa: - tuples.append(("ServiceAccount", "test-rbac-worker-celery")) - tuples.append(("ServiceAccount", "test-rbac-worker-kubernetes")) - else: - tuples.append(("ServiceAccount", "test-rbac-worker")) - return tuples @parametrize_version - @pytest.mark.parametrize( - "workers_values", - [ - {"serviceAccount": {"create": False}}, - { - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"create": False}}, - "kubernetes": {"serviceAccount": {"create": False}}, - }, - ], - ) - def test_deployments_no_rbac_no_sa(self, version, workers_values): + def test_deployments_no_rbac_no_sa(self, version): k8s_objects = render_chart( "test-rbac", values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", - "executor": "CeleryExecutor,KubernetesExecutor", "rbac": {"create": False}, "cleanup": { "enabled": True, @@ -196,7 +176,7 @@ def test_deployments_no_rbac_no_sa(self, version, workers_values): "dagProcessor": {"serviceAccount": {"create": False}}, "webserver": {"serviceAccount": {"create": False}}, "apiServer": {"serviceAccount": {"create": False}}, - "workers": workers_values, + "workers": {"serviceAccount": {"create": False}}, "triggerer": {"serviceAccount": {"create": False}}, "statsd": {"serviceAccount": {"create": False}}, "createUserJob": {"serviceAccount": {"create": False}}, @@ -212,20 +192,17 @@ def test_deployments_no_rbac_no_sa(self, version, workers_values): assert sorted(list_of_kind_names_tuples) == sorted(self._get_object_tuples(version, sa=False)) @parametrize_version - @pytest.mark.parametrize("dedicated_workers_sa", [False, True]) - def test_deployments_no_rbac_with_sa(self, version, dedicated_workers_sa): + def test_deployments_no_rbac_with_sa(self, version): k8s_objects = render_chart( "test-rbac", values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", - "executor": "CeleryExecutor,KubernetesExecutor", "rbac": {"create": False}, "cleanup": {"enabled": True}, "databaseCleanup": {"enabled": True}, "flower": {"enabled": True}, "pgbouncer": {"enabled": True}, - "workers": {"useWorkerDedicatedServiceAccounts": dedicated_workers_sa}, }, version=version, ), @@ -233,31 +210,16 @@ def test_deployments_no_rbac_with_sa(self, version, dedicated_workers_sa): list_of_kind_names_tuples = [ (k8s_object["kind"], k8s_object["metadata"]["name"]) for k8s_object in k8s_objects ] - real_list_of_kind_names = ( - self._get_object_tuples(version, dedicated_workers_sa=dedicated_workers_sa) - + SERVICE_ACCOUNT_NAME_TUPLES - ) + real_list_of_kind_names = self._get_object_tuples(version) + SERVICE_ACCOUNT_NAME_TUPLES assert sorted(list_of_kind_names_tuples) == sorted(real_list_of_kind_names) @parametrize_version - @pytest.mark.parametrize( - "workers_values", - [ - {"serviceAccount": {"create": False}}, - { - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"create": False}}, - "kubernetes": {"serviceAccount": {"create": False}}, - }, - ], - ) - def test_deployments_with_rbac_no_sa(self, version, workers_values): + def test_deployments_with_rbac_no_sa(self, version): k8s_objects = render_chart( "test-rbac", values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", - "executor": "CeleryExecutor,KubernetesExecutor", "cleanup": { "enabled": True, "serviceAccount": { @@ -274,7 +236,7 @@ def test_deployments_with_rbac_no_sa(self, version, workers_values): "dagProcessor": {"serviceAccount": {"create": False}}, "webserver": {"serviceAccount": {"create": False}}, "apiServer": {"serviceAccount": {"create": False}}, - "workers": workers_values, + "workers": {"serviceAccount": {"create": False}}, "triggerer": {"serviceAccount": {"create": False}}, "flower": {"enabled": True, "serviceAccount": {"create": False}}, "statsd": {"serviceAccount": {"create": False}}, @@ -298,19 +260,16 @@ def test_deployments_with_rbac_no_sa(self, version, workers_values): assert sorted(list_of_kind_names_tuples) == sorted(real_list_of_kind_names) @parametrize_version - @pytest.mark.parametrize("dedicated_workers_sa", [False, True]) - def test_deployments_with_rbac_with_sa(self, version, dedicated_workers_sa): + def test_deployments_with_rbac_with_sa(self, version): k8s_objects = render_chart( "test-rbac", values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", - "executor": "CeleryExecutor,KubernetesExecutor", "cleanup": {"enabled": True}, "databaseCleanup": {"enabled": True}, "flower": {"enabled": True}, "pgbouncer": {"enabled": True}, - "workers": {"useWorkerDedicatedServiceAccounts": dedicated_workers_sa}, }, version=version, ), @@ -319,9 +278,7 @@ def test_deployments_with_rbac_with_sa(self, version, dedicated_workers_sa): (k8s_object["kind"], k8s_object["metadata"]["name"]) for k8s_object in k8s_objects ] real_list_of_kind_names = ( - self._get_object_tuples(version, dedicated_workers_sa=dedicated_workers_sa) - + SERVICE_ACCOUNT_NAME_TUPLES - + RBAC_ENABLED_KIND_NAME_TUPLES + self._get_object_tuples(version) + SERVICE_ACCOUNT_NAME_TUPLES + RBAC_ENABLED_KIND_NAME_TUPLES ) assert sorted(list_of_kind_names_tuples) == sorted(real_list_of_kind_names) @@ -370,33 +327,6 @@ def test_service_account_custom_names(self): ] assert sorted(list_of_sa_names) == sorted(CUSTOM_SERVICE_ACCOUNT_NAMES) - def test_workers_service_account_custom_name(self): - k8s_objects = render_chart( - "test-rbac", - values={ - "airflowVersion": "3.0.0", - "fullnameOverride": "test-rbac", - "executor": "CeleryExecutor,KubernetesExecutor", - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"name": CUSTOM_WORKER_CELERY_NAME}}, - "kubernetes": {"serviceAccount": {"name": CUSTOM_WORKER_KUBERNETES_NAME}}, - }, - }, - show_only=[ - "templates/workers/worker-celery-serviceaccount.yaml", - "templates/workers/worker-kubernetes-serviceaccount.yaml", - ], - ) - - list_of_sa_names = [ - k8s_object["metadata"]["name"] - for k8s_object in k8s_objects - if k8s_object["kind"] == "ServiceAccount" - ] - assert len(k8s_objects) == 2 - assert sorted(list_of_sa_names) == [CUSTOM_WORKER_CELERY_NAME, CUSTOM_WORKER_KUBERNETES_NAME] - def test_webserver_service_account_name_airflow_2(self): k8s_objects = render_chart( "test-rbac", @@ -463,27 +393,6 @@ def test_service_account_custom_names_in_objects(self): assert sorted(list_of_sa_names_in_objects) == sorted(CUSTOM_SERVICE_ACCOUNT_NAMES) - def test_workers_celery_service_account_custom_names_in_objects(self): - k8s_objects = render_chart( - "test-rbac", - values={ - "airflowVersion": "3.0.0", - "fullnameOverride": "test-rbac", - "workers": { - "useWorkerDedicatedServiceAccounts": True, - "celery": {"serviceAccount": {"name": CUSTOM_WORKER_CELERY_NAME}}, - }, - }, - show_only=[ - "templates/workers/worker-deployment.yaml", - ], - ) - - assert ( - jmespath.search("spec.template.spec.serviceAccountName", k8s_objects[0]) - == CUSTOM_WORKER_CELERY_NAME - ) - def test_service_account_without_resource(self): k8s_objects = render_chart( "test-rbac", From b32c65c93ca378943b6083004a4aa2ca544ac889 Mon Sep 17 00:00:00 2001 From: Miretpl Date: Sat, 17 Jan 2026 19:00:37 +0100 Subject: [PATCH 2/3] Revert "Add newsfragment for 52357 PR (apache#59214)" --- chart/newsfragments/52357.improvement.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 chart/newsfragments/52357.improvement.rst diff --git a/chart/newsfragments/52357.improvement.rst b/chart/newsfragments/52357.improvement.rst deleted file mode 100644 index 717c600b6a00b..0000000000000 --- a/chart/newsfragments/52357.improvement.rst +++ /dev/null @@ -1 +0,0 @@ -With usage of ``workers.useWorkerDedicatedServiceAccounts`` flag and ``worker.celery.serviceAccountName``/``worker.kubernetes.serviceAccountName`` sections it is now possible to use different Kubernetes Service Accounts for Celery workers and KubernetesExecutor pods. From dcbfaf24ec6bd900ba1e09a7c215defc7461b399 Mon Sep 17 00:00:00 2001 From: Miretpl Date: Sat, 17 Jan 2026 19:13:28 +0100 Subject: [PATCH 3/3] Fix security tests --- .../tests/helm_tests/security/test_rbac.py | 4 ++++ .../security/test_rbac_pod_launcher.py | 23 ++----------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/helm-tests/tests/helm_tests/security/test_rbac.py b/helm-tests/tests/helm_tests/security/test_rbac.py index ec299eba176af..b3fb6830b4e38 100644 --- a/helm-tests/tests/helm_tests/security/test_rbac.py +++ b/helm-tests/tests/helm_tests/security/test_rbac.py @@ -152,6 +152,7 @@ def test_deployments_no_rbac_no_sa(self, version): values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", + "executor": "CeleryExecutor,KubernetesExecutor", "rbac": {"create": False}, "cleanup": { "enabled": True, @@ -198,6 +199,7 @@ def test_deployments_no_rbac_with_sa(self, version): values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", + "executor": "CeleryExecutor,KubernetesExecutor", "rbac": {"create": False}, "cleanup": {"enabled": True}, "databaseCleanup": {"enabled": True}, @@ -220,6 +222,7 @@ def test_deployments_with_rbac_no_sa(self, version): values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", + "executor": "CeleryExecutor,KubernetesExecutor", "cleanup": { "enabled": True, "serviceAccount": { @@ -266,6 +269,7 @@ def test_deployments_with_rbac_with_sa(self, version): values=self._get_values_with_version( values={ "fullnameOverride": "test-rbac", + "executor": "CeleryExecutor,KubernetesExecutor", "cleanup": {"enabled": True}, "databaseCleanup": {"enabled": True}, "flower": {"enabled": True}, diff --git a/helm-tests/tests/helm_tests/security/test_rbac_pod_launcher.py b/helm-tests/tests/helm_tests/security/test_rbac_pod_launcher.py index 6c0ba03f10b58..72467271d076b 100644 --- a/helm-tests/tests/helm_tests/security/test_rbac_pod_launcher.py +++ b/helm-tests/tests/helm_tests/security/test_rbac_pod_launcher.py @@ -55,7 +55,6 @@ def test_pod_launcher_role( "rbac_create", "allow_pod_launching", "executor", - "dedicated_sa", "triggerer_enabled", "multi_ns", "expected_subjects", @@ -68,29 +67,13 @@ def test_pod_launcher_role( "CeleryExecutor,KubernetesExecutor", False, False, - False, ["release-name-airflow-scheduler", "release-name-airflow-worker"], ), - # Dedicated worker SAs - ( - True, - True, - "CeleryExecutor,KubernetesExecutor", - True, - False, - False, - [ - "release-name-airflow-scheduler", - "release-name-airflow-worker-kubernetes", - "release-name-airflow-worker-celery", - ], - ), # Add triggerer SA if enabled ( True, True, "CeleryExecutor,KubernetesExecutor", - False, True, False, [ @@ -100,9 +83,9 @@ def test_pod_launcher_role( ], ), # RoleBinding not created if allowPodLaunching is False - (True, False, "CeleryExecutor,KubernetesExecutor", False, False, False, []), + (True, False, "CeleryExecutor,KubernetesExecutor", False, False, []), # RoleBinding not created if rbac.create is False - (False, True, "CeleryExecutor,KubernetesExecutor", False, False, False, []), + (False, True, "CeleryExecutor,KubernetesExecutor", False, False, []), ], ) def test_pod_launcher_rolebinding( @@ -110,7 +93,6 @@ def test_pod_launcher_rolebinding( rbac_create, allow_pod_launching, executor, - dedicated_sa, triggerer_enabled, multi_ns, expected_subjects, @@ -120,7 +102,6 @@ def test_pod_launcher_rolebinding( "rbac": {"create": rbac_create}, "allowPodLaunching": allow_pod_launching, "executor": executor, - "workers": {"useWorkerDedicatedServiceAccounts": dedicated_sa}, "triggerer": {"enabled": triggerer_enabled}, "multiNamespaceMode": multi_ns, },