Merge pull request #1378 from keleustes/nameprefix

Name Prefix and Suffix Transformation for multi level level kustomize context
2026-06-29 09:40:49 +00:00 · 2019-07-24 11:33:21 -07:00
parent 5a73f345fd 230090d790
commit ffd95ef5a9
6 changed files with 317 additions and 61 deletions
--- a/pkg/resmap/resmap.go
+++ b/pkg/resmap/resmap.go
@@ -552,26 +552,17 @@ func (m *resWrangler) makeCopy(copier resCopier) ResMap {
 // SubsetThatCouldBeReferencedByResource implements ResMap.
 func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
 	inputRes *resource.Resource) ResMap {
-	inputId := inputRes.OrgId()
-	if !inputId.IsNamespaceableKind() {
-		if inputRes.GetOutermostNamePrefix() == "" {
-			return m
-		}
-		result := New()
-		for _, r := range m.Resources() {
-			if r.GetOutermostNamePrefix() == inputRes.GetOutermostNamePrefix() &&
-				r.GetOutermostNameSuffix() == inputRes.GetOutermostNameSuffix() {
-				err := result.Append(r)
-				if err != nil {
-					panic(err)
-				}
-			}
-		}
-		return result
-	}
 	result := New()
+	inputId := inputRes.CurId()
+	isInputIdNamespaceable := inputId.IsNamespaceableKind()
+	rctxm := inputRes.PrefixesSuffixesEquals
 	for _, r := range m.Resources() {
-		if !r.OrgId().IsNamespaceableKind() || inputRes.InSameFuzzyNamespace(r) {
+		// 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) {
 			err := result.Append(r)
 			if err != nil {
 				panic(err)
--- a/pkg/resmap/resmap_test.go
+++ b/pkg/resmap/resmap_test.go
@@ -356,7 +356,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) {
 	r4 := rf.FromMap(
 		map[string]interface{}{
 			"apiVersion": "v1",
-			"kind":       "ConfigMap",
+			"kind":       "Deployment",
 			"metadata": map[string]interface{}{
 				"name":      "charlie",
 				"namespace": "happy",
@@ -365,7 +365,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) {
 	r5 := rf.FromMap(
 		map[string]interface{}{
 			"apiVersion": "v1",
-			"kind":       "Deployment",
+			"kind":       "ConfigMap",
 			"metadata": map[string]interface{}{
 				"name":      "charlie",
 				"namespace": "happy",
@@ -408,12 +408,12 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) {
 		"happy namespace no prefix": {
 			filter: r3,
 			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(r3).AddR(r4).AddR(r7).ResMap(),
+				AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap(),
 		},
 		"happy namespace with prefix": {
 			filter: r5,
 			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(r5).AddR(r6).AddR(r7).ResMap(),
+				AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap(),
 		},
 		"cluster level": {
 			filter: r7,
--- a/pkg/resource/resource.go
+++ b/pkg/resource/resource.go
@@ -27,6 +27,22 @@ type Resource struct {
 	nameSuffixes []string
 }

+// ResCtx is an interface describing the contextual added
+// kept kustomize in the context of each Resource object.
+// Currently mainly the name prefix and name suffix are added.
+type ResCtx interface {
+	AddNamePrefix(p string)
+	AddNameSuffix(s string)
+	GetOutermostNamePrefix() string
+	GetOutermostNameSuffix() string
+	GetNamePrefixes() []string
+	GetNameSuffixes() []string
+}
+
+// ResCtxMatcher returns true if two Resources are being
+// modified in the same kustomize context.
+type ResCtxMatcher func(ResCtx) bool
+
 // DeepCopy returns a new copy of resource
 func (r *Resource) DeepCopy() *Resource {
 	rc := &Resource{
@@ -104,14 +120,17 @@ func copyStringSlice(s []string) []string {
 	return c
 }

+// Implements ResCtx AddNamePrefix
 func (r *Resource) AddNamePrefix(p string) {
 	r.namePrefixes = append(r.namePrefixes, p)
 }

+// Implements ResCtx AddNameSuffix
 func (r *Resource) AddNameSuffix(s string) {
 	r.nameSuffixes = append(r.nameSuffixes, s)
 }

+// Implements ResCtx GetOutermostNamePrefix
 func (r *Resource) GetOutermostNamePrefix() string {
 	if len(r.namePrefixes) == 0 {
 		return ""
@@ -119,6 +138,7 @@ func (r *Resource) GetOutermostNamePrefix() string {
 	return r.namePrefixes[len(r.namePrefixes)-1]
 }

+// Implements ResCtx GetOutermostNameSuffix
 func (r *Resource) GetOutermostNameSuffix() string {
 	if len(r.nameSuffixes) == 0 {
 		return ""
@@ -126,10 +146,51 @@ func (r *Resource) GetOutermostNameSuffix() string {
 	return r.nameSuffixes[len(r.nameSuffixes)-1]
 }

-func (r *Resource) InSameFuzzyNamespace(o *Resource) bool {
-	return r.CurId().IsNsEquals(o.CurId()) &&
-		r.GetOutermostNamePrefix() == o.GetOutermostNamePrefix() &&
-		r.GetOutermostNameSuffix() == o.GetOutermostNameSuffix()
+func sameBeginningSubarray(a, b []string) bool {
+	maxlen := len(b)
+	if len(a) < len(b) {
+		maxlen = len(a)
+	}
+
+	if maxlen == 0 {
+		return true
+	}
+
+	for i, v := range a[0 : maxlen-1] {
+		if v != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+// Implements ResCtx GetNamePrefixes
+func (r *Resource) GetNamePrefixes() []string {
+	return r.namePrefixes
+}
+
+// Implements ResCtx GetNameSuffixes
+func (r *Resource) GetNameSuffixes() []string {
+	return r.nameSuffixes
+}
+
+// OutermostPrefixSuffixEquals returns true if both resources
+// outer suffix and prefix matches.
+func (r *Resource) OutermostPrefixSuffixEquals(o ResCtx) bool {
+	return (r.GetOutermostNamePrefix() == o.GetOutermostNamePrefix()) && (r.GetOutermostNameSuffix() == o.GetOutermostNameSuffix())
+}
+
+// PrefixesSuffixesEquals is conceptually doing the same task
+// as OutermostPrefixSuffix but performs a deeper comparison
+// of the the list of suffix and prefix.
+func (r *Resource) PrefixesSuffixesEquals(o ResCtx) bool {
+	return sameBeginningSubarray(r.GetNamePrefixes(), o.GetNamePrefixes()) && sameBeginningSubarray(r.GetNameSuffixes(), o.GetNameSuffixes())
+}
+
+// This is used to compute if a referrer could potentially be impacted
+// by the change of name of a referral.
+func (r *Resource) InSameKustomizeCtx(rctxm ResCtxMatcher) bool {
+	return rctxm(r)
 }

 func (r *Resource) GetOriginalName() string {
--- a/pkg/target/resourceconflict_test.go
+++ b/pkg/target/resourceconflict_test.go
@@ -4,7 +4,6 @@
 package target_test

 import (
-	"strings"
 	"testing"

 	"sigs.k8s.io/kustomize/v3/pkg/kusttest"
@@ -16,6 +15,7 @@ resources:
 - serviceaccount.yaml
 - rolebinding.yaml
 - clusterrolebinding.yaml
+- clusterrole.yaml
 namePrefix: pfx-
 nameSuffix: -sfx
 `)
@@ -32,7 +32,7 @@ metadata:
  name: rolebinding
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
+  kind: ClusterRole
  name: role
 subjects:
 - kind: ServiceAccount
@@ -45,11 +45,21 @@ metadata:
  name: rolebinding
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
+  kind: ClusterRole
  name: role
 subjects:
 - kind: ServiceAccount
  name: serviceaccount
+`)
+	th.WriteF("/app/base/clusterrole.yaml", `
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: role
+rules:
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["get", "watch", "list"]
 `)
 }

@@ -97,8 +107,8 @@ metadata:
  name: pfx-rolebinding-sfx
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: pfx-role-sfx
 subjects:
 - kind: ServiceAccount
  name: pfx-serviceaccount-sfx
@@ -109,11 +119,25 @@ metadata:
  name: pfx-rolebinding-sfx
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: pfx-role-sfx
 subjects:
 - kind: ServiceAccount
  name: pfx-serviceaccount-sfx
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: pfx-role-sfx
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
 `)
 }

@@ -137,8 +161,8 @@ metadata:
  name: a-pfx-rolebinding-sfx-suffixA
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: a-pfx-role-sfx-suffixA
 subjects:
 - kind: ServiceAccount
  name: a-pfx-serviceaccount-sfx-suffixA
@@ -149,11 +173,25 @@ metadata:
  name: a-pfx-rolebinding-sfx-suffixA
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: a-pfx-role-sfx-suffixA
 subjects:
 - kind: ServiceAccount
  name: a-pfx-serviceaccount-sfx-suffixA
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: a-pfx-role-sfx-suffixA
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
 `)
 }

@@ -177,8 +215,8 @@ metadata:
  name: b-pfx-rolebinding-sfx-suffixB
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: b-pfx-role-sfx-suffixB
 subjects:
 - kind: ServiceAccount
  name: b-pfx-serviceaccount-sfx-suffixB
@@ -189,11 +227,25 @@ metadata:
  name: b-pfx-rolebinding-sfx-suffixB
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: b-pfx-role-sfx-suffixB
 subjects:
 - kind: ServiceAccount
  name: b-pfx-serviceaccount-sfx-suffixB
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: b-pfx-role-sfx-suffixB
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
 `)
 }

@@ -218,8 +270,8 @@ metadata:
  name: a-pfx-rolebinding-sfx-suffixA
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: a-pfx-role-sfx-suffixA
 subjects:
 - kind: ServiceAccount
  name: a-pfx-serviceaccount-sfx-suffixA
@@ -230,12 +282,26 @@ metadata:
  name: a-pfx-rolebinding-sfx-suffixA
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: a-pfx-role-sfx-suffixA
 subjects:
 - kind: ServiceAccount
  name: a-pfx-serviceaccount-sfx-suffixA
 ---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: a-pfx-role-sfx-suffixA
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
+---
 apiVersion: v1
 kind: ServiceAccount
 metadata:
@@ -247,8 +313,8 @@ metadata:
  name: b-pfx-rolebinding-sfx-suffixB
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: b-pfx-role-sfx-suffixB
 subjects:
 - kind: ServiceAccount
  name: b-pfx-serviceaccount-sfx-suffixB
@@ -259,11 +325,25 @@ metadata:
  name: b-pfx-rolebinding-sfx-suffixB
 roleRef:
  apiGroup: rbac.authorization.k8s.io
-  kind: Role
-  name: role
+  kind: ClusterRole
+  name: b-pfx-role-sfx-suffixB
 subjects:
 - kind: ServiceAccount
  name: b-pfx-serviceaccount-sfx-suffixB
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: b-pfx-role-sfx-suffixB
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
 `)
 }

@@ -289,12 +369,100 @@ metadata:
  name: serviceaccount
 `)

-	_, err := th.MakeKustTarget().MakeCustomizedResMap()
-	if err == nil {
-		t.Fatalf("Expected resource conflict.")
-	}
-	if !strings.Contains(
-		err.Error(), "multiple matches for ~G_v1_ServiceAccount") {
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
 		t.Fatalf("Unexpected err: %v", err)
 	}
+	th.AssertActualEqualsExpected(m, `
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: a-serviceaccount-suffixA
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: a-pfx-serviceaccount-sfx-suffixA
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: a-pfx-rolebinding-sfx-suffixA
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: a-pfx-role-sfx-suffixA
+subjects:
+- kind: ServiceAccount
+  name: a-pfx-serviceaccount-sfx-suffixA
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: ClusterRoleBinding
+metadata:
+  name: a-pfx-rolebinding-sfx-suffixA
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: a-pfx-role-sfx-suffixA
+subjects:
+- kind: ServiceAccount
+  name: a-pfx-serviceaccount-sfx-suffixA
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: a-pfx-role-sfx-suffixA
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: b-pfx-serviceaccount-sfx-suffixB
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: RoleBinding
+metadata:
+  name: b-pfx-rolebinding-sfx-suffixB
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: b-pfx-role-sfx-suffixB
+subjects:
+- kind: ServiceAccount
+  name: b-pfx-serviceaccount-sfx-suffixB
+---
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: ClusterRoleBinding
+metadata:
+  name: b-pfx-rolebinding-sfx-suffixB
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: b-pfx-role-sfx-suffixB
+subjects:
+- kind: ServiceAccount
+  name: b-pfx-serviceaccount-sfx-suffixB
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: b-pfx-role-sfx-suffixB
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - watch
+  - list
+`)
 }
--- a/plugin/builtin/PrefixSuffixTransformer.go
+++ b/plugin/builtin/PrefixSuffixTransformer.go
@@ -49,22 +49,40 @@ func (p *PrefixSuffixTransformerPlugin) Config(
 }

 func (p *PrefixSuffixTransformerPlugin) Transform(m resmap.ResMap) error {
-	if len(p.Prefix) == 0 && len(p.Suffix) == 0 {
-		return nil
-	}
+
+	// Even if both the Prefix and Suffix are empty we want
+	// to proceed with the transformation. This allows to add contextual
+	// information to the resources (AddNamePrefix and AddNameSuffix).
+
 	for _, r := range m.Resources() {
 		if p.shouldSkip(r.OrgId()) {
+			// Don't change the actual definition
+			// of a CRD.
 			continue
 		}
 		id := r.OrgId()
+		// current default configuration contains
+		// only one entry: "metadata/name" with no GVK
 		for _, path := range p.FieldSpecs {
 			if !id.IsSelected(&path.Gvk) {
+				// With the currrent default configuration,
+				// because no Gvk is specified, so a wild
+				// card
 				continue
 			}
+
 			if smellsLikeANameChange(&path) {
+				// "metadata/name" is the only field.
+				// this will add a prefix and a suffix
+				// to the resource even if those are
+				// empty
 				r.AddNamePrefix(p.Prefix)
 				r.AddNameSuffix(p.Suffix)
 			}
+
+			// the addPrefixSuffix method will not
+			// change the name if both the prefix and suffix
+			// are empty.
 			err := transformers.MutateField(
 				r.Map(),
 				path.PathSlice(),
--- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go
+++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go
@@ -50,22 +50,40 @@ func (p *plugin) Config(
 }

 func (p *plugin) Transform(m resmap.ResMap) error {
-	if len(p.Prefix) == 0 && len(p.Suffix) == 0 {
-		return nil
-	}
+
+	// Even if both the Prefix and Suffix are empty we want
+	// to proceed with the transformation. This allows to add contextual
+	// information to the resources (AddNamePrefix and AddNameSuffix).
+
 	for _, r := range m.Resources() {
 		if p.shouldSkip(r.OrgId()) {
+			// Don't change the actual definition
+			// of a CRD.
 			continue
 		}
 		id := r.OrgId()
+		// current default configuration contains
+		// only one entry: "metadata/name" with no GVK
 		for _, path := range p.FieldSpecs {
 			if !id.IsSelected(&path.Gvk) {
+				// With the currrent default configuration,
+				// because no Gvk is specified, so a wild
+				// card
 				continue
 			}
+
 			if smellsLikeANameChange(&path) {
+				// "metadata/name" is the only field.
+				// this will add a prefix and a suffix
+				// to the resource even if those are
+				// empty
 				r.AddNamePrefix(p.Prefix)
 				r.AddNameSuffix(p.Suffix)
 			}
+
+			// the addPrefixSuffix method will not
+			// change the name if both the prefix and suffix
+			// are empty.
 			err := transformers.MutateField(
 				r.Map(),
 				path.PathSlice(),