Option to customize NamespaceTransformer overwrite behaviour (#4708)

* Option to customize NamespaceTransformer overwrite behaviour

* Code review feedback
This commit is contained in:
Katrina Verey
2022-07-14 15:00:58 -04:00
committed by GitHub
parent 17cbd96667
commit 0c6e827ab8
10 changed files with 378 additions and 82 deletions

View File

@@ -37,7 +37,7 @@ linters:
- gocyclo
# - godot
# - godox
- goerr113
# - goerr113
- gofmt
# - gofumpt
- goheader

View File

@@ -28,7 +28,7 @@ uninstall-out-of-tree-tools:
rm -f $(MYGOBIN)/stringer
$(MYGOBIN)/golangci-lint:
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.45.2
go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.2
$(MYGOBIN)/mdrip:
go install github.com/monopole/mdrip@v1.0.2

View File

@@ -12,13 +12,13 @@ type SetFn func(*yaml.RNode) error
// SetScalar returns a SetFn to set a scalar value
func SetScalar(value string) SetFn {
return func(node *yaml.RNode) error {
return node.PipeE(yaml.FieldSetter{StringValue: value})
}
return SetEntry("", value, yaml.NodeTagEmpty)
}
// SetEntry returns a SetFn to set an entry in a map
func SetEntry(key, value, tag string) SetFn {
// SetEntry returns a SetFn to set a field or a map entry to a value.
// It can be used with an empty name to set both a value and a tag on a scalar node.
// When setting only a value on a scalar node, use SetScalar instead.
func SetEntry(name, value, tag string) SetFn {
n := &yaml.Node{
Kind: yaml.ScalarNode,
Value: value,
@@ -26,7 +26,7 @@ func SetEntry(key, value, tag string) SetFn {
}
return func(node *yaml.RNode) error {
return node.PipeE(yaml.FieldSetter{
Name: key,
Name: name,
Value: yaml.NewRNode(n),
})
}
@@ -34,36 +34,72 @@ func SetEntry(key, value, tag string) SetFn {
type TrackableSetter struct {
// SetValueCallback will be invoked each time a field is set
setValueCallback func(key, value, tag string, node *yaml.RNode)
setValueCallback func(name, value, tag string, node *yaml.RNode)
}
// WithMutationTracker registers a callback which will be invoked each time a field is mutated
func (s *TrackableSetter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) {
func (s *TrackableSetter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) *TrackableSetter {
s.setValueCallback = callback
return s
}
// SetScalar returns a SetFn to set a scalar value
// SetScalar returns a SetFn to set a scalar value.
// if a mutation tracker has been registered, the tracker will be invoked each
// time a scalar is set
func (s TrackableSetter) SetScalar(value string) SetFn {
origSetScalar := SetScalar(value)
return s.SetEntry("", value, yaml.NodeTagEmpty)
}
// SetScalarIfEmpty returns a SetFn to set a scalar value only if it isn't already set.
// If a mutation tracker has been registered, the tracker will be invoked each
// time a scalar is actually set.
func (s TrackableSetter) SetScalarIfEmpty(value string) SetFn {
return s.SetEntryIfEmpty("", value, yaml.NodeTagEmpty)
}
// SetEntry returns a SetFn to set a field or a map entry to a value.
// It can be used with an empty name to set both a value and a tag on a scalar node.
// When setting only a value on a scalar node, use SetScalar instead.
// If a mutation tracker has been registered, the tracker will be invoked each
// time an entry is set.
func (s TrackableSetter) SetEntry(name, value, tag string) SetFn {
origSetEntry := SetEntry(name, value, tag)
return func(node *yaml.RNode) error {
if s.setValueCallback != nil {
s.setValueCallback("", value, "", node)
s.setValueCallback(name, value, tag, node)
}
return origSetScalar(node)
return origSetEntry(node)
}
}
// SetEntry returns a SetFn to set an entry in a map
// if a mutation tracker has been registered, the tracker will be invoked each
// time an entry is set
func (s TrackableSetter) SetEntry(key, value, tag string) SetFn {
// SetEntryIfEmpty returns a SetFn to set a field or a map entry to a value only if it isn't already set.
// It can be used with an empty name to set both a value and a tag on a scalar node.
// When setting only a value on a scalar node, use SetScalar instead.
// If a mutation tracker has been registered, the tracker will be invoked each
// time an entry is actually set.
func (s TrackableSetter) SetEntryIfEmpty(key, value, tag string) SetFn {
origSetEntry := SetEntry(key, value, tag)
return func(node *yaml.RNode) error {
if hasExistingValue(node, key) {
return nil
}
if s.setValueCallback != nil {
s.setValueCallback(key, value, tag, node)
}
return origSetEntry(node)
}
}
func hasExistingValue(node *yaml.RNode, key string) bool {
if node.IsNilOrEmpty() {
return false
}
if err := yaml.ErrorIfInvalid(node, yaml.ScalarNode); err == nil {
return yaml.GetValue(node) != ""
}
entry := node.Field(key)
if entry.IsNilOrEmpty() {
return false
}
return yaml.GetValue(entry.Value) != ""
}

View File

@@ -0,0 +1,106 @@
// Copyright 2022 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
package filtersutil_test
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sigs.k8s.io/kustomize/api/filters/filtersutil"
"sigs.k8s.io/kustomize/kyaml/yaml"
)
func TestTrackableSetter_SetScalarIfEmpty(t *testing.T) {
tests := []struct {
name string
input *yaml.RNode
value string
want string
}{
{
name: "sets null values",
input: yaml.MakeNullNode(),
value: "foo",
want: "foo",
},
{
name: "sets empty values",
input: yaml.NewScalarRNode(""),
value: "foo",
want: "foo",
},
{
name: "does not overwrite values",
input: yaml.NewStringRNode("a"),
value: "foo",
want: "a",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
wasSet := false
s := (&filtersutil.TrackableSetter{}).WithMutationTracker(func(_, _, _ string, _ *yaml.RNode) {
wasSet = true
})
wantSet := tt.value == tt.want
fn := s.SetScalarIfEmpty(tt.value)
require.NoError(t, fn(tt.input))
assert.Equal(t, tt.want, yaml.GetValue(tt.input))
assert.Equal(t, wantSet, wasSet, "tracker invoked even though value was not changed")
})
}
}
func TestTrackableSetter_SetEntryIfEmpty(t *testing.T) {
tests := []struct {
name string
input *yaml.RNode
key string
value string
want string
}{
{
name: "sets empty values",
input: yaml.NewMapRNode(&map[string]string{"setMe": ""}),
key: "setMe",
value: "foo",
want: "foo",
},
{
name: "sets missing keys",
input: yaml.NewMapRNode(&map[string]string{}),
key: "setMe",
value: "foo",
want: "foo",
},
{
name: "does not overwrite values",
input: yaml.NewMapRNode(&map[string]string{"existing": "original"}),
key: "existing",
value: "foo",
want: "original",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
wasSet := false
s := (&filtersutil.TrackableSetter{}).WithMutationTracker(func(_, _, _ string, _ *yaml.RNode) {
wasSet = true
})
wantSet := tt.value == tt.want
fn := s.SetEntryIfEmpty(tt.key, tt.value, "")
require.NoError(t, fn(tt.input))
assert.Equal(t, tt.want, yaml.GetValue(tt.input.Field(tt.key).Value))
assert.Equal(t, wantSet, wasSet, "tracker invoked even though value was not changed")
})
}
}
func TestTrackableSetter_SetEntryIfEmpty_BadInputNodeKind(t *testing.T) {
fn := filtersutil.TrackableSetter{}.SetEntryIfEmpty("foo", "false", yaml.NodeTagBool)
rn := yaml.NewListRNode("nope")
rn.AppendToFieldPath("dummy", "path")
assert.EqualError(t, fn(rn), "wrong Node Kind for dummy.path expected: MappingNode was SequenceNode: value: {- nope}")
}

View File

@@ -337,19 +337,16 @@ func (f Filter) selectReferral(
return candidates[0], nil
}
ids := getIds(candidates)
f.failureDetails(candidates)
return nil, fmt.Errorf(" found multiple possible referrals: %s", ids)
return nil, fmt.Errorf("found multiple possible referrals: %s\n%s", ids, f.failureDetails(candidates))
}
func (f Filter) failureDetails(resources []*resource.Resource) {
fmt.Printf(
"\n**** Too many possible referral targets to referrer:\n%s\n",
f.Referrer.MustYaml())
func (f Filter) failureDetails(resources []*resource.Resource) string {
msg := strings.Builder{}
msg.WriteString(fmt.Sprintf("\n**** Too many possible referral targets to referrer:\n%s\n", f.Referrer.MustYaml()))
for i, r := range resources {
fmt.Printf(
"--- possible referral %d:\n%s", i, r.MustYaml())
fmt.Println("------")
msg.WriteString(fmt.Sprintf("--- possible referral %d:\n%s\n", i, r.MustYaml()))
}
return msg.String()
}
func allNamesAreTheSame(resources []*resource.Resource) bool {

View File

@@ -19,6 +19,9 @@ type Filter struct {
// FsSlice contains the FieldSpecs to locate the namespace field
FsSlice types.FsSlice `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
// UnsetOnly means only blank namespace fields will be set
UnsetOnly bool `json:"unsetOnly" yaml:"unsetOnly"`
trackableSetter filtersutil.TrackableSetter
}
@@ -36,47 +39,34 @@ func (ns Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
// Run runs the filter on a single node rather than a slice
func (ns Filter) run(node *yaml.RNode) (*yaml.RNode, error) {
// hacks for hardcoded types -- :(
if err := ns.hacks(node); err != nil {
// Special handling for metadata.namespace -- :(
// never let SetEntry handle metadata.namespace--it will incorrectly include cluster-scoped resources
ns.FsSlice = ns.removeMetaNamespaceFieldSpecs(ns.FsSlice)
gvk := resid.GvkFromNode(node)
if err := ns.metaNamespaceHack(node, gvk); err != nil {
return nil, err
}
// Remove the fieldspecs that are for hardcoded fields. The fieldspecs
// exist for backwards compatibility with other implementations
// of this transformation.
// This implementation of the namespace transformation
// Does not use the fieldspecs for implementing cases which
// require hardcoded logic.
ns.FsSlice = ns.removeFieldSpecsForHacks(ns.FsSlice)
// Special handling for (cluster) role binding -- :(
if isRoleBinding(gvk.Kind) {
ns.FsSlice = ns.removeRoleBindingFieldSpecs(ns.FsSlice)
if err := ns.roleBindingHack(node, gvk); err != nil {
return nil, err
}
}
// transformations based on data -- :)
err := node.PipeE(fsslice.Filter{
FsSlice: ns.FsSlice,
SetValue: ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString),
SetValue: ns.fieldSetter(),
CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode
CreateTag: yaml.NodeTagString,
})
return node, err
}
// hacks applies the namespace transforms that are hardcoded rather
// than specified through FieldSpecs.
func (ns Filter) hacks(obj *yaml.RNode) error {
gvk := resid.GvkFromNode(obj)
if err := ns.metaNamespaceHack(obj, gvk); err != nil {
return err
}
return ns.roleBindingHack(obj, gvk)
}
// metaNamespaceHack is a hack for implementing the namespace transform
// for the metadata.namespace field on namespace scoped resources.
// namespace scoped resources are determined by NOT being present
// in a hard-coded list of cluster-scoped resource types (by apiVersion and kind).
//
// This hack should be updated to allow individual resources to specify
// if they are cluster scoped through either an annotation on the resources,
// or through inlined OpenAPI on the resource as a YAML comment.
func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error {
if gvk.IsClusterScoped() {
return nil
@@ -85,16 +75,16 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error {
FsSlice: []types.FieldSpec{
{Path: types.MetadataNamespacePath, CreateIfNotPresent: true},
},
SetValue: ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString),
SetValue: ns.fieldSetter(),
CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode
}
_, err := f.Filter(obj)
return err
}
// roleBindingHack is a hack for implementing the namespace transform
// roleBindingHack is a hack for implementing the transformer's DefaultSubjectsOnly mode
// for RoleBinding and ClusterRoleBinding resource types.
// RoleBinding and ClusterRoleBinding have namespace set on
// In this mode, RoleBinding and ClusterRoleBinding have namespace set on
// elements of the "subjects" field if and only if the subject elements
// "name" is "default". Otherwise the namespace is not set.
//
@@ -107,12 +97,11 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error {
// - name: "something-else" # this will not have the namespace set
// ...
func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error {
if gvk.Kind != roleBindingKind && gvk.Kind != clusterRoleBindingKind {
if !isRoleBinding(gvk.Kind) {
return nil
}
// Lookup the namespace field on all elements.
// We should change the fieldspec so this isn't necessary.
obj, err := obj.Pipe(yaml.Lookup(subjectsField))
if err != nil || yaml.IsMissingOrNull(obj) {
return err
@@ -138,27 +127,22 @@ func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error {
return err
}
return ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString)(node)
return ns.fieldSetter()(node)
})
return err
}
// removeFieldSpecsForHacks removes from the list fieldspecs that
func isRoleBinding(kind string) bool {
return kind == roleBindingKind || kind == clusterRoleBindingKind
}
// removeRoleBindingFieldSpecs removes from the list fieldspecs that
// have hardcoded implementations
func (ns Filter) removeFieldSpecsForHacks(fs types.FsSlice) types.FsSlice {
func (ns Filter) removeRoleBindingFieldSpecs(fs types.FsSlice) types.FsSlice {
var val types.FsSlice
for i := range fs {
// implemented by metaNamespaceHack
if fs[i].Path == types.MetadataNamespacePath {
continue
}
// implemented by roleBindingHack
if fs[i].Kind == roleBindingKind && fs[i].Path == subjectsField {
continue
}
// implemented by roleBindingHack
if fs[i].Kind == clusterRoleBindingKind && fs[i].Path == subjectsField {
if isRoleBinding(fs[i].Kind) && fs[i].Path == subjectsField {
continue
}
val = append(val, fs[i])
@@ -166,6 +150,24 @@ func (ns Filter) removeFieldSpecsForHacks(fs types.FsSlice) types.FsSlice {
return val
}
func (ns Filter) removeMetaNamespaceFieldSpecs(fs types.FsSlice) types.FsSlice {
var val types.FsSlice
for i := range fs {
if fs[i].Path == types.MetadataNamespacePath {
continue
}
val = append(val, fs[i])
}
return val
}
func (ns *Filter) fieldSetter() filtersutil.SetFn {
if ns.UnsetOnly {
return ns.trackableSetter.SetEntryIfEmpty("", ns.Namespace, yaml.NodeTagString)
}
return ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString)
}
const (
subjectsField = "subjects"
roleBindingKind = "RoleBinding"

View File

@@ -16,6 +16,7 @@ import (
type NamespaceTransformerPlugin struct {
types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
UnsetOnly bool `json:"unsetOnly" yaml:"unsetOnly"`
}
func (p *NamespaceTransformerPlugin) Config(
@@ -38,6 +39,7 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
if err := r.ApplyFilter(namespace.Filter{
Namespace: p.Namespace,
FsSlice: p.FieldSpecs,
UnsetOnly: p.UnsetOnly,
}); err != nil {
return err
}

View File

@@ -17,6 +17,7 @@ import (
type plugin struct {
types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
UnsetOnly bool `json:"unsetOnly" yaml:"unsetOnly"`
}
//noinspection GoUnusedGlobalVariable
@@ -42,6 +43,7 @@ func (p *plugin) Transform(m resmap.ResMap) error {
if err := r.ApplyFilter(namespace.Filter{
Namespace: p.Namespace,
FsSlice: p.FieldSpecs,
UnsetOnly: p.UnsetOnly,
}); err != nil {
return err
}

View File

@@ -10,6 +10,18 @@ import (
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
)
const defaultFieldSpecs = `
fieldSpecs:
- path: metadata/namespace
create: true
- path: subjects
kind: RoleBinding
group: rbac.authorization.k8s.io
- path: subjects
kind: ClusterRoleBinding
group: rbac.authorization.k8s.io
`
func TestNamespaceTransformer1(t *testing.T) {
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("NamespaceTransformer")
@@ -20,16 +32,7 @@ kind: NamespaceTransformer
metadata:
name: notImportantHere
namespace: test
fieldSpecs:
- path: metadata/namespace
create: true
- path: subjects
kind: RoleBinding
group: rbac.authorization.k8s.io
- path: subjects
kind: ClusterRoleBinding
group: rbac.authorization.k8s.io
`, `
`+defaultFieldSpecs, `
apiVersion: v1
kind: ConfigMap
metadata:
@@ -268,3 +271,151 @@ metadata:
}
})
}
func TestNamespaceTransformer_UnsetOnlyTrue(t *testing.T) {
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("NamespaceTransformer")
defer th.Reset()
th.RunTransformerAndCheckResult(`
apiVersion: builtin
kind: NamespaceTransformer
metadata:
name: notImportantHere
namespace: test
unsetOnly: true
fieldSpecs:
- path: metadata/namespace
create: true
- path: subjects/namespace
kind: RoleBinding
group: rbac.authorization.k8s.io
- path: subjects/namespace
kind: ClusterRoleBinding
group: rbac.authorization.k8s.io
- path: spec/template/namespace
kind: MyWeirdObject
- path: spec/template/altNamespace
kind: MyWeirdObject
create: true
`, `
apiVersion: v1
kind: ConfigMap
metadata:
name: cm1
---
apiVersion: v1
kind: ConfigMap
metadata:
name: cm2
namespace: foo
---
apiVersion: v1
kind: Service
metadata:
name: svc1
---
apiVersion: v1
kind: Service
metadata:
name: svc2
namespace: ""
---
apiVersion: v1
kind: Namespace
metadata:
name: ns1
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: manager-rolebinding
subjects:
- kind: ServiceAccount
name: default
namespace: other
- kind: ServiceAccount
name: default
namespace: ""
- kind: ServiceAccount
name: default
- kind: ServiceAccount
name: another
namespace: random
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: crd
---
apiVersion: v1
kind: MyWeirdObject
metadata:
name: custom
spec:
template:
namespace: original
`,
`
apiVersion: v1
kind: ConfigMap
metadata:
name: cm1
namespace: test
---
apiVersion: v1
kind: ConfigMap
metadata:
name: cm2
namespace: foo
---
apiVersion: v1
kind: Service
metadata:
name: svc1
namespace: test
---
apiVersion: v1
kind: Service
metadata:
name: svc2
namespace: test
---
apiVersion: v1
kind: Namespace
metadata:
name: ns1
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: manager-rolebinding
subjects:
- kind: ServiceAccount
name: default
namespace: other
- kind: ServiceAccount
name: default
namespace: test
- kind: ServiceAccount
name: default
namespace: test
- kind: ServiceAccount
name: another
namespace: random
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: crd
---
apiVersion: v1
kind: MyWeirdObject
metadata:
name: custom
namespace: test
spec:
template:
altNamespace: test
namespace: original
`)
}

View File

@@ -4,6 +4,7 @@ go 1.18
require (
sigs.k8s.io/kustomize/api v0.11.5
sigs.k8s.io/kustomize/kyaml v0.13.7
sigs.k8s.io/yaml v1.2.0
)
@@ -32,5 +33,4 @@ require (
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
k8s.io/kube-openapi v0.0.0-20220401212409-b28bf2818661 // indirect
sigs.k8s.io/kustomize/kyaml v0.13.7 // indirect
)