Fix some minor nits around namespace code.

pkg/commands/build/build.go

@@ -4,7 +4,6 @@
 package build
 
 import (
-	"fmt"
 	"io"
 	"path/filepath"
 	"strings"
@@ -18,6 +17,7 @@ import (
 	"sigs.k8s.io/kustomize/v3/pkg/pgmconfig"
 	"sigs.k8s.io/kustomize/v3/pkg/plugins"
 	"sigs.k8s.io/kustomize/v3/pkg/resmap"
+	"sigs.k8s.io/kustomize/v3/pkg/resource"
 	"sigs.k8s.io/kustomize/v3/pkg/target"
 	"sigs.k8s.io/kustomize/v3/plugin/builtin"
 	"sigs.k8s.io/yaml"
@@ -202,43 +202,38 @@ func NewCmdBuildPrune(
 
 func writeIndividualFiles(
 	fSys fs.FileSystem, folderPath string, m resmap.ResMap) error {
-
 	byNamespace := m.GroupedByNamespace()
-	nsNeeded := len(byNamespace) > 1
-	for namespace, nresources := range byNamespace {
-		for _, res := range nresources {
-			basename := fmt.Sprintf(
-				"%s_%s.yaml",
-				strings.ToLower(res.GetGvk().String()),
-				strings.ToLower(res.GetName()),
-			)
-
-			// Preserve backward compatibility with kustomize 2.1.0.
-			// No need to cluter filename with namespace if all the objects
-			// are in the same namespace. The not namespaceable objects
-			// are grouped in the "%no_namespace%" bucket.
-			if (nsNeeded) && (namespace != "%no_namespace%") {
-				basename = fmt.Sprintf(
-					"%s_%s",
-					strings.ToLower(namespace),
-					strings.ToLower(basename),
-				)
+	for namespace, resList := range byNamespace {
+		for _, res := range resList {
+			fName := fileName(res)
+			if len(byNamespace) > 1 {
+				fName = strings.ToLower(namespace) + "_" + fName
 			}
-
-			filename := filepath.Join(
-				folderPath,
-				basename,
-			)
-
-			out, err := yaml.Marshal(res.Map())
-			if err != nil {
-				return err
-			}
-			err = fSys.WriteFile(filename, out)
+			err := writeFile(fSys, folderPath, fName, res)
 			if err != nil {
 				return err
 			}
 		}
 	}
+	for _, res := range m.NonNamespaceable() {
+		err := writeFile(fSys, folderPath, fileName(res), res)
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
+
+func fileName(res *resource.Resource) string {
+	return strings.ToLower(res.GetGvk().String()) +
+		"_" + strings.ToLower(res.GetName()) + ".yaml"
+}
+
+func writeFile(
+	fSys fs.FileSystem, path, fName string, res *resource.Resource) error {
+	out, err := yaml.Marshal(res.Map())
+	if err != nil {
+		return err
+	}
+	return fSys.WriteFile(filepath.Join(path, fName), out)
+}

pkg/gvk/gvk.go

@@ -186,7 +186,7 @@ var notNamespaceableKinds = []string{
 	"VolumeAttachment",
 }
 
-// IsNamespaceableKind returns true if x is a namespeable Gvk
+// IsNamespaceableKind returns true if x is a namespaceable Gvk
 // Implements https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#not-all-objects-are-in-a-namespace
 func (x Gvk) IsNamespaceableKind() bool {
 	for _, k := range notNamespaceableKinds {

pkg/resid/resid.go

@@ -41,9 +41,11 @@ func NewResIdKindOnly(k string, n string) ResId {
 }
 
 const (
-	noNamespace = "~X"
-	noName      = "~N"
-	separator   = "|"
+	noNamespace          = "~X"
+	noName               = "~N"
+	separator            = "|"
+	TotallyNotANamespace = "_non_namespaceable_"
+	DefaultNamespace     = "default"
 )
 
 // String of ResId based on GVK, name and prefix
@@ -96,22 +98,32 @@ func (id ResId) Equals(o ResId) bool {
 	return id.IsNsEquals(o) && id.GvknEquals(o)
 }
 
-// IsNsEquals returns true if the other id matches namespace
-// or both are in the default namespace
-// or both are not namespaceable id.
+// IsNsEquals returns true if the id is in
+// the same effective namespace.
 func (id ResId) IsNsEquals(o ResId) bool {
-	return id.Namespace == o.Namespace ||
-		(id.IsInDefaultNs() && o.IsInDefaultNs()) ||
-		(!id.IsNamespaceable() && !o.IsNamespaceable())
+	return id.EffectiveNamespace() == o.EffectiveNamespace()
 }
 
-// IsInDefaultNs returns true if id is a namespable ResId and the Namespace
-// is either not set or set to "default"
+// IsInDefaultNs returns true if id is a namespaceable
+// ResId and the Namespace is either not set or set
+// to DefaultNamespace.
 func (id ResId) IsInDefaultNs() bool {
-	return id.IsNamespaceable() && (id.Namespace == "" || id.Namespace == "default")
+	return id.IsNamespaceableKind() && id.isPutativelyDefaultNs()
 }
 
-// IsNamespaceable returns true if id is a namespable ResId
-func (id ResId) IsNamespaceable() bool {
-	return id.IsNamespaceableKind()
+func (id ResId) isPutativelyDefaultNs() bool {
+	return id.Namespace == "" || id.Namespace == DefaultNamespace
+}
+
+// EffectiveNamespace returns a non-ambiguous, non-empty
+// namespace for use in reporting and equality tests.
+func (id ResId) EffectiveNamespace() string {
+	// The order of these checks matters.
+	if !id.IsNamespaceableKind() {
+		return TotallyNotANamespace
+	}
+	if id.isPutativelyDefaultNs() {
+		return DefaultNamespace
+	}
+	return id.Namespace
 }

pkg/resid/resid_test.go

@@ -161,104 +161,169 @@ func TestGvknString(t *testing.T) {
 	}
 }
 
-var GvknEqualsTest = []struct {
-	id1          ResId
-	id2          ResId
-	gVknResult   bool
-	nSgVknResult bool
-}{
-	{
-		id1: ResId{
-			Namespace: "X",
-			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		id2: ResId{
-			Namespace: "X",
-			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		gVknResult:   true,
-		nSgVknResult: true,
-	},
-	{
-		id1: ResId{
-			Namespace: "X",
-			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		id2: ResId{
-			Namespace: "Z",
-			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		gVknResult:   true,
-		nSgVknResult: false,
-	},
-	{
-		id1: ResId{
-			Namespace: "X",
-			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		id2: ResId{
-			Gvk:  gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
-			Name: "nm",
-		},
-		gVknResult:   true,
-		nSgVknResult: false,
-	},
-	{
-		id1: ResId{
-			Namespace: "X",
-			Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		id2: ResId{
-			Namespace: "Z",
-			Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
-			Name:      "nm",
-		},
-		gVknResult:   true,
-		nSgVknResult: false,
-	},
-	{
-		id1: ResId{
-			Namespace: "X",
-			Gvk:       gvk.Gvk{Kind: "k"},
-			Name:      "nm",
-		},
-		id2: ResId{
-			Namespace: "Z",
-			Gvk:       gvk.Gvk{Kind: "k"},
-			Name:      "nm",
-		},
-		gVknResult:   true,
-		nSgVknResult: false,
-	},
-	{
-		id1: ResId{
-			Namespace: "X",
-			Name:      "nm",
-		},
-		id2: ResId{
-			Namespace: "Z",
-			Name:      "nm",
-		},
-		gVknResult:   true,
-		nSgVknResult: false,
-	},
-}
-
 func TestEquals(t *testing.T) {
+
+	var GvknEqualsTest = []struct {
+		id1        ResId
+		id2        ResId
+		gVknResult bool
+		nsEquals   bool
+		equals     bool
+	}{
+		{
+			id1: ResId{
+				Namespace: "X",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			id2: ResId{
+				Namespace: "X",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			gVknResult: true,
+			nsEquals:   true,
+			equals:     true,
+		},
+		{
+			id1: ResId{
+				Namespace: "X",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			id2: ResId{
+				Namespace: "Z",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			gVknResult: true,
+			nsEquals:   false,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Namespace: "X",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			id2: ResId{
+				Gvk:  gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name: "nm",
+			},
+			gVknResult: true,
+			nsEquals:   false,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Namespace: "X",
+				Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			id2: ResId{
+				Namespace: "Z",
+				Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			gVknResult: true,
+			nsEquals:   false,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Namespace: "X",
+				Gvk:       gvk.Gvk{Kind: "k"},
+				Name:      "nm",
+			},
+			id2: ResId{
+				Namespace: "Z",
+				Gvk:       gvk.Gvk{Kind: "k"},
+				Name:      "nm",
+			},
+			gVknResult: true,
+			nsEquals:   false,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Gvk:  gvk.Gvk{Kind: "k"},
+				Name: "nm",
+			},
+			id2: ResId{
+				Gvk:  gvk.Gvk{Kind: "k"},
+				Name: "nm2",
+			},
+			gVknResult: false,
+			nsEquals:   true,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Gvk:  gvk.Gvk{Kind: "k"},
+				Name: "nm",
+			},
+			id2: ResId{
+				Gvk:  gvk.Gvk{Kind: "Node"},
+				Name: "nm",
+			},
+			gVknResult: false,
+			nsEquals:   false,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Gvk:  gvk.Gvk{Kind: "Node"},
+				Name: "nm1",
+			},
+			id2: ResId{
+				Gvk:  gvk.Gvk{Kind: "Node"},
+				Name: "nm2",
+			},
+			gVknResult: false,
+			nsEquals:   true,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Namespace: "default",
+				Gvk:       gvk.Gvk{Kind: "k"},
+				Name:      "nm1",
+			},
+			id2: ResId{
+				Gvk:  gvk.Gvk{Kind: "k"},
+				Name: "nm2",
+			},
+			gVknResult: false,
+			nsEquals:   true,
+			equals:     false,
+		},
+		{
+			id1: ResId{
+				Namespace: "X",
+				Name:      "nm",
+			},
+			id2: ResId{
+				Namespace: "Z",
+				Name:      "nm",
+			},
+			gVknResult: true,
+			nsEquals:   false,
+			equals:     false,
+		},
+	}
+
 	for _, tst := range GvknEqualsTest {
 		if tst.id1.GvknEquals(tst.id2) != tst.gVknResult {
 			t.Fatalf("GvknEquals(\n%v,\n%v\n) should be %v",
 				tst.id1, tst.id2, tst.gVknResult)
 		}
-		if tst.id1.Equals(tst.id2) != tst.nSgVknResult {
-			t.Fatalf("NsGvknEquals(\n%v,\n%v\n) should be %v",
-				tst.id1, tst.id2, tst.nSgVknResult)
+		if tst.id1.IsNsEquals(tst.id2) != tst.nsEquals {
+			t.Fatalf("IsNsEquals(\n%v,\n%v\n) should be %v",
+				tst.id1, tst.id2, tst.equals)
+		}
+		if tst.id1.Equals(tst.id2) != tst.equals {
+			t.Fatalf("Equals(\n%v,\n%v\n) should be %v",
+				tst.id1, tst.id2, tst.equals)
 		}
 	}
 }
@@ -305,3 +370,56 @@ func TestFromString(t *testing.T) {
 		}
 	}
 }
+
+func TestEffectiveNamespace(t *testing.T) {
+	var test = []struct {
+		id       ResId
+		expected string
+	}{
+		{
+			id: ResId{
+				Gvk:  gvk.Gvk{Group: "g", Version: "v", Kind: "Node"},
+				Name: "nm",
+			},
+			expected: TotallyNotANamespace,
+		},
+		{
+			id: ResId{
+				Namespace: "foo",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "Node"},
+				Name:      "nm",
+			},
+			expected: TotallyNotANamespace,
+		},
+		{
+			id: ResId{
+				Namespace: "foo",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			expected: "foo",
+		},
+		{
+			id: ResId{
+				Namespace: "",
+				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name:      "nm",
+			},
+			expected: DefaultNamespace,
+		},
+		{
+			id: ResId{
+				Gvk:  gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
+				Name: "nm",
+			},
+			expected: DefaultNamespace,
+		},
+	}
+
+	for _, tst := range test {
+		if actual := tst.id.EffectiveNamespace(); actual != tst.expected {
+			t.Fatalf("EffectiveNamespace was %s, expected %s",
+				actual, tst.expected)
+		}
+	}
+}

pkg/resmap/resmap.go

@@ -93,13 +93,19 @@ type ResMap interface {
 	// Same as GetByOriginalId.
 	GetById(resid.ResId) (*resource.Resource, error)
 
-	// GroupedByNamespace provides map of discardable slice
-	// of resource pointer
-	// The not namespaceable resources are added to the "cluster-wide" key.
-	// The resources in the default namespace are added to the "default" key.
-	// The rest of the resources are grouped in their respectiv namespace.
+	// GroupedByNamespace returns a map of namespace
+	// to a slice of *Resource in that namespace.
+	// Resources for whom IsNamespaceableKind is false are
+	// are not included at all (see NonNamespaceable).
+	// Resources with an empty namespace are placed
+	// in the resid.DefaultNamespace entry.
 	GroupedByNamespace() map[string][]*resource.Resource
 
+	// NonNamespaceable returns a slice of resources that
+	// cannot be placed in a namespace, e.g.
+	// Node, ClusterRole, Namespace itself, etc.
+	NonNamespaceable() []*resource.Resource
+
 	// AllIds returns all CurrentIds.
 	AllIds() []resid.ResId
 
@@ -352,25 +358,25 @@ func (m *resWrangler) GetById(id resid.ResId) (*resource.Resource, error) {
 
 // GroupedByNamespace implements ResMap.GroupByNamespace
 func (m *resWrangler) GroupedByNamespace() map[string][]*resource.Resource {
+	items := m.groupedByNamespace()
+	delete(items, resid.TotallyNotANamespace)
+	return items
+}
+
+// NonNamespaceable implements ResMap.NonNamespaceable
+func (m *resWrangler) NonNamespaceable() []*resource.Resource {
+	return m.groupedByNamespace()[resid.TotallyNotANamespace]
+}
+
+func (m *resWrangler) groupedByNamespace() map[string][]*resource.Resource {
 	byNamespace := make(map[string][]*resource.Resource)
 	for _, res := range m.rList {
-		// Add to the notNamespaceable bucket by default.
-		namespace := "%no_namespace%"
-
-		if res.OrgId().IsNamespaceable() {
-			namespace = res.OrgId().Namespace
-			if res.OrgId().IsInDefaultNs() {
-				namespace = "default"
-			}
-		}
-
+		namespace := res.CurId().EffectiveNamespace()
 		if _, found := byNamespace[namespace]; !found {
-			byNamespace[namespace] = make([]*resource.Resource, 0)
+			byNamespace[namespace] = []*resource.Resource{}
 		}
-
 		byNamespace[namespace] = append(byNamespace[namespace], res)
 	}
-
 	return byNamespace
 }