Merge pull request #56660 from hzxuzhonghu/recommened-admission

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>.

add admission into RecommendedOptions

**What this PR does / why we need it**:

make admission part of the RecommendedOptions

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #56627

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

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

@@ -98,7 +98,7 @@ func (o CustomResourceDefinitionsServerOptions) Config() (*apiserver.Config, err
 	}
 
 	serverConfig := genericapiserver.NewRecommendedConfig(apiserver.Codecs)
-	if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil {
+	if err := o.RecommendedOptions.ApplyTo(serverConfig, apiserver.Scheme); err != nil {
 		return nil, err
 	}
 

staging/src/k8s.io/apiextensions-apiserver/test/integration/testserver/start.go

@@ -45,6 +45,7 @@ func DefaultServerConfig() (*extensionsapiserver.Config, error) {
 	options.RecommendedOptions.SecureServing.BindPort = port
 	options.RecommendedOptions.Authentication = nil // disable
 	options.RecommendedOptions.Authorization = nil  // disable
+	options.RecommendedOptions.Admission = nil      // disable
 	options.RecommendedOptions.SecureServing.BindAddress = net.ParseIP("127.0.0.1")
 	etcdURL, ok := os.LookupEnv("KUBE_INTEGRATION_ETCD_URL")
 	if !ok {
@@ -58,7 +59,7 @@ func DefaultServerConfig() (*extensionsapiserver.Config, error) {
 	if err := options.RecommendedOptions.SecureServing.MaybeDefaultWithSelfSignedCerts("localhost", nil, []net.IP{net.ParseIP("127.0.0.1")}); err != nil {
 		return nil, fmt.Errorf("error creating self-signed certificates: %v", err)
 	}
-	if err := options.RecommendedOptions.ApplyTo(genericConfig); err != nil {
+	if err := options.RecommendedOptions.ApplyTo(genericConfig, nil); err != nil {
 		return nil, err
 	}
 

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

@@ -101,6 +101,20 @@ func (a *AdmissionOptions) ApplyTo(
 	scheme *runtime.Scheme,
 	pluginInitializers ...admission.PluginInitializer,
 ) error {
+	if a == nil {
+		return nil
+	}
+
+	// Admission need scheme to construct admission initializer.
+	if scheme == nil {
+		return fmt.Errorf("admission depends on a scheme, it cannot be nil")
+	}
+
+	// Admission depends on CoreAPI to set SharedInformerFactory and ClientConfig.
+	if informers == nil {
+		return fmt.Errorf("admission depends on a Kubernetes core API shared informer, it cannot be nil")
+	}
+
 	pluginNames := a.PluginNames
 	if len(a.PluginNames) == 0 {
 		pluginNames = a.enabledPluginNames()

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

@@ -20,6 +20,7 @@ import (
 	"github.com/spf13/pflag"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/server"
 	"k8s.io/apiserver/pkg/storage/storagebackend"
 )
@@ -35,6 +36,11 @@ type RecommendedOptions struct {
 	Audit          *AuditOptions
 	Features       *FeatureOptions
 	CoreAPI        *CoreAPIOptions
+
+	// ExtraAdmissionInitializers is called once after all ApplyTo from the options above, to pass the returned
+	// admission plugin initializers to Admission.ApplyTo.
+	ExtraAdmissionInitializers func(c *server.RecommendedConfig) ([]admission.PluginInitializer, error)
+	Admission                  *AdmissionOptions
 }
 
 func NewRecommendedOptions(prefix string, codec runtime.Codec) *RecommendedOptions {
@@ -46,6 +52,8 @@ func NewRecommendedOptions(prefix string, codec runtime.Codec) *RecommendedOptio
 		Audit:                      NewAuditOptions(),
 		Features:                   NewFeatureOptions(),
 		CoreAPI:                    NewCoreAPIOptions(),
+		ExtraAdmissionInitializers: func(c *server.RecommendedConfig) ([]admission.PluginInitializer, error) { return nil, nil },
+		Admission:                  NewAdmissionOptions(),
 	}
 }
 
@@ -57,9 +65,13 @@ func (o *RecommendedOptions) AddFlags(fs *pflag.FlagSet) {
 	o.Audit.AddFlags(fs)
 	o.Features.AddFlags(fs)
 	o.CoreAPI.AddFlags(fs)
+	o.Admission.AddFlags(fs)
 }
 
-func (o *RecommendedOptions) ApplyTo(config *server.RecommendedConfig) error {
+// ApplyTo adds RecommendedOptions to the server configuration.
+// scheme is the scheme of the apiserver types that are sent to the admission chain.
+// pluginInitializers can be empty, it is only need for additional initializers.
+func (o *RecommendedOptions) ApplyTo(config *server.RecommendedConfig, scheme *runtime.Scheme) error {
 	if err := o.Etcd.ApplyTo(&config.Config); err != nil {
 		return err
 	}
@@ -81,6 +93,12 @@ func (o *RecommendedOptions) ApplyTo(config *server.RecommendedConfig) error {
 	if err := o.CoreAPI.ApplyTo(config); err != nil {
 		return err
 	}
+	if initializers, err := o.ExtraAdmissionInitializers(config); err != nil {
+		return err
+	} else if err := o.Admission.ApplyTo(&config.Config, config.SharedInformerFactory, config.ClientConfig, scheme, initializers...); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -93,6 +111,7 @@ func (o *RecommendedOptions) Validate() []error {
 	errors = append(errors, o.Audit.Validate()...)
 	errors = append(errors, o.Features.Validate()...)
 	errors = append(errors, o.CoreAPI.Validate()...)
+	errors = append(errors, o.Admission.Validate()...)
 
 	return errors
 }

staging/src/k8s.io/kube-aggregator/pkg/cmd/server/start.go

@@ -109,7 +109,7 @@ func (o AggregatorOptions) RunAggregator(stopCh <-chan struct{}) error {
 
 	serverConfig := genericapiserver.NewRecommendedConfig(apiserver.Codecs)
 
-	if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil {
+	if err := o.RecommendedOptions.ApplyTo(serverConfig, apiserver.Scheme); err != nil {
 		return err
 	}
 	serverConfig.LongRunningFunc = filters.BasicLongRunningRequestCheck(

staging/src/k8s.io/sample-apiserver/pkg/admission/plugin/banflunder/admission_test.go

@@ -113,10 +113,7 @@ func TestBanflunderAdmissionPlugin(t *testing.T) {
 				t.Fatalf("scenario %d: failed to create banflunder admission plugin due to = %v", index, err)
 			}
 
-			targetInitializer, err := wardleinitializer.New(informersFactory)
+			targetInitializer := wardleinitializer.New(informersFactory)
-			if err != nil {
-				t.Fatalf("scenario %d: failed to crate wardle plugin initializer due to = %v", index, err)
-			}
 			targetInitializer.Initialize(target)
 
 			err = admission.ValidateInitialization(target)

staging/src/k8s.io/sample-apiserver/pkg/admission/wardleinitializer/wardleinitializer.go

@@ -28,10 +28,10 @@ type pluginInitializer struct {
 var _ admission.PluginInitializer = pluginInitializer{}
 
 // New creates an instance of wardle admission plugins initializer.
-func New(informers informers.SharedInformerFactory) (pluginInitializer, error) {
+func New(informers informers.SharedInformerFactory) pluginInitializer {
 	return pluginInitializer{
 		informers: informers,
-	}, nil
+	}
 }
 
 // Initialize checks the initialization interfaces implemented by a plugin

staging/src/k8s.io/sample-apiserver/pkg/admission/wardleinitializer/wardleinitializer_test.go

@@ -31,10 +31,8 @@ import (
 func TestWantsInternalWardleInformerFactory(t *testing.T) {
 	cs := &fake.Clientset{}
 	sf := informers.NewSharedInformerFactory(cs, time.Duration(1)*time.Second)
-	target, err := wardleinitializer.New(sf)
+	target := wardleinitializer.New(sf)
-	if err != nil {
+
-		t.Fatalf("expected to create an instance of initializer but got an error = %s", err.Error())
-	}
 	wantWardleInformerFactory := &wantInternalWardleInformerFactory{}
 	target.Initialize(wantWardleInformerFactory)
 	if wantWardleInformerFactory.sf != sf {

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

@@ -12,6 +12,7 @@ go_library(
     deps = [
         "//vendor/github.com/spf13/cobra:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
+        "//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/server:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/server/options:go_default_library",
         "//vendor/k8s.io/sample-apiserver/pkg/admission/plugin/banflunder:go_default_library",

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

@@ -24,6 +24,7 @@ import (
 	"github.com/spf13/cobra"
 
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apiserver/pkg/admission"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	genericoptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/sample-apiserver/pkg/admission/plugin/banflunder"
@@ -38,8 +39,8 @@ const defaultEtcdPathPrefix = "/registry/wardle.kubernetes.io"
 
 type WardleServerOptions struct {
 	RecommendedOptions *genericoptions.RecommendedOptions
-	Admission          *genericoptions.AdmissionOptions
 
+	SharedInformerFactory informers.SharedInformerFactory
 	StdOut                io.Writer
 	StdErr                io.Writer
 }
@@ -47,7 +48,6 @@ type WardleServerOptions struct {
 func NewWardleServerOptions(out, errOut io.Writer) *WardleServerOptions {
 	o := &WardleServerOptions{
 		RecommendedOptions: genericoptions.NewRecommendedOptions(defaultEtcdPathPrefix, apiserver.Codecs.LegacyCodec(v1alpha1.SchemeGroupVersion)),
-		Admission:          genericoptions.NewAdmissionOptions(),
 
 		StdOut: out,
 		StdErr: errOut,
@@ -79,7 +79,6 @@ func NewCommandStartWardleServer(out, errOut io.Writer, stopCh <-chan struct{})
 
 	flags := cmd.Flags()
 	o.RecommendedOptions.AddFlags(flags)
-	o.Admission.AddFlags(flags)
 
 	return cmd
 }
@@ -87,7 +86,6 @@ func NewCommandStartWardleServer(out, errOut io.Writer, stopCh <-chan struct{})
 func (o WardleServerOptions) Validate(args []string) error {
 	errors := []error{}
 	errors = append(errors, o.RecommendedOptions.Validate()...)
-	errors = append(errors, o.Admission.Validate()...)
 	return utilerrors.NewAggregate(errors)
 }
 
@@ -95,31 +93,27 @@ func (o *WardleServerOptions) Complete() error {
 	return nil
 }
 
-func (o WardleServerOptions) Config() (*apiserver.Config, error) {
+func (o *WardleServerOptions) Config() (*apiserver.Config, error) {
 	// register admission plugins
-	banflunder.Register(o.Admission.Plugins)
+	banflunder.Register(o.RecommendedOptions.Admission.Plugins)
 
 	// TODO have a "real" external address
 	if err := o.RecommendedOptions.SecureServing.MaybeDefaultWithSelfSignedCerts("localhost", nil, []net.IP{net.ParseIP("127.0.0.1")}); err != nil {
 		return nil, fmt.Errorf("error creating self-signed certificates: %v", err)
 	}
 
+	o.RecommendedOptions.ExtraAdmissionInitializers = func(c *genericapiserver.RecommendedConfig) ([]admission.PluginInitializer, error) {
+		client, err := clientset.NewForConfig(c.LoopbackClientConfig)
+		if err != nil {
+			return nil, err
+		}
+		informerFactory := informers.NewSharedInformerFactory(client, c.LoopbackClientConfig.Timeout)
+		o.SharedInformerFactory = informerFactory
+		return []admission.PluginInitializer{wardleinitializer.New(informerFactory)}, nil
+	}
+
 	serverConfig := genericapiserver.NewRecommendedConfig(apiserver.Codecs)
-	if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil {
+	if err := o.RecommendedOptions.ApplyTo(serverConfig, apiserver.Scheme); err != nil {
-		return nil, err
-	}
-
-	client, err := clientset.NewForConfig(serverConfig.LoopbackClientConfig)
-	if err != nil {
-		return nil, err
-	}
-	informerFactory := informers.NewSharedInformerFactory(client, serverConfig.LoopbackClientConfig.Timeout)
-	admissionInitializer, err := wardleinitializer.New(informerFactory)
-	if err != nil {
-		return nil, err
-	}
-
-	if err := o.Admission.ApplyTo(&serverConfig.Config, serverConfig.SharedInformerFactory, serverConfig.ClientConfig, apiserver.Scheme, admissionInitializer); err != nil {
 		return nil, err
 	}