Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion api/filters/nameref/nameref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
120 changes: 120 additions & 0 deletions api/filters/nameref/nameref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 74 additions & 0 deletions api/krusty/namereference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`)
}
12 changes: 12 additions & 0 deletions api/resmap/reswrangler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down
26 changes: 20 additions & 6 deletions api/resmap/reswrangler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
Loading