From 37c4a5ea52a2f8de8a28794058b78973763f769d Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Tue, 19 Nov 2024 08:52:08 +0200 Subject: [PATCH 1/4] remove scheduler automate serviceaccount token --- .../scheduler/scheduler-serviceaccount.yaml | 1 - chart/values.schema.json | 5 ----- chart/values.yaml | 3 --- helm_tests/airflow_core/test_scheduler.py | 22 ------------------- 4 files changed, 31 deletions(-) diff --git a/chart/templates/scheduler/scheduler-serviceaccount.yaml b/chart/templates/scheduler/scheduler-serviceaccount.yaml index 310f168496766..320015b7c7d91 100644 --- a/chart/templates/scheduler/scheduler-serviceaccount.yaml +++ b/chart/templates/scheduler/scheduler-serviceaccount.yaml @@ -23,7 +23,6 @@ {{- if and .Values.scheduler.enabled .Values.scheduler.serviceAccount.create }} apiVersion: v1 kind: ServiceAccount -automountServiceAccountToken: {{ .Values.scheduler.serviceAccount.automountServiceAccountToken }} metadata: name: {{ include "scheduler.serviceAccountName" . }} labels: diff --git a/chart/values.schema.json b/chart/values.schema.json index 8443b1a8cf655..367561be84305 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2592,11 +2592,6 @@ "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", diff --git a/chart/values.yaml b/chart/values.yaml index 2e27b7a4686ee..cc0ed119e0309 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -934,9 +934,6 @@ scheduler: # Create ServiceAccount 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. diff --git a/helm_tests/airflow_core/test_scheduler.py b/helm_tests/airflow_core/test_scheduler.py index 0bef3e7e1322b..8ed0803425fee 100644 --- a/helm_tests/airflow_core/test_scheduler.py +++ b/helm_tests/airflow_core/test_scheduler.py @@ -988,28 +988,6 @@ def test_should_add_component_specific_labels(self): assert "test_label" in jmespath.search("metadata.labels", docs[0]) assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value" - def test_default_automount_service_account_token(self): - docs = render_chart( - values={ - "scheduler": { - "serviceAccount": {"create": True}, - }, - }, - show_only=["templates/scheduler/scheduler-serviceaccount.yaml"], - ) - assert jmespath.search("automountServiceAccountToken", docs[0]) is True - - def test_overridden_automount_service_account_token(self): - docs = render_chart( - values={ - "scheduler": { - "serviceAccount": {"create": True, "automountServiceAccountToken": False}, - }, - }, - show_only=["templates/scheduler/scheduler-serviceaccount.yaml"], - ) - assert jmespath.search("automountServiceAccountToken", docs[0]) is False - class TestSchedulerCreation: """Tests scheduler deployment creation.""" From e7f3b1f205fd653f8148ecff8a54e4a418b9cd3d Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Fri, 22 Nov 2024 08:33:14 +0200 Subject: [PATCH 2/4] concider automount service account only if executor is CeleryExecutor --- .../scheduler/scheduler-serviceaccount.yaml | 3 ++ chart/values.schema.json | 5 ++ chart/values.yaml | 4 ++ helm_tests/airflow_core/test_scheduler.py | 49 +++++++++++++++++++ 4 files changed, 61 insertions(+) diff --git a/chart/templates/scheduler/scheduler-serviceaccount.yaml b/chart/templates/scheduler/scheduler-serviceaccount.yaml index 320015b7c7d91..0f4f8cfaa67e0 100644 --- a/chart/templates/scheduler/scheduler-serviceaccount.yaml +++ b/chart/templates/scheduler/scheduler-serviceaccount.yaml @@ -23,6 +23,9 @@ {{- if and .Values.scheduler.enabled .Values.scheduler.serviceAccount.create }} apiVersion: v1 kind: ServiceAccount +{{- if eq .Values.executor "CeleryExecutor" }} +automountServiceAccountToken: {{ .Values.scheduler.serviceAccount.automountServiceAccountToken }} +{{- end }} metadata: name: {{ include "scheduler.serviceAccountName" . }} labels: diff --git a/chart/values.schema.json b/chart/values.schema.json index 367561be84305..8443b1a8cf655 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -2592,6 +2592,11 @@ "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", diff --git a/chart/values.yaml b/chart/values.yaml index cc0ed119e0309..b0089a814e080 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -934,6 +934,10 @@ scheduler: # Create ServiceAccount serviceAccount: + # default value is true + # ref: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ + # only effect CeleryExecutor + automountServiceAccountToken: true # Specifies whether a ServiceAccount should be created create: true # The name of the ServiceAccount to use. diff --git a/helm_tests/airflow_core/test_scheduler.py b/helm_tests/airflow_core/test_scheduler.py index 8ed0803425fee..a3e1f1d0d627b 100644 --- a/helm_tests/airflow_core/test_scheduler.py +++ b/helm_tests/airflow_core/test_scheduler.py @@ -988,6 +988,55 @@ def test_should_add_component_specific_labels(self): assert "test_label" in jmespath.search("metadata.labels", docs[0]) assert jmespath.search("metadata.labels", docs[0])["test_label"] == "test_label_value" + @pytest.mark.parametrize( + "executor, default_automount_service_account", + [ + ("LocalExecutor", None), + ("CeleryExecutor", True), + ("CeleryKubernetesExecutor", None), + ("KubernetesExecutor", None), + ("LocalKubernetesExecutor", None), + ], + ) + def test_default_automount_service_account_token(self, executor, default_automount_service_account): + docs = render_chart( + values={ + "scheduler": { + "serviceAccount": {"create": True}, + }, + "executor": executor, + }, + show_only=["templates/scheduler/scheduler-serviceaccount.yaml"], + ) + assert jmespath.search("automountServiceAccountToken", docs[0]) is default_automount_service_account + + @pytest.mark.parametrize( + "executor, automount_service_account, shoud_automount_service_account", + [ + ("LocalExecutor", True, None), + ("CeleryExecutor", False, False), + ("CeleryKubernetesExecutor", False, None), + ("KubernetesExecutor", False, None), + ("LocalKubernetesExecutor", False, None), + ], + ) + def test_overridden_automount_service_account_token( + self, executor, automount_service_account, shoud_automount_service_account + ): + docs = render_chart( + values={ + "scheduler": { + "serviceAccount": { + "create": True, + "automountServiceAccountToken": automount_service_account, + }, + }, + "executor": executor, + }, + show_only=["templates/scheduler/scheduler-serviceaccount.yaml"], + ) + assert jmespath.search("automountServiceAccountToken", docs[0]) is shoud_automount_service_account + class TestSchedulerCreation: """Tests scheduler deployment creation.""" From 5949b3a15c2b0a99c528256646b86fecba11298b Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Fri, 22 Nov 2024 08:35:29 +0200 Subject: [PATCH 3/4] change comment --- chart/values.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/chart/values.yaml b/chart/values.yaml index b0089a814e080..c0764fb54a0d7 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -934,9 +934,8 @@ scheduler: # Create ServiceAccount serviceAccount: - # default value is true + # only affect CeleryExecutor, default value is true # ref: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/ - # only effect CeleryExecutor automountServiceAccountToken: true # Specifies whether a ServiceAccount should be created create: true From a0a6d5b478190539c249dbb16d94c2c549418acc Mon Sep 17 00:00:00 2001 From: romsharon98 Date: Mon, 16 Dec 2024 19:25:01 +0200 Subject: [PATCH 4/4] fix typo --- helm_tests/airflow_core/test_scheduler.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helm_tests/airflow_core/test_scheduler.py b/helm_tests/airflow_core/test_scheduler.py index a3e1f1d0d627b..7b3eae3fdada3 100644 --- a/helm_tests/airflow_core/test_scheduler.py +++ b/helm_tests/airflow_core/test_scheduler.py @@ -1011,7 +1011,7 @@ def test_default_automount_service_account_token(self, executor, default_automou assert jmespath.search("automountServiceAccountToken", docs[0]) is default_automount_service_account @pytest.mark.parametrize( - "executor, automount_service_account, shoud_automount_service_account", + "executor, automount_service_account, should_automount_service_account", [ ("LocalExecutor", True, None), ("CeleryExecutor", False, False), @@ -1021,7 +1021,7 @@ def test_default_automount_service_account_token(self, executor, default_automou ], ) def test_overridden_automount_service_account_token( - self, executor, automount_service_account, shoud_automount_service_account + self, executor, automount_service_account, should_automount_service_account ): docs = render_chart( values={ @@ -1035,7 +1035,7 @@ def test_overridden_automount_service_account_token( }, show_only=["templates/scheduler/scheduler-serviceaccount.yaml"], ) - assert jmespath.search("automountServiceAccountToken", docs[0]) is shoud_automount_service_account + assert jmespath.search("automountServiceAccountToken", docs[0]) is should_automount_service_account class TestSchedulerCreation: