Merge pull request #8353 from ywk253100/241010_discovery

Use aggregated discovery API to discovery API groups and resources

changelogs/unreleased/8353-ywk253100

@@ -0,0 +1 @@
+Use aggregated discovery API to discovery API groups and resources

pkg/client/factory.go

@@ -21,6 +21,7 @@ import (
 
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	"k8s.io/client-go/discovery"
 	k8scheme "k8s.io/client-go/kubernetes/scheme"
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -56,6 +57,9 @@ type Factory interface {
 	// It adds Kubernetes and Velero types to its scheme. It uses the following priority to specify the cluster
 	// configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration.
 	KubebuilderWatchClient() (kbclient.WithWatch, error)
+	// DiscoveryClient returns a Kubernetes discovery client. It uses the following priority to specify the cluster
+	// configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration.
+	DiscoveryClient() (discovery.AggregatedDiscoveryInterface, error)
 	// SetBasename changes the basename for an already-constructed client.
 	// This is useful for generating clients that require a different user-agent string below the root `velero`
 	// command, such as the server subcommand.
@@ -209,6 +213,14 @@ func (f *factory) KubebuilderWatchClient() (kbclient.WithWatch, error) {
 	return kubebuilderWatchClient, nil
 }
 
+func (f *factory) DiscoveryClient() (discovery.AggregatedDiscoveryInterface, error) {
+	clientConfig, err := f.ClientConfig()
+	if err != nil {
+		return nil, err
+	}
+	return discovery.NewDiscoveryClientForConfig(clientConfig)
+}
+
 func (f *factory) SetBasename(name string) {
 	f.baseName = name
 }

pkg/client/mocks/Factory.go

@@ -1,9 +1,11 @@
-// Code generated by mockery v2.20.0. DO NOT EDIT.
+// Code generated by mockery v2.28.1. DO NOT EDIT.
 
 package mocks
 
 import (
+	discovery "k8s.io/client-go/discovery"
 	dynamic "k8s.io/client-go/dynamic"
+
 	kubernetes "k8s.io/client-go/kubernetes"
 
 	mock "github.com/stretchr/testify/mock"
@@ -51,6 +53,32 @@ func (_m *Factory) ClientConfig() (*rest.Config, error) {
 	return r0, r1
 }
 
+// DiscoveryClient provides a mock function with given fields:
+func (_m *Factory) DiscoveryClient() (discovery.AggregatedDiscoveryInterface, error) {
+	ret := _m.Called()
+
+	var r0 discovery.AggregatedDiscoveryInterface
+	var r1 error
+	if rf, ok := ret.Get(0).(func() (discovery.AggregatedDiscoveryInterface, error)); ok {
+		return rf()
+	}
+	if rf, ok := ret.Get(0).(func() discovery.AggregatedDiscoveryInterface); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(discovery.AggregatedDiscoveryInterface)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func() error); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // DynamicClient provides a mock function with given fields:
 func (_m *Factory) DynamicClient() (dynamic.Interface, error) {
 	ret := _m.Called()

pkg/cmd/server/plugin/plugin.go

@@ -224,7 +224,12 @@ func newServiceAccountBackupItemAction(f client.Factory) plugincommon.HandlerIni
 			return nil, err
 		}
 
-		discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
+		discoveryClient, err := f.DiscoveryClient()
+		if err != nil {
+			return nil, err
+		}
+
+		discoveryHelper, err := velerodiscovery.NewHelper(discoveryClient, logger)
 		if err != nil {
 			return nil, err
 		}
@@ -253,11 +258,11 @@ func newRemapCRDVersionAction(f client.Factory) plugincommon.HandlerInitializer
 			return nil, err
 		}
 
-		clientset, err := f.KubeClient()
+		discoveryClient, err := f.DiscoveryClient()
 		if err != nil {
 			return nil, err
 		}
-		discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
+		discoveryHelper, err := velerodiscovery.NewHelper(discoveryClient, logger)
 		if err != nil {
 			return nil, err
 		}
@@ -470,7 +475,12 @@ func newServiceAccountItemBlockAction(f client.Factory) plugincommon.HandlerInit
 			return nil, err
 		}
 
-		discoveryHelper, err := velerodiscovery.NewHelper(clientset.Discovery(), logger)
+		discoveryClient, err := f.DiscoveryClient()
+		if err != nil {
+			return nil, err
+		}
+
+		discoveryHelper, err := velerodiscovery.NewHelper(discoveryClient, logger)
 		if err != nil {
 			return nil, err
 		}

pkg/cmd/server/server.go

@@ -135,7 +135,7 @@ type server struct {
 	metricsAddress   string
 	kubeClientConfig *rest.Config
 	kubeClient       kubernetes.Interface
-	discoveryClient  discovery.DiscoveryInterface
+	discoveryClient  discovery.AggregatedDiscoveryInterface
 	discoveryHelper  velerodiscovery.Helper
 	dynamicClient    dynamic.Interface
 	// controller-runtime client. the difference from the controller-manager's client
@@ -269,8 +269,8 @@ func newServer(f client.Factory, config *config.Config, logger *logrus.Logger) (
 		return nil, err
 	}
 
-	var discoveryClient *discovery.DiscoveryClient
-	if discoveryClient, err = discovery.NewDiscoveryClientForConfig(clientConfig); err != nil {
+	var discoveryClient discovery.AggregatedDiscoveryInterface
+	if discoveryClient, err = f.DiscoveryClient(); err != nil {
 		cancelFunc()
 		return nil, err
 	}

pkg/discovery/helper.go

@@ -74,7 +74,7 @@ type serverResourcesInterface interface {
 }
 
 type helper struct {
-	discoveryClient discovery.DiscoveryInterface
+	discoveryClient discovery.AggregatedDiscoveryInterface
 	logger          logrus.FieldLogger
 
 	// lock guards mapper, resources and resourcesMap
@@ -89,7 +89,7 @@ type helper struct {
 
 var _ Helper = &helper{}
 
-func NewHelper(discoveryClient discovery.DiscoveryInterface, logger logrus.FieldLogger) (Helper, error) {
+func NewHelper(discoveryClient discovery.AggregatedDiscoveryInterface, logger logrus.FieldLogger) (Helper, error) {
 	h := &helper{
 		discoveryClient: discoveryClient,
 		logger:          logger,

pkg/discovery/helper_test.go

@@ -224,14 +224,15 @@ func TestHelper_ResourceFor(t *testing.T) {
 			},
 		},
 	}
-
 	h := &helper{
-		discoveryClient: fakeDiscoveryClient,
-		lock:            sync.RWMutex{},
-		mapper:          nil,
-		resources:       fakeDiscoveryClient.Resources,
-		resourcesMap:    make(map[schema.GroupVersionResource]metav1.APIResource),
-		serverVersion:   &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
+		discoveryClient: &velerotest.DiscoveryClient{
+			FakeDiscovery: fakeDiscoveryClient,
+		},
+		lock:          sync.RWMutex{},
+		mapper:        nil,
+		resources:     fakeDiscoveryClient.Resources,
+		resourcesMap:  make(map[schema.GroupVersionResource]metav1.APIResource),
+		serverVersion: &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
 	}
 
 	for _, resourceList := range h.resources {
@@ -352,11 +353,13 @@ func TestHelper_KindFor(t *testing.T) {
 	}
 
 	h := &helper{
-		discoveryClient: fakeDiscoveryClient,
-		lock:            sync.RWMutex{},
-		resources:       fakeDiscoveryClient.Resources,
-		resourcesMap:    make(map[schema.GroupVersionResource]metav1.APIResource),
-		serverVersion:   &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
+		discoveryClient: &velerotest.DiscoveryClient{
+			FakeDiscovery: fakeDiscoveryClient,
+		},
+		lock:          sync.RWMutex{},
+		resources:     fakeDiscoveryClient.Resources,
+		resourcesMap:  make(map[schema.GroupVersionResource]metav1.APIResource),
+		serverVersion: &version.Info{Major: "1", Minor: "22", GitVersion: "v1.22.1"},
 	}
 
 	h.kindMap = map[schema.GroupVersionKind]metav1.APIResource{pvGVK: pvAPIRes}
@@ -517,9 +520,11 @@ func TestHelper_Refresh(t *testing.T) {
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
 			h := &helper{
-				lock:            sync.RWMutex{},
-				discoveryClient: fakeDiscoveryClient,
-				logger:          logrus.New(),
+				lock: sync.RWMutex{},
+				discoveryClient: &velerotest.DiscoveryClient{
+					FakeDiscovery: fakeDiscoveryClient,
+				},
+				logger: logrus.New(),
 			}
 			// Set feature flags
 			if testCase.features != "" {
@@ -637,7 +642,9 @@ func TestHelper(t *testing.T) {
 	fakeDiscoveryClient := &fake.FakeDiscovery{
 		Fake: &clientgotesting.Fake{},
 	}
-	h, err := NewHelper(fakeDiscoveryClient, logrus.New())
+	h, err := NewHelper(&velerotest.DiscoveryClient{
+		FakeDiscovery: fakeDiscoveryClient,
+	}, logrus.New())
 	assert.NoError(t, err)
 	// All below calls put together for the implementation are empty or just very simple, and just want to cover testing
 	// If wanting to write unit tests for some functions could remove it and with writing new function alone

pkg/test/discovery_client.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	discoveryfake "k8s.io/client-go/discovery/fake"
 )
@@ -80,3 +81,11 @@ func (c *DiscoveryClient) WithAPIResource(resource *APIResource) *DiscoveryClien
 
 	return c
 }
+
+func (c *DiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) {
+	apiGroupList, err := c.ServerGroups()
+	if err != nil {
+		return nil, nil, nil, err
+	}
+	return apiGroupList, nil, nil, nil
+}