Merge pull request #18218 from deads2k/admission-04-fix-inclusiontest

Automatic merge from submit-queue (batch tested with PRs 17953, 18218, 18247).

fix admission usage test and add missing plugins

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1536288

Our test for including upstream admission plugins wasn't good.  I fixed it to run based on the upstream registration functions.  I also added the EventRateLimiter to fix the perceived problem.

/assign soltysh
/assign sttts
@mfojtik

Author: openshift-merge-robot

pkg/cmd/server/origin/admission/chain_builder.go

@@ -49,7 +49,11 @@ var (
 
 	// kubeAdmissionPlugins gives the in-order default admission chain for kube resources.
 	kubeAdmissionPlugins = []string{
+		"AlwaysAdmit",
+		"NamespaceAutoProvision",
+		"NamespaceExists",
 		lifecycle.PluginName,
+		"EventRateLimit",
 		"RunOnceDuration",
 		"PodNodeConstraints",
 		"OriginPodNodeEnvironment",
@@ -61,10 +65,15 @@ var (
 		imagequalify.PluginName,
 		"ImagePolicyWebhook",
 		"PodPreset",
+		"InitialResources",
 		"LimitRanger",
 		"ServiceAccount",
 		noderestriction.PluginName,
+		"SecurityContextDeny",
 		sccadmission.PluginName,
+		"PodSecurityPolicy",
+		"DenyEscalatingExec",
+		"DenyExecOnPrivileged",
 		storageclassdefaultadmission.PluginName,
 		expandpvcadmission.PluginName,
 		"AlwaysPullImages",
@@ -73,13 +82,15 @@ var (
 		"PersistentVolumeLabel",
 		"OwnerReferencesPermissionEnforcement",
 		ingressadmission.IngressAdmission,
+		"Priority",
 		"ExtendedResourceToleration",
 		"DefaultTolerationSeconds",
 		"PVCProtection",
 		"Initializers",
 		"MutatingAdmissionWebhook",
 		"ValidatingAdmissionWebhook",
 		"PodTolerationRestriction",
+		"AlwaysDeny",
 		// NOTE: ResourceQuota and ClusterResourceQuota must be the last 2 plugins.
 		// DO NOT ADD ANY PLUGINS AFTER THIS LINE!
 		"ResourceQuota",
@@ -90,7 +101,11 @@ var (
 	// When possible, this list is used.  The set of openshift+kube chains must exactly match this set.  In addition,
 	// the order specified in the openshift and kube chains must match the order here.
 	combinedAdmissionControlPlugins = []string{
+		"AlwaysAdmit",
+		"NamespaceAutoProvision",
+		"NamespaceExists",
 		lifecycle.PluginName,
+		"EventRateLimit",
 		"ProjectRequestLimit",
 		"OriginNamespaceLifecycle",
 		"openshift.io/RestrictSubjectBindings",
@@ -109,10 +124,15 @@ var (
 		imagequalify.PluginName,
 		"ImagePolicyWebhook",
 		"PodPreset",
+		"InitialResources",
 		"LimitRanger",
 		"ServiceAccount",
 		noderestriction.PluginName,
+		"SecurityContextDeny",
 		sccadmission.PluginName,
+		"PodSecurityPolicy",
+		"DenyEscalatingExec",
+		"DenyExecOnPrivileged",
 		storageclassdefaultadmission.PluginName,
 		expandpvcadmission.PluginName,
 		"AlwaysPullImages",
@@ -121,13 +141,15 @@ var (
 		"PersistentVolumeLabel",
 		"OwnerReferencesPermissionEnforcement",
 		ingressadmission.IngressAdmission,
+		"Priority",
 		"ExtendedResourceToleration",
 		"DefaultTolerationSeconds",
 		"PVCProtection",
 		"Initializers",
 		"MutatingAdmissionWebhook",
 		"ValidatingAdmissionWebhook",
 		"PodTolerationRestriction",
+		"AlwaysDeny",
 		// NOTE: ResourceQuota and ClusterResourceQuota must be the last 2 plugins.
 		// DO NOT ADD ANY PLUGINS AFTER THIS LINE!
 		"ResourceQuota",

pkg/cmd/server/origin/admission/config_test.go

@@ -7,13 +7,11 @@ import (
 
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle"
 
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
 	overrideapi "github.com/openshift/origin/pkg/quota/admission/clusterresourceoverride/api"
 	sccadmission "github.com/openshift/origin/pkg/security/admission"
 	serviceadmit "github.com/openshift/origin/pkg/service/admission"
-	expandpvcadmission "k8s.io/kubernetes/plugin/pkg/admission/persistentvolume/resize"
 )
 
 // TestAdmissionPluginChains makes sure that the admission plugin lists are coherent.
@@ -73,41 +71,27 @@ var legacyOpenshiftAdmissionPlugins = sets.NewString(
 	"ResourceQuota",
 )
 
-// kubeAdmissionPlugins tracks kube plugins we use.  You may add to this list, but ONLY if they're from upstream kube
-var usedKubeAdmissionPlugins = sets.NewString(
-	lifecycle.PluginName,
-	"PodPreset",
-	"LimitRanger",
-	"ServiceAccount",
-	"DefaultStorageClass",
-	"ImagePolicyWebhook",
-	"AlwaysPullImages",
-	"LimitPodHardAntiAffinityTopology",
-	"SCCExecRestrictions",
-	"PersistentVolumeLabel",
-	"OwnerReferencesPermissionEnforcement",
-	"PodNodeSelector",
-	"DefaultTolerationSeconds",
-	"ResourceQuota",
-	"Initializers",
-	"ValidatingAdmissionWebhook",
-	"MutatingAdmissionWebhook",
-	"PodTolerationRestriction",
-	"ExtendedResourceToleration",
-	"PVCProtection",
-	"NodeRestriction",
-	expandpvcadmission.PluginName,
-)
-
 // TestAdmissionPluginNames makes sure that openshift admission plugins are prefixed with `openshift.io/`.
 func TestAdmissionPluginNames(t *testing.T) {
-	for _, plugin := range combinedAdmissionControlPlugins {
-		if !strings.HasPrefix(plugin, "openshift.io/") && !usedKubeAdmissionPlugins.Has(plugin) && !legacyOpenshiftAdmissionPlugins.Has(plugin) {
+	originAdmissionPlugins := admission.NewPlugins()
+	registerOpenshiftAdmissionPlugins(originAdmissionPlugins)
+
+	for _, plugin := range originAdmissionPlugins.Registered() {
+		if !strings.HasPrefix(plugin, "openshift.io/") && !legacyOpenshiftAdmissionPlugins.Has(plugin) {
 			t.Errorf("openshift admission plugins must be prefixed with openshift.io/ %v", plugin)
 		}
 	}
 }
 
+func TestUnusuedKubeAdmissionPlugins(t *testing.T) {
+	allAdmissionPlugins := sets.NewString(OriginAdmissionPlugins.Registered()...)
+	knownAdmissionPlugins := sets.NewString(combinedAdmissionControlPlugins...)
+
+	if unorderedPlugins := allAdmissionPlugins.Difference(knownAdmissionPlugins); len(unorderedPlugins) != 0 {
+		t.Errorf("%v need to be ordered and enabled/disabled", unorderedPlugins.List())
+	}
+}
+
 func TestSeparateAdmissionChainDetection(t *testing.T) {
 	testCases := []struct {
 		name                  string

pkg/cmd/server/origin/admission/register.go

@@ -52,6 +52,10 @@ func init() {
 func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
 	kubeapiserver.RegisterAllAdmissionPlugins(plugins)
 	genericapiserver.RegisterAllAdmissionPlugins(plugins)
+	registerOpenshiftAdmissionPlugins(plugins)
+}
+
+func registerOpenshiftAdmissionPlugins(plugins *admission.Plugins) {
 	authorizationrestrictusers.Register(plugins)
 	buildjenkinsbootstrapper.Register(plugins)
 	buildsecretinjector.Register(plugins)
@@ -114,15 +118,26 @@ var (
 		"LimitPodHardAntiAffinityTopology",
 		"DefaultTolerationSeconds",
 		"PodPreset", // default to off while PodPreset is alpha
-
-		// these are new, reassess post-rebase
+		"EventRateLimit",
+		"PodSecurityPolicy",
+		"Priority",
 		"Initializers",
 		"ValidatingAdmissionWebhook",
 		"MutatingAdmissionWebhook",
 		"PodTolerationRestriction",
 		"ExtendedResourceToleration",
 		"PVCProtection",
 		expandpvcadmission.PluginName,
+
+		// these should usually be off.
+		"AlwaysAdmit",
+		"AlwaysDeny",
+		"DenyEscalatingExec",
+		"DenyExecOnPrivileged",
+		"InitialResources",
+		"NamespaceAutoProvision",
+		"NamespaceExists",
+		"SecurityContextDeny",
 	)
 )
 

pkg/cmd/server/origin/admission/sync_test.go

@@ -69,8 +69,8 @@ func TestAdmissionOnOffCoverage(t *testing.T) {
 
 	if !configuredAdmissionPlugins.Equal(allCoveredAdmissionPlugins) {
 		t.Errorf("every admission plugin must be default on or default off. differences: %v and %v",
-			configuredAdmissionPlugins.Difference(allCoveredAdmissionPlugins),
-			allCoveredAdmissionPlugins.Difference(configuredAdmissionPlugins))
+			configuredAdmissionPlugins.Difference(allCoveredAdmissionPlugins).List(),
+			allCoveredAdmissionPlugins.Difference(configuredAdmissionPlugins).List())
 	}
 
 	for plugin := range DefaultOnPlugins {