Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent internal annotations from showing up in the errors thrown by filters #4352

Merged
merged 1 commit into from Dec 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions api/filters/fieldspec/fieldspec.go
Expand Up @@ -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
}
Expand Down
18 changes: 2 additions & 16 deletions api/filters/fieldspec/fieldspec_test.go
Expand Up @@ -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"),
},
Expand Down Expand Up @@ -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"),
},
Expand Down
27 changes: 2 additions & 25 deletions api/filters/refvar/refvar_test.go
Expand Up @@ -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),
Expand All @@ -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),
Expand Down
17 changes: 4 additions & 13 deletions api/internal/accumulator/namereferencetransformer_test.go
Expand Up @@ -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)
Expand Down
10 changes: 1 addition & 9 deletions api/internal/accumulator/refvartransformer_test.go
Expand Up @@ -120,15 +120,7 @@ func TestRefVarTransformer(t *testing.T) {
"slice": []interface{}{5}, // noticeably *not* a []string
}}).ResMap(),
},
errMessage: `considering field 'data/slice' of object
apiVersion: v1
data:
slice:
- 5
kind: ConfigMap
metadata:
name: cm1
: invalid value type expect a string`,
errMessage: `considering field 'data/slice' of object ConfigMap.v1.[noGrp]/cm1.[noNs]: invalid value type expect a string`,
},
"var replacement in nil": {
given: given{
Expand Down
11 changes: 10 additions & 1 deletion kyaml/resid/resid.go
Expand Up @@ -6,6 +6,8 @@ package resid
import (
"reflect"
"strings"

"sigs.k8s.io/kustomize/kyaml/yaml"
)

// ResId is an identifier of a k8s resource object.
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
13 changes: 6 additions & 7 deletions kyaml/resid/resid_test.go
Expand Up @@ -60,7 +60,6 @@ var resIdStringTests = []struct {
ResId{},
"[noKind].[noVer].[noGrp]/[noName].[noNs]",
},

}

func TestResIdString(t *testing.T) {
Expand Down Expand Up @@ -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",
Expand Down