From f4382738ab1eaddb4fb726a2612c85022143ff7c Mon Sep 17 00:00:00 2001 From: Yuwen Ma Date: Thu, 16 Sep 2021 11:15:05 -0700 Subject: [PATCH] [fix 4124] Skip local resource until all transformations have completed. Resources annotated as "local-config" are expected to be ignored. This skip local resource happens in "accumulateResources" which happens before any transformation operations. However, the local resource may be needed in transformations. Thus, this change removes the "drop local resource" logic from accumulateResources and removes these local resource after all transformation operations and var operations are done. Note: None of the existing ResMap functions can drop the resource slice easily: "Clear" will ruin the resource order, "AppendAll" only adds non-existing resource, "AbsorbAll" only add or modify but not delete. Thus, we introduce a new func "Intersection" for resourceAccumulator that specificaly removes the resource by ID and keep the original order. --- api/internal/accumulator/resaccumulator.go | 20 ++++ api/internal/target/kusttarget.go | 18 ++++ api/resource/factory.go | 62 +++++------ api/resource/factory_test.go | 115 ++++++++++++++++----- 4 files changed, 161 insertions(+), 54 deletions(-) diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index 2723feafc..934553757 100644 --- a/api/internal/accumulator/resaccumulator.go +++ b/api/internal/accumulator/resaccumulator.go @@ -168,3 +168,23 @@ func (ra *ResAccumulator) FixBackReferences() (err error) { return ra.Transform( newNameReferenceTransformer(ra.tConfig.NameReference)) } + +// Intersection drops the resources which "other" does not have. +func (ra *ResAccumulator) Intersection(other resmap.ResMap) error { + for _, curId := range ra.resMap.AllIds() { + toDelete := true + for _, otherId := range other.AllIds() { + if otherId == curId { + toDelete = false + break + } + } + if toDelete { + err := ra.resMap.Remove(curId) + if err != nil { + return err + } + } + } + return nil +} diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index 45e021e24..d2929a03f 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/pkg/errors" + "sigs.k8s.io/kustomize/api/builtins" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/accumulator" @@ -218,9 +219,26 @@ func (kt *KustTarget) accumulateTarget(ra *accumulator.ResAccumulator, origin *r return nil, errors.Wrapf( err, "merging vars %v", kt.kustomization.Vars) } + err = kt.IgnoreLocal(ra) + if err != nil { + return nil, err + } return ra, nil } +// IgnoreLocal drops the local resource by checking the annotation "config.kubernetes.io/local-config". +func (kt *KustTarget) IgnoreLocal(ra *accumulator.ResAccumulator) error { + rf := kt.rFactory.RF() + if rf.IncludeLocalConfigs { + return nil + } + remainRes, err := rf.DropLocalNodes(ra.ResMap().ToRNodeSlice()) + if err != nil { + return err + } + return ra.Intersection(kt.rFactory.FromResourceSlice(remainRes)) +} + func (kt *KustTarget) runGenerators( ra *accumulator.ResAccumulator) error { var generators []resmap.Generator diff --git a/api/resource/factory.go b/api/resource/factory.go index fb01bc8ce..9399dfa64 100644 --- a/api/resource/factory.go +++ b/api/resource/factory.go @@ -124,14 +124,34 @@ func (rf *Factory) SliceFromBytes(in []byte) ([]*Resource, error) { return rf.resourcesFromRNodes(nodes), nil } +// DropLocalNodes removes the local nodes by default. Local nodes are detected via the annotation `config.kubernetes.io/local-config: "true"` +func (rf *Factory) DropLocalNodes(nodes []*yaml.RNode) ([]*Resource, error) { + var result []*yaml.RNode + for _, node := range nodes { + if node.IsNilOrEmpty() { + continue + } + md, err := node.GetValidatedMetadata() + if err != nil { + return nil, err + } + + if rf.IncludeLocalConfigs { + result = append(result, node) + continue + } + localConfig, exist := md.ObjectMeta.Annotations[konfig.IgnoredByKustomizeAnnotation] + if !exist || localConfig == "false" { + result = append(result, node) + } + } + return rf.resourcesFromRNodes(result), nil +} + // ResourcesFromRNodes converts RNodes to Resources. func (rf *Factory) ResourcesFromRNodes( nodes []*yaml.RNode) (result []*Resource, err error) { - nodes, err = rf.dropBadNodes(nodes) - if err != nil { - return nil, err - } - return rf.resourcesFromRNodes(nodes), nil + return rf.DropLocalNodes(nodes) } // resourcesFromRNode assumes all nodes are good. @@ -216,38 +236,20 @@ func (rf *Factory) convertObjectSliceToNodeSlice( func (rf *Factory) dropBadNodes(nodes []*yaml.RNode) ([]*yaml.RNode, error) { var result []*yaml.RNode for _, n := range nodes { - ignore, err := rf.shouldIgnore(n) - if err != nil { + if n.IsNilOrEmpty() { + continue + } + if _, err := n.GetValidatedMetadata(); err != nil { return nil, err } - if !ignore { - result = append(result, n) + if foundNil, path := n.HasNilEntryInList(); foundNil { + return nil, fmt.Errorf("empty item at %v in object %v", path, n) } + result = append(result, n) } return result, nil } -// shouldIgnore returns true if there's some reason to ignore the node. -func (rf *Factory) shouldIgnore(n *yaml.RNode) (bool, error) { - if n.IsNilOrEmpty() { - return true, nil - } - if !rf.IncludeLocalConfigs { - md, err := n.GetValidatedMetadata() - if err != nil { - return true, err - } - _, ignore := md.ObjectMeta.Annotations[konfig.IgnoredByKustomizeAnnotation] - if ignore { - return true, nil - } - } - if foundNil, path := n.HasNilEntryInList(); foundNil { - return true, fmt.Errorf("empty item at %v in object %v", path, n) - } - return false, nil -} - // SliceFromBytesWithNames unmarshals bytes into a Resource slice with specified original // name. func (rf *Factory) SliceFromBytesWithNames(names []string, in []byte) ([]*Resource, error) { diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index 905b86202..873d6d9bf 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -13,6 +13,7 @@ import ( . "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/filesys" + "sigs.k8s.io/kustomize/kyaml/kio" ) func TestRNodesFromBytes(t *testing.T) { @@ -465,30 +466,6 @@ metadata: `, ` apiVersion: v1 kind: ConfigMap -metadata: - name: winnie -`}, - }, - }, - "localConfigYaml": { - input: []byte(` -apiVersion: v1 -kind: ConfigMap -metadata: - name: winnie-skip - annotations: - # this annotation causes the Resource to be ignored by kustomize - config.kubernetes.io/local-config: "" ---- -apiVersion: v1 -kind: ConfigMap -metadata: - name: winnie -`), - exp: expected{ - out: []string{` -apiVersion: v1 -kind: ConfigMap metadata: name: winnie `}, @@ -673,3 +650,93 @@ data: }) } } + +func TestDropLocalNodes(t *testing.T) { + testCases := map[string]struct { + input []byte + expected []byte + }{ + "localConfigUnset": { + input: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +`), + expected: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +`), + }, + "localConfigSet": { + input: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie-skip + annotations: + # this annotation causes the Resource to be ignored by kustomize + config.kubernetes.io/local-config: "" +`), + expected: nil, + }, + "localConfigSetToTrue": { + input: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie-skip + annotations: + config.kubernetes.io/local-config: "true" + `), + expected: nil, + }, + "localConfigSetToFalse": { + input: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie + annotations: + config.kubernetes.io/local-config: "false" +`), + expected: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + annotations: + config.kubernetes.io/local-config: "false" + name: winnie +`), + }, + "localConfigMultiInput": { + input: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie-skip + annotations: + config.kubernetes.io/local-config: "true" +`), + expected: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +`), + }, + } + for n := range testCases { + tc := testCases[n] + t.Run(n, func(t *testing.T) { + nin, _ := kio.FromBytes(tc.input) + res, err := factory.DropLocalNodes(nin) + assert.NoError(t, err) + if tc.expected == nil { + assert.Equal(t, 0, len(res)) + } else { + actual, _ := res[0].AsYAML() + assert.Equal(t, tc.expected, actual) + } + }) + } +}