From a563169461cd044be172ba4212f8217fe45285ef Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Fri, 24 Jul 2020 15:23:17 -0700 Subject: [PATCH] refactor namereftransformer with kyaml --- api/filters/nameref/doc.go | 3 + api/filters/nameref/nameref.go | 214 ++++++++++++++++++ api/filters/nameref/seqfilter.go | 57 +++++ api/filters/nameref/seqfilter_test.go | 80 +++++++ .../accumulator/namereferencetransformer.go | 184 +-------------- .../namereferencetransformer_test.go | 2 +- 6 files changed, 363 insertions(+), 177 deletions(-) create mode 100644 api/filters/nameref/doc.go create mode 100644 api/filters/nameref/nameref.go create mode 100644 api/filters/nameref/seqfilter.go create mode 100644 api/filters/nameref/seqfilter_test.go diff --git a/api/filters/nameref/doc.go b/api/filters/nameref/doc.go new file mode 100644 index 000000000..b78499d51 --- /dev/null +++ b/api/filters/nameref/doc.go @@ -0,0 +1,3 @@ +// Package nameref contains a kio.Filter implementation of the kustomize +// name reference transformer. +package nameref diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go new file mode 100644 index 000000000..d72967c6f --- /dev/null +++ b/api/filters/nameref/nameref.go @@ -0,0 +1,214 @@ +package nameref + +import ( + "fmt" + + "sigs.k8s.io/kustomize/api/filters/fieldspec" + "sigs.k8s.io/kustomize/api/filters/filtersutil" + "sigs.k8s.io/kustomize/api/resid" + "sigs.k8s.io/kustomize/api/resmap" + "sigs.k8s.io/kustomize/api/resource" + "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +// Filter will update the name reference +type Filter struct { + FieldSpec types.FieldSpec `json:"fieldSpec,omitempty" yaml:"fieldSpec,omitempty"` + Referrer *resource.Resource + Target resid.Gvk + ReferralCandidates resmap.ResMap +} + +func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + return kio.FilterAll(yaml.FilterFunc(f.run)).Filter(nodes) +} + +func (f Filter) run(node *yaml.RNode) (*yaml.RNode, error) { + err := node.PipeE(fieldspec.Filter{ + FieldSpec: f.FieldSpec, + SetValue: f.set, + }) + return node, err +} + +func (f Filter) set(node *yaml.RNode) error { + if yaml.IsMissingOrNull(node) { + return nil + } + switch node.YNode().Kind { + case yaml.ScalarNode: + return f.setScalar(node) + case yaml.MappingNode: + // Kind: ValidatingWebhookConfiguration + // FieldSpec is webhooks/clientConfig/service + return f.setMapping(node) + case yaml.SequenceNode: + return f.setSequence(node) + default: + return fmt.Errorf( + "%#v is expected to be either a string or a slice of string or a map of string", node) + } +} + +func (f Filter) setSequence(node *yaml.RNode) error { + return applyFilterToSeq(seqFilter{ + setScalarFn: f.setScalar, + setMappingFn: f.setMapping, + }, node) +} + +func (f Filter) setMapping(node *yaml.RNode) error { + return setNameAndNs( + node, + f.Referrer, + f.Target, + f.ReferralCandidates, + ) +} + +func (f Filter) setScalar(node *yaml.RNode) error { + newValue, err := getSimpleNameField( + node.YNode().Value, + f.Referrer, + f.Target, + f.ReferralCandidates, + f.ReferralCandidates.Resources(), + ) + if err != nil { + return err + } + err = filtersutil.SetScalar(newValue)(node) + if err != nil { + return err + } + return nil +} + +// selectReferral picks the referral among a subset of candidates. +// It returns the current name and namespace of the selected candidate. +// Note that the content of the referricalCandidateSubset slice is most of the time +// identical to the referralCandidates resmap. Still in some cases, such +// as ClusterRoleBinding, the subset only contains the resources of a specific +// namespace. +func selectReferral( + oldName string, + referrer *resource.Resource, + target resid.Gvk, + referralCandidates resmap.ResMap, + referralCandidateSubset []*resource.Resource) (string, string, error) { + + for _, res := range referralCandidateSubset { + id := res.OrgId() + if id.IsSelected(&target) && res.GetOriginalName() == oldName { + matches := referralCandidates.GetMatchingResourcesByOriginalId(id.Equals) + // If there's more than one match, there's no way + // to know which one to pick, so emit error. + if len(matches) > 1 { + return "", "", fmt.Errorf( + "multiple matches for %s:\n %v", + id, getIds(matches)) + } + // In the resource, note that it is referenced + // by the referrer. + res.AppendRefBy(referrer.CurId()) + // Return transformed name of the object, + // complete with prefixes, hashes, etc. + return res.GetName(), res.GetNamespace(), nil + } + } + + return oldName, "", nil +} + +// utility function to replace a simple string by the new name +func getSimpleNameField( + oldName string, + referrer *resource.Resource, + target resid.Gvk, + referralCandidates resmap.ResMap, + referralCandidateSubset []*resource.Resource) (string, error) { + + newName, _, err := selectReferral(oldName, referrer, target, + referralCandidates, referralCandidateSubset) + + return newName, err +} + +func getIds(rs []*resource.Resource) []string { + var result []string + for _, r := range rs { + result = append(result, r.CurId().String()+"\n") + } + return result +} + +// utility function to replace name field within a map RNode +// and leverage the namespace field. +func setNameAndNs( + in *yaml.RNode, + referrer *resource.Resource, + target resid.Gvk, + referralCandidates resmap.ResMap) error { + + if in.YNode().Kind != yaml.MappingNode { + return fmt.Errorf("expect a mapping node") + } + + // Get name field + nameNode, err := in.Pipe(yaml.FieldMatcher{ + Name: "name", + }) + if err != nil || nameNode == nil { + return fmt.Errorf("cannot find field 'name' in node") + } + + // Get namespace field + namespaceNode, err := in.Pipe(yaml.FieldMatcher{ + Name: "namespace", + }) + if err != nil { + return fmt.Errorf("error when find field 'namespace'") + } + + // check is namespace matched + // name will bot be updated if the namespace doesn't match + subset := referralCandidates.Resources() + if namespaceNode != nil { + namespace := namespaceNode.YNode().Value + bynamespace := referralCandidates.GroupedByOriginalNamespace() + if _, ok := bynamespace[namespace]; !ok { + return nil + } + subset = bynamespace[namespace] + } + + oldName := nameNode.YNode().Value + newname, newnamespace, err := selectReferral(oldName, referrer, target, + referralCandidates, subset) + if err != nil { + return err + } + + if (newname == oldName) && (newnamespace == "") { + // no candidate found. + return nil + } + + // set name + in.Pipe(yaml.FieldSetter{ + Name: "name", + StringValue: newname, + }) + if newnamespace != "" { + // 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. + in.Pipe(yaml.FieldSetter{ + Name: "namespace", + StringValue: newnamespace, + }) + } + return nil +} diff --git a/api/filters/nameref/seqfilter.go b/api/filters/nameref/seqfilter.go new file mode 100644 index 000000000..c880694d1 --- /dev/null +++ b/api/filters/nameref/seqfilter.go @@ -0,0 +1,57 @@ +package nameref + +import ( + "fmt" + + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +type setFn func(*yaml.RNode) error + +type seqFilter struct { + setScalarFn setFn + setMappingFn setFn +} + +func (sf seqFilter) Filter(node *yaml.RNode) (*yaml.RNode, error) { + if yaml.IsMissingOrNull(node) { + return node, nil + } + switch node.YNode().Kind { + case yaml.ScalarNode: + // Kind: Role/ClusterRole + // FieldSpec is rules.resourceNames + err := sf.setScalarFn(node) + return node, err + case yaml.MappingNode: + // Kind: RoleBinding/ClusterRoleBinding + // FieldSpec is subjects + // Note: The corresponding fieldSpec had been changed from + // from path: subjects/name to just path: subjects. This is + // what get mutatefield to request the mapping of the whole + // map containing namespace and name instead of just a simple + // string field containing the name + err := sf.setMappingFn(node) + return node, err + default: + return node, fmt.Errorf( + "%#v is expected to be either a string or a map of string", node) + } +} + +// applyFilterToSeq will apply the filter to each element in the sequence node +func applyFilterToSeq(filter yaml.Filter, node *yaml.RNode) error { + if node.YNode().Kind != yaml.SequenceNode { + return fmt.Errorf("expect a sequence node but got %v", node.YNode().Kind) + } + + for _, elem := range node.Content() { + rnode := yaml.NewRNode(elem) + err := rnode.PipeE(filter) + if err != nil { + return err + } + } + + return nil +} diff --git a/api/filters/nameref/seqfilter_test.go b/api/filters/nameref/seqfilter_test.go new file mode 100644 index 000000000..2dbf43c7d --- /dev/null +++ b/api/filters/nameref/seqfilter_test.go @@ -0,0 +1,80 @@ +package nameref + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +func SeqFilter(node *yaml.RNode) (*yaml.RNode, error) { + if node.YNode().Value == "aaa" { + node.YNode().SetString("ccc") + } + return node, nil +} + +func TestApplyFilterToSeq(t *testing.T) { + fltr := yaml.FilterFunc(SeqFilter) + + testCases := map[string]struct { + input string + expect string + }{ + "replace in seq": { + input: ` +- aaa +- bbb`, + expect: ` +- ccc +- bbb`, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + node, err := yaml.Parse(tc.input) + if err != nil { + t.Fatal(err) + } + err = applyFilterToSeq(fltr, node) + if err != nil { + t.Fatal(err) + } + if !assert.Equal(t, + strings.TrimSpace(tc.expect), + strings.TrimSpace(node.MustString())) { + t.Fatalf("expect:\n%s\nactual:\n%s", + strings.TrimSpace(tc.expect), + strings.TrimSpace(node.MustString())) + } + }) + } +} + +func TestApplyFilterToSeqUnhappy(t *testing.T) { + fltr := yaml.FilterFunc(SeqFilter) + + testCases := map[string]struct { + input string + }{ + "replace in seq": { + input: ` +aaa`, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + node, err := yaml.Parse(tc.input) + if err != nil { + t.Fatal(err) + } + err = applyFilterToSeq(fltr, node) + if err == nil { + t.Fatalf("expect an error") + } + }) + } +} diff --git a/api/internal/accumulator/namereferencetransformer.go b/api/internal/accumulator/namereferencetransformer.go index 6dbd73c1a..59497312a 100644 --- a/api/internal/accumulator/namereferencetransformer.go +++ b/api/internal/accumulator/namereferencetransformer.go @@ -4,14 +4,12 @@ package accumulator import ( - "fmt" "log" + "sigs.k8s.io/kustomize/api/filters/nameref" "sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig" - "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" - "sigs.k8s.io/kustomize/api/resource" - "sigs.k8s.io/kustomize/api/transform" + "sigs.k8s.io/kustomize/kyaml/filtersutil" ) type nameReferenceTransformer struct { @@ -86,16 +84,12 @@ func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { if candidates == nil { candidates = m.SubsetThatCouldBeReferencedByResource(referrer) } - err := transform.MutateField( - referrer.Map(), - fSpec.PathSlice(), - fSpec.CreateIfNotPresent, - o.getNewNameFunc( - // referrer could be an HPA instance, - // target could be Gvk for Deployment, - // candidate a list of resources "reachable" - // from the HPA. - referrer, target.Gvk, candidates)) + err := filtersutil.ApplyToJSON(nameref.Filter{ + FieldSpec: fSpec, + Referrer: referrer, + Target: target.Gvk, + ReferralCandidates: candidates, + }, referrer) if err != nil { return err } @@ -105,165 +99,3 @@ func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { } return nil } - -// selectReferral picks the referral among a subset of candidates. -// It returns the current name and namespace of the selected candidate. -// Note that the content of the referricalCandidateSubset slice is most of the time -// identical to the referralCandidates resmap. Still in some cases, such -// as ClusterRoleBinding, the subset only contains the resources of a specific -// namespace. -func (o *nameReferenceTransformer) selectReferral( - oldName string, - referrer *resource.Resource, - target resid.Gvk, - referralCandidates resmap.ResMap, - referralCandidateSubset []*resource.Resource) (interface{}, interface{}, error) { - - for _, res := range referralCandidateSubset { - id := res.OrgId() - if id.IsSelected(&target) && res.GetOriginalName() == oldName { - matches := referralCandidates.GetMatchingResourcesByOriginalId(id.Equals) - // If there's more than one match, there's no way - // to know which one to pick, so emit error. - if len(matches) > 1 { - return nil, nil, fmt.Errorf( - "multiple matches for %s:\n %v", - id, getIds(matches)) - } - // In the resource, note that it is referenced - // by the referrer. - res.AppendRefBy(referrer.CurId()) - // Return transformed name of the object, - // complete with prefixes, hashes, etc. - return res.GetName(), res.GetNamespace(), nil - } - } - - return oldName, nil, nil -} - -// utility function to replace a simple string by the new name -func (o *nameReferenceTransformer) getSimpleNameField( - oldName string, - referrer *resource.Resource, - target resid.Gvk, - referralCandidates resmap.ResMap, - referralCandidateSubset []*resource.Resource) (interface{}, error) { - - newName, _, err := o.selectReferral(oldName, referrer, target, - referralCandidates, referralCandidateSubset) - - return newName, err -} - -// utility function to replace name field within a map[string]interface{} -// and leverage the namespace field. -func (o *nameReferenceTransformer) getNameAndNsStruct( - inMap map[string]interface{}, - referrer *resource.Resource, - target resid.Gvk, - referralCandidates resmap.ResMap) (interface{}, error) { - - // Example: - if _, ok := inMap["name"]; !ok { - return nil, fmt.Errorf( - "%#v is expected to contain a name field", inMap) - } - oldName, ok := inMap["name"].(string) - if !ok { - return nil, fmt.Errorf( - "%#v is expected to contain a name field of type string", oldName) - } - - subset := referralCandidates.Resources() - if namespacevalue, ok := inMap["namespace"]; ok { - namespace := namespacevalue.(string) - bynamespace := referralCandidates.GroupedByOriginalNamespace() - if _, ok := bynamespace[namespace]; !ok { - return inMap, nil - } - subset = bynamespace[namespace] - } - - newname, newnamespace, err := o.selectReferral(oldName, referrer, target, - referralCandidates, subset) - if err != nil { - return nil, err - } - - if (newname == oldName) && (newnamespace == nil) { - // no candidate found. - return inMap, nil - } - - inMap["name"] = newname - if newnamespace != "" { - // 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. - inMap["namespace"] = newnamespace - } - return inMap, nil - -} - -func (o *nameReferenceTransformer) getNewNameFunc( - referrer *resource.Resource, - target resid.Gvk, - referralCandidates resmap.ResMap) func(in interface{}) (interface{}, error) { - return func(in interface{}) (interface{}, error) { - switch thing := in.(type) { - case string: - return o.getSimpleNameField(thing, referrer, target, - referralCandidates, referralCandidates.Resources()) - case map[string]interface{}: - // Kind: ValidatingWebhookConfiguration - // FieldSpec is webhooks/clientConfig/service - return o.getNameAndNsStruct(thing, referrer, target, - referralCandidates) - case []interface{}: - for idx, item := range thing { - switch value := item.(type) { - case string: - // Kind: Role/ClusterRole - // FieldSpec is rules.resourceNames - newName, err := o.getSimpleNameField(value, referrer, target, - referralCandidates, referralCandidates.Resources()) - if err != nil { - return nil, err - } - thing[idx] = newName - case map[string]interface{}: - // Kind: RoleBinding/ClusterRoleBinding - // FieldSpec is subjects - // Note: The corresponding fieldSpec had been changed from - // from path: subjects/name to just path: subjects. This is - // what get mutatefield to request the mapping of the whole - // map containing namespace and name instead of just a simple - // string field containing the name - newMap, err := o.getNameAndNsStruct(value, referrer, target, - referralCandidates) - if err != nil { - return nil, err - } - thing[idx] = newMap - default: - return nil, fmt.Errorf( - "%#v is expected to be either a []string or a []map[string]interface{}", in) - } - } - return in, nil - default: - return nil, fmt.Errorf( - "%#v is expected to be either a string or a []interface{}", in) - } - } -} - -func getIds(rs []*resource.Resource) []string { - var result []string - for _, r := range rs { - result = append(result, r.CurId().String()+"\n") - } - return result -} diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 49a117ea9..d6acbae2a 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -520,7 +520,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { }, }, }).ResMap(), - expectedErr: "is expected to contain a name field"}, + expectedErr: "cannot find field 'name' in node"}, } nrt := newNameReferenceTransformer(builtinconfig.MakeDefaultConfig().NameReference)