Merge pull request #53394 from p0lyn0mial/cleanup_kubeapi_admission_initializer

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

removes Authorizer and ExternalClientSet from kubeapiserver's admissi…

**What this PR does / why we need it**:
removes Authorizer and ExternalClientSet from kubeapiserver's admission initializer.


**Release note**:

```
NONE
```

cmd/kube-apiserver/app/server.go

@@ -57,7 +57,6 @@ import (
 	"k8s.io/apiserver/pkg/storage/etcd3/preflight"
 	clientgoinformers "k8s.io/client-go/informers"
 	clientgoclientset "k8s.io/client-go/kubernetes"
-	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/cmd/kube-apiserver/app/options"
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/apps"
@@ -417,13 +416,10 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp
 		// TODO: get rid of KUBE_API_VERSIONS or define sane behaviour if set
 		glog.Errorf("Failed to create clientset with KUBE_API_VERSIONS=%q. KUBE_API_VERSIONS is only for testing. Things will break.", kubeAPIVersions)
 	}
-	externalClient, err := clientset.NewForConfig(genericConfig.LoopbackClientConfig)
-	if err != nil {
-		return nil, nil, nil, nil, nil, fmt.Errorf("failed to create external clientset: %v", err)
-	}
-	sharedInformers := informers.NewSharedInformerFactory(client, 10*time.Minute)
 
-	clientgoExternalClient, err := clientgoclientset.NewForConfig(genericConfig.LoopbackClientConfig)
+	kubeClientConfig := genericConfig.LoopbackClientConfig
+	sharedInformers := informers.NewSharedInformerFactory(client, 10*time.Minute)
+	clientgoExternalClient, err := clientgoclientset.NewForConfig(kubeClientConfig)
 	if err != nil {
 		return nil, nil, nil, nil, nil, fmt.Errorf("failed to create real external clientset: %v", err)
 	}
@@ -457,9 +453,7 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp
 	pluginInitializer, err := BuildAdmissionPluginInitializer(
 		s,
 		client,
-		externalClient,
 		sharedInformers,
-		genericConfig.Authorizer,
 		serviceResolver,
 		proxyTransport,
 	)
@@ -489,6 +483,7 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp
 		versionedInformers,
 		certBytes,
 		keyBytes,
+		kubeClientConfig,
 		pluginInitializer)
 	if err != nil {
 		return nil, nil, nil, nil, nil, fmt.Errorf("failed to initialize admission: %v", err)
@@ -497,7 +492,7 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp
 }
 
 // BuildAdmissionPluginInitializer constructs the admission plugin initializer
-func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client internalclientset.Interface, externalClient clientset.Interface, sharedInformers informers.SharedInformerFactory, apiAuthorizer authorizer.Authorizer, serviceResolver aggregatorapiserver.ServiceResolver, proxyTransport *http.Transport) (admission.PluginInitializer, error) {
+func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client internalclientset.Interface, sharedInformers informers.SharedInformerFactory, serviceResolver aggregatorapiserver.ServiceResolver, proxyTransport *http.Transport) (admission.PluginInitializer, error) {
 	var cloudConfig []byte
 
 	if s.CloudProvider.CloudConfigFile != "" {
@@ -515,7 +510,7 @@ func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client interna
 	// do not require us to open watches for all items tracked by quota.
 	quotaRegistry := quotainstall.NewRegistry(nil, nil)
 
-	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, externalClient, sharedInformers, apiAuthorizer, cloudConfig, restMapper, quotaRegistry)
+	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, cloudConfig, restMapper, quotaRegistry)
 
 	pluginInitializer = pluginInitializer.SetServiceResolver(serviceResolver)
 	pluginInitializer = pluginInitializer.SetProxyTransport(proxyTransport)

federation/cmd/federation-apiserver/app/server.go

@@ -42,7 +42,6 @@ import (
 	serverstorage "k8s.io/apiserver/pkg/server/storage"
 	clientgoinformers "k8s.io/client-go/informers"
 	clientgoclientset "k8s.io/client-go/kubernetes"
-	clientset "k8s.io/client-go/kubernetes"
 	openapicommon "k8s.io/kube-openapi/pkg/common"
 	federationv1beta1 "k8s.io/kubernetes/federation/apis/federation/v1beta1"
 	"k8s.io/kubernetes/federation/cmd/federation-apiserver/app/options"
@@ -181,17 +180,14 @@ func NonBlockingRun(s *options.ServerRunOptions, stopCh <-chan struct{}) error {
 		return fmt.Errorf("invalid Authentication Config: %v", err)
 	}
 
-	client, err := internalclientset.NewForConfig(genericConfig.LoopbackClientConfig)
+	kubeClientConfig := genericConfig.LoopbackClientConfig
+	client, err := internalclientset.NewForConfig(kubeClientConfig)
 	if err != nil {
 		return fmt.Errorf("failed to create clientset: %v", err)
 	}
-	externalClient, err := clientset.NewForConfig(genericConfig.LoopbackClientConfig)
-	if err != nil {
-		return fmt.Errorf("failed to create external clientset: %v", err)
-	}
-	sharedInformers := informers.NewSharedInformerFactory(client, 10*time.Minute)
 
-	clientgoExternalClient, err := clientgoclientset.NewForConfig(genericConfig.LoopbackClientConfig)
+	sharedInformers := informers.NewSharedInformerFactory(client, 10*time.Minute)
+	clientgoExternalClient, err := clientgoclientset.NewForConfig(kubeClientConfig)
 	if err != nil {
 		return fmt.Errorf("failed to create real external clientset: %v", err)
 	}
@@ -214,13 +210,14 @@ func NonBlockingRun(s *options.ServerRunOptions, stopCh <-chan struct{}) error {
 	// NOTE: we do not provide informers to the quota registry because admission level decisions
 	// do not require us to open watches for all items tracked by quota.
 	quotaRegistry := quotainstall.NewRegistry(nil, nil)
-	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, externalClient, sharedInformers, apiAuthorizer, cloudConfig, nil, quotaRegistry)
+	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, cloudConfig, nil, quotaRegistry)
 
 	err = s.Admission.ApplyTo(
 		genericConfig,
 		versionedInformers,
 		nil,
 		nil,
+		kubeClientConfig,
 		pluginInitializer,
 	)
 	if err != nil {

pkg/kubeapiserver/admission/BUILD

@@ -10,11 +10,7 @@ go_test(
     name = "go_default_test",
     srcs = ["init_test.go"],
     library = ":go_default_library",
-    deps = [
+    deps = ["//vendor/k8s.io/apiserver/pkg/admission:go_default_library"],
-        "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
-        "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
-        "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
-    ],
 )
 
 go_library(
@@ -26,7 +22,6 @@ go_library(
         "//pkg/quota:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
-        "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
     ],

pkg/kubeapiserver/admission/init_test.go

@@ -21,50 +21,14 @@ import (
 	"testing"
 
 	"k8s.io/apiserver/pkg/admission"
-	genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer"
-	"k8s.io/apiserver/pkg/authorization/authorizer"
 )
 
-// TestAuthorizer is a testing struct for testing that fulfills the authorizer interface.
-type TestAuthorizer struct{}
-
-func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
-	return false, "", nil
-}
-
-var _ authorizer.Authorizer = &TestAuthorizer{}
-
 type doNothingAdmission struct{}
 
 func (doNothingAdmission) Admit(a admission.Attributes) error { return nil }
 func (doNothingAdmission) Handles(o admission.Operation) bool { return false }
 func (doNothingAdmission) Validate() error                    { return nil }
 
-// WantAuthorizerAdmission is a testing struct that fulfills the WantsAuthorizer
-// interface.
-type WantAuthorizerAdmission struct {
-	doNothingAdmission
-	auth authorizer.Authorizer
-}
-
-func (self *WantAuthorizerAdmission) SetAuthorizer(a authorizer.Authorizer) {
-	self.auth = a
-}
-
-var _ admission.Interface = &WantAuthorizerAdmission{}
-var _ genericadmissioninit.WantsAuthorizer = &WantAuthorizerAdmission{}
-
-// TestWantsAuthorizer ensures that the authorizer is injected when the WantsAuthorizer
-// interface is implemented.
-func TestWantsAuthorizer(t *testing.T) {
-	initializer := NewPluginInitializer(nil, nil, nil, &TestAuthorizer{}, nil, nil, nil)
-	wantAuthorizerAdmission := &WantAuthorizerAdmission{}
-	initializer.Initialize(wantAuthorizerAdmission)
-	if wantAuthorizerAdmission.auth == nil {
-		t.Errorf("expected authorizer to be initialized but found nil")
-	}
-}
-
 type WantsCloudConfigAdmissionPlugin struct {
 	doNothingAdmission
 	cloudConfig []byte
@@ -76,7 +40,7 @@ func (self *WantsCloudConfigAdmissionPlugin) SetCloudConfig(cloudConfig []byte)
 
 func TestCloudConfigAdmissionPlugin(t *testing.T) {
 	cloudConfig := []byte("cloud-configuration")
-	initializer := NewPluginInitializer(nil, nil, nil, &TestAuthorizer{}, cloudConfig, nil, nil)
+	initializer := NewPluginInitializer(nil, nil, cloudConfig, nil, nil)
 	wantsCloudConfigAdmission := &WantsCloudConfigAdmissionPlugin{}
 	initializer.Initialize(wantsCloudConfigAdmission)
 

pkg/kubeapiserver/admission/initializer.go

@@ -22,7 +22,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apiserver/pkg/admission"
-	admissioninit "k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
@@ -99,18 +98,14 @@ var _ admission.PluginInitializer = &PluginInitializer{}
 // all public, this construction method is pointless boilerplate.
 func NewPluginInitializer(
 	internalClient internalclientset.Interface,
-	externalClient clientset.Interface,
 	sharedInformers informers.SharedInformerFactory,
-	authz authorizer.Authorizer,
 	cloudConfig []byte,
 	restMapper meta.RESTMapper,
 	quotaRegistry quota.Registry,
 ) *PluginInitializer {
 	return &PluginInitializer{
 		internalClient: internalClient,
-		externalClient: externalClient,
 		informers:      sharedInformers,
-		authorizer:     authz,
 		cloudConfig:    cloudConfig,
 		restMapper:     restMapper,
 		quotaRegistry:  quotaRegistry,
@@ -136,18 +131,10 @@ func (i *PluginInitializer) Initialize(plugin admission.Interface) {
 		wants.SetInternalKubeClientSet(i.internalClient)
 	}
 
-	if wants, ok := plugin.(admissioninit.WantsExternalKubeClientSet); ok {
-		wants.SetExternalKubeClientSet(i.externalClient)
-	}
-
 	if wants, ok := plugin.(WantsInternalKubeInformerFactory); ok {
 		wants.SetInternalKubeInformerFactory(i.informers)
 	}
 
-	if wants, ok := plugin.(admissioninit.WantsAuthorizer); ok {
-		wants.SetAuthorizer(i.authorizer)
-	}
-
 	if wants, ok := plugin.(WantsCloudConfig); ok {
 		wants.SetCloudConfig(i.cloudConfig)
 	}

plugin/pkg/admission/gc/BUILD

@@ -32,6 +32,7 @@ go_test(
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
     ],

plugin/pkg/admission/gc/gc_admission_test.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/kubernetes/pkg/api"
@@ -69,7 +70,7 @@ func (fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
 }
 
 // newGCPermissionsEnforcement returns the admission controller configured for testing.
-func newGCPermissionsEnforcement() *gcPermissionsEnforcement {
+func newGCPermissionsEnforcement() (*gcPermissionsEnforcement, error) {
 	// the pods/status endpoint is ignored by this plugin since old kubelets
 	// corrupt them.  the pod status strategy ensures status updates cannot mutate
 	// ownerRef.
@@ -83,9 +84,18 @@ func newGCPermissionsEnforcement() *gcPermissionsEnforcement {
 		Handler:   admission.NewHandler(admission.Create, admission.Update),
 		whiteList: whiteList,
 	}
-	pluginInitializer := kubeadmission.NewPluginInitializer(nil, nil, nil, fakeAuthorizer{}, nil, api.Registry.RESTMapper(), nil)
+
-	pluginInitializer.Initialize(gcAdmit)
+	genericPluginInitializer, err := initializer.New(nil, nil, fakeAuthorizer{}, nil, nil)
-	return gcAdmit
+	if err != nil {
+		return nil, err
+	}
+	pluginInitializer := kubeadmission.NewPluginInitializer(nil, nil, nil, api.Registry.RESTMapper(), nil)
+	initializersChain := admission.PluginInitializers{}
+	initializersChain = append(initializersChain, genericPluginInitializer)
+	initializersChain = append(initializersChain, pluginInitializer)
+
+	initializersChain.Initialize(gcAdmit)
+	return gcAdmit, nil
 }
 
 func TestGCAdmission(t *testing.T) {
@@ -245,7 +255,10 @@ func TestGCAdmission(t *testing.T) {
 			checkError: expectNoError,
 		},
 	}
-	gcAdmit := newGCPermissionsEnforcement()
+	gcAdmit, err := newGCPermissionsEnforcement()
+	if err != nil {
+		t.Error(err)
+	}
 
 	for _, tc := range tests {
 		operation := admission.Create
@@ -490,7 +503,10 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) {
 			checkError: expectNoError,
 		},
 	}
-	gcAdmit := newGCPermissionsEnforcement()
+	gcAdmit, err := newGCPermissionsEnforcement()
+	if err != nil {
+		t.Error(err)
+	}
 
 	for _, tc := range tests {
 		operation := admission.Create

plugin/pkg/admission/limitranger/admission_test.go

@@ -744,7 +744,7 @@ func newHandlerForTest(c clientset.Interface) (admission.Interface, informers.Sh
 	if err != nil {
 		return nil, f, err
 	}
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, nil, f, nil, nil, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err = admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/namespace/autoprovision/admission_test.go

@@ -38,7 +38,7 @@ import (
 func newHandlerForTest(c clientset.Interface) (admission.Interface, informers.SharedInformerFactory, error) {
 	f := informers.NewSharedInformerFactory(c, 5*time.Minute)
 	handler := NewProvision()
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, nil, f, nil, nil, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err := admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/namespace/exists/admission_test.go

@@ -37,7 +37,7 @@ import (
 func newHandlerForTest(c clientset.Interface) (admission.Interface, informers.SharedInformerFactory, error) {
 	f := informers.NewSharedInformerFactory(c, 5*time.Minute)
 	handler := NewExists()
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, nil, f, nil, nil, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err := admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/podnodeselector/admission_test.go

@@ -241,7 +241,7 @@ func TestIgnoreUpdatingInitializedPod(t *testing.T) {
 func newHandlerForTest(c clientset.Interface) (*podNodeSelector, informers.SharedInformerFactory, error) {
 	f := informers.NewSharedInformerFactory(c, 5*time.Minute)
 	handler := NewPodNodeSelector(nil)
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, nil, f, nil, nil, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err := admission.Validate(handler)
 	return handler, f, err

plugin/pkg/admission/podtolerationrestriction/admission_test.go

@@ -342,7 +342,7 @@ func newHandlerForTest(c clientset.Interface) (*podTolerationsPlugin, informers.
 		return nil, nil, err
 	}
 	handler := NewPodTolerationsPlugin(pluginConfig)
-	pluginInitializer := kubeadmission.NewPluginInitializer(c, nil, f, nil, nil, nil, nil)
+	pluginInitializer := kubeadmission.NewPluginInitializer(c, f, nil, nil, nil)
 	pluginInitializer.Initialize(handler)
 	err = admission.Validate(handler)
 	return handler, f, err

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -192,6 +192,11 @@ type RecommendedConfig struct {
 	// RecommendedOptions.CoreAPI.ApplyTo called by RecommendedOptions.ApplyTo. It uses an in-cluster client config
 	// by default, or the kubeconfig given with kubeconfig command line flag.
 	SharedInformerFactory informers.SharedInformerFactory
+
+	// ClientConfig holds the kubernetes client configuration.
+	// This value is set by RecommendedOptions.CoreAPI.ApplyTo called by RecommendedOptions.ApplyTo.
+	// By default in-cluster client config is used.
+	ClientConfig *restclient.Config
 }
 
 type SecureServingInfo struct {

staging/src/k8s.io/apiserver/pkg/server/options/admission.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/apiserver/pkg/server"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 )
 
 // AdmissionOptions holds the admission options
@@ -72,13 +73,13 @@ func (a *AdmissionOptions) AddFlags(fs *pflag.FlagSet) {
 // In case admission plugin names were not provided by a custer-admin they will be prepared from the recommended/default values.
 // In addition the method lazily initializes a generic plugin that is appended to the list of pluginInitializers
 // note this method uses:
-//  genericconfig.LoopbackClientConfig
 //  genericconfig.Authorizer
 func (a *AdmissionOptions) ApplyTo(
 	c *server.Config,
 	informers informers.SharedInformerFactory,
 	serverIdentifyingClientCert []byte,
 	serverIdentifyingClientKey []byte,
+	clientConfig *rest.Config,
 	pluginInitializers ...admission.PluginInitializer,
 ) error {
 	pluginNames := a.PluginNames
@@ -91,7 +92,7 @@ func (a *AdmissionOptions) ApplyTo(
 		return fmt.Errorf("failed to read plugin config: %v", err)
 	}
 
-	clientset, err := kubernetes.NewForConfig(c.LoopbackClientConfig)
+	clientset, err := kubernetes.NewForConfig(clientConfig)
 	if err != nil {
 		return err
 	}

staging/src/k8s.io/apiserver/pkg/server/options/coreapi.go

@@ -73,6 +73,7 @@ func (o *CoreAPIOptions) ApplyTo(config *server.RecommendedConfig) error {
 	if err != nil {
 		return fmt.Errorf("failed to create Kubernetes clientset: %v", err)
 	}
+	config.ClientConfig = kubeconfig
 	config.SharedInformerFactory = clientgoinformers.NewSharedInformerFactory(clientgoExternalClient, 10*time.Minute)
 
 	return nil

staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go

@@ -119,7 +119,7 @@ func (o WardleServerOptions) Config() (*apiserver.Config, error) {
 		return nil, err
 	}
 
-	if err := o.Admission.ApplyTo(&serverConfig.Config, serverConfig.SharedInformerFactory, nil, nil, admissionInitializer); err != nil {
+	if err := o.Admission.ApplyTo(&serverConfig.Config, serverConfig.SharedInformerFactory, nil, nil, serverConfig.ClientConfig, admissionInitializer); err != nil {
 		return nil, err
 	}