diff --git a/changelogs/unreleased/8353-ywk253100 b/changelogs/unreleased/8353-ywk253100 new file mode 100644 index 0000000000..d6f34952da --- /dev/null +++ b/changelogs/unreleased/8353-ywk253100 @@ -0,0 +1 @@ +Use aggregated discovery API to discovery API groups and resources \ No newline at end of file diff --git a/pkg/client/factory.go b/pkg/client/factory.go index c74f43b889..f1524a5835 100644 --- a/pkg/client/factory.go +++ b/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 } diff --git a/pkg/client/mocks/Factory.go b/pkg/client/mocks/Factory.go index 1bec839cdd..60efeed6ff 100644 --- a/pkg/client/mocks/Factory.go +++ b/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() diff --git a/pkg/cmd/server/plugin/plugin.go b/pkg/cmd/server/plugin/plugin.go index 3688ea3544..1c5b061328 100644 --- a/pkg/cmd/server/plugin/plugin.go +++ b/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 } diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index a697e4d01b..776502f093 100644 --- a/pkg/cmd/server/server.go +++ b/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 } diff --git a/pkg/discovery/helper.go b/pkg/discovery/helper.go index e77a0dffd9..11c2d623be 100644 --- a/pkg/discovery/helper.go +++ b/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, diff --git a/pkg/discovery/helper_test.go b/pkg/discovery/helper_test.go index 37a16b5361..c4a7963e1f 100644 --- a/pkg/discovery/helper_test.go +++ b/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 diff --git a/pkg/test/discovery_client.go b/pkg/test/discovery_client.go index 3f65cf5ea4..fb463d5f87 100644 --- a/pkg/test/discovery_client.go +++ b/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 +}