diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index f2f53b9d6..02f6ece6a 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -1,7 +1,9 @@ package nameref import ( + "encoding/json" "fmt" + "strings" "sigs.k8s.io/kustomize/api/filters/fieldspec" "sigs.k8s.io/kustomize/api/filters/filtersutil" @@ -9,6 +11,7 @@ import ( "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" + kyaml_filtersutil "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -19,6 +22,7 @@ type Filter struct { Referrer *resource.Resource Target resid.Gvk ReferralCandidates resmap.ResMap + isRoleRef bool } func (f Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { @@ -37,6 +41,9 @@ func (f Filter) set(node *yaml.RNode) error { if yaml.IsMissingOrNull(node) { return nil } + if strings.HasSuffix(f.FieldSpec.Path, "roleRef/name") { + f.isRoleRef = true + } switch node.YNode().Kind { case yaml.ScalarNode: return f.setScalar(node) @@ -65,6 +72,7 @@ func (f Filter) setMapping(node *yaml.RNode) error { f.Referrer, f.Target, f.ReferralCandidates, + f.isRoleRef, ) } @@ -75,6 +83,7 @@ func (f Filter) setScalar(node *yaml.RNode) error { f.Target, f.ReferralCandidates, f.ReferralCandidates.Resources(), + f.isRoleRef, ) if err != nil { return err @@ -86,6 +95,40 @@ func (f Filter) setScalar(node *yaml.RNode) error { return nil } +// getRoleRefGvk returns a Gvk in the roleRef field. Return error +// if the roleRef, roleRef/apiGroup or roleRef/kind is missing. +func getRoleRefGvk(res json.Marshaler) (*resid.Gvk, error) { + n, err := kyaml_filtersutil.GetRNode(res) + if err != nil { + return nil, err + } + roleRef, err := n.Pipe(yaml.Lookup("roleRef")) + if err != nil { + return nil, err + } + if roleRef.IsNil() { + return nil, fmt.Errorf("roleRef cannot be found in %s", n.MustString()) + } + apiGroup, err := roleRef.Pipe(yaml.Lookup("apiGroup")) + if err != nil { + return nil, err + } + if apiGroup.IsNil() { + return nil, fmt.Errorf("apiGroup cannot be found in roleRef %s", roleRef.MustString()) + } + kind, err := roleRef.Pipe(yaml.Lookup("kind")) + if err != nil { + return nil, err + } + if kind.IsNil() { + return nil, fmt.Errorf("kind cannot be found in roleRef %s", roleRef.MustString()) + } + return &resid.Gvk{ + Group: apiGroup.YNode().Value, + Kind: kind.YNode().Value, + }, nil +} + func filterReferralCandidates( referrer *resource.Resource, matches []*resource.Resource, @@ -117,11 +160,22 @@ func selectReferral( referrer *resource.Resource, target resid.Gvk, referralCandidates resmap.ResMap, - referralCandidateSubset []*resource.Resource) (string, string, error) { - + referralCandidateSubset []*resource.Resource, + isRoleRef bool) (string, string, error) { + var roleRefGvk *resid.Gvk + if isRoleRef { + var err error + roleRefGvk, err = getRoleRefGvk(referrer) + if err != nil { + return "", "", err + } + } for _, res := range referralCandidateSubset { id := res.OrgId() - if id.IsSelected(&target) && res.GetOriginalName() == oldName { + // If the we are processing a roleRef, the apiGroup and Kind in the + // roleRef are needed to be considered. + if (!isRoleRef || id.IsSelected(roleRefGvk)) && + id.IsSelected(&target) && res.GetOriginalName() == oldName { matches := referralCandidates.GetMatchingResourcesByOriginalId(id.Equals) // If there's more than one match, // filter the matches by prefix and suffix @@ -155,10 +209,11 @@ func getSimpleNameField( referrer *resource.Resource, target resid.Gvk, referralCandidates resmap.ResMap, - referralCandidateSubset []*resource.Resource) (string, error) { + referralCandidateSubset []*resource.Resource, + isRoleRef bool) (string, error) { newName, _, err := selectReferral(oldName, referrer, target, - referralCandidates, referralCandidateSubset) + referralCandidates, referralCandidateSubset, isRoleRef) return newName, err } @@ -177,7 +232,8 @@ func setNameAndNs( in *yaml.RNode, referrer *resource.Resource, target resid.Gvk, - referralCandidates resmap.ResMap) error { + referralCandidates resmap.ResMap, + isRoleRef bool) error { if in.YNode().Kind != yaml.MappingNode { return fmt.Errorf("expect a mapping node") @@ -213,7 +269,7 @@ func setNameAndNs( oldName := nameNode.YNode().Value newname, newnamespace, err := selectReferral(oldName, referrer, target, - referralCandidates, subset) + referralCandidates, subset, isRoleRef) if err != nil { return err } diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index d6acbae2a..3d0dfaaf3 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -789,9 +789,9 @@ func TestNameReferenceClusterWide(t *testing.T) { "name": modifiedname, }, "roleRef": map[string]interface{}{ - "apiVersion": "rbac.authorization.k8s.io/v1", - "kind": "ClusterRole", - "name": orgname, + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": orgname, }, "subjects": []interface{}{ map[string]interface{}{ @@ -845,9 +845,9 @@ func TestNameReferenceClusterWide(t *testing.T) { "name": modifiedname, }, "roleRef": map[string]interface{}{ - "apiVersion": "rbac.authorization.k8s.io/v1", - "kind": "ClusterRole", - "name": modifiedname, + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": modifiedname, }, // The following tests required a change in // getNameFunc implementation in order to leverage @@ -937,9 +937,9 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) { "name": modifiedname, }, "roleRef": map[string]interface{}{ - "apiVersion": "rbac.authorization.k8s.io/v1", - "kind": "ClusterRole", - "name": orgname, + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": orgname, }, "subjects": []interface{}{ map[string]interface{}{ @@ -973,9 +973,9 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) { "name": modifiedname, }, "roleRef": map[string]interface{}{ - "apiVersion": "rbac.authorization.k8s.io/v1", - "kind": "ClusterRole", - "name": modifiedname, + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": modifiedname, }, // The following tests required a change in // getNameFunc implementation in order to leverage diff --git a/api/krusty/nameupdateinroleref_test.go b/api/krusty/nameupdateinroleref_test.go new file mode 100644 index 000000000..b816ddd9b --- /dev/null +++ b/api/krusty/nameupdateinroleref_test.go @@ -0,0 +1,287 @@ +package krusty_test + +import ( + "testing" + + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" +) + +// https://github.com/kubernetes-sigs/kustomize/issues/2640 +func TestNameUpdateInRoleRef(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("/app/rbac.yaml", ` +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: my-role +rules: +- apiGroups: + - '*' + resources: + - '*' + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: my-role +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: my-role +subjects: +- kind: ServiceAccount + name: default + namespace: foo +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: my-role +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: my-role +roleRef: + apiGroup: rbac.authorization.k8s.io + version: v1 + kind: Role + name: my-role +subjects: +- kind: ServiceAccount + name: default +`) + + th.WriteK("/app", ` +namespace: foo +resources: +- rbac.yaml + +patches: +- patch: |- + - op: add + path: /metadata/name + value: prefix_my-role + target: + group: rbac.authorization.k8s.io + version: v1 + kind: ClusterRole + name: my-role +`) + + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: prefix_my-role +rules: +- apiGroups: + - '*' + resources: + - '*' + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: my-role +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: prefix_my-role +subjects: +- kind: ServiceAccount + name: default + namespace: foo +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: my-role + namespace: foo +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: my-role + namespace: foo +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: my-role + version: v1 +subjects: +- kind: ServiceAccount + name: default + namespace: foo +`) +} + +// https://github.com/kubernetes-sigs/kustomize/issues/3073 +func TestNameUpdateInRoleRef2(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("/app/workloads.yaml", ` +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: myapp + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: myapp +rules: +- apiGroups: + - "" + resources: + - nodes/metrics + verbs: + - get + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: myapp +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: myapp +subjects: +- kind: ServiceAccount + name: myapp + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: myapp +rules: +- apiGroups: + - "" + resources: + - services + verbs: + - get + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: myapp +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: myapp +subjects: +- kind: ServiceAccount + name: myapp +`) + + th.WriteF("/app/suffixTransformer.yaml", ` +apiVersion: builtin +kind: PrefixSuffixTransformer +metadata: + name: notImportantHere +suffix: -suffix +fieldSpecs: +- path: metadata/name + kind: ClusterRole + name: myapp +- path: metadata/name + kind: ClusterRoleBinding + name: myapp +`) + + th.WriteK("/app", ` +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: +- workloads.yaml +transformers: +- suffixTransformer.yaml +namespace: test + +`) + + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: myapp + namespace: test +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: myapp-suffix +rules: +- apiGroups: + - "" + resources: + - nodes/metrics + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: myapp-suffix +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: myapp-suffix +subjects: +- kind: ServiceAccount + name: myapp + namespace: test +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: myapp + namespace: test +rules: +- apiGroups: + - "" + resources: + - services + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: myapp + namespace: test +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: myapp +subjects: +- kind: ServiceAccount + name: myapp + namespace: test +`) +}