Merge pull request #1221 from keleustes/mergens

ConfigMap Generators with identical name in different namespaces

pkg/commands/build/build.go

@@ -202,22 +202,42 @@ func NewCmdBuildPrune(
 
 func writeIndividualFiles(
 	fSys fs.FileSystem, folderPath string, m resmap.ResMap) error {
-	for _, res := range m.Resources() {
-		filename := filepath.Join(
-			folderPath,
-			fmt.Sprintf(
+
+	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()),
-			),
-		)
-		out, err := yaml.Marshal(res.Map())
-		if err != nil {
-			return err
-		}
-		err = fSys.WriteFile(filename, out)
-		if err != nil {
-			return err
+			)
+
+			// 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),
+				)
+			}
+
+			filename := filepath.Join(
+				folderPath,
+				basename,
+			)
+
+			out, err := yaml.Marshal(res.Map())
+			if err != nil {
+				return err
+			}
+			err = fSys.WriteFile(filename, out)
+			if err != nil {
+				return err
+			}
 		}
 	}
 	return nil

pkg/gvk/gvk.go

@@ -160,23 +160,39 @@ func (x Gvk) IsSelected(selector *Gvk) bool {
 	return true
 }
 
-var clusterLevelKinds = []string{
+var notNamespaceableKinds = []string{
 	"APIService",
-	"ClusterRoleBinding",
+	"CSIDriver",
+	"CSINode",
+	"CertificateSigningRequest",
 	"ClusterRole",
+	"ClusterRoleBinding",
+	"ComponentStatus",
 	"CustomResourceDefinition",
-	"Namespace",
-	"PersistentVolume",
 	"MutatingWebhookConfiguration",
+	"Namespace",
+	"Node",
+	"PersistentVolume",
+	"PodSecurityPolicy",
+	"PodSecurityPolicy",
+	"PriorityClass",
+	"RuntimeClass",
+	"SelfSubjectAccessReview",
+	"SelfSubjectRulesReview",
+	"StorageClass",
+	"SubjectAccessReview",
+	"TokenReview",
 	"ValidatingWebhookConfiguration",
+	"VolumeAttachment",
 }
 
-// IsClusterKind returns true if x is a cluster-level Gvk
-func (x Gvk) IsClusterKind() bool {
-	for _, k := range clusterLevelKinds {
+// IsNamespaceableKind returns true if x is a namespeable 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 {
 		if k == x.Kind {
-			return true
+			return false
 		}
 	}
-	return false
+	return true
 }

pkg/resid/resid.go

@@ -93,5 +93,25 @@ func (id ResId) GvknEquals(o ResId) bool {
 // Equals returns true if the other id matches
 // namespace/Group/Version/Kind/name.
 func (id ResId) Equals(o ResId) bool {
-	return id.Namespace == o.Namespace && id.GvknEquals(o)
+	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.
+func (id ResId) IsNsEquals(o ResId) bool {
+	return id.Namespace == o.Namespace ||
+		(id.IsInDefaultNs() && o.IsInDefaultNs()) ||
+		(!id.IsNamespaceable() && !o.IsNamespaceable())
+}
+
+// IsInDefaultNs returns true if id is a namespable ResId and the Namespace
+// is either not set or set to "default"
+func (id ResId) IsInDefaultNs() bool {
+	return id.IsNamespaceable() && (id.Namespace == "" || id.Namespace == "default")
+}
+
+// IsNamespaceable returns true if id is a namespable ResId
+func (id ResId) IsNamespaceable() bool {
+	return id.IsNamespaceableKind()
 }

pkg/resmap/resmap.go

@@ -93,6 +93,13 @@ 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() map[string][]*resource.Resource
+
 	// AllIds returns all CurrentIds.
 	AllIds() []resid.ResId
 
@@ -343,6 +350,30 @@ func (m *resWrangler) GetById(id resid.ResId) (*resource.Resource, error) {
 	return m.GetByCurrentId(id)
 }
 
+// GroupedByNamespace implements ResMap.GroupByNamespace
+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"
+			}
+		}
+
+		if _, found := byNamespace[namespace]; !found {
+			byNamespace[namespace] = make([]*resource.Resource, 0)
+		}
+
+		byNamespace[namespace] = append(byNamespace[namespace], res)
+	}
+
+	return byNamespace
+}
+
 // AsYaml implements ResMap.
 func (m *resWrangler) AsYaml() ([]byte, error) {
 	firstObj := true
@@ -458,12 +489,12 @@ func (m *resWrangler) makeCopy(copier resCopier) ResMap {
 func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
 	inputRes *resource.Resource) ResMap {
 	inputId := inputRes.OrgId()
-	if inputId.IsClusterKind() {
+	if !inputId.IsNamespaceableKind() {
 		return m
 	}
 	result := New()
 	for _, r := range m.Resources() {
-		if r.OrgId().IsClusterKind() || inputRes.InSameFuzzyNamespace(r) {
+		if !r.OrgId().IsNamespaceableKind() || inputRes.InSameFuzzyNamespace(r) {
 			err := result.Append(r)
 			if err != nil {
 				panic(err)
@@ -504,7 +535,7 @@ func (m *resWrangler) appendReplaceOrMerge(
 	res *resource.Resource) error {
 	id := res.CurId()
 	// Maybe also try by current id if nothing matches?
-	matches := m.GetMatchingResourcesByOriginalId(id.GvknEquals)
+	matches := m.GetMatchingResourcesByOriginalId(id.Equals)
 	switch len(matches) {
 	case 0:
 		switch res.Behavior() {

pkg/target/generatormergeandreplace_test.go

@@ -486,18 +486,108 @@ func TestGeneratingIntoNamespaces(t *testing.T) {
 	th.WriteK("/app", `
 configMapGenerator:
 - name: test
-  namespace: bob
+  namespace: default
+  literals:
+    - key=value
+- name: test
+  namespace: kube-system
+  literals:
+    - key=value
+secretGenerator:
+- name: test
+  namespace: default
+  literals:
+  - username=admin
+  - password=somepw
+- name: test
+  namespace: kube-system
+  literals:
+  - username=admin
+  - password=somepw
+`)
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
+		t.Fatalf("Err: %v", err)
+	}
+	th.AssertActualEqualsExpected(m, `
+apiVersion: v1
+data:
+  key: value
+kind: ConfigMap
+metadata:
+  name: test-t5t4md8fdm
+  namespace: default
+---
+apiVersion: v1
+data:
+  key: value
+kind: ConfigMap
+metadata:
+  name: test-t5t4md8fdm
+  namespace: kube-system
+---
+apiVersion: v1
+data:
+  password: c29tZXB3
+  username: YWRtaW4=
+kind: Secret
+metadata:
+  name: test-h65t9hg6kc
+  namespace: default
+type: Opaque
+---
+apiVersion: v1
+data:
+  password: c29tZXB3
+  username: YWRtaW4=
+kind: Secret
+metadata:
+  name: test-h65t9hg6kc
+  namespace: kube-system
+type: Opaque
+`)
+}
+
+// Valid that conflict is detected is the name are identical
+// and namespace left to default
+func TestConfigMapGeneratingIntoSameNamespace(t *testing.T) {
+	th := kusttest_test.NewKustTestHarness(t, "/app")
+	th.WriteK("/app", `
+configMapGenerator:
+- name: test
+  namespace: default
   literals:
   - key=value
 - name: test
-  namespace: kube-system
   literals:
   - key=value
 `)
 	_, err := th.MakeKustTarget().MakeCustomizedResMap()
-	// Document #1155
-	// This actually should be nil; it should work, and
-	// have some expected output.
+	if err == nil {
+		t.Fatalf("expected error")
+	}
+	if !strings.Contains(err.Error(), "must merge or replace") {
+		t.Fatalf("unexpected error %v", err)
+	}
+}
+
+// Valid that conflict is detected is the name are identical
+// and namespace left to default
+func TestSecretGeneratingIntoSameNamespace(t *testing.T) {
+	th := kusttest_test.NewKustTestHarness(t, "/app")
+	th.WriteK("/app", `
+secretGenerator:
+- name: test
+  namespace: default
+  literals:
+  - username=admin
+  - password=somepw
+- name: test
+  literals:
+  - username=admin
+  - password=somepw
+`)
+	_, err := th.MakeKustTarget().MakeCustomizedResMap()
 	if err == nil {
 		t.Fatalf("expected error")
 	}

plugin/builtin/NamespaceTransformer.go

@@ -62,7 +62,7 @@ const metaNamespace = "metadata/namespace"
 // object itself doesn't live in a namespace).
 func doIt(id resid.ResId, fs *config.FieldSpec) bool {
 	return fs.Path != metaNamespace ||
-		(fs.Path == metaNamespace && !id.IsClusterKind())
+		(fs.Path == metaNamespace && id.IsNamespaceableKind())
 }
 
 func (p *NamespaceTransformerPlugin) changeNamespace(

plugin/builtin/namespacetransformer/NamespaceTransformer.go

@@ -63,7 +63,7 @@ const metaNamespace = "metadata/namespace"
 // object itself doesn't live in a namespace).
 func doIt(id resid.ResId, fs *config.FieldSpec) bool {
 	return fs.Path != metaNamespace ||
-		(fs.Path == metaNamespace && !id.IsClusterKind())
+		(fs.Path == metaNamespace && id.IsNamespaceableKind())
 }
 
 func (p *plugin) changeNamespace(