Skip to content

Commit 3299760

Browse files
griffindvsgriffindvs
authored
🐛 Fix namespaced GVK check to use version (kubernetes-sigs#2875)
* Fix namespaced GVK check to use version A particular Kind may only be present in a specific version of a group. When querying the RESTMapper we should include the version to ensure the cached group is updated to pick up new versions as needed. Signed-off-by: Griffin Davis <gcd@ibm.com> * Add unit tests for IsGVKNamespaced Include unit tests for varying combinations of new GVKs introduced at runtime to validate cache updates. Signed-off-by: Griffin Davis <gcd@ibm.com> --------- Signed-off-by: Griffin Davis <gcd@ibm.com>
1 parent 3ec7800 commit 3299760

File tree

2 files changed

+192
-1
lines changed

2 files changed

+192
-1
lines changed

‎pkg/client/apiutil/apimachinery.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ func IsObjectNamespaced(obj runtime.Object, scheme *runtime.Scheme, restmapper m
7272
// IsGVKNamespaced returns true if the object having the provided
7373
// GVK is namespace scoped.
7474
func IsGVKNamespaced(gvk schema.GroupVersionKind, restmapper meta.RESTMapper) (bool, error) {
75-
restmapping, err := restmapper.RESTMapping(schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind})
75+
// Fetch the RESTMapping using the complete GVK. If we exclude the Version, the Version set
76+
// will be populated using the cached Group if available. This can lead to failures updating
77+
// the cache with new Versions of CRDs registered at runtime.
78+
restmapping, err := restmapper.RESTMapping(schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind}, gvk.Version)
7679
if err != nil {
7780
return false, fmt.Errorf("failed to get restmapping: %w", err)
7881
}

‎pkg/client/apiutil/apimachinery_test.go

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package apiutil_test
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
gmg "github.com/onsi/gomega"
24+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/apimachinery/pkg/runtime/schema"
27+
"k8s.io/client-go/discovery"
28+
"k8s.io/client-go/kubernetes/scheme"
29+
"k8s.io/client-go/rest"
30+
"sigs.k8s.io/controller-runtime/pkg/client"
31+
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
32+
)
33+
34+
func TestApiMachinery(t *testing.T) {
35+
restCfg, tearDownFn := setupEnvtest(t)
36+
defer tearDownFn(t)
37+
38+
// Details of the GVK registered at initialization.
39+
initialGvk := metav1.GroupVersionKind{
40+
Group: "crew.example.com",
41+
Version: "v1",
42+
Kind: "Driver",
43+
}
44+
45+
// A set of GVKs to register at runtime with varying properties.
46+
runtimeGvks := []struct {
47+
name string
48+
gvk metav1.GroupVersionKind
49+
plural string
50+
}{
51+
{
52+
name: "new Kind and Version added to existing Group",
53+
gvk: metav1.GroupVersionKind{
54+
Group: "crew.example.com",
55+
Version: "v1alpha1",
56+
Kind: "Passenger",
57+
},
58+
plural: "passengers",
59+
},
60+
{
61+
name: "new Kind added to existing Group and Version",
62+
gvk: metav1.GroupVersionKind{
63+
Group: "crew.example.com",
64+
Version: "v1",
65+
Kind: "Garage",
66+
},
67+
plural: "garages",
68+
},
69+
{
70+
name: "new GVK",
71+
gvk: metav1.GroupVersionKind{
72+
Group: "inventory.example.com",
73+
Version: "v1",
74+
Kind: "Taxi",
75+
},
76+
plural: "taxis",
77+
},
78+
}
79+
80+
t.Run("IsGVKNamespaced should report scope for GVK registered at initialization", func(t *testing.T) {
81+
g := gmg.NewWithT(t)
82+
83+
httpClient, err := rest.HTTPClientFor(restCfg)
84+
g.Expect(err).NotTo(gmg.HaveOccurred())
85+
86+
lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
87+
g.Expect(err).NotTo(gmg.HaveOccurred())
88+
89+
s := scheme.Scheme
90+
err = apiextensionsv1.AddToScheme(s)
91+
g.Expect(err).NotTo(gmg.HaveOccurred())
92+
93+
// Query the scope of a GVK that was registered at initialization.
94+
scope, err := apiutil.IsGVKNamespaced(
95+
schema.GroupVersionKind(initialGvk),
96+
lazyRestMapper,
97+
)
98+
g.Expect(err).NotTo(gmg.HaveOccurred())
99+
g.Expect(scope).To(gmg.BeTrue())
100+
})
101+
102+
for _, runtimeGvk := range runtimeGvks {
103+
t.Run("IsGVKNamespaced should report scope for "+runtimeGvk.name, func(t *testing.T) {
104+
g := gmg.NewWithT(t)
105+
ctx := context.Background()
106+
107+
httpClient, err := rest.HTTPClientFor(restCfg)
108+
g.Expect(err).NotTo(gmg.HaveOccurred())
109+
110+
lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient)
111+
g.Expect(err).NotTo(gmg.HaveOccurred())
112+
113+
s := scheme.Scheme
114+
err = apiextensionsv1.AddToScheme(s)
115+
g.Expect(err).NotTo(gmg.HaveOccurred())
116+
117+
c, err := client.New(restCfg, client.Options{Scheme: s})
118+
g.Expect(err).NotTo(gmg.HaveOccurred())
119+
120+
// Run a valid query to initialize cache.
121+
scope, err := apiutil.IsGVKNamespaced(
122+
schema.GroupVersionKind(initialGvk),
123+
lazyRestMapper,
124+
)
125+
g.Expect(err).NotTo(gmg.HaveOccurred())
126+
g.Expect(scope).To(gmg.BeTrue())
127+
128+
// Register a new CRD at runtime.
129+
crd := newCRD(ctx, g, c, runtimeGvk.gvk.Group, runtimeGvk.gvk.Kind, runtimeGvk.plural)
130+
version := crd.Spec.Versions[0]
131+
version.Name = runtimeGvk.gvk.Version
132+
version.Storage = true
133+
version.Served = true
134+
crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{version}
135+
crd.Spec.Scope = apiextensionsv1.NamespaceScoped
136+
137+
g.Expect(c.Create(ctx, crd)).To(gmg.Succeed())
138+
t.Cleanup(func() {
139+
g.Expect(c.Delete(ctx, crd)).To(gmg.Succeed())
140+
})
141+
142+
// Wait until the CRD is registered.
143+
g.Eventually(func(g gmg.Gomega) {
144+
isRegistered, err := isCrdRegistered(restCfg, runtimeGvk.gvk)
145+
g.Expect(err).NotTo(gmg.HaveOccurred())
146+
g.Expect(isRegistered).To(gmg.BeTrue())
147+
}).Should(gmg.Succeed(), "GVK should be available")
148+
149+
// Query the scope of the GVK registered at runtime.
150+
scope, err = apiutil.IsGVKNamespaced(
151+
schema.GroupVersionKind(runtimeGvk.gvk),
152+
lazyRestMapper,
153+
)
154+
g.Expect(err).NotTo(gmg.HaveOccurred())
155+
g.Expect(scope).To(gmg.BeTrue())
156+
})
157+
}
158+
}
159+
160+
// Check if a slice of APIResource contains a given Kind.
161+
func kindInAPIResources(resources *metav1.APIResourceList, kind string) bool {
162+
for _, res := range resources.APIResources {
163+
if res.Kind == kind {
164+
return true
165+
}
166+
}
167+
return false
168+
}
169+
170+
// Check if a CRD has registered with the API server using a DiscoveryClient.
171+
func isCrdRegistered(cfg *rest.Config, gvk metav1.GroupVersionKind) (bool, error) {
172+
discHTTP, err := rest.HTTPClientFor(cfg)
173+
if err != nil {
174+
return false, err
175+
}
176+
177+
discClient, err := discovery.NewDiscoveryClientForConfigAndClient(cfg, discHTTP)
178+
if err != nil {
179+
return false, err
180+
}
181+
182+
resources, err := discClient.ServerResourcesForGroupVersion(gvk.Group + "/" + gvk.Version)
183+
if err != nil {
184+
return false, err
185+
}
186+
187+
return kindInAPIResources(resources, gvk.Kind), nil
188+
}

0 commit comments

Comments
 (0)