Merge pull request #22025 from deads2k/admission-enable2

use the new enable/disable fields for admission

hack/openapi-violation.list

@@ -26,8 +26,11 @@ API rule violation: names_match,github.com/openshift/api/build/v1,BuildTriggerPo
 API rule violation: names_match,github.com/openshift/api/build/v1,BuildTriggerPolicy,GenericWebHook
 API rule violation: names_match,github.com/openshift/api/build/v1,BuildTriggerPolicy,GitHubWebHook
 API rule violation: names_match,github.com/openshift/api/build/v1,BuildTriggerPolicy,GitLabWebHook
+API rule violation: names_match,github.com/openshift/api/config/v1,AdmissionConfig,DisabledAdmissionPlugins
+API rule violation: names_match,github.com/openshift/api/config/v1,AdmissionConfig,EnabledAdmissionPlugins
 API rule violation: names_match,github.com/openshift/api/config/v1,AuthenticationSpec,OAuthMetadata
 API rule violation: names_match,github.com/openshift/api/config/v1,EtcdConnectionInfo,URLs
+API rule violation: names_match,github.com/openshift/api/config/v1,GenericAPIServerConfig,AdmissionConfig
 API rule violation: names_match,github.com/openshift/api/config/v1,IdentityProvider,UseAsChallenger
 API rule violation: names_match,github.com/openshift/api/config/v1,IdentityProvider,UseAsLogin
 API rule violation: names_match,github.com/openshift/api/config/v1,IdentityProviderConfig,GitHub

pkg/cmd/openshift-apiserver/openshiftadmission/chain_builder.go

@@ -51,7 +51,7 @@ func NewAdmissionChains(
 		admissionPluginConfigFilename = tempFile.Name()
 	}
 
-	allOffPlugins := append(DefaultOffPlugins.List(), explicitOff...)
+	allOffPlugins := append([]string{}, explicitOff...)
 	disabledPlugins := sets.NewString(allOffPlugins...)
 	enabledPlugins := sets.NewString(explicitOn...)
 	disabledPlugins = disabledPlugins.Difference(enabledPlugins)

pkg/cmd/openshift-apiserver/openshiftadmission/register.go

@@ -75,11 +75,6 @@ var (
 		validatingwebhook.PluginName,
 		"ResourceQuota",
 		"quota.openshift.io/ClusterResourceQuota",
-	)
-
-	// DefaultOffPlugins includes plugins which require explicit configuration to run
-	// if you wire them incorrectly, they may prevent the server from starting
-	DefaultOffPlugins = sets.NewString(
 		"project.openshift.io/ProjectRequestLimit",
 		"PodNodeConstraints",
 	)

pkg/cmd/openshift-apiserver/openshiftapiserver/config.go

@@ -172,22 +172,7 @@ func NewOpenshiftAPIConfig(config *openshiftcontrolplanev1.OpenShiftAPIServerCon
 	admissionDecorators := admission.Decorators{
 		admission.DecoratorFunc(admissionmetrics.WithControllerMetrics),
 	}
-	explicitOn := []string{}
-	explicitOff := []string{}
-	for plugin, config := range config.AdmissionPluginConfig {
-		enabled, err := isAdmissionPluginActivated(config)
-		if err != nil {
-			return nil, err
-		}
-		if enabled {
-			glog.V(2).Infof("Enabling %s", plugin)
-			explicitOn = append(explicitOn, plugin)
-		} else {
-			glog.V(2).Infof("Disabling %s", plugin)
-			explicitOff = append(explicitOff, plugin)
-		}
-	}
-	genericConfig.AdmissionControl, err = openshiftadmission.NewAdmissionChains([]string{}, explicitOn, explicitOff, config.AdmissionPluginConfig, admissionInitializer, admissionDecorators)
+	genericConfig.AdmissionControl, err = openshiftadmission.NewAdmissionChains([]string{}, config.AdmissionConfig.EnabledAdmissionPlugins, config.AdmissionConfig.DisabledAdmissionPlugins, config.AdmissionConfig.PluginConfig, admissionInitializer, admissionDecorators)
 	if err != nil {
 		return nil, err
 	}

pkg/cmd/openshift-kube-apiserver/kubeadmission/register.go

@@ -96,17 +96,11 @@ var (
 		"PodTolerationRestriction",
 		"quota.openshift.io/ClusterResourceQuota",
 		"route.openshift.io/IngressAdmission",
-	)
-
-	// additionalDefaultOffPlugins are admission plugins we choose not to enable by default in openshift
-	// you shouldn't put anything from kube in this list without api-approvers signing off on it.
-	additionalDefaultOffPlugins = sets.NewString(
 		"project.openshift.io/ProjectRequestLimit",
 		"autoscaling.openshift.io/RunOnceDuration",
 		"scheduling.openshift.io/PodNodeConstraints",
 		overrideapi.PluginName,
 		imagepolicyapi.PluginName,
-		"authorization.openshift.io/RestrictSubjectBindings",
 	)
 )
 
@@ -125,7 +119,7 @@ func NewOrderedKubeAdmissionPlugins(kubeAdmissionOrder []string) []string {
 func NewDefaultOffPluginsFunc(kubeDefaultOffAdmission sets.String) func() sets.String {
 	return func() sets.String {
 		kubeOff := sets.NewString(kubeDefaultOffAdmission.UnsortedList()...)
-		kubeOff.Insert(additionalDefaultOffPlugins.List()...)
+		kubeOff.Insert("authorization.openshift.io/RestrictSubjectBindings")
 		kubeOff.Delete(additionalDefaultOnPlugins.List()...)
 		return kubeOff
 	}

pkg/cmd/openshift-kube-apiserver/openshiftkubeapiserver/flags.go

@@ -1,7 +1,6 @@
 package openshiftkubeapiserver
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -10,10 +9,8 @@ import (
 	configv1 "github.com/openshift/api/config/v1"
 	kubecontrolplanev1 "github.com/openshift/api/kubecontrolplane/v1"
 	"github.com/openshift/origin/pkg/cmd/configflags"
-	configapi "github.com/openshift/origin/pkg/cmd/server/apis/config"
 	configapilatest "github.com/openshift/origin/pkg/cmd/server/apis/config/latest"
 	"github.com/openshift/origin/pkg/configconversion"
-	"k8s.io/apimachinery/pkg/runtime"
 )
 
 func ConfigToFlags(kubeAPIServerConfig *kubecontrolplanev1.KubeAPIServerConfig) ([]string, error) {
@@ -96,7 +93,7 @@ func ConfigToFlags(kubeAPIServerConfig *kubecontrolplanev1.KubeAPIServerConfig)
 
 	// TODO, we need to set these in order to enable the right admission plugins in each of the servers
 	// TODO this is needed for a viable cluster up
-	admissionFlags, err := admissionFlags(kubeAPIServerConfig.AdmissionPluginConfig)
+	admissionFlags, err := admissionFlags(kubeAPIServerConfig.AdmissionConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -154,71 +151,10 @@ func ConfigToFlags(kubeAPIServerConfig *kubecontrolplanev1.KubeAPIServerConfig)
 	return configflags.ToFlagSlice(args), nil
 }
 
-func admissionFlags(admissionPluginConfig map[string]configv1.AdmissionPluginConfig) (map[string][]string, error) {
+func admissionFlags(admissionConfig configv1.AdmissionConfig) (map[string][]string, error) {
 	args := map[string][]string{}
 
-	forceOn := []string{}
-	forceOff := []string{}
-	pluginConfig := map[string]configv1.AdmissionPluginConfig{}
-	for pluginName, origConfig := range admissionPluginConfig {
-		config := *origConfig.DeepCopy()
-		if len(config.Location) > 0 {
-			content, err := ioutil.ReadFile(config.Location)
-			if err != nil {
-				return nil, err
-			}
-			// if the config isn't a DefaultAdmissionConfig, then assume we're enabled (we were called out after all)
-			// if the config *is* a DefaultAdmissionConfig and it explicitly said to disable us, we are disabled
-			obj, err := configapilatest.ReadYAML(bytes.NewBuffer(content))
-			// if we can't read it, let the plugin deal with it
-			// if nothing was there, let the plugin deal with it
-			if err != nil || obj == nil {
-				forceOn = append(forceOn, pluginName)
-				config.Location = ""
-				config.Configuration = runtime.RawExtension{Raw: content}
-				pluginConfig[pluginName] = config
-				continue
-			}
-
-			if defaultConfig, ok := obj.(*configapi.DefaultAdmissionConfig); !ok {
-				forceOn = append(forceOn, pluginName)
-				config.Location = ""
-				config.Configuration = runtime.RawExtension{Raw: content}
-				pluginConfig[pluginName] = config
-
-			} else if defaultConfig.Disable {
-				forceOff = append(forceOff, pluginName)
-
-			} else {
-				forceOn = append(forceOn, pluginName)
-			}
-
-			continue
-		}
-
-		// if it wasn't a DefaultAdmissionConfig object, let the plugin deal with it
-		currConfig := &configapi.DefaultAdmissionConfig{}
-		uncastDefaultConfig, _, decodingErr := configapilatest.Codec.Decode(config.Configuration.Raw, nil, currConfig)
-		if decodingErr != nil {
-			forceOn = append(forceOn, pluginName)
-			pluginConfig[pluginName] = config
-			continue
-		}
-
-		defaultConfig, ok := uncastDefaultConfig.(*configapi.DefaultAdmissionConfig)
-		if !ok {
-			forceOn = append(forceOn, pluginName)
-			pluginConfig[pluginName] = config
-
-		} else if defaultConfig.Disable {
-			forceOff = append(forceOff, pluginName)
-
-		} else {
-			forceOn = append(forceOn, pluginName)
-		}
-
-	}
-	upstreamAdmissionConfig, err := configconversion.ConvertOpenshiftAdmissionConfigToKubeAdmissionConfig(pluginConfig)
+	upstreamAdmissionConfig, err := configconversion.ConvertOpenshiftAdmissionConfigToKubeAdmissionConfig(admissionConfig.PluginConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -237,8 +173,8 @@ func admissionFlags(admissionPluginConfig map[string]configv1.AdmissionPluginCon
 	tempFile.Close()
 
 	configflags.SetIfUnset(args, "admission-control-config-file", tempFile.Name())
-	configflags.SetIfUnset(args, "disable-admission-plugins", forceOff...)
-	configflags.SetIfUnset(args, "enable-admission-plugins", forceOn...)
+	configflags.SetIfUnset(args, "disable-admission-plugins", admissionConfig.DisabledAdmissionPlugins...)
+	configflags.SetIfUnset(args, "enable-admission-plugins", admissionConfig.EnabledAdmissionPlugins...)
 
 	return args, nil
 }

pkg/configconversion/legacyconfig_conversion.go

@@ -100,6 +100,8 @@ func ConvertMasterConfigToKubeAPIServerConfig(input *legacyconfigv1.MasterConfig
 	for k, v := range input.KubernetesMasterConfig.APIServerArguments {
 		ret.APIServerArguments[k] = v
 	}
+	ret.AdmissionConfig.EnabledAdmissionPlugins = ToKubeAdmissionPluginList(input.KubernetesMasterConfig.APIServerArguments["enable-admission-plugins"])
+	ret.AdmissionConfig.DisabledAdmissionPlugins = ToKubeAdmissionPluginList(input.KubernetesMasterConfig.APIServerArguments["disable-admission-plugins"])
 
 	// TODO this is likely to be a little weird.  I think we override most of this in the operator
 	ret.ServingInfo, err = ToHTTPServingInfo(&input.ServingInfo)
@@ -131,7 +133,7 @@ func ConvertMasterConfigToKubeAPIServerConfig(input *legacyconfigv1.MasterConfig
 	if err != nil {
 		return nil, err
 	}
-	ret.AdmissionPluginConfig, err = ToKubeAdmissionPluginConfigMap(input.AdmissionConfig.PluginConfig)
+	ret.AdmissionConfig.PluginConfig, err = ToKubeAdmissionPluginConfigMap(input.AdmissionConfig.PluginConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -177,6 +179,8 @@ func ConvertMasterConfigToOpenShiftAPIServerConfig(input *legacyconfigv1.MasterC
 	for k, v := range input.KubernetesMasterConfig.APIServerArguments {
 		ret.APIServerArguments[k] = v
 	}
+	ret.AdmissionConfig.EnabledAdmissionPlugins = ToOpenShiftAdmissionPluginList(input.KubernetesMasterConfig.APIServerArguments["enable-admission-plugins"])
+	ret.AdmissionConfig.DisabledAdmissionPlugins = ToOpenShiftAdmissionPluginList(input.KubernetesMasterConfig.APIServerArguments["disable-admission-plugins"])
 
 	// TODO this is likely to be a little weird.  I think we override most of this in the operator
 	ret.ServingInfo, err = ToHTTPServingInfo(&input.ServingInfo)
@@ -195,7 +199,7 @@ func ConvertMasterConfigToOpenShiftAPIServerConfig(input *legacyconfigv1.MasterC
 	if err != nil {
 		return nil, err
 	}
-	ret.AdmissionPluginConfig, err = ToOpenShiftAdmissionPluginConfigMap(input.AdmissionConfig.PluginConfig)
+	ret.AdmissionConfig.PluginConfig, err = ToOpenShiftAdmissionPluginConfigMap(input.AdmissionConfig.PluginConfig)
 	if err != nil {
 		return nil, err
 	}

pkg/configconversion/simple.go

@@ -80,6 +80,36 @@ func ToOpenShiftAdmissionPluginConfigMap(in map[string]*legacyconfigv1.Admission
 	return out, nil
 }
 
+func ToOpenShiftAdmissionPluginList(in []string) (out []string) {
+	if in == nil {
+		return nil
+	}
+
+	for _, name := range in {
+		if !isKnownOpenShiftAdmissionPlugin(name) {
+			continue
+		}
+		out = append(out, name)
+	}
+
+	return out
+}
+
+func ToKubeAdmissionPluginList(in []string) (out []string) {
+	if in == nil {
+		return nil
+	}
+
+	for _, name := range in {
+		if !isKnownKubeAdmissionPlugin(name) {
+			continue
+		}
+		out = append(out, name)
+	}
+
+	return out
+}
+
 func isKnownOpenShiftAdmissionPlugin(pluginName string) bool {
 	for _, plugin := range openshiftadmission.OpenShiftAdmissionPlugins {
 		if pluginName == plugin {