Merge pull request #2689 from g-gaston/fix-cache-invalidation-for-uns…

…ync-caches-backport-0-16

[release-0.16] 🐛 Fix lazy rest mapper cache invalidation

pkg/client/apiutil/restmapper.go

@@ -53,7 +53,7 @@ func NewDynamicRESTMapper(cfg *rest.Config, httpClient *http.Client) (meta.RESTM
 // client for discovery information to do REST mappings.
 type mapper struct {
 	mapper      meta.RESTMapper
-	client      *discovery.DiscoveryClient
+	client      discovery.DiscoveryInterface
 	knownGroups map[string]*restmapper.APIGroupResources
 	apiGroups   map[string]*metav1.APIGroup
 
@@ -278,15 +278,24 @@ func (m *mapper) fetchGroupVersionResourcesLocked(groupName string, versions ...
 		groupVersion := schema.GroupVersion{Group: groupName, Version: version}
 
 		apiResourceList, err := m.client.ServerResourcesForGroupVersion(groupVersion.String())
-		if apierrors.IsNotFound(err) && m.isGroupVersionCached(groupVersion) {
+		if apierrors.IsNotFound(err) {
 			// If the version is not found, we remove the group from the cache
 			// so it gets refreshed on the next call.
-			delete(m.apiGroups, groupName)
-			delete(m.knownGroups, groupName)
+			cacheInvalidated := false
+			if m.isAPIGroupCached(groupVersion) {
+				delete(m.apiGroups, groupName)
+				cacheInvalidated = true
+			}
+			if m.isGroupVersionCached(groupVersion) {
+				delete(m.knownGroups, groupName)
+				cacheInvalidated = true
+			}
 			// It's important to refresh the mapper after invalidating the cache, since returning an error
 			// aborts the call and leaves the underlying mapper unchanged. If not refreshed, the next call
 			// will still return a match for the NotFound version.
-			m.refreshMapper()
+			if cacheInvalidated {
+				m.refreshMapper()
+			}
 		}
 
 		if err != nil {
@@ -317,6 +326,22 @@ func (m *mapper) isGroupVersionCached(gv schema.GroupVersion) bool {
 	return false
 }
 
+// isAPIGroupCached checks if a version for a group is cached in the api groups cache.
+func (m *mapper) isAPIGroupCached(gv schema.GroupVersion) bool {
+	cachedGroup, ok := m.apiGroups[gv.Group]
+	if !ok {
+		return false
+	}
+
+	for _, version := range cachedGroup.Versions {
+		if version.Version == gv.Version {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (m *mapper) refreshMapper() {
 	updatedGroupResources := make([]*restmapper.APIGroupResources, 0, len(m.knownGroups))
 	for _, agr := range m.knownGroups {

pkg/client/apiutil/restmapper_wb_test.go

@@ -0,0 +1,204 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apiutil
+
+import (
+	"testing"
+
+	gmg "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/restmapper"
+)
+
+func TestLazyRestMapper_fetchGroupVersionResourcesLocked_CacheInvalidation(t *testing.T) {
+	tests := []struct {
+		name                                   string
+		groupName                              string
+		versions                               []string
+		cachedAPIGroups, expectedAPIGroups     map[string]*metav1.APIGroup
+		cachedKnownGroups, expectedKnownGroups map[string]*restmapper.APIGroupResources
+	}{
+		{
+			name:      "Not found version for cached groupVersion in apiGroups and knownGroups",
+			groupName: "group1",
+			versions:  []string{"v1", "v2"},
+			cachedAPIGroups: map[string]*metav1.APIGroup{
+				"group1": {
+					Name: "group1",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							Version: "v1",
+						},
+					},
+				},
+			},
+			cachedKnownGroups: map[string]*restmapper.APIGroupResources{
+				"group1": {
+					VersionedResources: map[string][]metav1.APIResource{
+						"v1": {
+							{
+								Name: "resource1",
+							},
+						},
+					},
+				},
+			},
+			expectedAPIGroups:   map[string]*metav1.APIGroup{},
+			expectedKnownGroups: map[string]*restmapper.APIGroupResources{},
+		},
+		{
+			name:      "Not found version for cached groupVersion only in apiGroups",
+			groupName: "group1",
+			versions:  []string{"v1", "v2"},
+			cachedAPIGroups: map[string]*metav1.APIGroup{
+				"group1": {
+					Name: "group1",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							Version: "v1",
+						},
+					},
+				},
+			},
+			cachedKnownGroups: map[string]*restmapper.APIGroupResources{
+				"group1": {
+					VersionedResources: map[string][]metav1.APIResource{
+						"v3": {
+							{
+								Name: "resource1",
+							},
+						},
+					},
+				},
+			},
+			expectedAPIGroups: map[string]*metav1.APIGroup{},
+			expectedKnownGroups: map[string]*restmapper.APIGroupResources{
+				"group1": {
+					VersionedResources: map[string][]metav1.APIResource{
+						"v3": {
+							{
+								Name: "resource1",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:      "Not found version for cached groupVersion only in knownGroups",
+			groupName: "group1",
+			versions:  []string{"v1", "v2"},
+			cachedAPIGroups: map[string]*metav1.APIGroup{
+				"group1": {
+					Name: "group1",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							Version: "v3",
+						},
+					},
+				},
+			},
+			cachedKnownGroups: map[string]*restmapper.APIGroupResources{
+				"group1": {
+					VersionedResources: map[string][]metav1.APIResource{
+						"v2": {
+							{
+								Name: "resource1",
+							},
+						},
+					},
+				},
+			},
+			expectedAPIGroups: map[string]*metav1.APIGroup{
+				"group1": {
+					Name: "group1",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							Version: "v3",
+						},
+					},
+				},
+			},
+			expectedKnownGroups: map[string]*restmapper.APIGroupResources{},
+		},
+		{
+			name:      "Not found version for non cached groupVersion",
+			groupName: "group1",
+			versions:  []string{"v1", "v2"},
+			cachedAPIGroups: map[string]*metav1.APIGroup{
+				"group1": {
+					Name: "group1",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							Version: "v3",
+						},
+					},
+				},
+			},
+			cachedKnownGroups: map[string]*restmapper.APIGroupResources{
+				"group1": {
+					VersionedResources: map[string][]metav1.APIResource{
+						"v3": {
+							{
+								Name: "resource1",
+							},
+						},
+					},
+				},
+			},
+			expectedAPIGroups: map[string]*metav1.APIGroup{
+				"group1": {
+					Name: "group1",
+					Versions: []metav1.GroupVersionForDiscovery{
+						{
+							Version: "v3",
+						},
+					},
+				},
+			},
+			expectedKnownGroups: map[string]*restmapper.APIGroupResources{
+				"group1": {
+					VersionedResources: map[string][]metav1.APIResource{
+						"v3": {
+							{
+								Name: "resource1",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := gmg.NewWithT(t)
+			m := &mapper{
+				mapper:      restmapper.NewDiscoveryRESTMapper([]*restmapper.APIGroupResources{}),
+				client:      fake.NewSimpleClientset().Discovery(),
+				apiGroups:   tt.cachedAPIGroups,
+				knownGroups: tt.cachedKnownGroups,
+			}
+			_, err := m.fetchGroupVersionResourcesLocked(tt.groupName, tt.versions...)
+			g.Expect(err).To(gmg.HaveOccurred())
+			g.Expect(err).To(gmg.BeAssignableToTypeOf(&ErrResourceDiscoveryFailed{}))
+			g.Expect(m.apiGroups).To(gmg.BeComparableTo(tt.expectedAPIGroups))
+			g.Expect(m.knownGroups).To(gmg.BeComparableTo(tt.expectedKnownGroups))
+		})
+	}
+}