Merge pull request #3835 from monopole/useResIdInReplacements

Use resid.ResId in replacements.

api/filters/replacement/replacement.go

@@ -43,8 +43,8 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.T
 			t.FieldPaths = []string{types.DefaultReplacementFieldPath}
 		}
 		for _, n := range nodes {
-			nodeId := getKrmId(n)
-			if t.Select.KrmId.Match(nodeId) && !rejectId(t.Reject, nodeId) {
+			id := makeResId(n)
+			if id.IsSelectedBy(t.Select.ResId) && !rejectId(t.Reject, id) {
 				err := applyToNode(n, value, t)
 				if err != nil {
 					return nil, err
@@ -55,9 +55,9 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.T
 	return nodes, nil
 }
 
-func rejectId(rejects []*types.Selector, nodeId *types.KrmId) bool {
+func rejectId(rejects []*types.Selector, id *resid.ResId) bool {
 	for _, r := range rejects {
-		if r.KrmId.Match(nodeId) {
+		if id.IsSelectedBy(r.ResId) {
 			return true
 		}
 	}
@@ -152,32 +152,33 @@ func getRefinedValue(options *types.FieldOptions, rn *yaml.RNode) (*yaml.RNode,
 func selectSourceNode(nodes []*yaml.RNode, selector *types.SourceSelector) (*yaml.RNode, error) {
 	var matches []*yaml.RNode
 	for _, n := range nodes {
-		if selector.KrmId.Match(getKrmId(n)) {
+		if makeResId(n).IsSelectedBy(selector.ResId) {
 			if len(matches) > 0 {
-				return nil, fmt.Errorf("more than one match for source %v", selector)
+				return nil, fmt.Errorf(
+					"multiple matches for selector %s", selector)
 			}
 			matches = append(matches, n)
 		}
 	}
 	if len(matches) == 0 {
-		return nil, fmt.Errorf("found no matches for source %v", selector)
+		return nil, fmt.Errorf("nothing selected by %s", selector)
 	}
 	return matches[0], nil
 }
 
-func getKrmId(n *yaml.RNode) *types.KrmId {
+// makeResId makes a ResId from an RNode.
+func makeResId(n *yaml.RNode) *resid.ResId {
 	ns, err := n.GetNamespace()
 	if err != nil {
 		// Resource has no metadata (no apiVersion, kind, nor metadata field).
-		// In this case, it cannot be selected.
-		return &types.KrmId{}
+		return nil
 	}
 	apiVersion := n.Field(yaml.APIVersionField)
 	var group, version string
 	if apiVersion != nil {
 		group, version = resid.ParseGroupVersion(yaml.GetValue(apiVersion.Value))
 	}
-	return &types.KrmId{
+	return &resid.ResId{
 		Gvk:       resid.Gvk{Group: group, Version: version, Kind: n.GetKind()},
 		Name:      n.GetName(),
 		Namespace: ns,

api/filters/replacement/replacement_test.go

@@ -269,7 +269,7 @@ spec:
   - select:
       kind: Deployment
 `,
-			expectedErr: "more than one match for source ~G_~V_Deployment",
+			expectedErr: "multiple matches for selector ~G_~V_Deployment|~X|~N",
 		},
 		"replacement has no source": {
 			input: `apiVersion: v1

api/krusty/replacementtransformer_test.go

@@ -68,8 +68,7 @@ resources:
 replacements:
 - path: replacement.yaml
 `)
-	th.WriteF("replacement.yaml",
-		`
+	th.WriteF("replacement.yaml", `
 source: 
   kind: Deployment
   fieldPath: spec.template.spec.containers.0.image
@@ -115,8 +114,7 @@ func TestReplacementTransformerWithDiamondShape(t *testing.T) {
 	th := kusttest_test.MakeEnhancedHarness(t)
 	defer th.Reset()
 
-	th.WriteF("base/deployment.yaml",
-		`
+	th.WriteF("base/deployments.yaml", `
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -150,21 +148,18 @@ spec:
       - image: nginx:newtagB
         name: nginx
 `)
-	th.WriteF("base/kustomization.yaml",
-		`
+	th.WriteK("base", `
 resources:
-- deployment.yaml
+- deployments.yaml
 `)
-	th.WriteF("a/kustomization.yaml",
-		`
+	th.WriteK("a", `
 namePrefix: a-
 resources:
 - ../base
 replacements:
 - path: replacement.yaml
 `)
-	th.WriteF("a/replacement.yaml",
-		`
+	th.WriteF("a/replacement.yaml", `
 source:
   name: a-sourceA
   fieldPath: spec.template.spec.containers.0.image
@@ -174,16 +169,14 @@ targets:
   fieldPaths:
   - spec.template.spec.containers.[name=nginx].image
 `)
-	th.WriteF("b/kustomization.yaml",
-		`
+	th.WriteK("b", `
 namePrefix: b-
 resources:
 - ../base
 replacements:
 - path: replacement.yaml
 `)
-	th.WriteF("b/replacement.yaml",
-		`
+	th.WriteF("b/replacement.yaml", `
 source:
   name: b-sourceB
   fieldPath: spec.template.spec.containers.0.image
@@ -193,14 +186,13 @@ targets:
   fieldPaths:
   - spec.template.spec.containers.[name=nginx].image
 `)
-	th.WriteF("combined/kustomization.yaml",
-		`
+	th.WriteK(".", `
 resources:
-- ../a
-- ../b
+- a
+- b
 `)
 
-	m := th.Run("combined", th.MakeDefaultOptions())
+	m := th.Run(".", th.MakeDefaultOptions())
 	th.AssertActualEqualsExpected(m, `
 apiVersion: apps/v1
 kind: Deployment

api/resid/resid.go

@@ -90,6 +90,13 @@ func (id ResId) GvknEquals(o ResId) bool {
 	return id.Name == o.Name && id.Gvk.Equals(o.Gvk)
 }
 
+// IsSelectedBy returns true if self is selected by the argument.
+func (id ResId) IsSelectedBy(selector ResId) bool {
+	return (selector.Name == "" || selector.Name == id.Name) &&
+		(selector.Namespace == "" || selector.IsNsEquals(id)) &&
+		id.Gvk.IsSelected(&selector.Gvk)
+}
+
 // Equals returns true if the other id matches
 // namespace/Group/Version/Kind/name.
 func (id ResId) Equals(o ResId) bool {

api/resid/resid_test.go

@@ -360,6 +360,100 @@ var ids = []ResId{
 	},
 }
 
+func TestResIdIsSelected(t *testing.T) {
+	type selectable struct {
+		id             ResId
+		expectSelected bool
+	}
+	var testCases = []struct {
+		selector    ResId
+		selectables []selectable
+	}{
+		{
+			selector: ResId{
+				Namespace: "X",
+				Name:      "nm",
+				Gvk:       Gvk{Group: "g", Version: "v", Kind: "k"},
+			},
+			selectables: []selectable{
+				{
+					id: ResId{
+						Namespace: "X",
+						Name:      "nm",
+						Gvk:       Gvk{Group: "g", Version: "v", Kind: "k"},
+					},
+					expectSelected: true,
+				},
+				{
+					id: ResId{
+						Namespace: "x",
+						Name:      "nm",
+						Gvk:       Gvk{Group: "g", Version: "v", Kind: "k"},
+					},
+					expectSelected: false,
+				},
+				{
+					id: ResId{
+						Name: "nm",
+						Gvk:  Gvk{Group: "g", Version: "v", Kind: "k"},
+					},
+					expectSelected: false,
+				},
+			},
+		},
+		{
+			selector: ResId{
+				/* Namespace wildcard */
+				Name: "nm",
+				Gvk:  Gvk{Group: "g" /* Version wildcard */, Kind: "k"},
+			},
+			selectables: []selectable{
+				{
+					id: ResId{
+						Namespace: "X",
+						Name:      "nm",
+						Gvk:       Gvk{Group: "g", Version: "v", Kind: "k"},
+					},
+					expectSelected: true,
+				},
+				{
+					id: ResId{
+						Namespace: "x",
+						Name:      "nm",
+						Gvk:       Gvk{Group: "g", Version: "v", Kind: "k"},
+					},
+					expectSelected: true,
+				},
+				{
+					id: ResId{
+						Name: "nm",
+						Gvk:  Gvk{Group: "g", Version: "VVV", Kind: "k"},
+					},
+					expectSelected: true,
+				},
+			},
+		},
+	}
+
+	for _, tst := range testCases {
+		for _, pair := range tst.selectables {
+			if pair.id.IsSelectedBy(tst.selector) {
+				if !pair.expectSelected {
+					t.Fatalf(
+						"expected id %s to NOT be selected by %s",
+						pair.id, tst.selector)
+				}
+			} else {
+				if pair.expectSelected {
+					t.Fatalf(
+						"expected id %s to be selected by %s",
+						pair.id, tst.selector)
+				}
+			}
+		}
+	}
+}
+
 func TestFromString(t *testing.T) {
 	for _, id := range ids {
 		newId := FromString(id.String())

api/resmap/selector_test.go

@@ -61,13 +61,13 @@ func TestFindPatchTargets(t *testing.T) {
 	}{
 		"select_01": {
 			target: types.Selector{
-				KrmId: types.KrmId{Name: "name.*"},
+				ResId: resid.ResId{Name: "name.*"},
 			},
 			count: 3,
 		},
 		"select_02": {
 			target: types.Selector{
-				KrmId:              types.KrmId{Name: "name.*"},
+				ResId:              resid.ResId{Name: "name.*"},
 				AnnotationSelector: "foo=bar",
 			},
 			count: 2,
@@ -80,7 +80,7 @@ func TestFindPatchTargets(t *testing.T) {
 		},
 		"select_04": {
 			target: types.Selector{
-				KrmId: types.KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Kind: "Kind1",
 					},
@@ -91,31 +91,31 @@ func TestFindPatchTargets(t *testing.T) {
 		},
 		"select_05": {
 			target: types.Selector{
-				KrmId: types.KrmId{Name: "NotMatched"},
+				ResId: resid.ResId{Name: "NotMatched"},
 			},
 			count: 0,
 		},
 		"select_06": {
 			target: types.Selector{
-				KrmId: types.KrmId{Name: ""},
+				ResId: resid.ResId{Name: ""},
 			},
 			count: 4,
 		},
 		"select_07": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "default"},
+				ResId: resid.ResId{Namespace: "default"},
 			},
 			count: 2,
 		},
 		"select_08": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: ""},
+				ResId: resid.ResId{Namespace: ""},
 			},
 			count: 4,
 		},
 		"select_09": {
 			target: types.Selector{
-				KrmId: types.KrmId{
+				ResId: resid.ResId{
 					Namespace: "default",
 					Name:      "name.*",
 					Gvk: resid.Gvk{
@@ -127,55 +127,55 @@ func TestFindPatchTargets(t *testing.T) {
 		},
 		"select_10": {
 			target: types.Selector{
-				KrmId: types.KrmId{Name: "^name.*"},
+				ResId: resid.ResId{Name: "^name.*"},
 			},
 			count: 3,
 		},
 		"select_11": {
 			target: types.Selector{
-				KrmId: types.KrmId{Name: "name.*$"},
+				ResId: resid.ResId{Name: "name.*$"},
 			},
 			count: 3,
 		},
 		"select_12": {
 			target: types.Selector{
-				KrmId: types.KrmId{Name: "^name.*$"},
+				ResId: resid.ResId{Name: "^name.*$"},
 			},
 			count: 3,
 		},
 		"select_13": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "^def.*"},
+				ResId: resid.ResId{Namespace: "^def.*"},
 			},
 			count: 2,
 		},
 		"select_14": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "def.*$"},
+				ResId: resid.ResId{Namespace: "def.*$"},
 			},
 			count: 2,
 		},
 		"select_15": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "^def.*$"},
+				ResId: resid.ResId{Namespace: "^def.*$"},
 			},
 			count: 2,
 		},
 		"select_16": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "default"},
+				ResId: resid.ResId{Namespace: "default"},
 			},
 			count: 2,
 		},
 		"select_17": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "NotMatched"},
+				ResId: resid.ResId{Namespace: "NotMatched"},
 			},
 			count: 0,
 		},
 		"select_18": {
 			target: types.Selector{
-				KrmId: types.KrmId{Namespace: "ns1"},
+				ResId: resid.ResId{Namespace: "ns1"},
 			},
 			count: 1,
 		},

api/types/patch_test.go

@@ -9,7 +9,7 @@ import (
 
 func TestPatchEquals(t *testing.T) {
 	selector := Selector{
-		KrmId: KrmId{
+		ResId: resid.ResId{
 			Gvk: resid.Gvk{
 				Group:   "group",
 				Version: "version",
@@ -40,7 +40,7 @@ func TestPatchEquals(t *testing.T) {
 				Path:  "foo",
 				Patch: "bar",
 				Target: &Selector{
-					KrmId: KrmId{
+					ResId: resid.ResId{
 						Gvk: resid.Gvk{
 							Group:   "group",
 							Version: "version",
@@ -57,7 +57,7 @@ func TestPatchEquals(t *testing.T) {
 				Path:  "foo",
 				Patch: "bar",
 				Target: &Selector{
-					KrmId: KrmId{
+					ResId: resid.ResId{
 						Gvk: resid.Gvk{
 							Group:   "group",
 							Version: "version",

api/types/replacement.go

@@ -3,6 +3,13 @@
 
 package types
 
+import (
+	"fmt"
+	"strings"
+
+	"sigs.k8s.io/kustomize/api/resid"
+)
+
 const DefaultReplacementFieldPath = "metadata.name"
 
 // Replacement defines how to perform a substitution
@@ -18,7 +25,7 @@ type Replacement struct {
 // SourceSelector is the source of the replacement transformer.
 type SourceSelector struct {
 	// A specific object to read it from.
-	KrmId `json:",inline,omitempty" yaml:",inline,omitempty"`
+	resid.ResId `json:",inline,omitempty" yaml:",inline,omitempty"`
 
 	// Structured field path expected in the allowed object.
 	FieldPath string `json:"fieldPath" yaml:"fieldPath"`
@@ -27,6 +34,20 @@ type SourceSelector struct {
 	Options *FieldOptions `json:"options" yaml:"options"`
 }
 
+func (s *SourceSelector) String() string {
+	if s == nil {
+		return ""
+	}
+	result := []string{s.ResId.String()}
+	if s.FieldPath != "" {
+		result = append(result, s.FieldPath)
+	}
+	if opts := s.Options.String(); opts != "" {
+		result = append(result, opts)
+	}
+	return strings.Join(result, ":")
+}
+
 // TargetSelector specifies fields in one or more objects.
 type TargetSelector struct {
 	// Include objects that match this.
@@ -57,3 +78,10 @@ type FieldOptions struct {
 	// If field missing, add it.
 	Create bool `json:"create" yaml:"create"`
 }
+
+func (fo *FieldOptions) String() string {
+	if fo == nil || fo.Delimiter == "" {
+		return ""
+	}
+	return fmt.Sprintf("%s(%d)", fo.Delimiter, fo.Index)
+}

api/types/selector.go

@@ -4,6 +4,7 @@
 package types
 
 import (
+	"fmt"
 	"regexp"
 
 	"sigs.k8s.io/kustomize/api/resid"
@@ -13,8 +14,8 @@ import (
 // Any resource that matches intersection of all conditions
 // is included in this set.
 type Selector struct {
-	// KrmId refers to a GVKN/Ns of a resource.
-	KrmId `json:",inline,omitempty" yaml:",inline,omitempty"`
+	// ResId refers to a GVKN/Ns of a resource.
+	resid.ResId `json:",inline,omitempty" yaml:",inline,omitempty"`
 
 	// AnnotationSelector is a string that follows the label selection expression
 	// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#api
@@ -27,21 +28,9 @@ type Selector struct {
 	LabelSelector string `json:"labelSelector,omitempty" yaml:"labelSelector,omitempty"`
 }
 
-// KrmId refers to a GVKN/Ns of a resource.
-type KrmId struct {
-	resid.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"`
-	Name      string `json:"name,omitempty" yaml:"name,omitempty"`
-	Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"`
-}
-
-// Match returns true if id selects other, i.e. id's fields
-// either match other's or are empty
-func (id *KrmId) Match(other *KrmId) bool {
-	return (id.Group == "" || id.Group == other.Group) &&
-		(id.Version == "" || id.Version == other.Version) &&
-		(id.Kind == "" || id.Kind == other.Kind) &&
-		(id.Name == "" || id.Name == other.Name) &&
-		(id.Namespace == "" || id.Namespace == other.Namespace)
+func (s *Selector) String() string {
+	return fmt.Sprintf(
+		"%s:a=%s:l=%s", s.ResId, s.AnnotationSelector, s.LabelSelector)
 }
 
 // SelectorRegex is a Selector with regex in GVK

api/types/selector_test.go

@@ -15,7 +15,7 @@ func TestSelectorRegexMatchGvk(t *testing.T) {
 	}{
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Group:   "group",
 						Version: "version",
@@ -32,7 +32,7 @@ func TestSelectorRegexMatchGvk(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Group:   "group",
 						Version: "",
@@ -49,7 +49,7 @@ func TestSelectorRegexMatchGvk(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Group:   "group",
 						Version: "version",
@@ -66,7 +66,7 @@ func TestSelectorRegexMatchGvk(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Group:   "group",
 						Version: "version",
@@ -83,7 +83,7 @@ func TestSelectorRegexMatchGvk(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Group:   "g.*",
 						Version: "\\d+",
@@ -100,7 +100,7 @@ func TestSelectorRegexMatchGvk(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Gvk: resid.Gvk{
 						Group:   "g.*",
 						Version: "\\d+",
@@ -137,7 +137,7 @@ func TestSelectorRegexMatchName(t *testing.T) {
 	}{
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Name:      "foo",
 					Namespace: "bar",
 				},
@@ -147,7 +147,7 @@ func TestSelectorRegexMatchName(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Name:      "foo",
 					Namespace: "bar",
 				},
@@ -157,7 +157,7 @@ func TestSelectorRegexMatchName(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Name: "f.*",
 				},
 			},
@@ -166,7 +166,7 @@ func TestSelectorRegexMatchName(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Name: "b.*",
 				},
 			},
@@ -194,7 +194,7 @@ func TestSelectorRegexMatchNamespace(t *testing.T) {
 	}{
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Name:      "bar",
 					Namespace: "foo",
 				},
@@ -204,7 +204,7 @@ func TestSelectorRegexMatchNamespace(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Name:      "foo",
 					Namespace: "bar",
 				},
@@ -214,7 +214,7 @@ func TestSelectorRegexMatchNamespace(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Namespace: "f.*",
 				},
 			},
@@ -223,7 +223,7 @@ func TestSelectorRegexMatchNamespace(t *testing.T) {
 		},
 		{
 			S: Selector{
-				KrmId: KrmId{
+				ResId: resid.ResId{
 					Namespace: "b.*",
 				},
 			},