diff --git a/api/filters/filtersutil/setters_test.go b/api/filters/filtersutil/setters_test.go index 862a968b3..50838076f 100644 --- a/api/filters/filtersutil/setters_test.go +++ b/api/filters/filtersutil/setters_test.go @@ -102,5 +102,7 @@ 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}") + assert.EqualError(t, fn(rn), `wrong node kind: expected MappingNode but got SequenceNode: node contents: +- nope +`) } diff --git a/api/filters/namespace/namespace.go b/api/filters/namespace/namespace.go index 5173a9554..7214f0760 100644 --- a/api/filters/namespace/namespace.go +++ b/api/filters/namespace/namespace.go @@ -79,7 +79,11 @@ func (ns Filter) run(node *yaml.RNode) (*yaml.RNode, error) { CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode CreateTag: yaml.NodeTagString, }) - return node, err + invalidKindErr := &yaml.InvalidNodeKindError{} + if err != nil && errors.As(err, &invalidKindErr) && invalidKindErr.ActualNodeKind() != yaml.ScalarNode { + return nil, errors.WrapPrefixf(err, "namespace field specs must target scalar nodes") + } + return node, errors.WrapPrefixf(err, "namespace transformation failed") } // metaNamespaceHack is a hack for implementing the namespace transform @@ -174,8 +178,7 @@ func setNamespaceField(node *yaml.RNode, setter filtersutil.SetFn) error { func (ns Filter) removeRoleBindingSubjectFieldSpecs(fs types.FsSlice) types.FsSlice { var val types.FsSlice for i := range fs { - if isRoleBinding(fs[i].Kind) && - (fs[i].Path == subjectsNamespacePath || fs[i].Path == subjectsField) { + if isRoleBinding(fs[i].Kind) && fs[i].Path == subjectsNamespacePath { continue } val = append(val, fs[i]) diff --git a/api/go.mod b/api/go.mod index e9e6816c9..1f816cf69 100644 --- a/api/go.mod +++ b/api/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( github.com/evanphx/json-patch v4.11.0+incompatible - github.com/go-errors/errors v1.0.1 + github.com/go-errors/errors v1.4.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/imdario/mergo v0.3.6 github.com/pkg/errors v0.9.1 diff --git a/cmd/config/go.mod b/cmd/config/go.mod index 5ce5fe071..56c0b9d8f 100644 --- a/cmd/config/go.mod +++ b/cmd/config/go.mod @@ -3,7 +3,7 @@ module sigs.k8s.io/kustomize/cmd/config go 1.18 require ( - github.com/go-errors/errors v1.0.1 + github.com/go-errors/errors v1.4.2 github.com/olekukonko/tablewriter v0.0.4 github.com/spf13/cobra v1.4.0 github.com/stretchr/testify v1.7.0 diff --git a/kyaml/errors/errors.go b/kyaml/errors/errors.go index f072c3c97..c292d7c68 100644 --- a/kyaml/errors/errors.go +++ b/kyaml/errors/errors.go @@ -31,6 +31,16 @@ func Errorf(msg string, args ...interface{}) error { return goerrors.Wrap(fmt.Errorf(msg, args...), 1) } +// As finds the targeted error in any wrapped error. +func As(err error, target interface{}) bool { + return goerrors.As(err, target) +} + +// Is detects whether the error is equal to a given error. +func Is(err error, target error) bool { + return goerrors.Is(err, target) +} + // GetStack returns a stack trace for the error if it has one func GetStack(err error) string { if e, ok := err.(*goerrors.Error); ok { diff --git a/kyaml/fn/framework/processors_test.go b/kyaml/fn/framework/processors_test.go index cbf83837d..6f44a55cc 100644 --- a/kyaml/fn/framework/processors_test.go +++ b/kyaml/fn/framework/processors_test.go @@ -491,7 +491,9 @@ another`), wantErr: `failed to parse rendered patch template into a resource: 001 aString 002 another -: wrong Node Kind for expected: MappingNode was ScalarNode: value: {aString another}`, +: wrong node kind: expected MappingNode but got ScalarNode: node contents: +aString another +`, }, { name: "ResourcePatchTemplate is invalid template", @@ -515,7 +517,9 @@ another`), wantErr: `failed to parse rendered patch template into a resource: 001 aString 002 another -: wrong Node Kind for expected: MappingNode was ScalarNode: value: {aString another}`, +: wrong node kind: expected MappingNode but got ScalarNode: node contents: +aString another +`, }, { name: "ContainerPatchTemplate is invalid template", @@ -538,7 +542,9 @@ another`), wantErr: `failed to parse rendered template into a resource: 001 aString 002 another -: wrong Node Kind for expected: MappingNode was ScalarNode: value: {aString another}`, +: wrong node kind: expected MappingNode but got ScalarNode: node contents: +aString another +`, }, { name: "ResourceTemplate is invalid template", diff --git a/kyaml/go.mod b/kyaml/go.mod index 624418412..ef49fdfab 100644 --- a/kyaml/go.mod +++ b/kyaml/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( github.com/davecgh/go-spew v1.1.1 - github.com/go-errors/errors v1.0.1 + github.com/go-errors/errors v1.4.2 github.com/google/gnostic v0.5.7-v3refs github.com/google/go-cmp v0.5.5 github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 3f8d22c21..9a70570b0 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -663,7 +663,7 @@ func TestByteReadWriter_WrapBareSeqNode(t *testing.T) { - foo - bar `, - readerErr: "wrong Node Kind for expected: MappingNode was SequenceNode", + readerErr: "wrong node kind: expected MappingNode but got SequenceNode", }, { name: "error round_trip bare seq node", @@ -686,7 +686,7 @@ func TestByteReadWriter_WrapBareSeqNode(t *testing.T) { served: true storage: false `, - readerErr: "wrong Node Kind for expected: MappingNode was SequenceNode", + readerErr: "wrong node kind: expected MappingNode but got SequenceNode", }, { name: "round_trip bare seq node json", @@ -698,7 +698,7 @@ func TestByteReadWriter_WrapBareSeqNode(t *testing.T) { name: "error round_trip invalid yaml node", wrapBareSeqNode: false, input: "I am not valid", - readerErr: "wrong Node Kind for expected: MappingNode was ScalarNode", + readerErr: "wrong node kind: expected MappingNode but got ScalarNode", }, } diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 48a025444..5f45424d9 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -87,6 +87,16 @@ var MappingNode yaml.Kind = yaml.MappingNode var ScalarNode yaml.Kind = yaml.ScalarNode var SequenceNode yaml.Kind = yaml.SequenceNode +func nodeKindString(k yaml.Kind) string { + return map[yaml.Kind]string{ + yaml.SequenceNode: "SequenceNode", + yaml.MappingNode: "MappingNode", + yaml.ScalarNode: "ScalarNode", + yaml.DocumentNode: "DocumentNode", + yaml.AliasNode: "AliasNode", + }[k] +} + var DoubleQuotedStyle yaml.Style = yaml.DoubleQuotedStyle var FlowStyle yaml.Style = yaml.FlowStyle var FoldedStyle yaml.Style = yaml.FoldedStyle diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index e693f88a1..b2ba181f7 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -794,12 +794,22 @@ func ErrorIfAnyInvalidAndNonNull(kind yaml.Kind, rn ...*RNode) error { return nil } -var nodeTypeIndex = map[yaml.Kind]string{ - yaml.SequenceNode: "SequenceNode", - yaml.MappingNode: "MappingNode", - yaml.ScalarNode: "ScalarNode", - yaml.DocumentNode: "DocumentNode", - yaml.AliasNode: "AliasNode", +type InvalidNodeKindError struct { + expectedKind yaml.Kind + node *RNode +} + +func (e *InvalidNodeKindError) Error() string { + msg := fmt.Sprintf("wrong node kind: expected %s but got %s", + nodeKindString(e.expectedKind), nodeKindString(e.node.YNode().Kind)) + if content, err := e.node.String(); err == nil { + msg += fmt.Sprintf(": node contents:\n%s", content) + } + return msg +} + +func (e *InvalidNodeKindError) ActualNodeKind() Kind { + return e.node.YNode().Kind } func ErrorIfInvalid(rn *RNode, kind yaml.Kind) error { @@ -809,11 +819,7 @@ func ErrorIfInvalid(rn *RNode, kind yaml.Kind) error { } if rn.YNode().Kind != kind { - s, _ := rn.String() - return errors.Errorf( - "wrong Node Kind for %s expected: %v was %v: value: {%s}", - strings.Join(rn.FieldPath(), "."), - nodeTypeIndex[kind], nodeTypeIndex[rn.YNode().Kind], strings.TrimSpace(s)) + return &InvalidNodeKindError{node: rn, expectedKind: kind} } if kind == yaml.MappingNode { diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index e217a07b1..f17b45534 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -45,7 +45,7 @@ func TestAppend(t *testing.T) { assert.NoError(t, err) rn, err := node.Pipe(Append(NewScalarRNode("").YNode())) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") } assert.Nil(t, rn) @@ -159,7 +159,9 @@ func TestElementSetter(t *testing.T) { // Return error because ElementSetter will assume all elements are scalar when // there is only value provided. _, err = node.Pipe(ElementSetter{Values: []string{"b"}}) - assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") + assert.EqualError(t, err, `wrong node kind: expected ScalarNode but got MappingNode: node contents: +a: b +`) node = MustParse(` - a @@ -455,7 +457,7 @@ a: b assert.NoError(t, err) rn, err = node.Pipe(FieldClearer{Name: "a"}) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") } assert.Nil(t, rn) assert.Equal(t, s, assertNoErrorString(t)(node.String())) @@ -855,7 +857,9 @@ func TestMapEntrySetter(t *testing.T) { Key: NewScalarRNode("foo"), Value: NewScalarRNode("bar"), }, - expectedErr: errors.Errorf("wrong Node Kind for expected: MappingNode was SequenceNode: value: {- foo: baz}"), + expectedErr: errors.Errorf(`wrong node kind: expected MappingNode but got SequenceNode: node contents: +- foo: baz +`), }, } for _, tc := range testCases { @@ -950,7 +954,7 @@ foo } k, err = instance.Filter(node) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") } assert.Nil(t, k) } @@ -1009,7 +1013,7 @@ foo: baz if !assert.Error(t, err) { return } - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") assert.Equal(t, `foo: baz `, assertNoErrorString(t)(node.String())) } @@ -1029,15 +1033,15 @@ func TestErrorIfInvalid(t *testing.T) { if !assert.Error(t, err) { t.FailNow() } - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") err = ErrorIfInvalid(NewRNode(&yaml.Node{}), yaml.SequenceNode) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") } err = ErrorIfInvalid(NewRNode(&yaml.Node{}), yaml.MappingNode) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") } err = ErrorIfInvalid(NewRNode(&yaml.Node{ @@ -1048,7 +1052,7 @@ func TestErrorIfInvalid(t *testing.T) { err = ErrorIfInvalid(NewRNode(&yaml.Node{}), yaml.SequenceNode) if assert.Error(t, err) { - assert.Contains(t, err.Error(), "wrong Node Kind") + assert.Contains(t, err.Error(), "wrong node kind") } err = ErrorIfInvalid(NewRNode(&yaml.Node{ diff --git a/kyaml/yaml/rnode_test.go b/kyaml/yaml/rnode_test.go index 5e490f532..506ccba9c 100644 --- a/kyaml/yaml/rnode_test.go +++ b/kyaml/yaml/rnode_test.go @@ -1372,7 +1372,9 @@ spec: { testName: "non mapping node error", input: `apiVersion`, - err: "wrong Node Kind for expected: MappingNode was ScalarNode: value: {apiVersion}", + err: `wrong node kind: expected MappingNode but got ScalarNode: node contents: +apiVersion +`, }, } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go index 0c1449082..6fc510c13 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -20,12 +21,6 @@ fieldSpecs: - path: subjects/namespace kind: ClusterRoleBinding group: rbac.authorization.k8s.io -- path: subjects - kind: RoleBinding - group: rbac.authorization.k8s.io -- path: subjects - kind: ClusterRoleBinding - group: rbac.authorization.k8s.io ` func TestNamespaceTransformer1(t *testing.T) { @@ -700,3 +695,70 @@ subjects: }) } } + +func TestNamespaceTransformer_InvalidFieldSpecs(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepBuiltin("NamespaceTransformer") + defer th.Reset() + th.RunTransformerAndCheckError(` +apiVersion: builtin +kind: NamespaceTransformer +metadata: + name: notImportantHere + namespace: test +unsetOnly: true +fieldSpecs: +- path: subjects + kind: RoleBinding + group: rbac.authorization.k8s.io +- path: subjects + kind: ClusterRoleBinding + group: rbac.authorization.k8s.io +`, ` +--- +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: example.com/v1 +kind: RoleBinding +metadata: + namespace: bar + name: rolebinding +subjects: +- name: default + namespace: bar +`, + func(t *testing.T, err error) { + t.Helper() + assert.EqualError(t, err, `namespace field specs must target scalar nodes: `+ + `considering field 'subjects' of object ClusterRoleBinding.v1.rbac.authorization.k8s.io/manager-rolebinding.[noNs]: `+ + `wrong node kind: expected ScalarNode but got SequenceNode: node contents: +- kind: ServiceAccount + name: default + namespace: other +- kind: ServiceAccount + name: default + namespace: "test" +- kind: ServiceAccount + name: default + namespace: test +- kind: ServiceAccount + name: another + namespace: random +`) + }) +} diff --git a/plugin/builtin/namespacetransformer/go.mod b/plugin/builtin/namespacetransformer/go.mod index daa24f62c..a0d9a5df6 100644 --- a/plugin/builtin/namespacetransformer/go.mod +++ b/plugin/builtin/namespacetransformer/go.mod @@ -3,6 +3,7 @@ module sigs.k8s.io/kustomize/plugin/builtin/namespacetransformer go 1.18 require ( + github.com/stretchr/testify v1.7.0 sigs.k8s.io/kustomize/api v0.11.5 sigs.k8s.io/kustomize/kyaml v0.13.7 sigs.k8s.io/yaml v1.2.0 @@ -25,6 +26,7 @@ require ( github.com/mailru/easyjson v0.7.6 // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/xlab/treeprint v1.1.0 // indirect go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect