Merge pull request #4312 from m-Bilal/fix-4240

Fixes 4240; added null check on namespace when resource is a RoleBinding

api/internal/accumulator/namereferencetransformer.go

@@ -52,7 +52,10 @@ func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error {
 	fMap := t.determineFilters(m.Resources())
 	debug(fMap)
 	for r, fList := range fMap {
-		c := m.SubsetThatCouldBeReferencedByResource(r)
+		c, err := m.SubsetThatCouldBeReferencedByResource(r)
+		if err != nil {
+			return err
+		}
 		for _, f := range fList {
 			f.Referrer = r
 			f.ReferralCandidates = c

api/krusty/blankvalues_test.go

@@ -0,0 +1,49 @@
+package krusty_test
+
+import (
+	"strings"
+	"testing"
+
+	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
+)
+
+// test for https://github.com/kubernetes-sigs/kustomize/issues/4240
+func TestBlankNamespace4240(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK(".", `
+resources:
+- resource.yaml
+`)
+
+	th.WriteF("resource.yaml", `
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: test
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: test
+rules: []
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: test
+subjects:
+- kind: ServiceAccount
+  name: test
+  namespace:
+roleRef:
+  kind: Role
+  name: test
+  apiGroup: rbac.authorization.k8s.io	
+`)
+
+	err := th.RunWithErr(".", th.MakeDefaultOptions())
+	if !strings.Contains(err.Error(), "Invalid Input: namespace is blank for resource") {
+		t.Fatalf("unexpected error: %q", err)
+	}
+}

api/resmap/resmap.go

@@ -224,7 +224,7 @@ type ResMap interface {
 	// This is a filter; it excludes things that cannot be
 	// referenced by the resource, e.g. objects in other
 	// namespaces. Cluster wide objects are never excluded.
-	SubsetThatCouldBeReferencedByResource(*resource.Resource) ResMap
+	SubsetThatCouldBeReferencedByResource(*resource.Resource) (ResMap, error)
 
 	// DeAnchor replaces YAML aliases with structured data copied from anchors.
 	// This cannot be undone; if desired, call DeepCopy first.

api/resmap/reswrangler.go

@@ -389,14 +389,17 @@ func (m *resWrangler) makeCopy(copier resCopier) ResMap {
 
 // SubsetThatCouldBeReferencedByResource implements ResMap.
 func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
-	referrer *resource.Resource) ResMap {
+	referrer *resource.Resource) (ResMap, error) {
 	referrerId := referrer.CurId()
 	if referrerId.IsClusterScoped() {
 		// A cluster scoped resource can refer to anything.
-		return m
+		return m, nil
 	}
 	result := newOne()
-	roleBindingNamespaces := getNamespacesForRoleBinding(referrer)
+	roleBindingNamespaces, err := getNamespacesForRoleBinding(referrer)
+	if err != nil {
+		return nil, err
+	}
 	for _, possibleTarget := range m.rList {
 		id := possibleTarget.CurId()
 		if id.IsClusterScoped() {
@@ -416,32 +419,36 @@ func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
 			result.append(possibleTarget)
 		}
 	}
-	return result
+	return result, nil
 }
 
 // getNamespacesForRoleBinding returns referenced ServiceAccount namespaces
 // if the resource is a RoleBinding
-func getNamespacesForRoleBinding(r *resource.Resource) map[string]bool {
+func getNamespacesForRoleBinding(r *resource.Resource) (map[string]bool, error) {
 	result := make(map[string]bool)
 	if r.GetKind() != "RoleBinding" {
-		return result
+		return result, nil
 	}
 	//nolint staticcheck
 	subjects, err := r.GetSlice("subjects")
 	if err != nil || subjects == nil {
-		return result
+		return result, nil
 	}
 	for _, s := range subjects {
 		subject := s.(map[string]interface{})
 		if ns, ok1 := subject["namespace"]; ok1 {
 			if kind, ok2 := subject["kind"]; ok2 {
 				if kind.(string) == "ServiceAccount" {
-					result[ns.(string)] = true
+					if n, ok3 := ns.(string); ok3 {
+						result[n] = true
+					} else {
+						return nil, errors.New(fmt.Sprintf("Invalid Input: namespace is blank for resource %q\n", r.CurId()))
+					}
 				}
 			}
 		}
 	}
-	return result
+	return result, nil
 }
 
 // AppendAll implements ResMap.

api/resmap/reswrangler_test.go

@@ -566,7 +566,10 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) {
 	for name, test := range tests {
 		test := test
 		t.Run(name, func(t *testing.T) {
-			got := m.SubsetThatCouldBeReferencedByResource(test.filter)
+			got, err1 := m.SubsetThatCouldBeReferencedByResource(test.filter)
+			if err1 != nil {
+				t.Fatalf("Expected error %v: ", err1)
+			}
 			err := test.expected.ErrorIfNotEqualLists(got)
 			if err != nil {
 				test.expected.Debug("expected")