Merge pull request #16682 from deads2k/server-47-collapse-init

Automatic merge from submit-queue (batch tested with PRs 16545, 16684, 16643, 16459, 16682).

 use the upstream admission plugin construction for most plugins

We still have four special ones that really need to take config.  That may or may not happen in 3.7

@aveshagarwal you can start to see the pay-off here.  This has exposed some debt-y config problems where the controller configuration was being built from the admission config.

/assign soltysh
/assign mfojtik

@mfojtik This collapses our admission path and eliminates drift for 3.7.  We probably need to fix the build controller config problem (spoke with @bparees already) for 3.7.

Author: openshift-merge-robot

pkg/assets/apiserver/asset_apiserver.go

@@ -32,7 +32,6 @@ import (
 	"github.com/openshift/origin/pkg/cmd/server/crypto"
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 	oauthutil "github.com/openshift/origin/pkg/oauth/util"
-	clusterresourceoverrideapi "github.com/openshift/origin/pkg/quota/admission/clusterresourceoverride/api"
 	"github.com/openshift/origin/pkg/util/httprequest"
 	oversion "github.com/openshift/origin/pkg/version"
 )
@@ -44,8 +43,7 @@ const (
 type AssetServerConfig struct {
 	GenericConfig *genericapiserver.Config
 
-	Options               oapi.AssetConfig
-	LimitRequestOverrides *clusterresourceoverrideapi.ClusterResourceOverrideConfig
+	Options oapi.AssetConfig
 
 	PublicURL url.URL
 }
@@ -218,20 +216,19 @@ func (c *completedAssetServerConfig) addWebConsoleConfig(serverMux *genericmux.P
 
 	// Generated web console config and server version
 	config := assets.WebConsoleConfig{
-		APIGroupAddr:          masterURL.Host,
-		APIGroupPrefix:        server.APIGroupPrefix,
-		MasterAddr:            masterURL.Host,
-		MasterPrefix:          api.Prefix,
-		KubernetesAddr:        masterURL.Host,
-		KubernetesPrefix:      server.DefaultLegacyAPIPrefix,
-		OAuthAuthorizeURI:     oauthutil.OpenShiftOAuthAuthorizeURL(masterURL.String()),
-		OAuthTokenURI:         oauthutil.OpenShiftOAuthTokenURL(masterURL.String()),
-		OAuthRedirectBase:     c.Options.PublicURL,
-		OAuthClientID:         OpenShiftWebConsoleClientID,
-		LogoutURI:             c.Options.LogoutURL,
-		LoggingURL:            c.Options.LoggingPublicURL,
-		MetricsURL:            c.Options.MetricsPublicURL,
-		LimitRequestOverrides: c.LimitRequestOverrides,
+		APIGroupAddr:      masterURL.Host,
+		APIGroupPrefix:    server.APIGroupPrefix,
+		MasterAddr:        masterURL.Host,
+		MasterPrefix:      api.Prefix,
+		KubernetesAddr:    masterURL.Host,
+		KubernetesPrefix:  server.DefaultLegacyAPIPrefix,
+		OAuthAuthorizeURI: oauthutil.OpenShiftOAuthAuthorizeURL(masterURL.String()),
+		OAuthTokenURI:     oauthutil.OpenShiftOAuthTokenURL(masterURL.String()),
+		OAuthRedirectBase: c.Options.PublicURL,
+		OAuthClientID:     OpenShiftWebConsoleClientID,
+		LogoutURI:         c.Options.LogoutURL,
+		LoggingURL:        c.Options.LoggingPublicURL,
+		MetricsURL:        c.Options.MetricsPublicURL,
 	}
 	kVersionInfo := kversion.Get()
 	oVersionInfo := oversion.Get()

pkg/cmd/server/api/install/install.go

@@ -5,6 +5,8 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/apis/apiserver"
+	apiserverv1alpha1 "k8s.io/apiserver/pkg/apis/apiserver/v1alpha1"
 	"k8s.io/apiserver/pkg/apis/audit"
 	auditv1alpha1 "k8s.io/apiserver/pkg/apis/audit/v1alpha1"
 
@@ -33,6 +35,8 @@ func init() {
 	// policy file inside master-config.yaml
 	audit.AddToScheme(configapi.Scheme)
 	auditv1alpha1.AddToScheme(configapi.Scheme)
+	apiserver.AddToScheme(configapi.Scheme)
+	apiserverv1alpha1.AddToScheme(configapi.Scheme)
 }
 
 func interfacesFor(version schema.GroupVersion) (*meta.VersionInterfaces, error) {

pkg/cmd/server/api/latest/helpers.go

@@ -11,7 +11,9 @@ import (
 	"github.com/ghodss/yaml"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	kyaml "k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/apiserver/pkg/apis/apiserver"
 
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
 )
@@ -158,3 +160,31 @@ func IsAdmissionPluginActivated(reader io.Reader, defaultValue bool) (bool, erro
 
 	return !activationConfig.Disable, nil
 }
+
+func ConvertOpenshiftAdmissionConfigToKubeAdmissionConfig(in map[string]configapi.AdmissionPluginConfig) (*apiserver.AdmissionConfiguration, error) {
+	ret := &apiserver.AdmissionConfiguration{}
+
+	for _, pluginName := range sets.StringKeySet(in).List() {
+		openshiftConfig := in[pluginName]
+
+		fmt.Printf("#### adding for %T\n", openshiftConfig.Configuration)
+		kubeConfig := apiserver.AdmissionPluginConfiguration{
+			Name: pluginName,
+			Path: openshiftConfig.Location,
+		}
+
+		if openshiftConfig.Configuration != nil {
+			configBytes, err := runtime.Encode(Codec, openshiftConfig.Configuration)
+			if err != nil {
+				return nil, err
+			}
+			kubeConfig.Configuration = &runtime.Unknown{
+				Raw: configBytes,
+			}
+		}
+
+		ret.Plugins = append(ret.Plugins, kubeConfig)
+	}
+
+	return ret, nil
+}

pkg/cmd/server/api/latest/latest.go

@@ -22,4 +22,8 @@ var OldestVersion = schema.GroupVersion{Group: "", Version: "v1"}
 // with a set of versions to choose.
 var Versions = []schema.GroupVersion{{Group: "", Version: "v1"}}
 
-var Codec = serializer.NewCodecFactory(configapi.Scheme).LegacyCodec(schema.GroupVersion{Group: "", Version: "v1"})
+var Codec = serializer.NewCodecFactory(configapi.Scheme).LegacyCodec(
+	schema.GroupVersion{Group: "", Version: "v1"},
+	schema.GroupVersion{Group: "apiserver.k8s.io", Version: "v1alpha1"},
+	schema.GroupVersion{Group: "audit.k8s.io", Version: "v1alpha1"},
+)

pkg/cmd/server/api/validation/master.go

@@ -190,7 +190,6 @@ func ValidateMasterConfig(config *api.MasterConfig, fldPath *field.Path) Validat
 
 	if config.AdmissionConfig.PluginConfig != nil {
 		validationResults.Append(ValidateAdmissionPluginConfig(config.AdmissionConfig.PluginConfig, fldPath.Child("admissionConfig", "pluginConfig")))
-		validationResults.Append(ValidateAdmissionPluginConfigConflicts(config))
 	}
 	if len(config.AdmissionConfig.PluginOrderOverride) != 0 {
 		validationResults.AddWarnings(field.Invalid(fldPath.Child("admissionConfig", "pluginOrderOverride"), config.AdmissionConfig.PluginOrderOverride, "specified admission ordering is being phased out.  Convert to DefaultAdmissionConfig in admissionConfig.pluginConfig."))
@@ -671,10 +670,10 @@ func ValidateKubernetesMasterConfig(config *api.KubernetesMasterConfig, fldPath
 	}
 
 	if config.AdmissionConfig.PluginConfig != nil {
-		validationResults.Append(ValidateAdmissionPluginConfig(config.AdmissionConfig.PluginConfig, fldPath.Child("admissionConfig", "pluginConfig")))
+		validationResults.AddErrors(field.Invalid(fldPath.Child("admissionConfig", "pluginConfig"), config.AdmissionConfig.PluginConfig, "separate admission chains are no longer allowed.  Convert to admissionConfig.pluginConfig."))
 	}
 	if len(config.AdmissionConfig.PluginOrderOverride) != 0 {
-		validationResults.AddWarnings(field.Invalid(fldPath.Child("admissionConfig", "pluginOrderOverride"), config.AdmissionConfig.PluginOrderOverride, "specified admission ordering is being phased out.  Convert to DefaultAdmissionConfig in admissionConfig.pluginConfig."))
+		validationResults.AddErrors(field.Invalid(fldPath.Child("admissionConfig", "pluginOrderOverride"), config.AdmissionConfig.PluginOrderOverride, "separate admission chains are no longer allowed.  Convert to DefaultAdmissionConfig in admissionConfig.pluginConfig."))
 	}
 
 	validationResults.Append(ValidateAPIServerExtendedArguments(config.APIServerArguments, fldPath.Child("apiServerArguments")))
@@ -795,21 +794,6 @@ func ValidateAdmissionPluginConfig(pluginConfig map[string]api.AdmissionPluginCo
 
 }
 
-func ValidateAdmissionPluginConfigConflicts(masterConfig *api.MasterConfig) ValidationResults {
-	validationResults := ValidationResults{}
-
-	if masterConfig.KubernetesMasterConfig != nil {
-		// check for collisions between openshift and kube plugin config
-		for pluginName, kubeConfig := range masterConfig.KubernetesMasterConfig.AdmissionConfig.PluginConfig {
-			if openshiftConfig, exists := masterConfig.AdmissionConfig.PluginConfig[pluginName]; exists && !reflect.DeepEqual(kubeConfig, openshiftConfig) {
-				validationResults.AddWarnings(field.Invalid(field.NewPath("kubernetesMasterConfig", "admissionConfig", "pluginConfig").Key(pluginName), masterConfig.AdmissionConfig.PluginConfig[pluginName], "conflicts with kubernetesMasterConfig.admissionConfig.pluginConfig.  Separate admission chains are being phased out.  Convert to admissionConfig.pluginConfig."))
-			}
-		}
-	}
-
-	return validationResults
-}
-
 func ValidateIngressIPNetworkCIDR(config *api.MasterConfig, fldPath *field.Path) (errors field.ErrorList) {
 	cidr := config.NetworkConfig.IngressIPNetworkCIDR
 	if len(cidr) == 0 {

pkg/cmd/server/api/validation/master_test.go

@@ -274,6 +274,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
 		options configapi.MasterConfig
 
 		warningFields []string
+		errorFields   []string
 	}{
 		{
 			name: "stock everything",
@@ -287,7 +288,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
 					},
 				},
 			},
-			warningFields: []string{"kubernetesMasterConfig.admissionConfig.pluginOrderOverride"},
+			errorFields: []string{"kubernetesMasterConfig.admissionConfig.pluginOrderOverride"},
 		},
 		{
 			name: "specified kube admission order 02",
@@ -393,7 +394,7 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
 					},
 				},
 			},
-			warningFields: []string{"kubernetesMasterConfig.admissionConfig.pluginConfig[foo]"},
+			errorFields: []string{"kubernetesMasterConfig.admissionConfig.pluginConfig"},
 		},
 	}
 
@@ -436,6 +437,21 @@ func TestValidateAdmissionPluginConfigConflicts(t *testing.T) {
 				t.Errorf("%s: didn't find %q", tc.name, expectedField)
 			}
 		}
+
+		for _, expectedField := range tc.errorFields {
+			found := false
+			for _, result := range results.Errors {
+				if result.Field == expectedField {
+					found = true
+					break
+				}
+			}
+
+			if !found {
+				t.Errorf("%s: didn't find %q", tc.name, expectedField)
+			}
+		}
+
 	}
 }