Merge pull request #3122 from Shell32-Natsu/ref-target

check Gvk in roleRef when update name reference
2026-05-17 18:25:26 +00:00 · 2020-10-21 11:19:20 -07:00
parent 287b38cc87 62a8a8c57d
commit 3c86d37148
3 changed files with 362 additions and 19 deletions
--- 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
 	}
--- 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
--- a/api/krusty/nameupdateinroleref_test.go
+++ 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
+`)
+}