From 3187687b761a74f4f416f715532da591a437a051 Mon Sep 17 00:00:00 2001 From: natasha41575 Date: Wed, 22 Dec 2021 15:41:22 -0800 Subject: [PATCH] don't surface entire node content in error message --- api/filters/fieldspec/fieldspec.go | 3 +-- api/filters/fieldspec/fieldspec_test.go | 18 ++----------- api/filters/refvar/refvar_test.go | 27 ++----------------- .../namereferencetransformer_test.go | 17 +++--------- kyaml/resid/resid.go | 11 +++++++- kyaml/resid/resid_test.go | 13 +++++---- 6 files changed, 25 insertions(+), 64 deletions(-) diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go index 8739d073336..152bbb1bc9e 100644 --- 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 index 43a4362c757..bfa3a4341b4 100644 --- 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 index fb29dbe9535..c80dc79bd1d 100644 --- 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 index 9c2e9605dc5..49169db9145 100644 --- 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 index a729cb78dd5..cddbcdde60f 100644 --- 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 index 041608f7cd7..08d5c4ad612 100644 --- 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",