From a60d99fdc9b876573301d606d88f42c173ec33a2 Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Thu, 27 Jun 2019 17:05:58 -0700 Subject: [PATCH] Fix 972 --- k8sdeps/transformer/patch/transformer.go | 18 +------------ pkg/patch/transformer/patchjson6902json.go | 30 +--------------------- pkg/resmap/resmap.go | 22 ++++++++++++++++ pkg/target/baseandoverlaysmall_test.go | 2 +- 4 files changed, 25 insertions(+), 47 deletions(-) diff --git a/k8sdeps/transformer/patch/transformer.go b/k8sdeps/transformer/patch/transformer.go index ea0d5c6dd..692b0b75e 100644 --- a/k8sdeps/transformer/patch/transformer.go +++ b/k8sdeps/transformer/patch/transformer.go @@ -13,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/kustomize/v3/pkg/gvk" - "sigs.k8s.io/kustomize/v3/pkg/resid" "sigs.k8s.io/kustomize/v3/pkg/resmap" "sigs.k8s.io/kustomize/v3/pkg/resource" "sigs.k8s.io/kustomize/v3/pkg/transformers" @@ -44,7 +43,7 @@ func (tf *transformer) Transform(m resmap.ResMap) error { return err } for _, patch := range patches.Resources() { - target, err := tf.findPatchTarget(m, patch.OrgId()) + target, err := m.GetById(patch.OrgId()) if err != nil { return err } @@ -96,21 +95,6 @@ func (tf *transformer) Transform(m resmap.ResMap) error { return nil } -func (tf *transformer) findPatchTarget( - m resmap.ResMap, id resid.ResId) (*resource.Resource, error) { - match, err1 := m.GetByOriginalId(id) - if err1 == nil { - return match, nil - } - match, err2 := m.GetByCurrentId(id) - if err2 == nil { - return match, nil - } - return nil, fmt.Errorf( - "%s; %s; failed to find unique target for patch %s", - err1.Error(), err2.Error(), id.GvknString()) -} - // mergePatches merge and index patches by OrgId. // It errors out if there is conflict between patches. func (tf *transformer) mergePatches() (resmap.ResMap, error) { diff --git a/pkg/patch/transformer/patchjson6902json.go b/pkg/patch/transformer/patchjson6902json.go index 8a76952f1..0f44e9393 100644 --- a/pkg/patch/transformer/patchjson6902json.go +++ b/pkg/patch/transformer/patchjson6902json.go @@ -23,7 +23,6 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/kustomize/v3/pkg/resid" "sigs.k8s.io/kustomize/v3/pkg/resmap" - "sigs.k8s.io/kustomize/v3/pkg/resource" "sigs.k8s.io/kustomize/v3/pkg/transformers" "sigs.k8s.io/yaml" ) @@ -66,7 +65,7 @@ func newPatchJson6902JSONTransformer( // Transform apply the json patches on top of the base resources. func (t *patchJson6902JSONTransformer) Transform(m resmap.ResMap) error { - obj, err := t.findTargetObj(m) + obj, err := m.GetById(t.target) if err != nil { return err } @@ -84,30 +83,3 @@ func (t *patchJson6902JSONTransformer) Transform(m resmap.ResMap) error { } return nil } - -func (t *patchJson6902JSONTransformer) findTargetObj( - m resmap.ResMap) (*resource.Resource, error) { - var matched []*resource.Resource - // TODO(monopole): namespace bug in json patch? - // Since introduction in PR #300 - // (see pkg/patch/transformer/util.go), - // this code has treated an empty namespace like a wildcard - // rather than like an additional restriction to match - // only the empty namespace. No test coverage to confirm. - // Not sure if desired, keeping it for now. - if t.target.Namespace != "" { - matched = m.GetMatchingResourcesByOriginalId(t.target.Equals) - } else { - matched = m.GetMatchingResourcesByOriginalId(t.target.GvknEquals) - } - if len(matched) == 0 { - return nil, fmt.Errorf( - "couldn't find target %v for json patch", t.target) - } - if len(matched) > 1 { - return nil, fmt.Errorf( - "found multiple targets %v matching %v for json patch", - matched, t.target) - } - return matched[0], nil -} diff --git a/pkg/resmap/resmap.go b/pkg/resmap/resmap.go index 2aec36b00..3d09595ad 100644 --- a/pkg/resmap/resmap.go +++ b/pkg/resmap/resmap.go @@ -89,6 +89,12 @@ type ResMap interface { // an exact match, returning an error on multiple or no matches. GetByOriginalId(resid.ResId) (*resource.Resource, error) + // GetById is a helper function which first + // attempts GetByOriginalId, then GetByCurrentId, + // returning an error if both fail to find a single + // match. + GetById(resid.ResId) (*resource.Resource, error) + // GroupedByNamespace returns a map of namespace // to a slice of *Resource in that namespace. // Resources for whom IsNamespaceableKind is false are @@ -333,6 +339,22 @@ func (m *resWrangler) GetByOriginalId( return demandOneMatch(m.GetMatchingResourcesByOriginalId, id, "Original") } +// GetById implements ResMap. +func (m *resWrangler) GetById( + id resid.ResId) (*resource.Resource, error) { + match, err1 := m.GetByOriginalId(id) + if err1 == nil { + return match, nil + } + match, err2 := m.GetByCurrentId(id) + if err2 == nil { + return match, nil + } + return nil, fmt.Errorf( + "%s; %s; failed to find unique target for patch %s", + err1.Error(), err2.Error(), id.GvknString()) +} + type resFinder func(IdMatcher) []*resource.Resource func demandOneMatch( diff --git a/pkg/target/baseandoverlaysmall_test.go b/pkg/target/baseandoverlaysmall_test.go index 3bc99b75f..73d71c645 100644 --- a/pkg/target/baseandoverlaysmall_test.go +++ b/pkg/target/baseandoverlaysmall_test.go @@ -419,7 +419,7 @@ patchesJson6902: - target: version: v1 kind: Service - name: myService # BUG (https://github.com/kubernetes-sigs/kustomize/issues/972): this should be a-myService, because that is what the output for the base contains + name: a-myService path: service-patch.yaml `)