From 15bc399d5a74a9cb4e9ad1d117429ba75a7b0fd7 Mon Sep 17 00:00:00 2001
From: Donny Xia <donnyxia@google.com>
Date: Mon, 20 Jul 2020 13:04:01 -0700
Subject: [PATCH] Support RoleBinding for ServiceAccount across namespace

---
 api/krusty/rolebindingacrossnamespace_test.go | 135 +++++++++++++++++-
 api/resmap/resmap.go                          |  30 +++-
 2 files changed, 159 insertions(+), 6 deletions(-)

diff --git a/api/krusty/rolebindingacrossnamespace_test.go b/api/krusty/rolebindingacrossnamespace_test.go
index 267c824b2..08d1e6204 100644
--- a/api/krusty/rolebindingacrossnamespace_test.go
+++ b/api/krusty/rolebindingacrossnamespace_test.go
@@ -19,7 +19,25 @@ nameSuffix: -ns2
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  name: my-sa
+  name: my-sa1
+  namespace: ns1
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: my-sa2
+  namespace: ns2
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: my-sa3
+  namespace: ns3
+---
+apiVersion: v1
+kind: NotServiceAccount
+metadata:
+  name: my-nsa
   namespace: ns1
 ---
 apiVersion: rbac.authorization.k8s.io/v1
@@ -46,7 +64,16 @@ roleRef:
   name: my-role
 subjects:
   - kind: ServiceAccount
-    name: my-sa
+    name: my-sa1
+    namespace: ns1
+  - kind: ServiceAccount
+    name: my-sa2
+    namespace: ns2
+  - kind: ServiceAccount
+    name: my-sa3
+    namespace: ns3
+  - kind: NotServiceAccount
+    name: my-nsa
     namespace: ns1
 `)
 
@@ -55,7 +82,25 @@ subjects:
 apiVersion: v1
 kind: ServiceAccount
 metadata:
-  name: my-sa-ns2
+  name: my-sa1-ns2
+  namespace: ns1
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: my-sa2-ns2
+  namespace: ns2
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: my-sa3-ns2
+  namespace: ns3
+---
+apiVersion: v1
+kind: NotServiceAccount
+metadata:
+  name: my-nsa-ns2
   namespace: ns1
 ---
 apiVersion: rbac.authorization.k8s.io/v1
@@ -82,7 +127,89 @@ roleRef:
   name: my-role-ns2
 subjects:
 - kind: ServiceAccount
-  name: my-sa
+  name: my-sa1-ns2
+  namespace: ns1
+- kind: ServiceAccount
+  name: my-sa2-ns2
+  namespace: ns2
+- kind: ServiceAccount
+  name: my-sa3-ns2
+  namespace: ns3
+- kind: NotServiceAccount
+  name: my-nsa
   namespace: ns1
 `)
 }
+
+func TestRoleBindingAcrossNamespaceWoSubjects(t *testing.T) {
+	th := kusttest_test.MakeEnhancedHarness(t)
+	defer th.Reset()
+
+	th.WriteK("/app", `
+resources:
+- resource.yaml
+nameSuffix: -ns2
+`)
+	th.WriteF("/app/resource.yaml", `
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: my-sa1
+  namespace: ns1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: my-role
+  namespace: ns2
+rules:
+  - apiGroups:
+      - '*'
+    resources:
+      - '*'
+    verbs:
+      - '*'
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: my-role-binding
+  namespace: ns2
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: my-role
+`)
+
+	m := th.Run("/app", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: my-sa1-ns2
+  namespace: ns1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: my-role-ns2
+  namespace: ns2
+rules:
+- apiGroups:
+  - '*'
+  resources:
+  - '*'
+  verbs:
+  - '*'
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: my-role-binding-ns2
+  namespace: ns2
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: my-role-ns2
+`)
+}
diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go
index 91e87df66..dd7ee729b 100644
--- a/api/resmap/resmap.go
+++ b/api/resmap/resmap.go
@@ -611,19 +611,45 @@ func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
 	inputId := inputRes.CurId()
 	isInputIdNamespaceable := inputId.IsNamespaceableKind()
 	rctxm := inputRes.PrefixesSuffixesEquals
+	subjectNamespaces := getNamespacesForRoleBinding(inputRes)
 	for _, r := range m.Resources() {
 		// Need to match more accuratly both at the time of selection and transformation.
 		// OutmostPrefixSuffixEquals is not accurate enough since it is only using
 		// the outer most suffix and the last prefix. Use PrefixedSuffixesEquals instead.
 		resId := r.CurId()
-		if (!isInputIdNamespaceable || !resId.IsNamespaceableKind() || resId.IsNsEquals(inputId)) &&
-			r.InSameKustomizeCtx(rctxm) {
+		if (!isInputIdNamespaceable || !resId.IsNamespaceableKind() || resId.IsNsEquals(inputId) ||
+			subjectNamespaces[r.GetNamespace()]) && r.InSameKustomizeCtx(rctxm) {
 			result.append(r)
 		}
 	}
 	return result
 }
 
+// getNamespacesForRoleBinding returns referenced namespace map if the inputRes is
+// a RoleBinding and the subject is ServiceAccount in another namespace. Otherwise it returns
+// {}.
+func getNamespacesForRoleBinding(inputRes *resource.Resource) map[string]bool {
+	res := make(map[string]bool)
+	if inputRes.GetKind() != "RoleBinding" {
+		return res
+	}
+	subjects, err := inputRes.GetSlice("subjects")
+	if err != nil || subjects == nil {
+		return res
+	}
+
+	for _, s := range subjects {
+		subject := s.(map[string]interface{})
+		if subject["namespace"] == nil || subject["kind"] == nil ||
+			subject["kind"].(string) != "ServiceAccount" {
+			continue
+		}
+		res[subject["namespace"].(string)] = true
+	}
+
+	return res
+}
+
 func (m *resWrangler) append(res *resource.Resource) {
 	m.rList = append(m.rList, res)
 }