Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chart: Fix cluster-wide RBAC naming clash when using multiple multiNamespace releases with the same name #37197

Merged
merged 6 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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