OCPBUGS-74511: remove RouteExternalCertificate feature gate

pkg/cmd/openshift-apiserver/cmd.go

@@ -104,7 +104,7 @@ func (o *OpenShiftAPIServer) Validate() error {
 
 // RunAPIServer takes the options, starts the API server and waits until stopCh is closed or initial listening fails.
 func (o *OpenShiftAPIServer) RunAPIServer(stopCh <-chan struct{}) error {
-	if err := features.InitializeFeatureGates(feature.DefaultMutableFeatureGate, openshiftfeatures.SelfManaged, openshiftfeatures.FeatureGateRouteExternalCertificate); err != nil {
+	if err := features.InitializeFeatureGates(feature.DefaultMutableFeatureGate, openshiftfeatures.SelfManaged); err != nil {
 		return err
 	}
 

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

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"time"
 
-	openshiftfeatures "github.com/openshift/api/features"
 	openshiftcontrolplanev1 "github.com/openshift/api/openshiftcontrolplane/v1"
 	"github.com/openshift/apiserver-library-go/pkg/configflags"
 	"github.com/openshift/library-go/pkg/apiserver/admission/admissiontimeout"
@@ -35,7 +34,6 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/restmapper"
 	"k8s.io/component-base/compatibility"
-	"k8s.io/component-base/featuregate"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 )
@@ -266,7 +264,6 @@ func NewOpenshiftAPIConfig(config *openshiftcontrolplanev1.OpenShiftAPIServerCon
 			AdditionalTrustedCA:                caData,
 			ImageStreamImportMode:              apisimage.ImportModeType(config.ImagePolicyConfig.ImageStreamImportMode),
 			RouteAllocator:                     routeAllocator,
-			AllowRouteExternalCertificates:     feature.DefaultFeatureGate.Enabled(featuregate.Feature(openshiftfeatures.FeatureGateRouteExternalCertificate)),
 			ProjectAuthorizationCache:          projectAuthorizationCache,
 			ProjectCache:                       projectCache,
 			ProjectRequestTemplate:             config.ProjectConfig.ProjectRequestTemplate,

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

@@ -90,8 +90,7 @@ type OpenshiftAPIExtraConfig struct {
 	AdditionalTrustedCA                []byte
 	ImageStreamImportMode              apisimage.ImportModeType
 
-	RouteAllocator                 *routehostassignment.SimpleAllocationPlugin
-	AllowRouteExternalCertificates bool
+	RouteAllocator *routehostassignment.SimpleAllocationPlugin
 
 	ProjectAuthorizationCache *projectauth.AuthorizationCache
 	ProjectCache              *projectcache.ProjectCache
@@ -328,7 +327,6 @@ func (c *completedConfig) withRouteAPIServer(delegateAPIServer genericapiserver.
 		ExtraConfig: routeapiserver.ExtraConfig{
 			KubeAPIServerClientConfig: c.ExtraConfig.KubeAPIServerClientConfig,
 			RouteAllocator:            c.ExtraConfig.RouteAllocator,
-			AllowExternalCertificates: c.ExtraConfig.AllowRouteExternalCertificates,
 			Codecs:                    legacyscheme.Codecs,
 			Scheme:                    legacyscheme.Scheme,
 		},

pkg/route/apiserver/apiserver.go

@@ -22,8 +22,6 @@ type ExtraConfig struct {
 	KubeAPIServerClientConfig *restclient.Config
 	RouteAllocator            *routehostassignment.SimpleAllocationPlugin
 
-	AllowExternalCertificates bool
-
 	// TODO these should all become local eventually
 	Scheme *runtime.Scheme
 	Codecs serializer.CodecFactory
@@ -111,7 +109,6 @@ func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) {
 		c.ExtraConfig.RouteAllocator,
 		authorizationClient.SubjectAccessReviews(),
 		kubeClient.CoreV1(),
-		c.ExtraConfig.AllowExternalCertificates,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("error building REST storage: %v", err)

pkg/route/apiserver/registry/route/etcd/etcd.go

@@ -38,8 +38,8 @@ type HostnameGenerator interface {
 }
 
 // NewREST returns a RESTStorage object that will work against routes.
-func NewREST(optsGetter generic.RESTOptionsGetter, allocator HostnameGenerator, sarClient routeregistry.SubjectAccessReviewInterface, secretsGetter corev1client.SecretsGetter, allowExternalCertificates bool) (*REST, *StatusREST, error) {
-	strategy := routeregistry.NewStrategy(allocator, sarClient, secretsGetter, allowExternalCertificates)
+func NewREST(optsGetter generic.RESTOptionsGetter, allocator HostnameGenerator, sarClient routeregistry.SubjectAccessReviewInterface, secretsGetter corev1client.SecretsGetter) (*REST, *StatusREST, error) {
+	strategy := routeregistry.NewStrategy(allocator, sarClient, secretsGetter)
 
 	store := &registry.Store{
 		NewFunc:                   func() runtime.Object { return &routeapi.Route{} },

pkg/route/apiserver/registry/route/etcd/etcd_test.go

@@ -58,7 +58,7 @@ func newStorage(t *testing.T, allocator HostnameGenerator) (*REST, *etcdtesting.
 	etcdStorageConfigForRoutes := &storagebackend.ConfigForResource{Config: *etcdStorage, GroupResource: schema.GroupResource{Group: "route.openshift.io", Resource: "routes"}}
 	restOptions := generic.RESTOptions{StorageConfig: etcdStorageConfigForRoutes, Decorator: generic.UndecoratedStorage, DeleteCollectionWorkers: 1, ResourcePrefix: "routes"}
 	fake := fake.NewSimpleClientset(&corev1.SecretList{})
-	storage, _, err := NewREST(restOptions, allocator, &testSAR{allow: true}, fake.CoreV1(), true)
+	storage, _, err := NewREST(restOptions, allocator, &testSAR{allow: true}, fake.CoreV1())
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/route/apiserver/registry/route/strategy.go

@@ -35,22 +35,20 @@ type HostnameGenerator interface {
 type routeStrategy struct {
 	runtime.ObjectTyper
 	names.NameGenerator
-	hostnameGenerator         HostnameGenerator
-	sarClient                 SubjectAccessReviewInterface
-	secretsGetter             corev1client.SecretsGetter
-	allowExternalCertificates bool
+	hostnameGenerator HostnameGenerator
+	sarClient         SubjectAccessReviewInterface
+	secretsGetter     corev1client.SecretsGetter
 }
 
 // NewStrategy initializes the default logic that applies when creating and updating
 // Route objects via the REST API.
-func NewStrategy(allocator HostnameGenerator, sarClient SubjectAccessReviewInterface, secretsGetter corev1client.SecretsGetter, allowExternalCertificates bool) routeStrategy {
+func NewStrategy(allocator HostnameGenerator, sarClient SubjectAccessReviewInterface, secretsGetter corev1client.SecretsGetter) routeStrategy {
 	return routeStrategy{
-		ObjectTyper:               legacyscheme.Scheme,
-		NameGenerator:             names.SimpleNameGenerator,
-		hostnameGenerator:         allocator,
-		sarClient:                 sarClient,
-		secretsGetter:             secretsGetter,
-		allowExternalCertificates: allowExternalCertificates,
+		ObjectTyper:       legacyscheme.Scheme,
+		NameGenerator:     names.SimpleNameGenerator,
+		hostnameGenerator: allocator,
+		sarClient:         sarClient,
+		secretsGetter:     secretsGetter,
 	}
 }
 
@@ -60,7 +58,7 @@ func (routeStrategy) NamespaceScoped() bool {
 
 func (s routeStrategy) routeValidationOptions() routecommon.RouteValidationOptions {
 	return routecommon.RouteValidationOptions{
-		AllowExternalCertificates: s.allowExternalCertificates,
+		AllowExternalCertificates: true,
 	}
 }
 
@@ -69,13 +67,6 @@ func (s routeStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object)
 	route.Status = routeapi.RouteStatus{}
 	route.Generation = 1
 	stripEmptyDestinationCACertificate(route)
-
-	// In kube APIs, disabled fields are stripped from inbound objects.
-	// This provides parity with prior releases and other unknown fields in kube.
-	// Example of stripping these values in pods: https://github.com/kubernetes/kubernetes/blob/master/pkg/registry/core/pod/strategy.go#L108
-	if !s.allowExternalCertificates && route.Spec.TLS != nil && route.Spec.TLS.ExternalCertificate != nil {
-		route.Spec.TLS.ExternalCertificate = nil
-	}
 }
 
 func (s routeStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
@@ -90,15 +81,6 @@ func (s routeStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob
 		route.Spec.Host = oldRoute.Spec.Host
 	}
 
-	// strip the field if it wasn't previously set and it was disabled.
-	// I didn't bother to do this in the initial PR since OCP doesn't allow featuregates to transition back to disabled
-	// but since I'm in the code adding comments, this an easy thing to fix up now and future-us may allow the transition.
-	if !s.allowExternalCertificates && (oldRoute.Spec.TLS == nil || oldRoute.Spec.TLS.ExternalCertificate == nil) {
-		if route.Spec.TLS != nil && route.Spec.TLS.ExternalCertificate != nil {
-			route.Spec.TLS.ExternalCertificate = nil
-		}
-	}
-
 	// Any changes to the spec increment the generation number.
 	// Changes to status, or to any metadata field (eg.: labels and annotations)
 	// should not impact on the generation
@@ -150,7 +132,7 @@ type routeStatusStrategy struct {
 	routeStrategy
 }
 
-var StatusStrategy = routeStatusStrategy{NewStrategy(nil, nil, nil, false)}
+var StatusStrategy = routeStatusStrategy{NewStrategy(nil, nil, nil)}
 
 func (routeStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	newRoute := obj.(*routeapi.Route)