don't surface entire node content in error message

kubernetes-sigs · Dec 23, 2021 · 3187687 · 3187687
1 parent 233f1a3
commit 3187687
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 64 deletions.
diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go
@@ -51,9 +51,8 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) {
 	}
 	fltr.path = utils.PathSplitter(fltr.FieldSpec.Path, "/")
 	if err := fltr.filter(obj); err != nil {
-		s, _ := obj.String()
 		return nil, errors.WrapPrefixf(err,
-			"considering field '%s' of object\n%v", fltr.FieldSpec.Path, s)
+			"considering field '%s' of object %s", fltr.FieldSpec.Path, resid.FromRNode(obj))
 	}
 	return obj, nil
 }

diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go
@@ -59,14 +59,7 @@ apiVersion: foo
 kind: Bar
 xxx:
 `,
-			error: `considering field '' of object
-apiVersion: foo/v1
-kind: Bar
-xxx:
-metadata:
-  annotations:
-    internal.config.kubernetes.io/annotations-migration-resource-id: '0'
-: cannot set or create an empty field name`,
+			error: `considering field '' of object Bar.v1.foo/[noName].[noNs]: cannot set or create an empty field name`,
 			filter: fieldspec.Filter{
 				SetValue: filtersutil.SetScalar("e"),
 			},
@@ -219,14 +212,7 @@ kind: Bar
 a:
   b: a
 `,
-			error: `considering field 'a/b/c' of object
-kind: Bar
-a:
-  b: a
-metadata:
-  annotations:
-    internal.config.kubernetes.io/annotations-migration-resource-id: '0'
-: expected sequence or mapping node`,
+			error: `considering field 'a/b/c' of object Bar.[noVer].[noGrp]/[noName].[noNs]: expected sequence or mapping node`,
 			filter: fieldspec.Filter{
 				SetValue: filtersutil.SetScalar("e"),
 			},

diff --git a/api/filters/refvar/refvar_test.go b/api/filters/refvar/refvar_test.go
@@ -243,19 +243,7 @@ metadata:
 data:
   slice:
   - false`,
-			expectedError: `considering field 'data/slice' of object
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: dep
-  annotations:
-    config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/annotations-migration-resource-id: '0'
-data:
-  slice:
-  - false
-: invalid value type expect a string`,
+			expectedError: `considering field 'data/slice' of object Deployment.v1.apps/dep.[noNs]: invalid value type expect a string`,
 			filter: Filter{
 				MappingFunc: makeMf(map[string]interface{}{
 					"VAR": int64(5),
@@ -271,18 +259,7 @@ metadata:
   name: dep
 data:
   1: str`,
-			expectedError: `considering field 'data' of object
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: dep
-  annotations:
-    config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/index: '0'
-    internal.config.kubernetes.io/annotations-migration-resource-id: '0'
-data:
-  1: str
-: invalid map key: value='1', tag='` + yaml.NodeTagInt + `'`,
+			expectedError: `considering field 'data' of object Deployment.v1.apps/dep.[noNs]: invalid map key: value='1', tag='` + yaml.NodeTagInt + `'`,
 			filter: Filter{
 				MappingFunc: makeMf(map[string]interface{}{
 					"VAR": int64(5),

diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go
@@ -520,19 +520,10 @@ func TestNameReferenceUnhappyRun(t *testing.T) {
 						},
 					},
 				}).ResMap(),
-			expectedErr: `updating name reference in 'rules/resourceNames' field of ` +
-				`'ClusterRole.v1.rbac.authorization.k8s.io/cr.[noNs]'` +
-				`: considering field 'rules/resourceNames' of object
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
-  name: cr
-rules:
-- resourceNames:
-    foo: bar
-  resources:
-  - secrets
-: visit traversal on path: [resourceNames]: path config error; no 'name' field in node`},
+			expectedErr: `updating name reference in 'rules/resourceNames' field of 'ClusterRole.v1.rbac.authorization.k8s.io/cr.[noNs]': ` +
+				`considering field 'rules/resourceNames' of object ClusterRole.v1.rbac.authorization.k8s.io/cr.[noNs]: visit traversal on ` +
+				`path: [resourceNames]: path config error; no 'name' field in node`,
+		},
 	}
 
 	nrt := newNameReferenceTransformer(builtinconfig.MakeDefaultConfig().NameReference)

diff --git a/kyaml/resid/resid.go b/kyaml/resid/resid.go
@@ -6,6 +6,8 @@ package resid
 import (
 	"reflect"
 	"strings"
+
+	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
 // ResId is an identifier of a k8s resource object.
@@ -63,7 +65,7 @@ func FromString(s string) ResId {
 	gvk := GvkFromString(values[0])
 
 	values = strings.Split(values[1], fieldSep)
-	last := len(values)-1
+	last := len(values) - 1
 
 	ns := values[last]
 	if ns == noNamespace {
@@ -80,6 +82,13 @@ func FromString(s string) ResId {
 	}
 }
 
+// FromRNode returns the ResId for the RNode
+func FromRNode(rn *yaml.RNode) ResId {
+	group, version := ParseGroupVersion(rn.GetApiVersion())
+	return NewResIdWithNamespace(
+		Gvk{Group: group, Version: version, Kind: rn.GetKind()}, rn.GetName(), rn.GetNamespace())
+}
+
 // GvknEquals returns true if the other id matches
 // Group/Version/Kind/name.
 func (id ResId) GvknEquals(o ResId) bool {

diff --git a/kyaml/resid/resid_test.go b/kyaml/resid/resid_test.go
@@ -60,7 +60,6 @@ var resIdStringTests = []struct {
 		ResId{},
 		"[noKind].[noVer].[noGrp]/[noName].[noNs]",
 	},
-
 }
 
 func TestResIdString(t *testing.T) {
@@ -271,18 +270,18 @@ var ids = []ResId{
 	},
 	{
 		Gvk: Gvk{
-			Group: "rbac.authorization.k8s.io",
-			Version: "v1",
-			Kind: "ClusterRole",
+			Group:           "rbac.authorization.k8s.io",
+			Version:         "v1",
+			Kind:            "ClusterRole",
 			isClusterScoped: true,
 		},
 		Name: "nm",
 	},
 	{
 		Gvk: Gvk{
-			Group: "rbac.authorization.k8s.io",
-			Version: "v1",
-			Kind: "ClusterRole",
+			Group:           "rbac.authorization.k8s.io",
+			Version:         "v1",
+			Kind:            "ClusterRole",
 			isClusterScoped: true,
 		},
 		Name: "my.name",