diff --git a/chart/newsfragments/37197.significant.rst b/chart/newsfragments/37197.significant.rst new file mode 100644 index 0000000000000..68f3aad567c7f --- /dev/null +++ b/chart/newsfragments/37197.significant.rst @@ -0,0 +1,8 @@ +Fixed name clashes when using multiple Airflow deployments in ``multiNamespaceMode`` across several namespaces. + +``ClusterRole``s and ``ClusterRoleBinding``s created when ``multiNamespaceMode`` is enabled have been renamed to ensure unique names: +* ``{{ include "airflow.fullname" . }}-pod-launcher-role`` has been renamed to ``{{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-launcher-role`` +* ``{{ include "airflow.fullname" . }}-pod-launcher-rolebinding`` has been renamed to ``{{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-launcher-rolebinding`` +* ``{{ include "airflow.fullname" . }}-pod-log-reader-role`` has been renamed to ``{{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-log-reader-role`` +* ``{{ include "airflow.fullname" . }}-pod-log-reader-rolebinding`` has been renamed to ``{{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-log-reader-rolebinding`` +* ``{{ include "airflow.fullname" . }}-scc-rolebinding`` has been renamed to ``{{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-scc-rolebinding`` diff --git a/chart/templates/rbac/pod-launcher-role.yaml b/chart/templates/rbac/pod-launcher-role.yaml index 271394db07017..454c1d5f31bf2 100644 --- a/chart/templates/rbac/pod-launcher-role.yaml +++ b/chart/templates/rbac/pod-launcher-role.yaml @@ -28,15 +28,20 @@ kind: ClusterRole kind: Role {{- end }} metadata: - name: {{ include "airflow.fullname" . }}-pod-launcher-role {{- if not .Values.multiNamespaceMode }} + name: {{ include "airflow.fullname" . }}-pod-launcher-role namespace: "{{ .Release.Namespace }}" + {{- else }} + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-launcher-role {{- end }} labels: tier: airflow release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- if .Values.multiNamespaceMode }} + namespace: "{{ .Release.Namespace }}" + {{- end }} {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} diff --git a/chart/templates/rbac/pod-launcher-rolebinding.yaml b/chart/templates/rbac/pod-launcher-rolebinding.yaml index 60705c8993a77..e03c122cba2eb 100644 --- a/chart/templates/rbac/pod-launcher-rolebinding.yaml +++ b/chart/templates/rbac/pod-launcher-rolebinding.yaml @@ -32,13 +32,18 @@ kind: RoleBinding metadata: {{- if not .Values.multiNamespaceMode }} namespace: "{{ .Release.Namespace }}" - {{- end }} name: {{ include "airflow.fullname" . }}-pod-launcher-rolebinding + {{- else }} + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-launcher-rolebinding + {{- end }} labels: tier: airflow release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- if .Values.multiNamespaceMode }} + namespace: "{{ .Release.Namespace }}" + {{- end }} {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} @@ -46,10 +51,11 @@ roleRef: apiGroup: rbac.authorization.k8s.io {{- if .Values.multiNamespaceMode }} kind: ClusterRole + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-launcher-role {{- else }} kind: Role - {{- end }} name: {{ include "airflow.fullname" . }}-pod-launcher-role + {{- end }} subjects: {{- if has .Values.executor $schedulerLaunchExecutors }} - kind: ServiceAccount diff --git a/chart/templates/rbac/pod-log-reader-role.yaml b/chart/templates/rbac/pod-log-reader-role.yaml index d048d36fa2b06..dbc9386af48c9 100644 --- a/chart/templates/rbac/pod-log-reader-role.yaml +++ b/chart/templates/rbac/pod-log-reader-role.yaml @@ -28,15 +28,20 @@ kind: ClusterRole kind: Role {{- end }} metadata: - name: {{ include "airflow.fullname" . }}-pod-log-reader-role {{- if not .Values.multiNamespaceMode }} + name: {{ include "airflow.fullname" . }}-pod-log-reader-role namespace: "{{ .Release.Namespace }}" + {{- else }} + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-log-reader-role {{- end }} labels: tier: airflow release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- if .Values.multiNamespaceMode }} + namespace: "{{ .Release.Namespace }}" + {{- end }} {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} diff --git a/chart/templates/rbac/pod-log-reader-rolebinding.yaml b/chart/templates/rbac/pod-log-reader-rolebinding.yaml index e441f9af85ef9..33b030789c92a 100644 --- a/chart/templates/rbac/pod-log-reader-rolebinding.yaml +++ b/chart/templates/rbac/pod-log-reader-rolebinding.yaml @@ -30,13 +30,18 @@ kind: RoleBinding metadata: {{- if not .Values.multiNamespaceMode }} namespace: "{{ .Release.Namespace }}" - {{- end }} name: {{ include "airflow.fullname" . }}-pod-log-reader-rolebinding + {{- else }} + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-log-reader-rolebinding + {{- end }} labels: tier: airflow release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- if .Values.multiNamespaceMode }} + namespace: "{{ .Release.Namespace }}" + {{- end }} {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} @@ -44,10 +49,11 @@ roleRef: apiGroup: rbac.authorization.k8s.io {{- if .Values.multiNamespaceMode }} kind: ClusterRole + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-pod-log-reader-role {{- else }} kind: Role - {{- end }} name: {{ include "airflow.fullname" . }}-pod-log-reader-role + {{- end }} subjects: {{- if .Values.webserver.allowPodLogReading }} - kind: ServiceAccount diff --git a/chart/templates/rbac/security-context-constraint-rolebinding.yaml b/chart/templates/rbac/security-context-constraint-rolebinding.yaml index 070a8e8169ae8..bd95c5b779bb0 100644 --- a/chart/templates/rbac/security-context-constraint-rolebinding.yaml +++ b/chart/templates/rbac/security-context-constraint-rolebinding.yaml @@ -30,14 +30,19 @@ kind: RoleBinding {{- end }} metadata: {{- if not .Values.multiNamespaceMode }} + name: {{ include "airflow.fullname" . }}-scc-rolebinding namespace: "{{ .Release.Namespace }}" + {{- else }} + name: {{ .Release.Namespace }}-{{ include "airflow.fullname" . }}-scc-rolebinding {{- end }} - name: {{ include "airflow.fullname" . }}-scc-rolebinding labels: tier: airflow release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- if .Values.multiNamespaceMode }} + namespace: "{{ .Release.Namespace }}" + {{- end }} {{- with .Values.labels }} {{- toYaml . | nindent 4 }} {{- end }} diff --git a/helm_tests/airflow_aux/test_pod_launcher_role.py b/helm_tests/airflow_aux/test_pod_launcher_role.py index 64131fa6f2048..c1336268824c6 100644 --- a/helm_tests/airflow_aux/test_pod_launcher_role.py +++ b/helm_tests/airflow_aux/test_pod_launcher_role.py @@ -49,3 +49,69 @@ def test_pod_launcher_role(self, executor, rbac, allow, expected_accounts): assert f"release-name-airflow-{suffix}" == jmespath.search(f"subjects[{idx}].name", docs[0]) else: assert [] == docs + + @pytest.mark.parametrize( + "multiNamespaceMode, namespace, expectedRole, expectedRoleBinding", + [ + ( + True, + "namespace", + "namespace-release-name-pod-launcher-role", + "namespace-release-name-pod-launcher-rolebinding", + ), + ( + True, + "other-ns", + "other-ns-release-name-pod-launcher-role", + "other-ns-release-name-pod-launcher-rolebinding", + ), + (False, "namespace", "release-name-pod-launcher-role", "release-name-pod-launcher-rolebinding"), + ], + ) + def test_pod_launcher_rolebinding_multi_namespace( + self, multiNamespaceMode, namespace, expectedRole, expectedRoleBinding + ): + docs = render_chart( + namespace=namespace, + values={"webserver": {"allowPodLogReading": True}, "multiNamespaceMode": multiNamespaceMode}, + show_only=["templates/rbac/pod-launcher-rolebinding.yaml"], + ) + + actualRoleBinding = jmespath.search("metadata.name", docs[0]) + assert actualRoleBinding == expectedRoleBinding + + actualRoleRef = jmespath.search("roleRef.name", docs[0]) + assert actualRoleRef == expectedRole + + actualKind = jmespath.search("kind", docs[0]) + actualRoleRefKind = jmespath.search("roleRef.kind", docs[0]) + if multiNamespaceMode: + assert actualKind == "ClusterRoleBinding" + assert actualRoleRefKind == "ClusterRole" + else: + assert actualKind == "RoleBinding" + assert actualRoleRefKind == "Role" + + @pytest.mark.parametrize( + "multiNamespaceMode, namespace, expectedRole", + [ + (True, "namespace", "namespace-release-name-pod-launcher-role"), + (True, "other-ns", "other-ns-release-name-pod-launcher-role"), + (False, "namespace", "release-name-pod-launcher-role"), + ], + ) + def test_pod_launcher_role_multi_namespace(self, multiNamespaceMode, namespace, expectedRole): + docs = render_chart( + namespace=namespace, + values={"webserver": {"allowPodLogReading": True}, "multiNamespaceMode": multiNamespaceMode}, + show_only=["templates/rbac/pod-launcher-role.yaml"], + ) + + actualRole = jmespath.search("metadata.name", docs[0]) + assert actualRole == expectedRole + + actualKind = jmespath.search("kind", docs[0]) + if multiNamespaceMode: + assert actualKind == "ClusterRole" + else: + assert actualKind == "Role" diff --git a/helm_tests/security/test_rbac_pod_log_reader.py b/helm_tests/security/test_rbac_pod_log_reader.py index 82ec13fcc1339..a6acc2dee452d 100644 --- a/helm_tests/security/test_rbac_pod_log_reader.py +++ b/helm_tests/security/test_rbac_pod_log_reader.py @@ -64,3 +64,74 @@ def test_pod_log_reader_role(self, triggerer, webserver, expected): ) actual = jmespath.search("metadata.name", docs[0]) if docs else None assert actual == expected + + @pytest.mark.parametrize( + "multiNamespaceMode, namespace, expectedRole, expectedRoleBinding", + [ + ( + True, + "namespace", + "namespace-release-name-pod-log-reader-role", + "namespace-release-name-pod-log-reader-rolebinding", + ), + ( + True, + "other-ns", + "other-ns-release-name-pod-log-reader-role", + "other-ns-release-name-pod-log-reader-rolebinding", + ), + ( + False, + "namespace", + "release-name-pod-log-reader-role", + "release-name-pod-log-reader-rolebinding", + ), + ], + ) + def test_pod_log_reader_rolebinding_multi_namespace( + self, multiNamespaceMode, namespace, expectedRole, expectedRoleBinding + ): + docs = render_chart( + namespace=namespace, + values={"webserver": {"allowPodLogReading": True}, "multiNamespaceMode": multiNamespaceMode}, + show_only=["templates/rbac/pod-log-reader-rolebinding.yaml"], + ) + + actualRoleBinding = jmespath.search("metadata.name", docs[0]) + assert actualRoleBinding == expectedRoleBinding + + actualRoleRef = jmespath.search("roleRef.name", docs[0]) + assert actualRoleRef == expectedRole + + actualKind = jmespath.search("kind", docs[0]) + actualRoleRefKind = jmespath.search("roleRef.kind", docs[0]) + if multiNamespaceMode: + assert actualKind == "ClusterRoleBinding" + assert actualRoleRefKind == "ClusterRole" + else: + assert actualKind == "RoleBinding" + assert actualRoleRefKind == "Role" + + @pytest.mark.parametrize( + "multiNamespaceMode, namespace, expectedRole", + [ + (True, "namespace", "namespace-release-name-pod-log-reader-role"), + (True, "other-ns", "other-ns-release-name-pod-log-reader-role"), + (False, "namespace", "release-name-pod-log-reader-role"), + ], + ) + def test_pod_log_reader_role_multi_namespace(self, multiNamespaceMode, namespace, expectedRole): + docs = render_chart( + namespace=namespace, + values={"webserver": {"allowPodLogReading": True}, "multiNamespaceMode": multiNamespaceMode}, + show_only=["templates/rbac/pod-log-reader-role.yaml"], + ) + + actualRole = jmespath.search("metadata.name", docs[0]) + assert actualRole == expectedRole + + actualKind = jmespath.search("kind", docs[0]) + if multiNamespaceMode: + assert actualKind == "ClusterRole" + else: + assert actualKind == "Role" diff --git a/helm_tests/security/test_scc_rolebinding.py b/helm_tests/security/test_scc_rolebinding.py index cb674f931e95b..e77845811929d 100644 --- a/helm_tests/security/test_scc_rolebinding.py +++ b/helm_tests/security/test_scc_rolebinding.py @@ -63,13 +63,15 @@ def test_create_scc(self, rbac_enabled, scc_enabled, created): assert "release-name-airflow-cleanup" == jmespath.search("subjects[8].name", docs[0]) @pytest.mark.parametrize( - "rbac_enabled,scc_enabled,created", + "rbac_enabled,scc_enabled,created,namespace,expected_name", [ - (True, True, True), + (True, True, True, "default", "default-release-name-scc-rolebinding"), + (True, True, True, "other-ns", "other-ns-release-name-scc-rolebinding"), ], ) - def test_create_scc_multinamespace(self, rbac_enabled, scc_enabled, created): + def test_create_scc_multinamespace(self, rbac_enabled, scc_enabled, created, namespace, expected_name): docs = render_chart( + namespace=namespace, values={ "multiNamespaceMode": True, "webserver": {"defaultUser": {"enabled": False}}, @@ -84,7 +86,7 @@ def test_create_scc_multinamespace(self, rbac_enabled, scc_enabled, created): if created: assert "ClusterRoleBinding" == jmespath.search("kind", docs[0]) assert "ClusterRole" == jmespath.search("roleRef.kind", docs[0]) - assert "release-name-scc-rolebinding" == jmespath.search("metadata.name", docs[0]) + assert expected_name == jmespath.search("metadata.name", docs[0]) assert "system:openshift:scc:anyuid" == jmespath.search("roleRef.name", docs[0]) @pytest.mark.parametrize(