From d04877a9e7486f6c0f336a4b63397bd37ebe36c6 Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Tue, 30 Oct 2018 17:31:28 -0700 Subject: [PATCH] Simplify some code and add TODOs. --- k8sdeps/transformer/hash/namehash.go | 16 ++----- .../testcase-multibases-conflict/test.yaml | 2 +- .../{transformer.go => factory.go} | 0 pkg/target/kusttarget.go | 3 -- pkg/transformers/namereference.go | 46 +++++++++---------- 5 files changed, 25 insertions(+), 42 deletions(-) rename pkg/ifc/transformer/{transformer.go => factory.go} (100%) diff --git a/k8sdeps/transformer/hash/namehash.go b/k8sdeps/transformer/hash/namehash.go index 126623b64..75b9e893c 100644 --- a/k8sdeps/transformer/hash/namehash.go +++ b/k8sdeps/transformer/hash/namehash.go @@ -20,7 +20,6 @@ import ( "fmt" "sigs.k8s.io/kustomize/pkg/resmap" - "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/transformers" ) @@ -33,25 +32,16 @@ func NewNameHashTransformer() transformers.Transformer { return &nameHashTransformer{} } -// Transform appends hash to configmaps and secrets. +// Transform appends hash to generated resources. func (o *nameHashTransformer) Transform(m resmap.ResMap) error { for _, res := range m { if res.IsGenerated() { - err := o.appendHash(res) + h, err := NewKustHash().Hash(res.Map()) if err != nil { return err } + res.SetName(fmt.Sprintf("%s-%s", res.GetName(), h)) } } return nil } - -func (o *nameHashTransformer) appendHash(res *resource.Resource) error { - h, err := NewKustHash().Hash(res.Map()) - if err != nil { - return err - } - nameWithHash := fmt.Sprintf("%s-%s", res.GetName(), h) - res.SetName(nameWithHash) - return nil -} diff --git a/pkg/commands/build/testdata/testcase-multibases-conflict/test.yaml b/pkg/commands/build/testdata/testcase-multibases-conflict/test.yaml index 496ce95b8..bda51d606 100644 --- a/pkg/commands/build/testdata/testcase-multibases-conflict/test.yaml +++ b/pkg/commands/build/testdata/testcase-multibases-conflict/test.yaml @@ -1,4 +1,4 @@ description: multibases with name reference args: [] filename: testdata/testcase-multibases-conflict/combined -expectedError: detected conflicts when resolving name references serviceaccount +expectedError: Multiple matches for name noGroup_v1_ServiceAccount diff --git a/pkg/ifc/transformer/transformer.go b/pkg/ifc/transformer/factory.go similarity index 100% rename from pkg/ifc/transformer/transformer.go rename to pkg/ifc/transformer/factory.go diff --git a/pkg/target/kusttarget.go b/pkg/target/kusttarget.go index bc38bddf6..f665344c4 100644 --- a/pkg/target/kusttarget.go +++ b/pkg/target/kusttarget.go @@ -82,7 +82,6 @@ func unmarshal(y []byte, o interface{}) error { if err != nil { return err } - dec := json.NewDecoder(bytes.NewReader(j)) dec.DisallowUnknownFields() return dec.Decode(o) @@ -111,7 +110,6 @@ func (kt *KustTarget) resolveRefsToGeneratedResources(m resmap.ResMap) (resmap.R return nil, err } } - var r []transformers.Transformer t, err := transformers.NewNameReferenceTransformer(kt.tConfig.NameReference) if err != nil { @@ -159,7 +157,6 @@ func (kt *KustTarget) loadCustomizedResMap() (resmap.ResMap, error) { if err != nil { errs.Append(errors.Wrap(err, "SliceFromPatches")) } - if len(errs.Get()) > 0 { return nil, errs } diff --git a/pkg/transformers/namereference.go b/pkg/transformers/namereference.go index de494584e..300226e33 100644 --- a/pkg/transformers/namereference.go +++ b/pkg/transformers/namereference.go @@ -21,7 +21,6 @@ import ( "fmt" "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/transformers/config" ) @@ -47,17 +46,21 @@ func NewNameReferenceTransformer( // associated with the key. e.g. if is one of the key-value pair in the map, // then the old name is k.Name and the new name is v.GetName() func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { + // TODO: Too much looping. + // Even more hidden loops in FilterBy, + // updateNameReference and FindByGVKN. for id := range m { - objMap := m[id].Map() for _, backRef := range o.backRefs { for _, fSpec := range backRef.FieldSpecs { - if !id.Gvk().IsSelected(&fSpec.Gvk) { - continue - } - err := mutateField(objMap, fSpec.PathSlice(), fSpec.CreateIfNotPresent, - o.updateNameReference(backRef.Gvk, m.FilterBy(id))) - if err != nil { - return err + if id.Gvk().IsSelected(&fSpec.Gvk) { + err := mutateField( + m[id].Map(), fSpec.PathSlice(), + fSpec.CreateIfNotPresent, + o.updateNameReference( + backRef.Gvk, m.FilterBy(id))) + if err != nil { + return err + } } } } @@ -72,27 +75,20 @@ func (o *nameReferenceTransformer) updateNameReference( if !ok { return nil, fmt.Errorf("%#v is expectd to be %T", in, s) } - for id, res := range m { - if !id.Gvk().IsSelected(&backRef) { - continue - } - if id.Name() == s { - err := o.detectConflict(id, m, s) - if err != nil { - return nil, err + if id.Gvk().IsSelected(&backRef) && id.Name() == s { + matchedIds := m.FindByGVKN(id) + // If there's more than one match, there's no way + // to know which one to pick, so emit error. + if len(matchedIds) > 1 { + return nil, fmt.Errorf( + "Multiple matches for name %s:\n %v", id, matchedIds) } + // Return transformed name of the object, + // complete with prefixes, hashes, etc. return res.GetName(), nil } } return in, nil } } - -func (o *nameReferenceTransformer) detectConflict(id resid.ResId, m resmap.ResMap, name string) error { - matchedIds := m.FindByGVKN(id) - if len(matchedIds) > 1 { - return fmt.Errorf("detected conflicts when resolving name references %s:\n%v", name, matchedIds) - } - return nil -}