Update Namespace and Name simultaneously (1/2)

- Removed RoleBinding and Webhook specific code in the namespacetransformer.
  That code was attempting to perform the task of the namereference
- Updated namereference transformer configuration to suppport the
  Webhooks.
- Prevent the namereference from wiping out the namespace value if
  no referral candidate was selected
- Added unit tests.

pkg/transformers/config/defaultconfig/namereference.go

@@ -272,10 +272,10 @@ nameReference:
   - path: spec/service/name
     kind: APIService
     group: apiregistration.k8s.io
-  - path: webhooks/clientConfig/service/name
+  - path: webhooks/clientConfig/service
     kind: ValidatingWebhookConfiguration
     group: admissionregistration.k8s.io
-  - path: webhooks/clientConfig/service/name
+  - path: webhooks/clientConfig/service
     kind: MutatingWebhookConfiguration
     group: admissionregistration.k8s.io
 

pkg/transformers/namereference.go

@@ -191,6 +191,11 @@ func (o *nameReferenceTransformer) getNameAndNsStruct(
 		return nil, err
 	}
 
+	if (newname == oldName) && (newnamespace == nil) {
+		// no candidate found.
+		return inMap, nil
+	}
+
 	inMap["name"] = newname
 	if newnamespace != "" {
 		// We don't want value "" to replace value "default" since
@@ -212,6 +217,12 @@ func (o *nameReferenceTransformer) getNewNameFunc(
 			oldName, _ := in.(string)
 			return o.getSimpleNameField(oldName, referrer, target,
 				referralCandidates, referralCandidates.Resources())
+		case map[string]interface{}:
+			// Kind: ValidatingWebhookConfiguration
+			// FieldSpec is webhooks/clientConfig/service
+			oldMap, _ := in.(map[string]interface{})
+			return o.getNameAndNsStruct(oldMap, referrer, target,
+				referralCandidates)
 		case []interface{}:
 			l, _ := in.([]interface{})
 			for idx, item := range l {

pkg/transformers/namereference_test.go

@@ -520,7 +520,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) {
 						},
 					},
 				}).ResMap(),
-			expectedErr: "is expected to be"},
+			expectedErr: "is expected to contain a name field"},
 	}
 
 	nrt := NewNameReferenceTransformer(defaultTransformerConfig.NameReference)
@@ -652,6 +652,7 @@ const (
 	ns1       = "ns1"
 	ns2       = "ns2"
 	ns3       = "ns3"
+	ns4       = "ns4"
 
 	orgname      = "uniquename"
 	prefixedname = "prefix-uniquename"
@@ -808,6 +809,11 @@ func TestNameReferenceClusterWide(t *testing.T) {
 					"name":      orgname,
 					"namespace": ns2,
 				},
+				map[string]interface{}{
+					"kind":      "ServiceAccount",
+					"name":      orgname,
+					"namespace": "random",
+				},
 			}}).ResMap()
 
 	expected := resmaptest_test.NewSeededRmBuilder(t, rf, m.ShallowCopy()).
@@ -862,6 +868,11 @@ func TestNameReferenceClusterWide(t *testing.T) {
 						"name":      suffixedname,
 						"namespace": ns2,
 					},
+					map[string]interface{}{
+						"kind":      "ServiceAccount",
+						"name":      orgname,
+						"namespace": "random",
+					},
 				},
 			}).ResMap()
 
@@ -890,6 +901,13 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) {
 	rf := resource.NewFactory(
 		kunstruct.NewKunstructuredFactoryImpl())
 	m := resmaptest_test.NewRmBuilder(t, rf).
+		AddWithNsAndName(ns4, orgname, map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "Secret",
+			"metadata": map[string]interface{}{
+				"name":      orgname,
+				"namespace": ns4,
+			}}).
 		// Add ServiceAccount with the same org name in "ns1" namespaces
 		AddWithNsAndName(ns1, orgname, map[string]interface{}{
 			"apiVersion": "v1",
@@ -934,6 +952,16 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) {
 					"name":      orgname,
 					"namespace": ns3,
 				},
+				map[string]interface{}{
+					"kind":      "ServiceAccount",
+					"name":      orgname,
+					"namespace": "random",
+				},
+				map[string]interface{}{
+					"kind":      "ServiceAccount",
+					"name":      orgname,
+					"namespace": ns4,
+				},
 			}}).ResMap()
 
 	expected := resmaptest_test.NewSeededRmBuilder(t, rf, m.ShallowCopy()).
@@ -963,6 +991,16 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) {
 						"name":      suffixedname,
 						"namespace": ns2,
 					},
+					map[string]interface{}{
+						"kind":      "ServiceAccount",
+						"name":      orgname,
+						"namespace": "random",
+					},
+					map[string]interface{}{
+						"kind":      "ServiceAccount",
+						"name":      orgname,
+						"namespace": ns4,
+					},
 				},
 			}).ResMap()
 

plugin/builtin/NamespaceTransformer.go

@@ -2,7 +2,6 @@
 package builtin
 
 import (
-	"sigs.k8s.io/kustomize/v3/pkg/gvk"
 	"sigs.k8s.io/kustomize/v3/pkg/ifc"
 	"sigs.k8s.io/kustomize/v3/pkg/resid"
 	"sigs.k8s.io/kustomize/v3/pkg/resmap"
@@ -51,8 +50,6 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
 			}
 		}
 	}
-	p.updateClusterRoleBinding(m)
-	p.updateServiceReference(m)
 	return nil
 }
 
@@ -85,81 +82,3 @@ func (p *NamespaceTransformerPlugin) isSelected(
 	}
 	return nil, false
 }
-
-func (p *NamespaceTransformerPlugin) updateClusterRoleBinding(m resmap.ResMap) {
-	srvAccount := gvk.Gvk{Version: "v1", Kind: "ServiceAccount"}
-	saMap := map[string]bool{}
-	for _, id := range m.AllIds() {
-		if id.Gvk.Equals(srvAccount) {
-			saMap[id.Name] = true
-		}
-	}
-
-	for _, res := range m.Resources() {
-		if res.OrgId().Kind != "ClusterRoleBinding" &&
-			res.OrgId().Kind != "RoleBinding" {
-			continue
-		}
-		objMap := res.Map()
-		subjects, ok := objMap["subjects"].([]interface{})
-		if subjects == nil || !ok {
-			continue
-		}
-		for i := range subjects {
-			subject := subjects[i].(map[string]interface{})
-			kind, foundK := subject["kind"]
-			name, foundN := subject["name"]
-			if !foundK || !foundN || kind.(string) != srvAccount.Kind {
-				continue
-			}
-			// a ServiceAccount named “default” exists in every active namespace
-			if name.(string) == "default" || saMap[name.(string)] {
-				subject := subjects[i].(map[string]interface{})
-				transformers.MutateField(
-					subject, []string{"namespace"},
-					true, func(_ interface{}) (interface{}, error) {
-						return p.Namespace, nil
-					})
-				subjects[i] = subject
-			}
-		}
-		objMap["subjects"] = subjects
-	}
-}
-
-func (p *NamespaceTransformerPlugin) updateServiceReference(m resmap.ResMap) {
-	svc := gvk.Gvk{Version: "v1", Kind: "Service"}
-	svcMap := map[string]bool{}
-	for _, id := range m.AllIds() {
-		if id.Gvk.Equals(svc) {
-			svcMap[id.Name] = true
-		}
-	}
-
-	for _, res := range m.Resources() {
-		if res.OrgId().Kind != "ValidatingWebhookConfiguration" &&
-			res.OrgId().Kind != "MutatingWebhookConfiguration" {
-			continue
-		}
-		objMap := res.Map()
-		webhooks, ok := objMap["webhooks"].([]interface{})
-		if webhooks == nil || !ok {
-			continue
-		}
-		for i := range webhooks {
-			webhook := webhooks[i].(map[string]interface{})
-			transformers.MutateField(
-				webhook, []string{"clientConfig", "service"},
-				false, func(obj interface{}) (interface{}, error) {
-					svc := obj.(map[string]interface{})
-					svcName, foundN := svc["name"]
-					if foundN && svcMap[svcName.(string)] {
-						svc["namespace"] = p.Namespace
-					}
-					return svc, nil
-				})
-			webhooks[i] = webhook
-		}
-		objMap["webhooks"] = webhooks
-	}
-}

plugin/builtin/namespacetransformer/NamespaceTransformer.go

@@ -5,7 +5,6 @@
 package main
 
 import (
-	"sigs.k8s.io/kustomize/v3/pkg/gvk"
 	"sigs.k8s.io/kustomize/v3/pkg/ifc"
 	"sigs.k8s.io/kustomize/v3/pkg/resid"
 	"sigs.k8s.io/kustomize/v3/pkg/resmap"
@@ -52,8 +51,6 @@ func (p *plugin) Transform(m resmap.ResMap) error {
 			}
 		}
 	}
-	p.updateClusterRoleBinding(m)
-	p.updateServiceReference(m)
 	return nil
 }
 
@@ -86,81 +83,3 @@ func (p *plugin) isSelected(
 	}
 	return nil, false
 }
-
-func (p *plugin) updateClusterRoleBinding(m resmap.ResMap) {
-	srvAccount := gvk.Gvk{Version: "v1", Kind: "ServiceAccount"}
-	saMap := map[string]bool{}
-	for _, id := range m.AllIds() {
-		if id.Gvk.Equals(srvAccount) {
-			saMap[id.Name] = true
-		}
-	}
-
-	for _, res := range m.Resources() {
-		if res.OrgId().Kind != "ClusterRoleBinding" &&
-			res.OrgId().Kind != "RoleBinding" {
-			continue
-		}
-		objMap := res.Map()
-		subjects, ok := objMap["subjects"].([]interface{})
-		if subjects == nil || !ok {
-			continue
-		}
-		for i := range subjects {
-			subject := subjects[i].(map[string]interface{})
-			kind, foundK := subject["kind"]
-			name, foundN := subject["name"]
-			if !foundK || !foundN || kind.(string) != srvAccount.Kind {
-				continue
-			}
-			// a ServiceAccount named “default” exists in every active namespace
-			if name.(string) == "default" || saMap[name.(string)] {
-				subject := subjects[i].(map[string]interface{})
-				transformers.MutateField(
-					subject, []string{"namespace"},
-					true, func(_ interface{}) (interface{}, error) {
-						return p.Namespace, nil
-					})
-				subjects[i] = subject
-			}
-		}
-		objMap["subjects"] = subjects
-	}
-}
-
-func (p *plugin) updateServiceReference(m resmap.ResMap) {
-	svc := gvk.Gvk{Version: "v1", Kind: "Service"}
-	svcMap := map[string]bool{}
-	for _, id := range m.AllIds() {
-		if id.Gvk.Equals(svc) {
-			svcMap[id.Name] = true
-		}
-	}
-
-	for _, res := range m.Resources() {
-		if res.OrgId().Kind != "ValidatingWebhookConfiguration" &&
-			res.OrgId().Kind != "MutatingWebhookConfiguration" {
-			continue
-		}
-		objMap := res.Map()
-		webhooks, ok := objMap["webhooks"].([]interface{})
-		if webhooks == nil || !ok {
-			continue
-		}
-		for i := range webhooks {
-			webhook := webhooks[i].(map[string]interface{})
-			transformers.MutateField(
-				webhook, []string{"clientConfig", "service"},
-				false, func(obj interface{}) (interface{}, error) {
-					svc := obj.(map[string]interface{})
-					svcName, foundN := svc["name"]
-					if foundN && svcMap[svcName.(string)] {
-						svc["namespace"] = p.Namespace
-					}
-					return svc, nil
-				})
-			webhooks[i] = webhook
-		}
-		objMap["webhooks"] = webhooks
-	}
-}

plugin/builtin/namespacetransformer/NamespaceTransformer_test.go

@@ -99,6 +99,15 @@ metadata:
   name: crd
 `)
 
+	// Import note: The namespace transformer is in charge of
+	// the metadata.namespace field. The namespace transformer SHOULD
+	// NOT modify neither the "namespace" subfield within the
+	// ClusterRoleBinding.subjects field nor the "namespace"
+	// subfield in the ValidatingWebhookConfiguration.webhooks field.
+	// This is the role of the namereference Transformer to handle
+	// object reference changes (prefix/suffix and namespace).
+	// For use cases involving simultaneous change of name and namespace,
+	// refer to namespaces tests in pkg/target test suites.
 	th.AssertActualEqualsExpected(rm, `
 apiVersion: v1
 kind: ConfigMap
@@ -142,10 +151,10 @@ metadata:
 subjects:
 - kind: ServiceAccount
   name: default
-  namespace: test
+  namespace: system
 - kind: ServiceAccount
   name: service-account
-  namespace: test
+  namespace: system
 - kind: ServiceAccount
   name: another
   namespace: random
@@ -158,7 +167,7 @@ webhooks:
 - clientConfig:
     service:
       name: svc1
-      namespace: test
+      namespace: system
   name: example1
 - clientConfig:
     service: