diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index c25cf82823..54db81b3ba 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/resid" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -309,6 +310,15 @@ func (f Filter) sameCurrentNamespaceAsReferrer() sieveFunc { } } +// scopeUnknown returns true if the openapi data does not know whether the +// resource's kind is namespace-scoped or cluster-scoped. Kustomize assumes +// such kinds are namespace-scoped, but they may in fact be cluster-scoped +// (e.g. a cluster-scoped custom resource). +func scopeUnknown(r *resource.Resource) bool { + _, found := openapi.IsNamespaceScoped(r.GetGvk().AsTypeMeta()) + return !found +} + // selectReferral picks the best referral from a list of candidates. func (f Filter) selectReferral( // The name referral that may need to be updated. @@ -319,7 +329,18 @@ func (f Filter) selectReferral( candidates = doSieve(candidates, previousNameMatches(oldName)) candidates = doSieve(candidates, previousIdSelectedByGvk(&f.ReferralTarget)) candidates = doSieve(candidates, f.roleRefFilter()) - candidates = doSieve(candidates, f.sameCurrentNamespaceAsReferrer()) + if sameNsCandidates := doSieve( + candidates, f.sameCurrentNamespaceAsReferrer()); len(sameNsCandidates) > 0 { + candidates = sameNsCandidates + } else { + // No candidate appears to share the referrer's namespace, but the + // namespace comparison above is meaningless for candidates whose + // scope had to be guessed at: they may actually be cluster-scoped, + // or a namespace transformation may have moved them. Suppressing + // the update in that case would leave a dangling name reference. + // See issue #5696. + candidates = doSieve(candidates, scopeUnknown) + } if len(candidates) == 1 { return candidates[0], nil } diff --git a/api/filters/nameref/nameref_test.go b/api/filters/nameref/nameref_test.go index e1a2029ee7..ce1e6a8bec 100644 --- a/api/filters/nameref/nameref_test.go +++ b/api/filters/nameref/nameref_test.go @@ -187,6 +187,126 @@ map: }, }, }, + // A candidate of a kind whose scope is not known from the openapi + // data may be cluster-scoped, so a namespace mismatch with the + // referrer must not suppress the update. See issue #5696. + "scalar with unknown scope candidate in another namespace": { + referrerOriginal: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dep + namespace: kuma +ref: + name: oldName +`, + candidates: ` +apiVersion: example.com/v1 +kind: CustomKind +metadata: + name: newName + namespace: bar +`, + originalNames: []string{"oldName"}, + referrerFinal: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dep + namespace: kuma +ref: + name: newName +`, + filter: Filter{ + NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, + ReferralTarget: resid.Gvk{ + Group: "example.com", + Version: "v1", + Kind: "CustomKind", + }, + }, + }, + // A candidate of a kind known to be namespace-scoped cannot be + // referred to from another namespace; the reference is left alone. + "scalar with known namespaced candidate in another namespace": { + referrerOriginal: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dep + namespace: kuma +ref: + name: oldName +`, + candidates: ` +apiVersion: v1 +kind: Secret +metadata: + name: newName + namespace: bar +`, + originalNames: []string{"oldName"}, + referrerFinal: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dep + namespace: kuma +ref: + name: oldName +`, + filter: Filter{ + NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, + ReferralTarget: resid.Gvk{ + Version: "v1", + Kind: "Secret", + }, + }, + }, + // A candidate in the referrer's namespace wins over an + // unknown scope candidate in another namespace. + "scalar with unknown scope candidates in several namespaces": { + referrerOriginal: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dep + namespace: kuma +ref: + name: oldName +`, + candidates: ` +apiVersion: example.com/v1 +kind: CustomKind +metadata: + name: newName + namespace: kuma +--- +apiVersion: example.com/v1 +kind: CustomKind +metadata: + name: otherName + namespace: bar +`, + originalNames: []string{"oldName", "oldName"}, + referrerFinal: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: dep + namespace: kuma +ref: + name: newName +`, + filter: Filter{ + NameFieldToUpdate: types.FieldSpec{Path: "ref/name"}, + ReferralTarget: resid.Gvk{ + Group: "example.com", + Version: "v1", + Kind: "CustomKind", + }, + }, + }, "null value": { referrerOriginal: ` apiVersion: apps/v1 diff --git a/api/krusty/namereference_test.go b/api/krusty/namereference_test.go index 843b815bfa..e7b2ab55c9 100644 --- a/api/krusty/namereference_test.go +++ b/api/krusty/namereference_test.go @@ -868,3 +868,77 @@ spec: - Deny `) } + +// Regression test for issue #5696. +// +// The Mesh kind is a custom resource whose scope is unknown to kustomize, +// so kustomize assumes it is namespace-scoped and the NamespaceTransformer +// moves it into namespace "bar", while the MeshMetric keeps its explicit +// "kuma" namespace. The custom nameReference configuration must still +// update the back-reference held in the MeshMetric label, even though the +// two objects end up in different namespaces: the namespace comparison is +// meaningless when the referral's scope is only guessed at. +func TestIssue5696NameReferenceWithNamespaceTransformer(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK(".", ` +namePrefix: foo- +resources: +- resources.yaml +configurations: +- config.yaml +transformers: +- |- + apiVersion: builtin + kind: NamespaceTransformer + metadata: + name: notImportantHere + namespace: bar + unsetOnly: true +`) + th.WriteF("resources.yaml", ` +apiVersion: kuma.io/v1alpha1 +kind: Mesh +metadata: + name: test +spec: + stuff: true +--- +apiVersion: kuma.io/v1alpha1 +kind: MeshMetric +metadata: + name: test-metrics + namespace: kuma + labels: + kuma.io/mesh: test +spec: + moreStuff: true +`) + th.WriteF("config.yaml", ` +nameReference: +- kind: Mesh + version: v1alpha1 + fieldSpecs: + - path: metadata/labels/kuma.io\/mesh + kind: MeshMetric +`) + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: kuma.io/v1alpha1 +kind: Mesh +metadata: + name: foo-test + namespace: bar +spec: + stuff: true +--- +apiVersion: kuma.io/v1alpha1 +kind: MeshMetric +metadata: + labels: + kuma.io/mesh: foo-test + name: foo-test-metrics + namespace: kuma +spec: + moreStuff: true +`) +} diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 33578a0bbe..980ae89924 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/resid" kyaml "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -421,6 +422,17 @@ func (m *resWrangler) SubsetThatCouldBeReferencedByResource( // There's still a chance they can refer to each other. if roleBindingNamespaces[possibleTarget.GetNamespace()] { result.append(possibleTarget) + continue + } + // If the scope of the possible target's kind is not known from + // the openapi data, it was only assumed to be namespace-scoped + // and may in fact be cluster-scoped (e.g. a cluster-scoped custom + // resource), so the namespace comparison above is meaningless. + // Include it, and let the nameref filter decide (it prefers + // candidates in the referrer's namespace). See issue #5696. + if _, found := openapi.IsNamespaceScoped( + id.Gvk.AsTypeMeta()); !found { + result.append(possibleTarget) } } return result, nil diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index 73c7d71425..77a9251367 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -615,6 +615,20 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { if err7 != nil { t.Fatalf("failed to get new instance: %v", err7) } + // The scope of this kind is unknown to the openapi data, so it + // could be cluster-scoped and thus referenced from any namespace. + r8, err8 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "CustomKind", + "metadata": map[string]interface{}{ + "name": "zoe", + "namespace": "other", + }, + }) + if err8 != nil { + t.Fatalf("failed to get new instance: %v", err8) + } tests := map[string]struct { filter *resource.Resource @@ -623,31 +637,31 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "default namespace 1": { filter: r2, expected: resmaptest_test.NewRmBuilder(t, rf). - AddR(r1).AddR(r2).AddR(r7).ResMap(), + AddR(r1).AddR(r2).AddR(r7).AddR(r8).ResMap(), }, "default namespace 2": { filter: r1, expected: resmaptest_test.NewRmBuilder(t, rf). - AddR(r1).AddR(r2).AddR(r7).ResMap(), + AddR(r1).AddR(r2).AddR(r7).AddR(r8).ResMap(), }, "happy namespace no prefix": { filter: r3, expected: resmaptest_test.NewRmBuilder(t, rf). - AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap(), + AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).AddR(r8).ResMap(), }, "happy namespace with prefix": { filter: r5, expected: resmaptest_test.NewRmBuilder(t, rf). - AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap(), + AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).AddR(r8).ResMap(), }, "cluster level": { filter: r7, expected: resmaptest_test.NewRmBuilder(t, rf). - AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap(), + AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).AddR(r8).ResMap(), }, } m := resmaptest_test.NewRmBuilder(t, rf). - AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap() + AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).AddR(r8).ResMap() for name, test := range tests { test := test t.Run(name, func(t *testing.T) {