Improve error message when namespace transformer is given invalid fieldspecs

Also remove invalid+ignored fieldspecs from the defaults

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
+`)
 }

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])

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

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

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 {

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",

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

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",
 		},
 	}
 

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

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 {

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{

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
+`,
 		},
 	}
 

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
+`)
+		})
+}

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