Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix 4124] Skip local resource until all transformations have completed. #4180

Merged
merged 2 commits into from Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 {
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -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) {
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 @@ -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
`),
yuwenma marked this conversation as resolved.
Show resolved Hide resolved
},
"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)
}
})
}
}