diff --git a/pkg/client/apiutil/restmapper.go b/pkg/client/apiutil/restmapper.go index 2e899fbbc9..24af330c8b 100644 --- a/pkg/client/apiutil/restmapper.go +++ b/pkg/client/apiutil/restmapper.go @@ -185,7 +185,11 @@ func (m *mapper) addKnownGroupAndReload(groupName string, versions ...string) er // Update information for group resources about versioned resources. // The number of API calls is equal to the number of versions: /apis//. - groupVersionResources, err := m.fetchGroupVersionResources(groupName, versions...) + // If we encounter a missing API version (NotFound error), we will remove the group from + // the m.apiGroups and m.knownGroups caches. + // If this happens, in the next call the group will be added back to apiGroups + // and only the existing versions will be loaded in knownGroups. + groupVersionResources, err := m.fetchGroupVersionResourcesLocked(groupName, versions...) if err != nil { return fmt.Errorf("failed to get API group resources: %w", err) } @@ -194,14 +198,12 @@ func (m *mapper) addKnownGroupAndReload(groupName string, versions ...string) er groupResources = m.knownGroups[groupName] } - for version, resources := range groupVersionResources { - groupResources.VersionedResources[version.Version] = resources.APIResources - } - // Update information for group resources about the API group by adding new versions. // Ignore the versions that are already registered. - for groupVersion := range groupVersionResources { + for groupVersion, resources := range groupVersionResources { version := groupVersion.Version + + groupResources.VersionedResources[version] = resources.APIResources found := false for _, v := range groupResources.Group.Versions { if v.Version == version { @@ -268,9 +270,9 @@ func (m *mapper) findAPIGroupByName(groupName string) (*metav1.APIGroup, error) return m.apiGroups[groupName], nil } -// fetchGroupVersionResources fetches the resources for the specified group and its versions. +// fetchGroupVersionResourcesLocked fetches the resources for the specified group and its versions. // This method might modify the cache so it needs to be called under the lock. -func (m *mapper) fetchGroupVersionResources(groupName string, versions ...string) (map[schema.GroupVersion]*metav1.APIResourceList, error) { +func (m *mapper) fetchGroupVersionResourcesLocked(groupName string, versions ...string) (map[schema.GroupVersion]*metav1.APIResourceList, error) { groupVersionResources := make(map[schema.GroupVersion]*metav1.APIResourceList) failedGroups := make(map[schema.GroupVersion]error) @@ -283,6 +285,7 @@ func (m *mapper) fetchGroupVersionResources(groupName string, versions ...string // so it gets refreshed on the next call. delete(m.apiGroups, groupName) delete(m.knownGroups, groupName) + continue } else if err != nil { failedGroups[groupVersion] = err } diff --git a/pkg/client/apiutil/restmapper_test.go b/pkg/client/apiutil/restmapper_test.go index ec41c242e6..2e34a98735 100644 --- a/pkg/client/apiutil/restmapper_test.go +++ b/pkg/client/apiutil/restmapper_test.go @@ -571,12 +571,24 @@ func TestLazyRestMapperProvider(t *testing.T) { c, err := client.New(restCfg, client.Options{Scheme: s}) g.Expect(err).NotTo(gmg.HaveOccurred()) - // Register a new CRD ina new group to avoid collisions when deleting versions - "taxi.inventory.example.com". + // Register a new CRD ina new group to avoid collisions when deleting versions - "taxis.inventory.example.com". group := "inventory.example.com" kind := "Taxi" plural := "taxis" crdName := plural + "." + group - crd := createNewCRD(ctx, g, c, group, kind, plural) + // Create a CRD with two versions: v1alpha1 and v1 where both are served and + // v1 is the storage version so we can easily remove v1alpha1 later. + crd := newCRD(ctx, g, c, group, kind, plural) + v1alpha1 := crd.Spec.Versions[0] + v1alpha1.Name = "v1alpha1" + v1alpha1.Storage = false + v1alpha1.Served = true + v1 := crd.Spec.Versions[0] + v1.Name = "v1" + v1.Storage = true + v1.Served = true + crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{v1alpha1, v1} + g.Expect(c.Create(ctx, crd)).To(gmg.Succeed()) t.Cleanup(func() { g.Expect(c.Delete(ctx, crd)).To(gmg.Succeed()) }) @@ -599,8 +611,8 @@ func TestLazyRestMapperProvider(t *testing.T) { // #1: GET https://host/api // #2: GET https://host/apis // Then, for all available versions: - // #3: GET https://host/apis/inventory.example.com/v1 - // #4: GET https://host/apis/inventory.example.com/v2 + // #3: GET https://host/apis/inventory.example.com/v1alpha1 + // #4: GET https://host/apis/inventory.example.com/v1 // This should fill the cache for apiGroups and versions. mapping, err := lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}) g.Expect(err).NotTo(gmg.HaveOccurred()) @@ -608,54 +620,67 @@ func TestLazyRestMapperProvider(t *testing.T) { g.Expect(crt.GetRequestCount()).To(gmg.Equal(4)) crt.Reset() // We reset the counter to check how many additional requests are made later. - // At this point v2 should be cached - _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}, "v2") + // At this point v1alpha1 should be cached + _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}, "v1alpha1") g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(0)) // We update the CRD to only have v1 version. g.Expect(c.Get(ctx, types.NamespacedName{Name: crdName}, crd)).To(gmg.Succeed()) - var v1 apiextensionsv1.CustomResourceDefinitionVersion - for i, version := range crd.Spec.Versions { + for _, version := range crd.Spec.Versions { if version.Name == "v1" { - crd.Spec.Versions[i].Storage = true v1 = version - v1.Storage = true + break } } crd.Spec.Versions = []apiextensionsv1.CustomResourceDefinitionVersion{v1} g.Expect(c.Update(ctx, crd)).To(gmg.Succeed()) - // We wait until v2 is not available anymore. + // We wait until v1alpha1 is not available anymore. g.Eventually(func(g gmg.Gomega) { - _, err = discClient.ServerResourcesForGroupVersion(group + "/v2") - g.Expect(apierrors.IsNotFound(err)).To(gmg.BeTrue(), "v2 should not be available anymore") + _, err = discClient.ServerResourcesForGroupVersion(group + "/v1alpha1") + g.Expect(apierrors.IsNotFound(err)).To(gmg.BeTrue(), "v1alpha1 should not be available anymore") }).Should(gmg.Succeed()) - // Although v2 is not available anymore, the cache is not invalidated yet so it should return a mapping. - _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}, "v2") + // Although v1alpha1 is not available anymore, the cache is not invalidated yet so it should return a mapping. + _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}, "v1alpha1") g.Expect(err).NotTo(gmg.HaveOccurred()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(0)) // We request Limo, which is not in the mapper because it doesn't exist. // This will trigger a reload of the lazy mapper cache. // Reloading the cache will read v2 again and since it's not available anymore, it should invalidate the cache. - // #1: GET https://host/apis/inventory.example.com/v1 - // #2: GET https://host/apis/inventory.example.com/v2 + // #1: GET https://host/apis/inventory.example.com/v1alpha1 + // #2: GET https://host/apis/inventory.example.com/v1 _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: "Limo"}) g.Expect(err).To(beNoMatchError()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(2)) crt.Reset() - // Now we request v2 again and it should return an error since the cache was invalidated. - // #1: GET https://host/apis/inventory.example.com/v2 - _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}, "v2") + // Now we request v1alpha1 again and it should return an error since the cache was invalidated. + // #1: GET https://host/apis/inventory.example.com/v1alpha1 + _, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}, "v1alpha1") g.Expect(err).To(beNoMatchError()) g.Expect(crt.GetRequestCount()).To(gmg.Equal(1)) + + // Verify that when requesting the mapping without a version, it doesn't error + // and it returns v1. + mapping, err = lazyRestMapper.RESTMapping(schema.GroupKind{Group: group, Kind: kind}) + g.Expect(err).NotTo(gmg.HaveOccurred()) + g.Expect(mapping.Resource.Version).To(gmg.Equal("v1")) }) } +// createNewCRD creates a new CRD with the given group, kind, and plural and returns it. func createNewCRD(ctx context.Context, g gmg.Gomega, c client.Client, group, kind, plural string) *apiextensionsv1.CustomResourceDefinition { + newCRD := newCRD(ctx, g, c, group, kind, plural) + g.Expect(c.Create(ctx, newCRD)).To(gmg.Succeed()) + + return newCRD +} + +// newCRD returns a new CRD with the given group, kind, and plural. +func newCRD(ctx context.Context, g gmg.Gomega, c client.Client, group, kind, plural string) *apiextensionsv1.CustomResourceDefinition { crd := &apiextensionsv1.CustomResourceDefinition{} err := c.Get(ctx, types.NamespacedName{Name: "drivers.crew.example.com"}, crd) g.Expect(err).NotTo(gmg.HaveOccurred()) @@ -671,9 +696,6 @@ func createNewCRD(ctx context.Context, g gmg.Gomega, c client.Client, group, kin } newCRD.ResourceVersion = "" - // Create the new CRD. - g.Expect(c.Create(ctx, newCRD)).To(gmg.Succeed()) - return newCRD }