From 5ca2cc4dc64afc8a4f31277fd5b1a518c7b5b447 Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Wed, 24 May 2023 16:49:24 +0200 Subject: [PATCH 1/6] Fix unspecified KindsFor version --- pkg/client/apiutil/restmapper.go | 6 ++++++ pkg/client/apiutil/restmapper_test.go | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkg/client/apiutil/restmapper.go b/pkg/client/apiutil/restmapper.go index f14f8a9f59..8961800e31 100644 --- a/pkg/client/apiutil/restmapper.go +++ b/pkg/client/apiutil/restmapper.go @@ -152,6 +152,12 @@ func (m *mapper) getMapper() meta.RESTMapper { // addKnownGroupAndReload reloads the mapper with updated information about missing API group. // versions can be specified for partial updates, for instance for v1beta1 version only. func (m *mapper) addKnownGroupAndReload(groupName string, versions ...string) error { + // versions will here be [""] if the forwarded Version value of + // GroupVersionResource (in calling method) was not specified. + if len(versions) == 1 && versions[0] == "" { + versions = []string{} + } + // If no specific versions are set by user, we will scan all available ones for the API group. // This operation requires 2 requests: /api and /apis, but only once. For all subsequent calls // this data will be taken from cache. diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index 99ea5a79d8..e62348b274 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -210,6 +210,28 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) }) + t.Run("LazyRESTMapper KindsFor should work correctly with empty versions list", func(t *testing.T) { + g := gmg.NewWithT(t) + + httpClient, err := rest.HTTPClientFor(restCfg) + g.Expect(err).NotTo(gmg.HaveOccurred()) + + crt := newCountingRoundTripper(httpClient.Transport) + httpClient.Transport = crt + + lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient) + g.Expect(err).NotTo(gmg.HaveOccurred()) + + g.Expect(crt.GetRequestCount()).To(gmg.Equal(0)) + + kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "autoscaling", Resource: "horizontalpodautoscaler"}) + g.Expect(err).NotTo(gmg.HaveOccurred()) + g.Expect(len(kinds)).To(gmg.Equal(2)) + g.Expect(kinds[0].Kind).To(gmg.Equal("HorizontalPodAutoscaler")) + g.Expect(kinds[1].Kind).To(gmg.Equal("HorizontalPodAutoscaler")) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) + }) + t.Run("LazyRESTMapper should work correctly with multiple API group versions", func(t *testing.T) { g := gmg.NewWithT(t) From 93393632a35f1aa79743702088c41a715bc58002 Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Wed, 24 May 2023 20:21:34 +0200 Subject: [PATCH 2/6] Dont allocate empty list Co-authored-by: Alvaro Aleman --- pkg/client/apiutil/restmapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/apiutil/restmapper.go b/pkg/client/apiutil/restmapper.go index 8961800e31..e0ff72dc13 100644 --- a/pkg/client/apiutil/restmapper.go +++ b/pkg/client/apiutil/restmapper.go @@ -155,7 +155,7 @@ func (m *mapper) addKnownGroupAndReload(groupName string, versions ...string) er // versions will here be [""] if the forwarded Version value of // GroupVersionResource (in calling method) was not specified. if len(versions) == 1 && versions[0] == "" { - versions = []string{} + versions = nil } // If no specific versions are set by user, we will scan all available ones for the API group. From 64be9dd3ebc3bf1fa0d4971bc838771029d6d5b1 Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Wed, 24 May 2023 21:05:00 +0200 Subject: [PATCH 3/6] Test all four relevant interface methods, and do not assert count --- pkg/client/apiutil/restmapper_test.go | 61 +++++++++++++++++---------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index e62348b274..5b0ab0fcba 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -210,28 +210,6 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) }) - t.Run("LazyRESTMapper KindsFor should work correctly with empty versions list", func(t *testing.T) { - g := gmg.NewWithT(t) - - httpClient, err := rest.HTTPClientFor(restCfg) - g.Expect(err).NotTo(gmg.HaveOccurred()) - - crt := newCountingRoundTripper(httpClient.Transport) - httpClient.Transport = crt - - lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient) - g.Expect(err).NotTo(gmg.HaveOccurred()) - - g.Expect(crt.GetRequestCount()).To(gmg.Equal(0)) - - kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "autoscaling", Resource: "horizontalpodautoscaler"}) - g.Expect(err).NotTo(gmg.HaveOccurred()) - g.Expect(len(kinds)).To(gmg.Equal(2)) - g.Expect(kinds[0].Kind).To(gmg.Equal("HorizontalPodAutoscaler")) - g.Expect(kinds[1].Kind).To(gmg.Equal("HorizontalPodAutoscaler")) - g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) - }) - t.Run("LazyRESTMapper should work correctly with multiple API group versions", func(t *testing.T) { g := gmg.NewWithT(t) @@ -425,6 +403,45 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(err).To(gmg.HaveOccurred()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(6)) }) + + t.Run("LazyRESTMapper should work correctly if the version isn't specified", func(t *testing.T) { + g := gmg.NewWithT(t) + + httpClient, err := rest.HTTPClientFor(restCfg) + g.Expect(err).NotTo(gmg.HaveOccurred()) + + crt := newCountingRoundTripper(httpClient.Transport) + httpClient.Transport = crt + + lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient) + g.Expect(err).NotTo(gmg.HaveOccurred()) + + g.Expect(crt.GetRequestCount()).To(gmg.Equal(0)) + + kind, err := lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Resource: "ingress"}) + g.Expect(err).NotTo(gmg.HaveOccurred()) + g.Expect(kind.Version).ToNot(gmg.BeEmpty()) + g.Expect(kind.Kind).ToNot(gmg.BeEmpty()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(3)) + + kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Resource: "tokenreviews"}) + g.Expect(err).NotTo(gmg.HaveOccurred()) + g.Expect(kinds).ToNot(gmg.BeEmpty()) + g.Expect(kinds[0].Version).ToNot(gmg.BeEmpty()) + g.Expect(kinds[0].Kind).ToNot(gmg.BeEmpty()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) + + resorce, err := lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Resource: "priorityclasses"}) + g.Expect(err).NotTo(gmg.HaveOccurred()) + g.Expect(resorce.Version).ToNot(gmg.BeEmpty()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(5)) + + resorces, err := lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "policy", Resource: "poddisruptionbudgets"}) + g.Expect(err).NotTo(gmg.HaveOccurred()) + g.Expect(kinds).ToNot(gmg.BeEmpty()) + g.Expect(resorces[0].Version).ToNot(gmg.BeEmpty()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(6)) + }) t.Run("LazyRESTMapper can fetch CRDs if they were created at runtime", func(t *testing.T) { g := gmg.NewWithT(t) From 133fb123dab872550a05a3f480981913e9353c4d Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Wed, 24 May 2023 21:16:46 +0200 Subject: [PATCH 4/6] Attempt to format correctly in web editor --- pkg/client/apiutil/restmapper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index 5b0ab0fcba..a58608994e 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -403,7 +403,7 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(err).To(gmg.HaveOccurred()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(6)) }) - + t.Run("LazyRESTMapper should work correctly if the version isn't specified", func(t *testing.T) { g := gmg.NewWithT(t) From af4988199a8fa98efb20adf60eb9d30bbe6ea05c Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Wed, 24 May 2023 21:26:02 +0200 Subject: [PATCH 5/6] Remove request count asserts --- pkg/client/apiutil/restmapper_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index a58608994e..f215d807ba 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -410,37 +410,28 @@ func TestLazyRestMapperProvider(t *testing.T) { httpClient, err := rest.HTTPClientFor(restCfg) g.Expect(err).NotTo(gmg.HaveOccurred()) - crt := newCountingRoundTripper(httpClient.Transport) - httpClient.Transport = crt - lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient) g.Expect(err).NotTo(gmg.HaveOccurred()) - g.Expect(crt.GetRequestCount()).To(gmg.Equal(0)) - kind, err := lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Resource: "ingress"}) g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(kind.Version).ToNot(gmg.BeEmpty()) g.Expect(kind.Kind).ToNot(gmg.BeEmpty()) - g.Expect(crt.GetRequestCount()).To(gmg.Equal(3)) kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Resource: "tokenreviews"}) g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(kinds).ToNot(gmg.BeEmpty()) g.Expect(kinds[0].Version).ToNot(gmg.BeEmpty()) g.Expect(kinds[0].Kind).ToNot(gmg.BeEmpty()) - g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) resorce, err := lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Resource: "priorityclasses"}) g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(resorce.Version).ToNot(gmg.BeEmpty()) - g.Expect(crt.GetRequestCount()).To(gmg.Equal(5)) resorces, err := lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "policy", Resource: "poddisruptionbudgets"}) g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(kinds).ToNot(gmg.BeEmpty()) g.Expect(resorces[0].Version).ToNot(gmg.BeEmpty()) - g.Expect(crt.GetRequestCount()).To(gmg.Equal(6)) }) t.Run("LazyRESTMapper can fetch CRDs if they were created at runtime", func(t *testing.T) { From 5bdac8f0638b1b3e167e75b8750e30b5773d0ad7 Mon Sep 17 00:00:00 2001 From: Amund Tenstad Date: Wed, 24 May 2023 21:32:30 +0200 Subject: [PATCH 6/6] Remove non-version item asserts --- pkg/client/apiutil/restmapper_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index f215d807ba..782a8ce478 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -416,13 +416,11 @@ func TestLazyRestMapperProvider(t *testing.T) { kind, err := lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "networking.k8s.io", Resource: "ingress"}) g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(kind.Version).ToNot(gmg.BeEmpty()) - g.Expect(kind.Kind).ToNot(gmg.BeEmpty()) kinds, err := lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "authentication.k8s.io", Resource: "tokenreviews"}) g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(kinds).ToNot(gmg.BeEmpty()) g.Expect(kinds[0].Version).ToNot(gmg.BeEmpty()) - g.Expect(kinds[0].Kind).ToNot(gmg.BeEmpty()) resorce, err := lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "scheduling.k8s.io", Resource: "priorityclasses"}) g.Expect(err).NotTo(gmg.HaveOccurred())