Skip to content

Commit

Permalink
Merge pull request #4180 from yuwenma/fix-4124
Browse files Browse the repository at this point in the history
[Fix 4124]  Skip local resource until all transformations have completed.
  • Loading branch information
monopole committed Sep 24, 2021
2 parents 22b7358 + f438273 commit dd579c9
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 54 deletions.
20 changes: 20 additions & 0 deletions api/internal/accumulator/resaccumulator.go
Expand Up @@ -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
}
18 changes: 18 additions & 0 deletions api/internal/target/kusttarget.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
70 changes: 70 additions & 0 deletions api/krusty/localconfig_test.go
@@ -0,0 +1,70 @@
// Copyright 2019 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0

package krusty_test

import (
"testing"

kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
)

// This test checks that if a resource is annotated as "local-config", this resource won't be ignored until
// all transformations have completed. This makes sure the local resource can be used as a transformation input.
// See https://github.com/kubernetes-sigs/kustomize/issues/4124 for details.
func TestSKipLocalConfigAfterTransform(t *testing.T) {
th := kusttest_test.MakeHarness(t)
th.WriteK(".", `apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- pod.yaml
- deployment.yaml
transformers:
- replacement.yaml
`)
th.WriteF("pod.yaml", `apiVersion: v1
kind: Pod
metadata:
name: buildup
annotations:
config.kubernetes.io/local-config: "true"
spec:
containers:
- name: app
image: nginx
`)
th.WriteF("deployment.yaml", `apiVersion: apps/v1
kind: Deployment
metadata:
name: buildup
`)
th.WriteF("replacement.yaml", `
apiVersion: builtin
kind: ReplacementTransformer
metadata:
name: buildup
replacements:
- source:
kind: Pod
fieldPath: spec
targets:
- select:
kind: Deployment
fieldPaths:
- spec.template.spec
options:
create: true
`)
m := th.Run(".", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `apiVersion: apps/v1
kind: Deployment
metadata:
name: buildup
spec:
template:
spec:
containers:
- image: nginx
name: app
`)
}
62 changes: 32 additions & 30 deletions api/resource/factory.go
Expand Up @@ -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.
Expand Down Expand Up @@ -224,38 +244,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) {
Expand Down
115 changes: 91 additions & 24 deletions api/resource/factory_test.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
`},
Expand Down Expand Up @@ -670,3 +647,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)
}
})
}
}

0 comments on commit dd579c9

Please sign in to comment.