From 33edb4f31532263ce3ae4fe1c97b58b81f822c58 Mon Sep 17 00:00:00 2001 From: Yuwen Ma Date: Tue, 14 Sep 2021 10:37:02 -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 | 19 +++++ api/resource/factory.go | 62 ++++++++------- api/resource/factory_test.go | 92 +++++++++++++++++----- 4 files changed, 144 insertions(+), 49 deletions(-) diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index 2723feafcf9..9345537572d 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 45e021e245b..68872e64a92 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" @@ -22,6 +23,7 @@ import ( "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/yaml" ) @@ -218,9 +220,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 kt.rFactory.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 fb01bc8ceec..9399dfa6473 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 7d01eb695bf..6ecb1714418 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -9,10 +9,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/api/loader" . "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 TestSliceFromBytes(t *testing.T) { @@ -504,25 +506,6 @@ metadata: out: []map[string]interface{}{testConfigMap, testConfigMap}, }, }, - "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: []map[string]interface{}{testConfigMap}, - }, - }, "garbageInOneOfTwoObjects": { input: []byte(` apiVersion: v1 @@ -679,3 +662,74 @@ items: }) } } + +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 +`), + }, + } + 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) + } + }) + } +}