From 07a0171e98d7d32667d96f915662154fd085fb66 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 16 Dec 2021 12:41:04 +0000 Subject: [PATCH] Use regular discovery client instead of cache Signed-off-by: James Munnelly --- .../certificaterequest/approval/BUILD.bazel | 1 + .../approval/certificaterequest_approval.go | 7 ++-- internal/webhook/BUILD.bazel | 1 - internal/webhook/webhook.go | 4 +- pkg/webhook/admission/initializer/BUILD.bazel | 3 -- .../admission/initializer/initializer.go | 15 +------ .../admission/initializer/initializer_test.go | 40 ++----------------- .../admission/initializer/interfaces.go | 7 ---- pkg/webhook/admission/plugins_test.go | 8 ++-- 9 files changed, 15 insertions(+), 71 deletions(-) diff --git a/internal/plugin/admission/certificaterequest/approval/BUILD.bazel b/internal/plugin/admission/certificaterequest/approval/BUILD.bazel index 7a2031f81..6ee62d549 100644 --- a/internal/plugin/admission/certificaterequest/approval/BUILD.bazel +++ b/internal/plugin/admission/certificaterequest/approval/BUILD.bazel @@ -18,6 +18,7 @@ go_library( "@io_k8s_apiserver//pkg/authentication/user:go_default_library", "@io_k8s_apiserver//pkg/authorization/authorizer:go_default_library", "@io_k8s_client_go//discovery:go_default_library", + "@io_k8s_client_go//kubernetes:go_default_library", ], ) diff --git a/internal/plugin/admission/certificaterequest/approval/certificaterequest_approval.go b/internal/plugin/admission/certificaterequest/approval/certificaterequest_approval.go index 35bf7ad06..2e3e5600e 100644 --- a/internal/plugin/admission/certificaterequest/approval/certificaterequest_approval.go +++ b/internal/plugin/admission/certificaterequest/approval/certificaterequest_approval.go @@ -38,6 +38,7 @@ import ( "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" "github.com/jetstack/cert-manager/internal/apis/certmanager" "github.com/jetstack/cert-manager/internal/apis/certmanager/validation/util" @@ -61,7 +62,7 @@ type certificateRequestApproval struct { var _ admission.ValidationInterface = &certificateRequestApproval{} var _ initializer.WantsAuthorizer = &certificateRequestApproval{} -var _ initializer.WantsDiscoveryCache = &certificateRequestApproval{} +var _ initializer.WantsExternalKubeClientSet = &certificateRequestApproval{} func Register(plugins *admission.Plugins) { plugins.Register(PluginName, func() (admission.Interface, error) { @@ -265,8 +266,8 @@ func (c *certificateRequestApproval) SetAuthorizer(a authorizer.Authorizer) { c.authorizer = a } -func (c *certificateRequestApproval) SetDiscoveryCache(discovery discovery.CachedDiscoveryInterface) { - c.discovery = discovery +func (c *certificateRequestApproval) SetExternalKubeClientSet(client kubernetes.Interface) { + c.discovery = client.Discovery() } func (c *certificateRequestApproval) ValidateInitialization() error { diff --git a/internal/webhook/BUILD.bazel b/internal/webhook/BUILD.bazel index 8dc17c90f..35dc0e2e9 100644 --- a/internal/webhook/BUILD.bazel +++ b/internal/webhook/BUILD.bazel @@ -26,7 +26,6 @@ go_library( "@io_k8s_apimachinery//pkg/runtime:go_default_library", "@io_k8s_apimachinery//pkg/util/wait:go_default_library", "@io_k8s_apiserver//pkg/authorization/authorizerfactory:go_default_library", - "@io_k8s_client_go//discovery/cached/memory:go_default_library", "@io_k8s_client_go//kubernetes:go_default_library", "@io_k8s_client_go//rest:go_default_library", "@io_k8s_client_go//tools/clientcmd:go_default_library", diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go index d22e31aa5..ef589c3ce 100644 --- a/internal/webhook/webhook.go +++ b/internal/webhook/webhook.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authorization/authorizerfactory" - "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" @@ -109,11 +108,10 @@ func buildAdmissionChain(client kubernetes.Interface) (*admission.RequestHandler Cap: time.Second * 5, }, }.New() - discoveryClient := memory.NewMemCacheClient(client.Discovery()) if err != nil { return nil, fmt.Errorf("error creating authorization handler: %v", err) } - pluginInitializer := initializer.New(client, nil, authorizer, nil, discoveryClient) + pluginInitializer := initializer.New(client, nil, authorizer, nil) pluginChain, err := pluginHandler.NewFromPlugins(plugin.DefaultOnAdmissionPlugins().List(), pluginInitializer) if err != nil { return nil, fmt.Errorf("error building admission chain: %v", err) diff --git a/pkg/webhook/admission/initializer/BUILD.bazel b/pkg/webhook/admission/initializer/BUILD.bazel index 63e59f6da..95922beb2 100644 --- a/pkg/webhook/admission/initializer/BUILD.bazel +++ b/pkg/webhook/admission/initializer/BUILD.bazel @@ -12,7 +12,6 @@ go_library( "//pkg/webhook/admission:go_default_library", "@io_k8s_apiserver//pkg/authorization/authorizer:go_default_library", "@io_k8s_apiserver//pkg/quota/v1:go_default_library", - "@io_k8s_client_go//discovery:go_default_library", "@io_k8s_client_go//informers:go_default_library", "@io_k8s_client_go//kubernetes:go_default_library", "@io_k8s_component_base//featuregate:go_default_library", @@ -28,8 +27,6 @@ go_test( "@io_k8s_api//admission/v1:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", "@io_k8s_apiserver//pkg/authorization/authorizer:go_default_library", - "@io_k8s_client_go//discovery:go_default_library", - "@io_k8s_client_go//discovery/cached/memory:go_default_library", "@io_k8s_client_go//informers:go_default_library", "@io_k8s_client_go//kubernetes:go_default_library", "@io_k8s_client_go//kubernetes/fake:go_default_library", diff --git a/pkg/webhook/admission/initializer/initializer.go b/pkg/webhook/admission/initializer/initializer.go index 489946183..5ffad803e 100644 --- a/pkg/webhook/admission/initializer/initializer.go +++ b/pkg/webhook/admission/initializer/initializer.go @@ -18,7 +18,6 @@ package initializer import ( "k8s.io/apiserver/pkg/authorization/authorizer" - "k8s.io/client-go/discovery" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/component-base/featuregate" @@ -31,25 +30,17 @@ type pluginInitializer struct { externalInformers informers.SharedInformerFactory authorizer authorizer.Authorizer featureGates featuregate.FeatureGate - discoveryCache discovery.CachedDiscoveryInterface } // New creates an instance of admission plugins initializer. // This constructor is public with a long param list so that callers immediately know that new information can be expected // during compilation when they update a level. -func New( - extClientset kubernetes.Interface, - extInformers informers.SharedInformerFactory, - authz authorizer.Authorizer, - featureGates featuregate.FeatureGate, - discoveryCache discovery.CachedDiscoveryInterface, -) pluginInitializer { +func New(extClientset kubernetes.Interface, extInformers informers.SharedInformerFactory, authz authorizer.Authorizer, featureGates featuregate.FeatureGate) pluginInitializer { return pluginInitializer{ externalClient: extClientset, externalInformers: extInformers, authorizer: authz, featureGates: featureGates, - discoveryCache: discoveryCache, } } @@ -72,10 +63,6 @@ func (i pluginInitializer) Initialize(plugin admission.Interface) { if wants, ok := plugin.(WantsAuthorizer); ok { wants.SetAuthorizer(i.authorizer) } - - if wants, ok := plugin.(WantsDiscoveryCache); ok { - wants.SetDiscoveryCache(i.discoveryCache) - } } var _ admission.PluginInitializer = pluginInitializer{} diff --git a/pkg/webhook/admission/initializer/initializer_test.go b/pkg/webhook/admission/initializer/initializer_test.go index 377b500c2..c15ac2d8d 100644 --- a/pkg/webhook/admission/initializer/initializer_test.go +++ b/pkg/webhook/admission/initializer/initializer_test.go @@ -24,8 +24,6 @@ import ( admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authorization/authorizer" - "k8s.io/client-go/discovery" - "k8s.io/client-go/discovery/cached/memory" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" @@ -38,7 +36,7 @@ import ( // TestWantsFeature ensures that the feature gates are injected // when the WantsFeatures interface is implemented by a plugin. func TestWantsFeatures(t *testing.T) { - target := initializer.New(nil, nil, nil, featuregate.NewFeatureGate(), nil) + target := initializer.New(nil, nil, nil, featuregate.NewFeatureGate()) wantFeaturesAdmission := &WantsFeaturesAdmission{} target.Initialize(wantFeaturesAdmission) if wantFeaturesAdmission.features == nil { @@ -49,7 +47,7 @@ func TestWantsFeatures(t *testing.T) { // TestWantsAuthorizer ensures that the authorizer is injected // when the WantsAuthorizer interface is implemented by a plugin. func TestWantsAuthorizer(t *testing.T) { - target := initializer.New(nil, nil, &TestAuthorizer{}, nil, nil) + target := initializer.New(nil, nil, &TestAuthorizer{}, nil) wantAuthorizerAdmission := &WantAuthorizerAdmission{} target.Initialize(wantAuthorizerAdmission) if wantAuthorizerAdmission.auth == nil { @@ -61,7 +59,7 @@ func TestWantsAuthorizer(t *testing.T) { // when the WantsExternalKubeClientSet interface is implemented by a plugin. func TestWantsExternalKubeClientSet(t *testing.T) { cs := &fake.Clientset{} - target := initializer.New(cs, nil, &TestAuthorizer{}, nil, nil) + target := initializer.New(cs, nil, &TestAuthorizer{}, nil) wantExternalKubeClientSet := &WantExternalKubeClientSet{} target.Initialize(wantExternalKubeClientSet) if wantExternalKubeClientSet.cs != cs { @@ -74,7 +72,7 @@ func TestWantsExternalKubeClientSet(t *testing.T) { func TestWantsExternalKubeInformerFactory(t *testing.T) { cs := &fake.Clientset{} sf := informers.NewSharedInformerFactory(cs, time.Duration(1)*time.Second) - target := initializer.New(cs, sf, &TestAuthorizer{}, nil, nil) + target := initializer.New(cs, sf, &TestAuthorizer{}, nil) wantExternalKubeInformerFactory := &WantExternalKubeInformerFactory{} target.Initialize(wantExternalKubeInformerFactory) if wantExternalKubeInformerFactory.sf != sf { @@ -82,18 +80,6 @@ func TestWantsExternalKubeInformerFactory(t *testing.T) { } } -// TestWantsDiscoveryCache ensures that the discovery client is injected -// when the WantsDiscoveryCache interface is implemented by a plugin. -func TestWantsDiscoveryCache(t *testing.T) { - discoveryInterface := memory.NewMemCacheClient((&fake.Clientset{}).Discovery()) - target := initializer.New(nil, nil, nil, nil, discoveryInterface) - wantDiscoveryCache := &WantsDiscoveryCacheAdmission{} - target.Initialize(wantDiscoveryCache) - if wantDiscoveryCache.discoveryInterface != discoveryInterface { - t.Errorf("expected discovery cache to be initialized") - } -} - // WantExternalKubeInformerFactory is a test stub that fulfills the WantsExternalKubeInformerFactory interface type WantExternalKubeInformerFactory struct { sf informers.SharedInformerFactory @@ -166,21 +152,3 @@ func (self *WantsFeaturesAdmission) ValidateInitialization() error { retu var _ admission.Interface = &WantsFeaturesAdmission{} var _ initializer.WantsFeatures = &WantsFeaturesAdmission{} - -// TestDiscoveryCache is a test stub that fulfills the WantsDiscoveryCache interface. -type TestDiscoveryCache struct{} - -// WantsDiscoveryCacheAdmission is a test stub that fulfills the WantsFeatures interface. -type WantsDiscoveryCacheAdmission struct { - discoveryInterface discovery.CachedDiscoveryInterface -} - -func (self *WantsDiscoveryCacheAdmission) SetDiscoveryCache(discoveryInterface discovery.CachedDiscoveryInterface) { - self.discoveryInterface = discoveryInterface -} - -func (self *WantsDiscoveryCacheAdmission) Handles(o admissionv1.Operation) bool { return false } -func (self *WantsDiscoveryCacheAdmission) ValidateInitialization() error { return nil } - -var _ admission.Interface = &WantAuthorizerAdmission{} -var _ initializer.WantsDiscoveryCache = &WantsDiscoveryCacheAdmission{} diff --git a/pkg/webhook/admission/initializer/interfaces.go b/pkg/webhook/admission/initializer/interfaces.go index e43451e00..9515a4ec4 100644 --- a/pkg/webhook/admission/initializer/interfaces.go +++ b/pkg/webhook/admission/initializer/interfaces.go @@ -19,7 +19,6 @@ package initializer import ( "k8s.io/apiserver/pkg/authorization/authorizer" quota "k8s.io/apiserver/pkg/quota/v1" - "k8s.io/client-go/discovery" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/component-base/featuregate" @@ -51,12 +50,6 @@ type WantsQuotaConfiguration interface { admission.InitializationValidator } -// WantsDiscoveryCache defines a function which sets a discovery information cache for admission plugins that need it. -type WantsDiscoveryCache interface { - SetDiscoveryCache(discovery.CachedDiscoveryInterface) - admission.InitializationValidator -} - // WantsFeatures defines a function which passes the featureGates for inspection by an admission plugin. // Admission plugins should not hold a reference to the featureGates. Instead, they should query a particular one // and assign it to a simple bool in the admission plugin struct. diff --git a/pkg/webhook/admission/plugins_test.go b/pkg/webhook/admission/plugins_test.go index 180ba2fa2..92118159d 100644 --- a/pkg/webhook/admission/plugins_test.go +++ b/pkg/webhook/admission/plugins_test.go @@ -46,7 +46,7 @@ func TestPlugins_InitializesNamedOnly(t *testing.T) { }) // only initialize TestPlugin1 - _, err := p.NewFromPlugins([]string{"TestPlugin1"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil, nil)) + _, err := p.NewFromPlugins([]string{"TestPlugin1"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil)) if err != nil { t.Errorf("got unexpected error: %v", err) } @@ -75,7 +75,7 @@ func TestPlugins_FailsIfAnyPluginFails(t *testing.T) { }) // only initialize TestPlugin1 - _, err := p.NewFromPlugins([]string{"TestPlugin1", "TestPlugin2"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil, nil)) + _, err := p.NewFromPlugins([]string{"TestPlugin1", "TestPlugin2"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil)) if err == nil { t.Errorf("expected an error but got none") } @@ -97,7 +97,7 @@ func TestPlugins_FailsNonExistingPlugin(t *testing.T) { }) // only initialize TestPlugin1 - _, err := p.NewFromPlugins([]string{"TestPlugin1", "TestPluginDoesNotExist"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil, nil)) + _, err := p.NewFromPlugins([]string{"TestPlugin1", "TestPluginDoesNotExist"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil)) if err == nil { t.Errorf("expected an error but got none") } @@ -116,7 +116,7 @@ func TestPlugins_FailsIfPluginFailsToBuild(t *testing.T) { }) // only initialize TestPlugin1 - _, err := p.NewFromPlugins([]string{"TestPlugin1"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil, nil)) + _, err := p.NewFromPlugins([]string{"TestPlugin1"}, initializer.New(fake.NewSimpleClientset(), nil, nil, nil)) if err == nil { t.Errorf("expected an error but got none") }