[release-4.19] OCPBUGS-66920: Add ValidatingAdmissionPolicy and check for omissions next time.

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

@@ -1,6 +1,9 @@
 package openshiftadmission
 
 import (
+	"fmt"
+	"slices"
+
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/admission/plugin/resourcequota"
 	genericapiserver "k8s.io/apiserver/pkg/server"
@@ -13,6 +16,7 @@ import (
 	imageadmission "github.com/openshift/openshift-apiserver/pkg/image/apiserver/admission/limitrange"
 	projectrequestlimit "github.com/openshift/openshift-apiserver/pkg/project/apiserver/admission/requestlimit"
 	requiredrouteannotations "github.com/openshift/openshift-apiserver/pkg/route/apiserver/admission/requiredrouteannotations"
+	"k8s.io/apiserver/pkg/server/options"
 )
 
 // TODO register this per apiserver or at least per process
@@ -24,7 +28,7 @@ func init() {
 
 // RegisterAllAdmissionPlugins registers all admission plugins
 func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
-	// kube admission plugins that we rely up.  These should move to generic
+	// Kube (k8s.io/kubernetes) admission plugins that we rely on. These should move to generic (k8s.io/apiserver).
 	gc.Register(plugins)
 	resourcequota.Register(plugins)
 
@@ -44,23 +48,78 @@ func RegisterOpenshiftAdmissionPlugins(plugins *admission.Plugins) {
 
 var (
 	// OpenShiftAdmissionPlugins gives the in-order default admission chain for openshift resources.
-	OpenShiftAdmissionPlugins = []string{
-		// these are from the kbue chain
-		"NamespaceLifecycle",
-		"OwnerReferencesPermissionEnforcement",
-
-		// all custom admission goes here to simulate being part of a webhook
-		"project.openshift.io/ProjectRequestLimit",
-		"build.openshift.io/BuildConfigSecretInjector",
-		"build.openshift.io/BuildByStrategy",
-		"image.openshift.io/ImageLimitRange",
-		"image.openshift.io/ImagePolicy",
-		"quota.openshift.io/ClusterResourceQuota",
-		"route.openshift.io/RequiredRouteAnnotations",
-
-		// the rest of the kube chain goes here
-		"MutatingAdmissionWebhook",
-		"ValidatingAdmissionWebhook",
-		"ResourceQuota",
-	}
+	OpenShiftAdmissionPlugins = func() []string {
+		// The effective admission chain.
+		downstreamPlugins := []string{
+			// these are from the kube chain
+			"NamespaceLifecycle",
+			"OwnerReferencesPermissionEnforcement",
+
+			// all custom admission goes here to simulate being part of a webhook
+			"project.openshift.io/ProjectRequestLimit",
+			"build.openshift.io/BuildConfigSecretInjector",
+			"build.openshift.io/BuildByStrategy",
+			"image.openshift.io/ImageLimitRange",
+			"image.openshift.io/ImagePolicy",
+			"quota.openshift.io/ClusterResourceQuota",
+			"route.openshift.io/RequiredRouteAnnotations",
+
+			// the rest of the kube chain goes here
+			"MutatingAdmissionWebhook",
+			"ValidatingAdmissionPolicy",
+			"ValidatingAdmissionWebhook",
+			"ResourceQuota",
+		}
+
+		// Upstream plugins that are omitted intentionally. If a given plugin is enabled by
+		// default for generic API servers, it must either be enabled by default downstream
+		// or included in this list with an explanation.
+		omittedPlugins := []string{}
+
+		// Plugins that are enabled downstream but are not included in the upstream
+		// defaults. If an admission plugin is _removed_ from the upstream defaults, this
+		// should generate an error during the corresponding k8s.io/apiserver dependency
+		// bump, and the removed plugin must either be removed from `downstreamPlugins` (to
+		// remain in sync with upstream) or added to `downstreamOnlyPlugins` (to
+		// intentionally diverge from the upstream).
+		downstreamOnlyPlugins := []string{
+			"project.openshift.io/ProjectRequestLimit",
+			"build.openshift.io/BuildConfigSecretInjector",
+			"build.openshift.io/BuildByStrategy",
+			"image.openshift.io/ImageLimitRange",
+			"image.openshift.io/ImagePolicy",
+			"quota.openshift.io/ClusterResourceQuota",
+			"route.openshift.io/RequiredRouteAnnotations",
+
+			"OwnerReferencesPermissionEnforcement",
+			"ResourceQuota",
+		}
+
+		upstreamPlugins := options.NewAdmissionOptions().RecommendedPluginOrder
+		for _, upstreamPluginName := range upstreamPlugins {
+			if !slices.Contains(downstreamPlugins, upstreamPluginName) && !slices.Contains(omittedPlugins, upstreamPluginName) {
+				// If you are reading this because you are changing the version of
+				// the k8s.io/apiserver dependency, upstream may have introduced a
+				// new default-enabled admission plugin. If there is a good reason
+				// against enabling it in openshift-apiserver, its name must be
+				// included in omittedPlugins, otherwise, it should in the
+				// appropriate position in downstreamPlugins.
+				panic(fmt.Sprintf("k8s.io/apiserver default admission plugins includes %q which is in neither downstreamPlugins nor omittedPlugins: %v", upstreamPluginName, upstreamPlugins))
+			}
+		}
+
+		for _, downstreamPluginName := range downstreamPlugins {
+			if !slices.Contains(upstreamPlugins, downstreamPluginName) && !slices.Contains(downstreamOnlyPlugins, downstreamPluginName) {
+				// If you are reading this because you are changing the version of
+				// the k8s.io/apiserver dependency, upstream may have removed a
+				// default-enabled admission plugin. If there is a good reason to
+				// keep it enabled in openshift-apiserver, its name must be included
+				// in downstreamOnlyPlugins, otherwise, it should be removed from
+				// downstreamPlugins.
+				panic(fmt.Sprintf("downstreamPlugins includes %q which is in neither downstreamOnlyPlugins nor the k8s.io/apiserver default admission plugins: %v", downstreamPluginName, upstreamPlugins))
+			}
+		}
+
+		return downstreamPlugins
+	}()
 )

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

@@ -214,8 +214,7 @@ func NewOpenshiftAPIConfig(config *openshiftcontrolplanev1.OpenShiftAPIServerCon
 	}
 	admissionOptions.DisablePlugins = config.AdmissionConfig.DisabledAdmissionPlugins
 	admissionOptions.ConfigFile = admissionConfigFile
-	// TODO:
-	if err := admissionOptions.ApplyTo(&genericConfig.Config, kubeInformers, kubeClient, dynamicClient, nil, admissionInitializer); err != nil {
+	if err := admissionOptions.ApplyTo(&genericConfig.Config, kubeInformers, kubeClient, dynamicClient, feature.DefaultFeatureGate, admissionInitializer); err != nil {
 		return nil, err
 	}