diff --git a/pkg/cmd/openshift-apiserver/openshiftadmission/register.go b/pkg/cmd/openshift-apiserver/openshiftadmission/register.go index 99a5647d7..7d0e66a37 100644 --- a/pkg/cmd/openshift-apiserver/openshiftadmission/register.go +++ b/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 + }() ) diff --git a/pkg/cmd/openshift-apiserver/openshiftapiserver/config.go b/pkg/cmd/openshift-apiserver/openshiftapiserver/config.go index ceb3a7e47..9774056f9 100644 --- a/pkg/cmd/openshift-apiserver/openshiftapiserver/config.go +++ b/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 }