From 30dcf38609c5c8d1afe4bb399698323230f0e25c Mon Sep 17 00:00:00 2001 From: monopole Date: Sun, 10 Jan 2021 09:16:52 -0800 Subject: [PATCH] Add var ref replacement tests and more doc. --- api/filters/nameref/nameref.go | 124 ++++++++++-------- .../accumulator/namereferencetransformer.go | 13 +- api/internal/accumulator/refvartransformer.go | 3 +- api/internal/accumulator/resaccumulator.go | 4 +- api/krusty/variableref_test.go | 59 ++++++++- 5 files changed, 144 insertions(+), 59 deletions(-) diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 1a55c33c4..543a7bed7 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -6,7 +6,6 @@ import ( "strings" "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" @@ -33,6 +32,8 @@ type Filter struct { ReferralCandidates resmap.ResMap } +// At time of writing, in practice this is called with a slice with only +// one entry, the node also referred to be the resource in the Referrer field. func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { return kio.FilterAll(yaml.FilterFunc(f.run)).Filter(nodes) } @@ -40,7 +41,7 @@ func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { // The node passed in here is the same node as held in Referrer, and // that's how the referrer's name field is updated. // However, this filter still needs the extra methods on Referrer -// to consult things like the resource Id, it's namespace, etc. +// to consult things like the resource Id, its namespace, etc. func (f Filter) run(node *yaml.RNode) (*yaml.RNode, error) { err := node.PipeE(fieldspec.Filter{ FieldSpec: f.NameFieldToUpdate, @@ -95,41 +96,53 @@ func (f Filter) setMapping(node *yaml.RNode) error { } oldName := nameNode.YNode().Value - newName, newNamespace, err := f.selectReferral(oldName, subset) + res, err := f.selectReferral(oldName, subset) + if err != nil || res == nil { + // Nil res means nothing to do. + return err + } + f.recordTheReferral(res) + if res.GetName() == oldName && res.GetNamespace() == "" { + // The name has not changed, nothing to do. + return nil + } + err = node.PipeE(yaml.FieldSetter{ + Name: "name", + StringValue: res.GetName(), + }) if err != nil { return err } - - if newName == oldName && newNamespace == "" { - // Nothing to do. - return nil - } - - // set name - node.Pipe(yaml.FieldSetter{ - Name: "name", - StringValue: newName, - }) - if newNamespace != "" { + if res.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. - node.Pipe(yaml.FieldSetter{ + err = node.PipeE(yaml.FieldSetter{ Name: "namespace", - StringValue: newNamespace, + StringValue: res.GetNamespace(), }) } - return nil + return err } func (f Filter) setScalar(node *yaml.RNode) error { - newValue, _, err := f.selectReferral( - node.YNode().Value, - f.ReferralCandidates.Resources()) - if err != nil { + res, err := f.selectReferral( + node.YNode().Value, f.ReferralCandidates.Resources()) + if err != nil || res == nil { + // Nil res means nothing to do. return err } - return filtersutil.SetScalar(newValue)(node) + f.recordTheReferral(res) + if res.GetName() == node.YNode().Value { + // The name has not changed, nothing to do. + return nil + } + return node.PipeE(yaml.FieldSetter{StringValue: res.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()) } func (f Filter) isRoleRef() bool { @@ -191,52 +204,57 @@ func (f Filter) filterReferralCandidates( } // selectReferral picks the referral among a subset of candidates. -// It returns the current name and namespace of the selected candidate. -// The content of the referricalCandidateSubset slice is most of the time -// identical to the referralCandidates resmap. Still in some cases, such +// The content of the candidateSubset 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 (f Filter) selectReferral( oldName string, - referralCandidateSubset []*resource.Resource) (string, string, error) { + candidateSubset []*resource.Resource) (*resource.Resource, error) { var roleRefGvk *resid.Gvk if f.isRoleRef() { var err error roleRefGvk, err = getRoleRefGvk(f.Referrer) if err != nil { - return "", "", err + return nil, err } } - for _, res := range referralCandidateSubset { + for _, res := range candidateSubset { + if res.GetOriginalName() != oldName { + continue + } id := res.OrgId() + if !id.IsSelected(&f.ReferralTarget) { + continue + } // If the we are processing a roleRef, the apiGroup and Kind in the // roleRef are needed to be considered. - if (!f.isRoleRef() || id.IsSelected(roleRefGvk)) && - id.IsSelected(&f.ReferralTarget) && res.GetOriginalName() == oldName { - matches := f.ReferralCandidates.GetMatchingResourcesByOriginalId(id.Equals) - // If there's more than one match, - // filter the matches by prefix and suffix - if len(matches) > 1 { - filteredMatches := f.filterReferralCandidates(matches) - if len(filteredMatches) > 1 { - return "", "", fmt.Errorf( - "multiple matches for %s:\n %v", - id, getIds(filteredMatches)) - } - // Check is the match the resource we are working on - if len(filteredMatches) == 0 || res != 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.GetName(), res.GetNamespace(), nil + if f.isRoleRef() && !id.IsSelected(roleRefGvk) { + continue } + matches := f.ReferralCandidates.GetMatchingResourcesByOriginalId(id.Equals) + // If there's more than one match, + // filter the matches by prefix and suffix + if len(matches) > 1 { + filteredMatches := f.filterReferralCandidates(matches) + if len(filteredMatches) > 1 { + return nil, fmt.Errorf( + "multiple matches for %s:\n %v", + id, getIds(filteredMatches)) + } + // Check is the match the resource we are working on + if len(filteredMatches) == 0 || res != 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 oldName, "", nil + return nil, nil } func getIds(rs []*resource.Resource) []string { diff --git a/api/internal/accumulator/namereferencetransformer.go b/api/internal/accumulator/namereferencetransformer.go index ebaba29a5..d929e3560 100644 --- a/api/internal/accumulator/namereferencetransformer.go +++ b/api/internal/accumulator/namereferencetransformer.go @@ -19,7 +19,8 @@ var _ resmap.Transformer = &nameReferenceTransformer{} // newNameReferenceTransformer constructs a nameReferenceTransformer // with a given slice of NameBackReferences. -func newNameReferenceTransformer(br []builtinconfig.NameBackReferences) resmap.Transformer { +func newNameReferenceTransformer( + br []builtinconfig.NameBackReferences) resmap.Transformer { if br == nil { log.Fatal("backrefs not expected to be nil") } @@ -81,8 +82,18 @@ func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error { for _, fSpec := range referralTarget.FieldSpecs { if referrer.OrgId().IsSelected(&fSpec.Gvk) { if candidates == nil { + // This excludes objects from other namespaces. + // In most realistic uses, it returns all elements of m, + // (since they're all in the same namespace). candidates = m.SubsetThatCouldBeReferencedByResource(referrer) } + // One way to get here is with, say, a referrer that's an + // HPA, and a target that's a Deployment (one of the + // Deployment's fieldSpecs selects an HPA). Now we look + // through the candidates to see if one is a Deployment + // (the target), and if so, get the Deployment's name and + // write it into the referrer, at the field specfied in + // fSpec. err := referrer.ApplyFilter(nameref.Filter{ Referrer: referrer, NameFieldToUpdate: fSpec, diff --git a/api/internal/accumulator/refvartransformer.go b/api/internal/accumulator/refvartransformer.go index 405041b93..5a4da71be 100644 --- a/api/internal/accumulator/refvartransformer.go +++ b/api/internal/accumulator/refvartransformer.go @@ -4,9 +4,8 @@ package accumulator import ( - expansion2 "sigs.k8s.io/kustomize/api/internal/accumulator/expansion" - "sigs.k8s.io/kustomize/api/filters/refvar" + expansion2 "sigs.k8s.io/kustomize/api/internal/accumulator/expansion" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" ) diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index 1e4804e36..f9c0f0764 100644 --- a/api/internal/accumulator/resaccumulator.go +++ b/api/internal/accumulator/resaccumulator.go @@ -164,6 +164,6 @@ func (ra *ResAccumulator) FixBackReferences() (err error) { if ra.tConfig.NameReference == nil { return nil } - return ra.Transform(newNameReferenceTransformer( - ra.tConfig.NameReference)) + return ra.Transform( + newNameReferenceTransformer(ra.tConfig.NameReference)) } diff --git a/api/krusty/variableref_test.go b/api/krusty/variableref_test.go index 19379aa4e..973f4ffbb 100644 --- a/api/krusty/variableref_test.go +++ b/api/krusty/variableref_test.go @@ -929,7 +929,64 @@ metadata: `) } -func TestVariableRefIngress(t *testing.T) { +func TestVariableRefIngressBasic(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK(".", ` +resources: +- ingress.yaml +- deployment.yaml + +vars: +- name: DEPLOYMENT_NAME + objref: + apiVersion: apps/v1 + kind: Deployment + name: nginxDep + fieldref: + fieldpath: metadata.name +`) + th.WriteF("deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginxDep +`) + + th.WriteF("ingress.yaml", ` +apiVersion: networking.k8s.io/v1beta1 +kind: Ingress +metadata: + name: nginxIngress +spec: + rules: + - host: $(DEPLOYMENT_NAME).example.com + tls: + - hosts: + - $(DEPLOYMENT_NAME).example.com + secretName: $(DEPLOYMENT_NAME).example.com-tls +`) + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: networking.k8s.io/v1beta1 +kind: Ingress +metadata: + name: nginxIngress +spec: + rules: + - host: nginxDep.example.com + tls: + - hosts: + - nginxDep.example.com + secretName: nginxDep.example.com-tls +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginxDep +`) +} + +func TestVariableRefIngressOverlay(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("/app/base", ` resources: