diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go index 9b2cc37ae..00026b923 100644 --- a/api/filters/fieldspec/fieldspec.go +++ b/api/filters/fieldspec/fieldspec.go @@ -41,7 +41,7 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) { if err := fltr.filter(obj); err != nil { s, _ := obj.String() return nil, errors.WrapPrefixf(err, - "obj '%s' at path '%v'", s, fltr.FieldSpec.Path) + "considering field '%s' of object\n%v", fltr.FieldSpec.Path, s) } return obj, nil } diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go index 30efb8522..5726e75e9 100644 --- a/api/filters/fieldspec/fieldspec_test.go +++ b/api/filters/fieldspec/fieldspec_test.go @@ -176,8 +176,11 @@ kind: Bar a: b: a `, - error: "obj 'kind: Bar\na:\n b: a\n' at path 'a/b/c': " + - "expected sequence or mapping node", + error: `considering field 'a/b/c' of object +kind: Bar +a: + b: a +: expected sequence or mapping node`, filter: fieldspec.Filter{ SetValue: filtersutil.SetScalar("e"), }, diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index ef279498d..e6172feb0 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -5,12 +5,13 @@ import ( "fmt" "strings" + "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/filters/fieldspec" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" - kyaml_filtersutil "sigs.k8s.io/kustomize/kyaml/filtersutil" + "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -38,18 +39,28 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { return kio.FilterAll(yaml.FilterFunc(f.run)).Filter(nodes) } -// The node passed in here is the same node as held in Referrer, and +// The node passed in here is the same node as held in Referrer; // that's how the referrer's name field is updated. -// However, this filter still needs the extra methods on Referrer +// Currently, however, this filter still needs the extra methods on Referrer // to consult things like the resource Id, its namespace, etc. +// TODO(3455): No filter should use the Resource api; all information +// about names should come from annotations, with helper methods +// on the RNode object. Resource should get stupider, RNode smarter. func (f Filter) run(node *yaml.RNode) (*yaml.RNode, error) { - err := node.PipeE(fieldspec.Filter{ + if err := node.PipeE(fieldspec.Filter{ FieldSpec: f.NameFieldToUpdate, SetValue: f.set, - }) - return node, err + }); err != nil { + return nil, errors.Wrapf( + err, "updating name reference in '%s' field of '%s'", + f.NameFieldToUpdate.Path, f.Referrer.CurId().String()) + } + return node, nil } +// This function is called at many nodes in the YAML doc tree. +// Only on first entry can one expect the argument to match the +// top-level node backing the Referrer. func (f Filter) set(node *yaml.RNode) error { if yaml.IsMissingOrNull(node) { return nil @@ -65,8 +76,7 @@ func (f Filter) set(node *yaml.RNode) error { setMappingFn: f.setMapping, }, node) default: - return fmt.Errorf( - "node is expected to be either a string or a slice of string or a map of string") + return fmt.Errorf("node must be a scalar, sequence or map") } } @@ -76,16 +86,19 @@ func (f Filter) setMapping(node *yaml.RNode) error { return fmt.Errorf("expect a mapping node") } nameNode, err := node.Pipe(yaml.FieldMatcher{Name: "name"}) - if err != nil || nameNode == nil { - return fmt.Errorf("cannot find field 'name' in node") + if err != nil { + return errors.Wrap(err, "trying to match 'name' field") + } + if nameNode == nil { + return fmt.Errorf("no 'name' field in node") } namespaceNode, err := node.Pipe(yaml.FieldMatcher{Name: "namespace"}) if err != nil { - return fmt.Errorf("error when find field 'namespace'") + return errors.Wrap(err, "trying to match 'namespace' field") } // name will not be updated if the namespace doesn't match - subset := f.ReferralCandidates.Resources() + candidates := f.ReferralCandidates.Resources() if namespaceNode != nil { namespace := namespaceNode.YNode().Value bynamespace := f.ReferralCandidates.GroupedByOriginalNamespace() @@ -95,57 +108,57 @@ func (f Filter) setMapping(node *yaml.RNode) error { return nil } } - subset = bynamespace[namespace] + candidates = bynamespace[namespace] } oldName := nameNode.YNode().Value - res, err := f.selectReferral(oldName, subset) - if err != nil || res == nil { - // Nil res means nothing to do. + referral, err := f.selectReferral(oldName, candidates) + if err != nil || referral == nil { + // Nil referral means nothing to do. return err } - f.recordTheReferral(res) - if res.GetName() == oldName && res.GetNamespace() == "" { + f.recordTheReferral(referral) + if referral.GetName() == oldName && referral.GetNamespace() == "" { // The name has not changed, nothing to do. return nil } err = node.PipeE(yaml.FieldSetter{ Name: "name", - StringValue: res.GetName(), + StringValue: referral.GetName(), }) if err != nil { return err } - if res.GetNamespace() != "" { + if referral.GetNamespace() != "" { // We don't want value "" to replace value "default" since // the empty string is handled as a wild card here not default namespace // by kubernetes. err = node.PipeE(yaml.FieldSetter{ Name: "namespace", - StringValue: res.GetNamespace(), + StringValue: referral.GetNamespace(), }) } return err } func (f Filter) setScalar(node *yaml.RNode) error { - res, err := f.selectReferral( + referral, err := f.selectReferral( node.YNode().Value, f.ReferralCandidates.Resources()) - if err != nil || res == nil { - // Nil res means nothing to do. + if err != nil || referral == nil { + // Nil referral means nothing to do. return err } - f.recordTheReferral(res) - if res.GetName() == node.YNode().Value { + f.recordTheReferral(referral) + if referral.GetName() == node.YNode().Value { // The name has not changed, nothing to do. return nil } - return node.PipeE(yaml.FieldSetter{StringValue: res.GetName()}) + return node.PipeE(yaml.FieldSetter{StringValue: referral.GetName()}) } -// In the resource, make a note that it is referred to by the referrer. -func (f Filter) recordTheReferral(res *resource.Resource) { - res.AppendRefBy(f.Referrer.CurId()) +// In the resource, make a note that it is referred to by the Referrer. +func (f Filter) recordTheReferral(referral *resource.Resource) { + referral.AppendRefBy(f.Referrer.CurId()) } func (f Filter) isRoleRef() bool { @@ -155,7 +168,7 @@ func (f Filter) isRoleRef() bool { // getRoleRefGvk returns a Gvk in the roleRef field. Return error // if the roleRef, roleRef/apiGroup or roleRef/kind is missing. func getRoleRefGvk(res json.Marshaler) (*resid.Gvk, error) { - n, err := kyaml_filtersutil.GetRNode(res) + n, err := filtersutil.GetRNode(res) if err != nil { return nil, err } @@ -213,7 +226,7 @@ func (f Filter) filterReferralCandidates( // namespace. func (f Filter) selectReferral( oldName string, - candidateSubset []*resource.Resource) (*resource.Resource, error) { + referralCandidates []*resource.Resource) (*resource.Resource, error) { var roleRefGvk *resid.Gvk if f.isRoleRef() { var err error @@ -222,11 +235,11 @@ func (f Filter) selectReferral( return nil, err } } - for _, res := range candidateSubset { - if res.GetOriginalName() != oldName { + for _, candidate := range referralCandidates { + if candidate.GetOriginalName() != oldName { continue } - id := res.OrgId() + id := candidate.OrgId() if !id.IsSelected(&f.ReferralTarget) { continue } @@ -242,28 +255,26 @@ func (f Filter) selectReferral( filteredMatches := f.filterReferralCandidates(matches) if len(filteredMatches) > 1 { return nil, fmt.Errorf( - "multiple matches for %s:\n %v", - id, getIds(filteredMatches)) + "cannot fix name in '%s' field of referrer '%s';"+ + " found multiple possible referrals: %v", + f.NameFieldToUpdate.Path, + f.Referrer.CurId(), + getIds(filteredMatches)) } // Check is the match the resource we are working on - if len(filteredMatches) == 0 || res != filteredMatches[0] { + if len(filteredMatches) == 0 || candidate != filteredMatches[0] { continue } } - // In the resource, note that it is referenced - // by the referrer. - res.AppendRefBy(f.Referrer.CurId()) - // Return transformed name of the object, - // complete with prefixes, hashes, etc. - return res, nil + return candidate, nil } return nil, nil } -func getIds(rs []*resource.Resource) []string { +func getIds(rs []*resource.Resource) string { var result []string for _, r := range rs { - result = append(result, r.CurId().String()+"\n") + result = append(result, r.CurId().String()) } - return result + return strings.Join(result, ", ") } diff --git a/api/filters/nameref/nameref_test.go b/api/filters/nameref/nameref_test.go index 7609e541c..ed38e4381 100644 --- a/api/filters/nameref/nameref_test.go +++ b/api/filters/nameref/nameref_test.go @@ -14,14 +14,14 @@ import ( func TestNamerefFilter(t *testing.T) { testCases := map[string]struct { - input string - candidates string - expected string - filter Filter - originalNames []string + referrerOriginal string + candidates string + referrerFinal string + filter Filter + originalNames []string }{ "simple scalar": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -41,7 +41,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName", ""}, - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -59,7 +59,7 @@ ref: }, }, "sequence": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -80,7 +80,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName1", ""}, - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -99,7 +99,7 @@ seq: }, }, "mapping": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -119,7 +119,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName", ""}, - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -137,7 +137,7 @@ map: }, }, "mapping with namespace": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -159,7 +159,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName", ""}, - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -178,7 +178,7 @@ map: }, }, "null value": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -198,7 +198,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName", ""}, - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -220,7 +220,7 @@ map: for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { factory := provider.NewDefaultDepProvider().GetResourceFactory() - referrer, err := factory.FromBytes([]byte(tc.input)) + referrer, err := factory.FromBytes([]byte(tc.referrerOriginal)) if err != nil { t.Fatal(err) } @@ -237,9 +237,9 @@ map: tc.filter.ReferralCandidates = candidates if !assert.Equal(t, - strings.TrimSpace(tc.expected), + strings.TrimSpace(tc.referrerFinal), strings.TrimSpace( - filtertest_test.RunFilter(t, tc.input, tc.filter))) { + filtertest_test.RunFilter(t, tc.referrerOriginal, tc.filter))) { t.FailNow() } }) @@ -248,14 +248,14 @@ map: func TestNamerefFilterUnhappy(t *testing.T) { testCases := map[string]struct { - input string - candidates string - expected string - filter Filter - originalNames []string + referrerOriginal string + candidates string + referrerFinal string + filter Filter + originalNames []string }{ "multiple match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -275,7 +275,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName", "oldName"}, - expected: "", + referrerFinal: "", filter: Filter{ NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, ReferralTarget: resid.Gvk{ @@ -286,7 +286,7 @@ metadata: }, }, "no name": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -306,7 +306,7 @@ metadata: name: newName2 `, originalNames: []string{"oldName", "oldName"}, - expected: "", + referrerFinal: "", filter: Filter{ NameFieldToUpdate: types.FieldSpec{Path: "ref"}, ReferralTarget: resid.Gvk{ @@ -321,7 +321,7 @@ metadata: for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { factory := provider.NewDefaultDepProvider().GetResourceFactory() - referrer, err := factory.FromBytes([]byte(tc.input)) + referrer, err := factory.FromBytes([]byte(tc.referrerOriginal)) if err != nil { t.Fatal(err) } @@ -337,11 +337,11 @@ metadata: candidates := resMapFactory.FromResourceSlice(candidatesRes) tc.filter.ReferralCandidates = candidates - _, err = filtertest_test.RunFilterE(t, tc.input, tc.filter) + _, err = filtertest_test.RunFilterE(t, tc.referrerOriginal, tc.filter) if err == nil { t.Fatalf("expect an error") } - if tc.expected != "" && !assert.EqualError(t, err, tc.expected) { + if tc.referrerFinal != "" && !assert.EqualError(t, err, tc.referrerFinal) { t.FailNow() } }) @@ -350,19 +350,19 @@ metadata: func TestCandidatesWithDifferentPrefixSuffix(t *testing.T) { testCases := map[string]struct { - input string - candidates string - expected string - filter Filter - originalNames []string - prefix []string - suffix []string - inputPrefix string - inputSuffix string - err bool + referrerOriginal string + candidates string + referrerFinal string + filter Filter + originalNames []string + prefix []string + suffix []string + inputPrefix string + inputSuffix string + err bool }{ "prefix match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -386,7 +386,7 @@ metadata: suffix: []string{"", "suffix2"}, inputPrefix: "prefix1", inputSuffix: "", - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -405,7 +405,7 @@ ref: err: false, }, "suffix match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -429,7 +429,7 @@ metadata: suffix: []string{"suffix1", "suffix2"}, inputPrefix: "", inputSuffix: "suffix1", - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -448,7 +448,7 @@ ref: err: false, }, "prefix suffix both match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -472,7 +472,7 @@ metadata: suffix: []string{"suffix1", "suffix2"}, inputPrefix: "prefix1", inputSuffix: "suffix1", - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -491,7 +491,7 @@ ref: err: false, }, "multiple match: both": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -515,7 +515,7 @@ metadata: suffix: []string{"suffix", "suffix"}, inputPrefix: "prefix", inputSuffix: "suffix", - expected: "", + referrerFinal: "", filter: Filter{ NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, ReferralTarget: resid.Gvk{ @@ -527,7 +527,7 @@ metadata: err: true, }, "multiple match: only prefix": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -551,7 +551,7 @@ metadata: suffix: []string{"", ""}, inputPrefix: "prefix", inputSuffix: "", - expected: "", + referrerFinal: "", filter: Filter{ NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, ReferralTarget: resid.Gvk{ @@ -563,7 +563,7 @@ metadata: err: true, }, "multiple match: only suffix": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -587,7 +587,7 @@ metadata: suffix: []string{"suffix", "suffix"}, inputPrefix: "", inputSuffix: "suffix", - expected: "", + referrerFinal: "", filter: Filter{ NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, ReferralTarget: resid.Gvk{ @@ -599,7 +599,7 @@ metadata: err: true, }, "no match: neither match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -623,7 +623,7 @@ metadata: suffix: []string{"suffix1", "suffix2"}, inputPrefix: "prefix", inputSuffix: "suffix", - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -642,7 +642,7 @@ ref: err: false, }, "no match: prefix doesn't match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -666,7 +666,7 @@ metadata: suffix: []string{"suffix", "suffix"}, inputPrefix: "prefix", inputSuffix: "suffix", - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -685,7 +685,7 @@ ref: err: false, }, "no match: suffix doesn't match": { - input: ` + referrerOriginal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -709,7 +709,7 @@ metadata: suffix: []string{"suffix1", "suffix2"}, inputPrefix: "prefix", inputSuffix: "suffix", - expected: ` + referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: @@ -732,7 +732,7 @@ ref: for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { factory := provider.NewDefaultDepProvider().GetResourceFactory() - referrer, err := factory.FromBytes([]byte(tc.input)) + referrer, err := factory.FromBytes([]byte(tc.referrerOriginal)) if err != nil { t.Fatal(err) } @@ -764,13 +764,15 @@ ref: if !tc.err { if !assert.Equal(t, - strings.TrimSpace(tc.expected), + strings.TrimSpace(tc.referrerFinal), strings.TrimSpace( - filtertest_test.RunFilter(t, tc.input, tc.filter))) { + filtertest_test.RunFilter( + t, tc.referrerOriginal, tc.filter))) { t.FailNow() } } else { - _, err := filtertest_test.RunFilterE(t, tc.input, tc.filter) + _, err := filtertest_test.RunFilterE( + t, tc.referrerOriginal, tc.filter) if err == nil { t.Fatalf("an error is expected") } diff --git a/api/filters/refvar/refvar_test.go b/api/filters/refvar/refvar_test.go index cdaf95278..a350b1ab8 100644 --- a/api/filters/refvar/refvar_test.go +++ b/api/filters/refvar/refvar_test.go @@ -243,7 +243,8 @@ metadata: data: slice: - false`, - expectedError: `obj 'apiVersion: apps/v1 + expectedError: `considering field 'data/slice' of object +apiVersion: apps/v1 kind: Deployment metadata: name: dep @@ -252,7 +253,7 @@ metadata: data: slice: - false -' at path 'data/slice': invalid value type expect a string`, +: invalid value type expect a string`, filter: Filter{ MappingFunc: makeMf(map[string]interface{}{ "VAR": int64(5), @@ -268,7 +269,8 @@ metadata: name: dep data: 1: str`, - expectedError: `obj 'apiVersion: apps/v1 + expectedError: `considering field 'data' of object +apiVersion: apps/v1 kind: Deployment metadata: name: dep @@ -276,7 +278,7 @@ metadata: config.kubernetes.io/index: '0' data: 1: str -' at path 'data': invalid map key: value='1', tag='` + yaml.NodeTagInt + `'`, +: invalid map key: value='1', tag='` + yaml.NodeTagInt + `'`, filter: Filter{ MappingFunc: makeMf(map[string]interface{}{ "VAR": int64(5), diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 41327acfc..c9fedeb4b 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" + "sigs.k8s.io/kustomize/api/konfig" + "sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig" "sigs.k8s.io/kustomize/api/provider" "sigs.k8s.io/kustomize/api/resid" @@ -518,7 +520,27 @@ func TestNameReferenceUnhappyRun(t *testing.T) { }, }, }).ResMap(), - expectedErr: "cannot find field 'name' in node"}, + // TODO(#3304): DECISION - kyaml better; not a bug. + expectedErr: konfig.IfApiMachineryElseKyaml( + `updating name reference in 'rules/resourceNames' field of `+ + `'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'`+ + `: considering field 'rules/resourceNames' of object +{"apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRole", "metadata": { + "name": "cr"}, "rules": [{"resourceNames": {"foo": "bar"}, "resources": ["secrets"]}]} +: visit traversal on path: [resourceNames]: no 'name' field in node`, + `updating name reference in 'rules/resourceNames' field of `+ + `'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'`+ + `: considering field 'rules/resourceNames' of object +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: cr +rules: +- resourceNames: + foo: bar + resources: + - secrets +: visit traversal on path: [resourceNames]: no 'name' field in node`)}, } nrt := newNameReferenceTransformer(builtinconfig.MakeDefaultConfig().NameReference) @@ -529,7 +551,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { } if !strings.Contains(err.Error(), test.expectedErr) { - t.Fatalf("Incorrect error.\nExpected: %s, but got %v", + t.Fatalf("Incorrect error.\nExpected:\n %s\nGot:\n%v", test.expectedErr, err) } } diff --git a/api/internal/accumulator/refvartransformer_test.go b/api/internal/accumulator/refvartransformer_test.go index 85b8a115c..8b4d31c16 100644 --- a/api/internal/accumulator/refvartransformer_test.go +++ b/api/internal/accumulator/refvartransformer_test.go @@ -24,14 +24,12 @@ func TestRefVarTransformer(t *testing.T) { res resmap.ResMap unused []string } - testCases := []struct { - description string - given given - expected expected - errMessage string + testCases := map[string]struct { + given given + expected expected + errMessage string }{ - { - description: "var replacement in map[string]", + "var replacement in map[string]": { given: given{ varMap: map[string]interface{}{ "FOO": "replacementForFoo", @@ -106,8 +104,7 @@ func TestRefVarTransformer(t *testing.T) { unused: []string{"BAR"}, }, }, - { - description: "var replacement panic in map[string]", + "var replacement panic in map[string]": { given: given{ varMap: map[string]interface{}{}, fs: []types.FieldSpec{ @@ -126,20 +123,21 @@ func TestRefVarTransformer(t *testing.T) { }, // TODO(#3304): DECISION - kyaml better; not a bug. errMessage: konfig.IfApiMachineryElseKyaml( - `obj '{"apiVersion": "v1", "data": {"slice": [5]}, "kind": "ConfigMap", "metadata": {"name": "cm1"}} -' at path 'data/slice': invalid value type expect a string`, - `obj 'apiVersion: v1 + `considering field 'data/slice' of object +{"apiVersion": "v1", "data": {"slice": [5]}, "kind": "ConfigMap", "metadata": {"name": "cm1"}} +: invalid value type expect a string`, + `considering field 'data/slice' of object +apiVersion: v1 data: slice: - 5 kind: ConfigMap metadata: name: cm1 -' at path 'data/slice': invalid value type expect a string`, +: invalid value type expect a string`, ), }, - { - description: "var replacement in nil", + "var replacement in nil": { given: given{ varMap: map[string]interface{}{}, fs: []types.FieldSpec{ @@ -171,20 +169,18 @@ metadata: }, } - for _, tc := range testCases { - t.Run(tc.description, func(t *testing.T) { - // arrange + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { tr := newRefVarTransformer(tc.given.varMap, tc.given.fs) - - // act err := tr.Transform(tc.given.res) - - // assert if tc.errMessage != "" { if err == nil { t.Fatalf("missing expected error %v", tc.errMessage) } else if err.Error() != tc.errMessage { - t.Fatalf("actual error doesn't match expected error: \nACTUAL: %v\nEXPECTED: %v", err.Error(), tc.errMessage) + t.Fatalf(`actual error doesn't match expected error: +ACTUAL: %v +EXPECTED: %v`, + err.Error(), tc.errMessage) } } else { if err != nil { @@ -194,7 +190,13 @@ metadata: a, e := tc.given.res, tc.expected.res if !reflect.DeepEqual(a, e) { err = e.ErrorIfNotEqualLists(a) - t.Fatalf("actual doesn't match expected: \nACTUAL:\n%v\nEXPECTED:\n%v\nERR: %v", a, e, err) + t.Fatalf(`actual doesn't match expected: +ACTUAL: +%v +EXPECTED: +%v +ERR: %v`, + a, e, err) } } }) diff --git a/api/krusty/namereference_test.go b/api/krusty/namereference_test.go index b3a715d60..c32e1cce2 100644 --- a/api/krusty/namereference_test.go +++ b/api/krusty/namereference_test.go @@ -1,11 +1,194 @@ package krusty_test import ( + "strings" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) +func TestIssue3489(t *testing.T) { + const assets = `{ + "tenantId": "XXXXX-XXXXXX-XXXXX-XXXXXX-XXXXXX", + "subscriptionId": "XXXXX-XXXXXX-XXXXX-XXXXXX-XXXXXX", + "resourceGroup": "DNS-EUW-XXX-RG", + "useManagedIdentityExtension": true, + "userAssignedIdentityID": "XXXXX-XXXXXX-XXXXX-XXXXXX-XXXXXX" +} +` + th := kusttest_test.MakeHarness(t) + th.WriteK(".", ` +namespace: kube-system +resources: +- external-dns +- external-dns-private +`) + th.WriteK("external-dns", ` +resources: +- ../base +commonLabels: + app: external-dns + instance: public +images: +- name: k8s.gcr.io/external-dns/external-dns + newName: xxx.azurecr.io/external-dns + newTag: v0.7.4_sylr.1 +- name: quay.io/sylr/external-dns + newName: xxx.azurecr.io/external-dns + newTag: v0.7.4_sylr.1 +secretGenerator: +- name: azure-config-file + behavior: replace + files: + - assets/azure.json +patches: +- target: + group: apps + version: v1 + kind: Deployment + name: external-dns + patch: |- + - op: replace + path: /spec/template/spec/containers/0/args + value: + - --txt-owner-id="aks" + - --txt-prefix=external-dns- + - --source=service + - --provider=azure + - --registry=txt + - --domain-filter=dev.company.com +`) + + th.WriteF("external-dns/assets/azure.json", assets) + th.WriteK("external-dns-private", ` +resources: +- ../base +nameSuffix: -private +commonLabels: + app: external-dns + instance: private +images: +- name: k8s.gcr.io/external-dns/external-dns + newName: xxx.azurecr.io/external-dns + newTag: v0.7.4_sylr.1 +- name: quay.io/sylr/external-dns + newName: xxx.azurecr.io/external-dns + newTag: v0.7.4_sylr.1 +secretGenerator: +- name: azure-config-file + behavior: replace + files: + - assets/azure.json +patches: +- target: + group: apps + version: v1 + kind: Deployment + name: external-dns + patch: |- + - op: replace + path: /spec/template/spec/containers/0/args + value: + - --txt-owner-id="aks" + - --txt-prefix=external-dns-private- + - --source=service + - --provider=azure-private-dns + - --registry=txt + - --domain-filter=static.company.az +`) + th.WriteF("external-dns-private/assets/azure.json", assets) + th.WriteK("base", ` +resources: +- clusterrole.yaml +- clusterrolebinding.yaml +- deployment.yaml +- serviceaccount.yaml +commonLabels: + app: external-dns + instance: public +images: +- name: k8s.gcr.io/external-dns/external-dns + newName: quay.io/sylr/external-dns + newTag: v0.7.4-73-g00a9a0c7 +secretGenerator: +- name: azure-config-file + files: + - assets/azure.json +`) + th.WriteF("base/assets/azure.json", assets) + th.WriteF("base/clusterrolebinding.yaml", ` +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: external-dns-viewer +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: external-dns +subjects: +- kind: ServiceAccount + name: external-dns +`) + th.WriteF("base/clusterrole.yaml", ` +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: external-dns +rules: +- apiGroups: [''] + resources: ['endpoints', 'pods', 'services', 'nodes'] + verbs: ['get', 'watch', 'list'] +- apiGroups: ['extensions', 'networking.k8s.io'] + resources: ['ingresses'] + verbs: ['get', 'watch', 'list'] +`) + th.WriteF("base/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: external-dns +spec: + strategy: + type: Recreate + selector: + matchLabels: {} + template: + metadata: {} + spec: + serviceAccountName: external-dns + containers: + - name: external-dns + image: k8s.gcr.io/external-dns/external-dns + args: + - --domain-filter="" + - --txt-owner-id="" + - --txt-prefix=external-dns- + - --source=service + - --provider=azure + - --registry=txt + resources: {} + volumeMounts: + - name: azure-config-file + mountPath: /etc/kubernetes + readOnly: true + volumes: + - name: azure-config-file + secret: + secretName: azure-config-file +`) + th.WriteF("base/serviceaccount.yaml", ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: external-dns +`) + // TODO(3489): This shouldn't be an error. + err := th.RunWithErr(".", th.MakeDefaultOptions()) + if !strings.Contains(err.Error(), "found multiple possible referrals") { + t.Fatalf("unexpected error: %q", err) + } +} + func TestEmptyFieldSpecValue(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("/app", ` diff --git a/api/krusty/resourceconflict_test.go b/api/krusty/resourceconflict_test.go index eb53f1638..8f6a6bd0f 100644 --- a/api/krusty/resourceconflict_test.go +++ b/api/krusty/resourceconflict_test.go @@ -362,7 +362,7 @@ metadata: if err == nil { t.Fatalf("expected error") } - if !strings.Contains(err.Error(), "multiple matches for ~G_v1_ServiceAccount|~X|serviceaccount") { + if !strings.Contains(err.Error(), "found multiple possible referrals") { t.Fatalf("unexpected error %v", err) } }