Merge pull request #321 from Liujingfang1/multibases

filter by namespace, prefix and detect conflict when resolving name reference

pkg/commands/testdata/testcase-multibases-conflict/base/kustomization.yaml

@@ -0,0 +1,4 @@
+resources:
+- serviceaccount.yaml
+- rolebinding.yaml
+namePrefix: base-

pkg/commands/testdata/testcase-multibases-conflict/base/rolebinding.yaml

@@ -0,0 +1,11 @@
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: role
+subjects:
+- kind: ServiceAccount
+  name: serviceaccount

pkg/commands/testdata/testcase-multibases-conflict/base/serviceaccount.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: serviceaccount

pkg/commands/testdata/testcase-multibases-conflict/combined/kustomization.yaml

@@ -0,0 +1,3 @@
+bases:
+- ../overlays/a
+- ../overlays/b

pkg/commands/testdata/testcase-multibases-conflict/overlays/a/kustomization.yaml

@@ -0,0 +1,7 @@
+bases:
+- ../../base/
+
+namePrefix: a-
+
+resources:
+- serviceaccount.yaml

pkg/commands/testdata/testcase-multibases-conflict/overlays/a/serviceaccount.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: serviceaccount

pkg/commands/testdata/testcase-multibases-conflict/overlays/b/kustomization.yaml

@@ -0,0 +1,4 @@
+bases:
+- ../../base/
+
+namePrefix: b-

pkg/commands/testdata/testcase-multibases-conflict/test.yaml

@@ -0,0 +1,4 @@
+description: multibases with name reference
+args: []
+filename: testdata/testcase-multibases-conflict/combined
+expectedError: detected conflicts when resolving name references serviceaccount

pkg/commands/testdata/testcase-multibases-nonconflict/base/kustomization.yaml

@@ -0,0 +1,4 @@
+resources:
+- serviceaccount.yaml
+- rolebinding.yaml
+namePrefix: base-

pkg/commands/testdata/testcase-multibases-nonconflict/base/rolebinding.yaml

@@ -0,0 +1,11 @@
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: role
+subjects:
+- kind: ServiceAccount
+  name: serviceaccount

pkg/commands/testdata/testcase-multibases-nonconflict/base/serviceaccount.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: serviceaccount

pkg/commands/testdata/testcase-multibases-nonconflict/combined/kustomization.yaml

@@ -0,0 +1,3 @@
+bases:
+- ../overlays/a
+- ../overlays/b

pkg/commands/testdata/testcase-multibases-nonconflict/expected.yaml

@@ -0,0 +1,33 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: a-base-serviceaccount
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: b-base-serviceaccount
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: a-base-rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: role
+subjects:
+- kind: ServiceAccount
+  name: a-base-serviceaccount
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: b-base-rolebinding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: role
+subjects:
+- kind: ServiceAccount
+  name: b-base-serviceaccount

pkg/commands/testdata/testcase-multibases-nonconflict/overlays/a/kustomization.yaml

@@ -0,0 +1,4 @@
+bases:
+- ../../base/
+
+namePrefix: a-

pkg/commands/testdata/testcase-multibases-nonconflict/overlays/b/kustomization.yaml

@@ -0,0 +1,4 @@
+bases:
+- ../../base/
+
+namePrefix: b-

pkg/commands/testdata/testcase-multibases-nonconflict/test.yaml

@@ -0,0 +1,4 @@
+description: multibases with name reference
+args: []
+filename: testdata/testcase-multibases-nonconflict/combined
+expectedStdout: testdata/testcase-multibases-nonconflict/expected.yaml

pkg/commands/testdata/testcase-variable-ref/expected.yaml

@@ -78,16 +78,10 @@ metadata:
 apiVersion: v1
 kind: Service
 metadata:
-  annotations:
-    prometheus.io/path: _status/vars
-    prometheus.io/port: "8080"
-    prometheus.io/scrape: "true"
-    service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"
   labels:
     app: cockroachdb
-  name: dev-base-cockroachdb
+  name: dev-base-cockroachdb-public
 spec:
-  clusterIP: None
   ports:
   - name: grpc
     port: 26257
@@ -101,10 +95,16 @@ spec:
 apiVersion: v1
 kind: Service
 metadata:
+  annotations:
+    prometheus.io/path: _status/vars
+    prometheus.io/port: "8080"
+    prometheus.io/scrape: "true"
+    service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"
   labels:
     app: cockroachdb
-  name: dev-base-cockroachdb-public
+  name: dev-base-cockroachdb
 spec:
+  clusterIP: None
   ports:
   - name: grpc
     port: 26257

pkg/resmap/idslice.go

@@ -34,7 +34,7 @@ func (a IdSlice) Less(i, j int) bool {
 	if a[i].Gvk().String() != a[j].Gvk().String() {
 		return gvkLess(a[i].Gvk(), a[j].Gvk())
 	}
-	return a[i].Name() < a[j].Name()
+	return a[i].String() < a[j].String()
 }
 
 var order = []string{"Namespace", "CustomResourceDefinition", "ServiceAccount",

pkg/resmap/resmap.go

@@ -140,6 +140,18 @@ func (m ResMap) insert(newName string, obj *unstructured.Unstructured) error {
 	return nil
 }
 
+// FilterBy returns a ResMap containing ResIds with the same namespace and nameprefix
+// with the inputId
+func (m ResMap) FilterBy(inputId resource.ResId) ResMap {
+	result := ResMap{}
+	for id, res := range m {
+		if id.Namespace() == inputId.Namespace() && id.HasSameLeftmostPrefix(inputId) {
+			result[id] = res
+		}
+	}
+	return result
+}
+
 // NewResourceSliceFromPatches returns a slice of resources given a patch path slice from a kustomization file.
 func NewResourceSliceFromPatches(
 	loader loader.Loader, paths []patch.PatchStrategicMerge) ([]*resource.Resource, error) {

pkg/resource/resid.go

@@ -97,10 +97,32 @@ func (n ResId) Namespace() string {
 
 // CopyWithNewPrefix make a new copy from current ResId and append a new prefix
 func (n ResId) CopyWithNewPrefix(p string) ResId {
-	return ResId{gvk: n.gvk, name: n.name, prefix: p + n.prefix, namespace: n.namespace}
+	return ResId{gvk: n.gvk, name: n.name, prefix: n.concatPrefix(p), namespace: n.namespace}
 }
 
 // CopyWithNewNamespace make a new copy from current ResId and set a new namespace
 func (n ResId) CopyWithNewNamespace(ns string) ResId {
 	return ResId{gvk: n.gvk, name: n.name, prefix: n.prefix, namespace: ns}
 }
+
+// HasSameLeftmostPrefix check if two ResIds have the same
+// left most prefix.
+func (n ResId) HasSameLeftmostPrefix(id ResId) bool {
+	prefixes1 := n.prefixList()
+	prefixes2 := id.prefixList()
+	return prefixes1[0] == prefixes2[0]
+}
+
+func (n ResId) concatPrefix(p string) string {
+	if p == "" {
+		return n.prefix
+	}
+	if n.prefix == "" {
+		return p
+	}
+	return p + ":" + n.prefix
+}
+
+func (n ResId) prefixList() []string {
+	return strings.Split(n.prefix, ":")
+}

pkg/transformers/namereference.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/kubernetes-sigs/kustomize/pkg/resmap"
+	"github.com/kubernetes-sigs/kustomize/pkg/resource"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
@@ -58,7 +59,7 @@ func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error {
 					continue
 				}
 				err := mutateField(objMap, path.Path, path.CreateIfNotPresent,
-					o.updateNameReference(referencePathConfig.referencedGVK, m))
+					o.updateNameReference(referencePathConfig.referencedGVK, m.FilterBy(id)))
 				if err != nil {
 					return err
 				}
@@ -81,9 +82,21 @@ func (o *nameReferenceTransformer) updateNameReference(
 				continue
 			}
 			if id.Name() == s {
+				err := o.detectConflict(id, m, s)
+				if err != nil {
+					return nil, err
+				}
 				return res.GetName(), nil
 			}
 		}
 		return in, nil
 	}
 }
+
+func (o *nameReferenceTransformer) detectConflict(id resource.ResId, m resmap.ResMap, name string) error {
+	matchedIds := m.FindByGVKN(id)
+	if len(matchedIds) > 1 {
+		return fmt.Errorf("detected conflicts when resolving name references %s:\n%v", name, matchedIds)
+	}
+	return nil
+}