diff --git a/pkg/client/apiutil/restmapper.go b/pkg/client/apiutil/restmapper.go index d5e03b2b19..5af02063b8 100644 --- a/pkg/client/apiutil/restmapper.go +++ b/pkg/client/apiutil/restmapper.go @@ -21,6 +21,7 @@ import ( "net/http" "sync" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -166,8 +167,10 @@ func (m *mapper) addKnownGroupAndReload(groupName string, versions ...string) er if err != nil { return err } - for _, version := range apiGroup.Versions { - versions = append(versions, version.Version) + if apiGroup != nil { + for _, version := range apiGroup.Versions { + versions = append(versions, version.Version) + } } } @@ -254,17 +257,12 @@ func (m *mapper) findAPIGroupByName(groupName string) (*metav1.APIGroup, error) m.mu.Unlock() // Looking in the cache again. - { - m.mu.RLock() - group, ok := m.apiGroups[groupName] - m.mu.RUnlock() - if ok { - return group, nil - } - } + m.mu.RLock() + defer m.mu.RUnlock() - // If there is still nothing, return an error. - return nil, fmt.Errorf("failed to find API group %q", groupName) + // Don't return an error here if the API group is not present. + // The reloaded RESTMapper will take care of returning a NoMatchError. + return m.apiGroups[groupName], nil } // fetchGroupVersionResources fetches the resources for the specified group and its versions. @@ -276,7 +274,7 @@ func (m *mapper) fetchGroupVersionResources(groupName string, versions ...string groupVersion := schema.GroupVersion{Group: groupName, Version: version} apiResourceList, err := m.client.ServerResourcesForGroupVersion(groupVersion.String()) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { failedGroups[groupVersion] = err } if apiResourceList != nil { diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index bee63bf240..e520c24de5 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -18,19 +18,22 @@ package apiutil_test import ( "context" + "fmt" "net/http" "testing" - "k8s.io/apimachinery/pkg/api/meta" - _ "github.com/onsi/ginkgo/v2" gmg "github.com/onsi/gomega" + "github.com/onsi/gomega/format" + gomegatypes "github.com/onsi/gomega/types" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -303,6 +306,10 @@ func TestLazyRestMapperProvider(t *testing.T) { lazyRestMapper, err := apiutil.NewDynamicRESTMapper(restCfg, httpClient) g.Expect(err).NotTo(gmg.HaveOccurred()) + // A version is specified but the group doesn't exist. + // For each group, we expect 1 call to the version-specific discovery endpoint: + // #1: GET https://host/apis// + _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "INVALID1"}, "v1") g.Expect(err).To(gmg.HaveOccurred()) g.Expect(meta.IsNoMatchError(err)).To(gmg.BeTrue()) @@ -332,6 +339,35 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(err).To(gmg.HaveOccurred()) g.Expect(meta.IsNoMatchError(err)).To(gmg.BeTrue()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(6)) + + // No version is specified but the group doesn't exist. + // For each group, we expect 2 calls to discover all group versions: + // #1: GET https://host/api + // #2: GET https://host/apis + + _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: "INVALID7"}) + g.Expect(err).To(beNoMatchError()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(8)) + + _, err = lazyRestMapper.RESTMappings(schema.GroupKind{Group: "INVALID8"}) + g.Expect(err).To(beNoMatchError()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(10)) + + _, err = lazyRestMapper.KindFor(schema.GroupVersionResource{Group: "INVALID9"}) + g.Expect(err).To(beNoMatchError()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(12)) + + _, err = lazyRestMapper.KindsFor(schema.GroupVersionResource{Group: "INVALID10"}) + g.Expect(err).To(beNoMatchError()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(14)) + + _, err = lazyRestMapper.ResourceFor(schema.GroupVersionResource{Group: "INVALID11"}) + g.Expect(err).To(beNoMatchError()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(16)) + + _, err = lazyRestMapper.ResourcesFor(schema.GroupVersionResource{Group: "INVALID12"}) + g.Expect(err).To(beNoMatchError()) + g.Expect(crt.GetRequestCount()).To(gmg.Equal(18)) }) t.Run("LazyRESTMapper should return an error if a resource doesn't exist", func(t *testing.T) { @@ -529,3 +565,35 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(mapping.GroupVersionKind.Kind).To(gmg.Equal("rider")) }) } + +func beNoMatchError() gomegatypes.GomegaMatcher { + return &errorMatcher{ + checkFunc: meta.IsNoMatchError, + message: "NoMatch", + } +} + +type errorMatcher struct { + checkFunc func(error) bool + message string +} + +func (e *errorMatcher) Match(actual interface{}) (success bool, err error) { + if actual == nil { + return false, nil + } + + actualErr, actualOk := actual.(error) + if !actualOk { + return false, fmt.Errorf("expected an error-type. got:\n%s", format.Object(actual, 1)) + } + + return e.checkFunc(actualErr), nil +} + +func (e *errorMatcher) FailureMessage(actual interface{}) (message string) { + return format.Message(actual, fmt.Sprintf("to be %s error", e.message)) +} +func (e *errorMatcher) NegatedFailureMessage(actual interface{}) (message string) { + return format.Message(actual, fmt.Sprintf("not to be %s error", e.message)) +}