From 540eb26c28f1582a5b5199be0e5f4dc7ce1551b7 Mon Sep 17 00:00:00 2001 From: Bob Put Date: Sun, 24 May 2026 16:13:34 -0400 Subject: [PATCH] Fix Celery worker service account in RBAC --- chart/templates/_helpers.yaml | 11 +++++ .../rbac/job-launcher-rolebinding.yaml | 11 ++++- .../rbac/pod-launcher-rolebinding.yaml | 11 ++++- ...curity-context-constraint-rolebinding.yaml | 11 ++++- .../airflow_aux/test_job_launcher_role.py | 44 +++++++++++++++++++ .../airflow_aux/test_pod_launcher_role.py | 44 +++++++++++++++++++ .../security/test_scc_rolebinding.py | 19 ++++++++ 7 files changed, 145 insertions(+), 6 deletions(-) diff --git a/chart/templates/_helpers.yaml b/chart/templates/_helpers.yaml index 3445f256a0f3f..762133c6bdae7 100644 --- a/chart/templates/_helpers.yaml +++ b/chart/templates/_helpers.yaml @@ -726,6 +726,17 @@ server_tls_key_file = /etc/pgbouncer/server.key {{- end }} {{- end }} +{{/* Create the name of the worker celery service account to use */}} +{{- define "worker.celery.serviceAccountName" -}} + {{- $filteredCeleryServiceAccount := include "removeNilFields" .Values.workers.celery.serviceAccount | fromYaml -}} + {{- $serviceAccount := include "workersMergeValues" (list .Values.workers.serviceAccount $filteredCeleryServiceAccount "" list) | fromYaml -}} + {{- if and (hasKey .Values.workers "name") (ne .Values.workers.name "default") }} + {{- include "_serviceAccountNameGen" (merge (dict "sa" $serviceAccount "key" "workers" "nameSuffix" (printf "%s-%s" "worker" .Values.workers.name)) .) -}} + {{- else }} + {{- include "_serviceAccountNameGen" (merge (dict "sa" $serviceAccount "key" "workers" "nameSuffix" "worker") .) -}} + {{- end }} +{{- 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") .) -}} diff --git a/chart/templates/rbac/job-launcher-rolebinding.yaml b/chart/templates/rbac/job-launcher-rolebinding.yaml index 6749b13986691..634a536ca5caa 100644 --- a/chart/templates/rbac/job-launcher-rolebinding.yaml +++ b/chart/templates/rbac/job-launcher-rolebinding.yaml @@ -55,14 +55,21 @@ roleRef: name: {{ include "airflow.fullname" . }}-job-launcher-role {{- end }} subjects: + {{- $workerServiceAccountName := include "worker.serviceAccountName" $ }} + {{- $workerCeleryServiceAccountName := include "worker.celery.serviceAccountName" $ }} {{- if and .Values.scheduler.enabled (or (contains "LocalExecutor" .Values.executor) (contains "KubernetesExecutor" .Values.executor)) }} - kind: ServiceAccount name: {{ include "scheduler.serviceAccountName" $ }} namespace: "{{ $.Release.Namespace }}" {{- end }} - {{- if or (contains "CeleryExecutor" .Values.executor) (and (contains "KubernetesExecutor" .Values.executor) (eq .Values.workers.kubernetes.serviceAccount.create nil)) }} + {{- if contains "CeleryExecutor" .Values.executor }} - kind: ServiceAccount - name: {{ include "worker.serviceAccountName" $ }} + name: {{ $workerCeleryServiceAccountName }} + namespace: "{{ $.Release.Namespace }}" + {{- end }} + {{- if and (contains "KubernetesExecutor" .Values.executor) (eq .Values.workers.kubernetes.serviceAccount.create nil) (or (not (contains "CeleryExecutor" .Values.executor)) (ne $workerServiceAccountName $workerCeleryServiceAccountName)) }} + - kind: ServiceAccount + name: {{ $workerServiceAccountName }} namespace: "{{ $.Release.Namespace }}" {{- end }} {{- if and (or .Values.workers.kubernetes.serviceAccount.create .Values.workers.kubernetes.serviceAccount.name) (contains "KubernetesExecutor" .Values.executor) }} diff --git a/chart/templates/rbac/pod-launcher-rolebinding.yaml b/chart/templates/rbac/pod-launcher-rolebinding.yaml index f05f897f728aa..0137d233d4565 100644 --- a/chart/templates/rbac/pod-launcher-rolebinding.yaml +++ b/chart/templates/rbac/pod-launcher-rolebinding.yaml @@ -55,14 +55,21 @@ roleRef: name: {{ include "airflow.fullname" . }}-pod-launcher-role {{- end }} subjects: + {{- $workerServiceAccountName := include "worker.serviceAccountName" $ }} + {{- $workerCeleryServiceAccountName := include "worker.celery.serviceAccountName" $ }} {{- if and .Values.scheduler.enabled (or (contains "LocalExecutor" .Values.executor) (contains "KubernetesExecutor" .Values.executor)) }} - kind: ServiceAccount name: {{ include "scheduler.serviceAccountName" $ }} namespace: "{{ $.Release.Namespace }}" {{- end }} - {{- if or (contains "CeleryExecutor" .Values.executor) (and (contains "KubernetesExecutor" .Values.executor) (eq .Values.workers.kubernetes.serviceAccount.create nil)) }} + {{- if contains "CeleryExecutor" .Values.executor }} - kind: ServiceAccount - name: {{ include "worker.serviceAccountName" $ }} + name: {{ $workerCeleryServiceAccountName }} + namespace: "{{ $.Release.Namespace }}" + {{- end }} + {{- if and (contains "KubernetesExecutor" .Values.executor) (eq .Values.workers.kubernetes.serviceAccount.create nil) (or (not (contains "CeleryExecutor" .Values.executor)) (ne $workerServiceAccountName $workerCeleryServiceAccountName)) }} + - kind: ServiceAccount + name: {{ $workerServiceAccountName }} namespace: "{{ $.Release.Namespace }}" {{- end }} {{- if and (or .Values.workers.kubernetes.serviceAccount.create .Values.workers.kubernetes.serviceAccount.name) (contains "KubernetesExecutor" .Values.executor) }} diff --git a/chart/templates/rbac/security-context-constraint-rolebinding.yaml b/chart/templates/rbac/security-context-constraint-rolebinding.yaml index cccf2d9fa422a..7982972acb8b9 100644 --- a/chart/templates/rbac/security-context-constraint-rolebinding.yaml +++ b/chart/templates/rbac/security-context-constraint-rolebinding.yaml @@ -50,9 +50,16 @@ roleRef: kind: ClusterRole name: system:openshift:scc:anyuid subjects: - {{- if or (contains "CeleryExecutor" .Values.executor) (and (contains "KubernetesExecutor" .Values.executor) (eq .Values.workers.kubernetes.serviceAccount.create nil)) }} + {{- $workerServiceAccountName := include "worker.serviceAccountName" . }} + {{- $workerCeleryServiceAccountName := include "worker.celery.serviceAccountName" . }} + {{- if contains "CeleryExecutor" .Values.executor }} - kind: ServiceAccount - name: {{ include "worker.serviceAccountName" . }} + name: {{ $workerCeleryServiceAccountName }} + namespace: "{{ .Release.Namespace }}" + {{- end }} + {{- if and (contains "KubernetesExecutor" .Values.executor) (eq .Values.workers.kubernetes.serviceAccount.create nil) (or (not (contains "CeleryExecutor" .Values.executor)) (ne $workerServiceAccountName $workerCeleryServiceAccountName)) }} + - kind: ServiceAccount + name: {{ $workerServiceAccountName }} namespace: "{{ .Release.Namespace }}" {{- end }} {{- if and (or .Values.workers.kubernetes.serviceAccount.create .Values.workers.kubernetes.serviceAccount.name) (contains "KubernetesExecutor" .Values.executor) }} diff --git a/chart/tests/helm_tests/airflow_aux/test_job_launcher_role.py b/chart/tests/helm_tests/airflow_aux/test_job_launcher_role.py index 18bac0e3a9eb0..2f1eabec01ec5 100644 --- a/chart/tests/helm_tests/airflow_aux/test_job_launcher_role.py +++ b/chart/tests/helm_tests/airflow_aux/test_job_launcher_role.py @@ -147,6 +147,50 @@ def test_worker_role_binding_should_exists(self, executor): "namespace": "airflow", } + def test_worker_role_binding_uses_celery_service_account_name(self): + docs = render_chart( + name="prod", + namespace="airflow", + values={ + "rbac": {"create": True}, + "allowJobLaunching": True, + "executor": "CeleryExecutor", + "workers": {"celery": {"serviceAccount": {"name": "custom-worker"}}}, + }, + show_only=["templates/rbac/job-launcher-rolebinding.yaml"], + ) + + assert jmespath.search("subjects[?name=='custom-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "custom-worker", + "namespace": "airflow", + } + assert jmespath.search("subjects[?name=='prod-airflow-worker']", docs[0]) == [] + + def test_worker_role_binding_keeps_kubernetes_fallback_service_account_when_celery_differs(self): + docs = render_chart( + name="prod", + namespace="airflow", + values={ + "rbac": {"create": True}, + "allowJobLaunching": True, + "executor": "CeleryExecutor,KubernetesExecutor", + "workers": {"celery": {"serviceAccount": {"name": "custom-worker"}}}, + }, + show_only=["templates/rbac/job-launcher-rolebinding.yaml"], + ) + + assert jmespath.search("subjects[?name=='custom-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "custom-worker", + "namespace": "airflow", + } + assert jmespath.search("subjects[?name=='prod-airflow-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "prod-airflow-worker", + "namespace": "airflow", + } + def test_worker_role_binding_should_not_exists(self): docs = render_chart( name="prod", diff --git a/chart/tests/helm_tests/airflow_aux/test_pod_launcher_role.py b/chart/tests/helm_tests/airflow_aux/test_pod_launcher_role.py index bf66464bd50f9..07acbde57b472 100644 --- a/chart/tests/helm_tests/airflow_aux/test_pod_launcher_role.py +++ b/chart/tests/helm_tests/airflow_aux/test_pod_launcher_role.py @@ -148,6 +148,50 @@ def test_worker_role_binding_should_exists(self, executor): "namespace": "airflow", } + def test_worker_role_binding_uses_celery_service_account_name(self): + docs = render_chart( + name="prod", + namespace="airflow", + values={ + "rbac": {"create": True}, + "allowPodLaunching": True, + "executor": "CeleryExecutor", + "workers": {"celery": {"serviceAccount": {"name": "custom-worker"}}}, + }, + show_only=["templates/rbac/pod-launcher-rolebinding.yaml"], + ) + + assert jmespath.search("subjects[?name=='custom-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "custom-worker", + "namespace": "airflow", + } + assert jmespath.search("subjects[?name=='prod-airflow-worker']", docs[0]) == [] + + def test_worker_role_binding_keeps_kubernetes_fallback_service_account_when_celery_differs(self): + docs = render_chart( + name="prod", + namespace="airflow", + values={ + "rbac": {"create": True}, + "allowPodLaunching": True, + "executor": "CeleryExecutor,KubernetesExecutor", + "workers": {"celery": {"serviceAccount": {"name": "custom-worker"}}}, + }, + show_only=["templates/rbac/pod-launcher-rolebinding.yaml"], + ) + + assert jmespath.search("subjects[?name=='custom-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "custom-worker", + "namespace": "airflow", + } + assert jmespath.search("subjects[?name=='prod-airflow-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "prod-airflow-worker", + "namespace": "airflow", + } + def test_worker_role_binding_should_not_exists(self): docs = render_chart( name="prod", diff --git a/chart/tests/helm_tests/security/test_scc_rolebinding.py b/chart/tests/helm_tests/security/test_scc_rolebinding.py index a9b1719a2da64..cf260635c5a5d 100644 --- a/chart/tests/helm_tests/security/test_scc_rolebinding.py +++ b/chart/tests/helm_tests/security/test_scc_rolebinding.py @@ -130,6 +130,25 @@ def test_worker_role_binding_should_exists(self, executor): "namespace": "airflow", } + def test_worker_role_binding_uses_celery_service_account_name(self): + docs = render_chart( + name="prod", + namespace="airflow", + values={ + "rbac": {"create": True, "createSCCRoleBinding": True}, + "executor": "CeleryExecutor", + "workers": {"celery": {"serviceAccount": {"name": "custom-worker"}}}, + }, + show_only=["templates/rbac/security-context-constraint-rolebinding.yaml"], + ) + + assert jmespath.search("subjects[?name=='custom-worker'] | [0]", docs[0]) == { + "kind": "ServiceAccount", + "name": "custom-worker", + "namespace": "airflow", + } + assert jmespath.search("subjects[?name=='prod-airflow-worker']", docs[0]) == [] + def test_worker_role_binding_should_not_exists(self): docs = render_chart( name="prod",