From e2598a43a37886d04b09821e4fbc9dd912802b47 Mon Sep 17 00:00:00 2001
From: Maciej Szulik <maszulik@redhat.com>
Date: Sat, 21 Nov 2020 00:10:47 +0100
Subject: [PATCH] restmapper: re-try shortcut expander after not-found error

Kubernetes-commit: f7d1eb7726fa511b6439b7f8162138742ac639ab
---
 restmapper/shortcut.go      | 13 +++++-
 restmapper/shortcut_test.go | 81 +++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/restmapper/shortcut.go b/restmapper/shortcut.go
index 714ba90a..7ab3cd46 100644
--- a/restmapper/shortcut.go
+++ b/restmapper/shortcut.go
@@ -43,7 +43,18 @@ func NewShortcutExpander(delegate meta.RESTMapper, client discovery.DiscoveryInt
 
 // KindFor fulfills meta.RESTMapper
 func (e shortcutExpander) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) {
-	return e.RESTMapper.KindFor(e.expandResourceShortcut(resource))
+	// expandResourceShortcut works with current API resources as read from discovery cache.
+	// In case of new CRDs this means we potentially don't have current state of discovery.
+	// In the current wiring in k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go#toRESTMapper,
+	// we are using DeferredDiscoveryRESTMapper which on KindFor failure will clear the
+	// cache and fetch all data from a cluster (see vendor/k8s.io/client-go/restmapper/discovery.go#KindFor).
+	// Thus another call to expandResourceShortcut, after a NoMatchError should successfully
+	// read Kind to the user or an error.
+	gvk, err := e.RESTMapper.KindFor(e.expandResourceShortcut(resource))
+	if meta.IsNoMatchError(err) {
+		return e.RESTMapper.KindFor(e.expandResourceShortcut(resource))
+	}
+	return gvk, err
 }
 
 // KindsFor fulfills meta.RESTMapper
diff --git a/restmapper/shortcut_test.go b/restmapper/shortcut_test.go
index 515565d3..95b865fe 100644
--- a/restmapper/shortcut_test.go
+++ b/restmapper/shortcut_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	openapi_v2 "github.com/google/gnostic/openapiv2"
+	"github.com/google/go-cmp/cmp"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -195,6 +196,65 @@ func TestKindFor(t *testing.T) {
 	}
 }
 
+func TestKindForWithNewCRDs(t *testing.T) {
+	tests := map[string]struct {
+		in       schema.GroupVersionResource
+		expected schema.GroupVersionKind
+		srvRes   []*metav1.APIResourceList
+	}{
+		"": {
+			in:       schema.GroupVersionResource{Group: "a", Version: "", Resource: "sc"},
+			expected: schema.GroupVersionKind{Group: "a", Version: "v1", Kind: "StorageClass"},
+			srvRes: []*metav1.APIResourceList{
+				{
+					GroupVersion: "a/v1",
+					APIResources: []metav1.APIResource{
+						{
+							Name:       "storageclasses",
+							ShortNames: []string{"sc"},
+							Kind:       "StorageClass",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			invalidateCalled := false
+			fakeDiscovery := &fakeDiscoveryClient{}
+			fakeDiscovery.serverResourcesHandler = func() ([]*metav1.APIResourceList, error) {
+				if invalidateCalled {
+					return test.srvRes, nil
+				}
+				return []*metav1.APIResourceList{}, nil
+			}
+			fakeCachedDiscovery := &fakeCachedDiscoveryClient{DiscoveryInterface: fakeDiscovery}
+			fakeCachedDiscovery.invalidateHandler = func() {
+				invalidateCalled = true
+			}
+			fakeCachedDiscovery.freshHandler = func() bool {
+				return invalidateCalled
+			}
+
+			// in real world the discovery client is fronted with a cache which
+			// will answer the initial request, only failure to match will trigger
+			// the cache invalidation and live discovery call
+			delegate := NewDeferredDiscoveryRESTMapper(fakeCachedDiscovery)
+			mapper := NewShortcutExpander(delegate, fakeCachedDiscovery)
+
+			gvk, err := mapper.KindFor(test.in)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+			if diff := cmp.Equal(gvk, test.expected); !diff {
+				t.Errorf("unexpected data returned %#v, expected %#v", gvk, test.expected)
+			}
+		})
+	}
+}
+
 type fakeRESTMapper struct {
 	kindForInput schema.GroupVersionResource
 }
@@ -301,3 +361,24 @@ func (c *fakeDiscoveryClient) OpenAPISchema() (*openapi_v2.Document, error) {
 func (c *fakeDiscoveryClient) OpenAPIV3() openapi.Client {
 	panic("implement me")
 }
+
+type fakeCachedDiscoveryClient struct {
+	discovery.DiscoveryInterface
+	freshHandler      func() bool
+	invalidateHandler func()
+}
+
+var _ discovery.CachedDiscoveryInterface = &fakeCachedDiscoveryClient{}
+
+func (c *fakeCachedDiscoveryClient) Fresh() bool {
+	if c.freshHandler != nil {
+		return c.freshHandler()
+	}
+	return true
+}
+
+func (c *fakeCachedDiscoveryClient) Invalidate() {
+	if c.invalidateHandler != nil {
+		c.invalidateHandler()
+	}
+}