Skip to content

Commit

Permalink
Chart: Fix cluster-wide RBAC naming clash when using multiple multiNa…
Browse files Browse the repository at this point in the history
…mespace releases with the same name (#37197)
  • Loading branch information
mewa committed Mar 21, 2024
1 parent ea951af commit a0c1985
Show file tree
Hide file tree
Showing 9 changed files with 185 additions and 11 deletions.
8 changes: 8 additions & 0 deletions chart/newsfragments/37197.significant.rst
Original file line number Diff line number Diff line change
@@ -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``
7 changes: 6 additions & 1 deletion chart/templates/rbac/pod-launcher-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
10 changes: 8 additions & 2 deletions chart/templates/rbac/pod-launcher-rolebinding.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,30 @@ 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 }}
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
Expand Down
7 changes: 6 additions & 1 deletion chart/templates/rbac/pod-log-reader-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
10 changes: 8 additions & 2 deletions chart/templates/rbac/pod-log-reader-rolebinding.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,30 @@ 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 }}
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
66 changes: 66 additions & 0 deletions helm_tests/airflow_aux/test_pod_launcher_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
71 changes: 71 additions & 0 deletions helm_tests/security/test_rbac_pod_log_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
10 changes: 6 additions & 4 deletions helm_tests/security/test_scc_rolebinding.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
Expand All @@ -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(
Expand Down

0 comments on commit a0c1985

Please sign in to comment.