diff --git a/pkg/target/namespaces_test.go b/pkg/target/namespaces_test.go index 582879ad5..835d28fd1 100644 --- a/pkg/target/namespaces_test.go +++ b/pkg/target/namespaces_test.go @@ -95,6 +95,206 @@ rules: `) } +// TestNameAndNsTransformation validates that NamespaceTransformer, +// PrefixSuffixTransformer and namereference transformers are +// able to deal with simultaneous change of namespace and name. +func TestNameAndNsTransformation(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/nameandns") + + th.WriteK("/nameandns", ` +namePrefix: p1- +nameSuffix: -s1 +namespace: newnamespace +resources: +- resources.yaml +`) + + th.WriteF("/nameandns/resources.yaml", ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm2 + namespace: ns1 +--- +apiVersion: v1 +kind: Service +metadata: + name: svc1 + namespace: ns1 +--- +apiVersion: v1 +kind: Service +metadata: + name: svc2 + namespace: ns1 +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa1 + namespace: ns1 +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa2 + namespace: ns1 +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: manager-rolebinding +subjects: +- kind: ServiceAccount + name: sa1 + namespace: ns1 +- kind: ServiceAccount + name: sa2 + namespace: ns1 +- kind: ServiceAccount + name: sa3 + namespace: random +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: ValidatingWebhookConfiguration +metadata: + name: example +webhooks: + - name: example1 + clientConfig: + service: + name: svc1 + namespace: ns1 + - name: example2 + clientConfig: + service: + name: svc2 + namespace: ns1 + - name: example3 + clientConfig: + service: + name: svc3 + namespace: random +--- +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: crds.my.org +--- +kind: ClusterRole +metadata: + name: cr1 +--- +kind: ClusterRoleBinding +metadata: + name: crb1 +--- +kind: PersistentVolume +metadata: + name: pv1 +`) + + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: p1-cm1-s1 + namespace: newnamespace +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: p1-cm2-s1 + namespace: newnamespace +--- +apiVersion: v1 +kind: Service +metadata: + name: p1-svc1-s1 + namespace: newnamespace +--- +apiVersion: v1 +kind: Service +metadata: + name: p1-svc2-s1 + namespace: newnamespace +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: p1-sa1-s1 + namespace: newnamespace +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: p1-sa2-s1 + namespace: newnamespace +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: p1-manager-rolebinding-s1 +subjects: +- kind: ServiceAccount + name: p1-sa1-s1 + namespace: newnamespace +- kind: ServiceAccount + name: p1-sa2-s1 + namespace: newnamespace +- kind: ServiceAccount + name: sa3 + namespace: random +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: ValidatingWebhookConfiguration +metadata: + name: p1-example-s1 +webhooks: +- clientConfig: + service: + name: p1-svc1-s1 + namespace: newnamespace + name: example1 +- clientConfig: + service: + name: p1-svc2-s1 + namespace: newnamespace + name: example2 +- clientConfig: + service: + name: svc3 + namespace: random + name: example3 +--- +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: crds.my.org +--- +kind: ClusterRole +metadata: + name: p1-cr1-s1 +--- +kind: ClusterRoleBinding +metadata: + name: p1-crb1-s1 +--- +kind: PersistentVolume +metadata: + name: p1-pv1-s1 +`) +} + // This serie of constants is used to prove the need of // the namespace field in the objref field of the var declaration. // The following tests demonstrate that it creates umbiguous variable diff --git a/pkg/transformers/config/defaultconfig/namereference.go b/pkg/transformers/config/defaultconfig/namereference.go index 1b4c8d1eb..598aaec1d 100644 --- a/pkg/transformers/config/defaultconfig/namereference.go +++ b/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 diff --git a/pkg/transformers/namereference.go b/pkg/transformers/namereference.go index 7aaf00b51..6dbb4545e 100644 --- a/pkg/transformers/namereference.go +++ b/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 { diff --git a/pkg/transformers/namereference_test.go b/pkg/transformers/namereference_test.go index 7ae02cc4f..623860bff 100644 --- a/pkg/transformers/namereference_test.go +++ b/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() diff --git a/plugin/builtin/NamespaceTransformer.go b/plugin/builtin/NamespaceTransformer.go index 84797fef0..199a70bf7 100644 --- a/plugin/builtin/NamespaceTransformer.go +++ b/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 - } -} diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index 0516d3c89..906e6e564 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/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 - } -} diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go b/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go index d0d393235..ce3043a0e 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer_test.go +++ b/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: