OCPBUGS-77056: Make external cert validation asynchronous

hack/Makefile.debug

@@ -1,6 +1,7 @@
 # -*- mode: makefile -*-
 
 export GOOS=linux
+export GOARCH ?= amd64
 
 REGISTRY ?= quay.io
 IMAGE ?= openshift/openshift-router
@@ -11,8 +12,8 @@ REMOTE_IMAGE ?= $(REGISTRY)/$(IMAGE):$(TAG)
 OPENSHIFT_ENDPOINT ?= $(shell oc config view --minify --template '{{(index .clusters 0).cluster.server}}' | grep -o '//[^ :]*' | sed 's/^..//')
 
 new-openshift-router-image:
-	GO111MODULE=on CGO_ENABLED=0 GOFLAGS=-mod=vendor go build -o openshift-router -gcflags=all="-N -l" ./cmd/openshift-router
-	$(IMAGEBUILDER) build -t $(LOCAL_IMAGE) -f hack/Dockerfile.debug .
+	GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOFLAGS=-mod=vendor go build -o openshift-router -gcflags=all="-N -l" ./cmd/openshift-router
+	$(IMAGEBUILDER) build --arch $(GOARCH) -t $(LOCAL_IMAGE) -f hack/Dockerfile.debug .
 
 push:
 	$(IMAGEBUILDER) tag $(LOCAL_IMAGE) $(REMOTE_IMAGE)

pkg/cmd/infra/router/clientcmd.go

@@ -60,6 +60,13 @@ func (cfg *Config) KubeConfig() (*restclient.Config, string, error) {
 	if err != nil {
 		return nil, "", err
 	}
+
+	// Increase client-side rate limiting to support higher throughput during
+	// router startup, especially when many external certificate routes are
+	// present.
+	clientConfig.QPS = 50
+	clientConfig.Burst = 100
+
 	return clientConfig, namespace, nil
 }
 

pkg/cmd/infra/router/template.go

@@ -36,13 +36,13 @@ import (
 	routelisters "github.com/openshift/client-go/route/listers/route/v1"
 	"github.com/openshift/library-go/pkg/crypto"
 	"github.com/openshift/library-go/pkg/proc"
-	"github.com/openshift/library-go/pkg/route/secretmanager"
 
 	"github.com/openshift/router/pkg/router"
 	"github.com/openshift/router/pkg/router/client"
 	"github.com/openshift/router/pkg/router/controller"
 	"github.com/openshift/router/pkg/router/metrics"
 	"github.com/openshift/router/pkg/router/metrics/haproxy"
+	"github.com/openshift/router/pkg/router/routeapihelpers"
 	"github.com/openshift/router/pkg/router/shutdown"
 	templateplugin "github.com/openshift/router/pkg/router/template"
 	haproxyconfigmanager "github.com/openshift/router/pkg/router/template/configmanager/haproxy"
@@ -769,7 +769,7 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
 		return err
 	}
 
-	secretManager := secretmanager.NewManager(kc, nil)
+	secretManager := controller.NewSharedSecretManager(kc, nil)
 
 	pluginCfg := templateplugin.TemplatePluginConfig{
 		AppCtx:                        ctx,
@@ -815,7 +815,7 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
 	informer := factory.CreateRoutesSharedInformer()
 	routeLister := routelisters.NewRouteLister(informer.GetIndexer())
 	if o.UpdateStatus {
-		lease := writerlease.New(time.Minute, 3*time.Second)
+		lease := writerlease.New(time.Minute, 3*time.Second, routeapihelpers.MaxConcurrentSARChecks)
 		go lease.Run(stopCh)
 		tracker := controller.NewSimpleContentionTracker(informer, o.RouterName, o.ResyncInterval/10)
 		tracker.SetConflictMessage(fmt.Sprintf("The router detected another process is writing conflicting updates to route status with name %q. Please ensure that the configuration of all routers is consistent. Route status will not be updated as long as conflicts are detected.", o.RouterName))

pkg/router/controller/contention.go

@@ -46,6 +46,7 @@ var (
 		ExtCrtStatusReasonSecretRecreated,
 		ExtCrtStatusReasonSecretUpdated,
 		ExtCrtStatusReasonSecretDeleted,
+		ExtCrtStatusReasonSARCompleted,
 	)
 )
 
@@ -294,8 +295,10 @@ func ingressConditionsEqual(a, b []routev1.RouteIngressCondition) bool {
 
 // conditionsEqual compares two RouteIngressConditions, ignoring LastTransitionTime and any reason in ignoreIngressConditionReason.
 func conditionsEqual(a, b *routev1.RouteIngressCondition) bool {
-	if ignoreIngressConditionReason.Has(a.Reason) || ignoreIngressConditionReason.Has(b.Reason) {
-		return true
+	if a.Type == b.Type && a.Status == b.Status {
+		if ignoreIngressConditionReason.Has(a.Reason) || ignoreIngressConditionReason.Has(b.Reason) {
+			return true
+		}
 	}
 	return a.Type == b.Type &&
 		a.Status == b.Status &&

pkg/router/controller/route_secret_manager.go

@@ -25,6 +25,7 @@ const (
 	ExtCrtStatusReasonSecretUpdated    = "ExternalCertificateSecretUpdated"
 	ExtCrtStatusReasonSecretDeleted    = "ExternalCertificateSecretDeleted"
 	ExtCrtStatusReasonGetFailed        = "ExternalCertificateGetFailed"
+	ExtCrtStatusReasonSARCompleted     = "ExternalCertificateSARCompleted"
 )
 
 // RouteSecretManager implements the router.Plugin interface to register
@@ -157,14 +158,15 @@ func (p *RouteSecretManager) HandleRoute(eventType watch.EventType, route *route
 				// Therefore, it is essential to re-sync the secret to ensure the plugin chain correctly handles the route.
 
 				log.V(4).Info("Re-validating existing external certificate", "namespace", route.Namespace, "secret", oldSecret, "route", route.Name)
-				// re-validate
+				// re-validate (synchronous, throttled by semaphore)
 				if err := p.validate(route); err != nil {
 					return err
 				}
 				// read referenced secret and update TLS certificate and key
 				if err := p.populateRouteTLSFromSecret(route); err != nil {
 					return err
 				}
+
 			}
 
 		case newHasExt && !oldHadExt:
@@ -198,13 +200,24 @@ func (p *RouteSecretManager) HandleRoute(eventType watch.EventType, route *route
 	}
 
 	// call next plugin
-	return p.plugin.HandleRoute(eventType, route)
+	err := p.plugin.HandleRoute(eventType, route)
+
+	// If the route was accepted by the downstream plugin chain and it has an external certificate
+	// that we successfully validated in this pass, emit the SARCompleted status.
+	// This ensures we do not prematurely mark structurally invalid routes as Admitted=True
+	// before the ExtendedValidator has checked the cert/key contents.
+	if err == nil && hasExternalCertificate(route) && (eventType == watch.Added || eventType == watch.Modified) {
+		msg := fmt.Sprintf("SAR check and secret load completed for secret %q", route.Spec.TLS.ExternalCertificate.Name)
+		p.recorder.RecordRouteUpdate(route, ExtCrtStatusReasonSARCompleted, msg)
+	}
+
+	return err
 }
 
 // validateAndRegister validates the route's externalCertificate configuration and registers it with the secret manager.
 // It also updates the in-memory TLS certificate and key after reading from secret informer's cache.
 func (p *RouteSecretManager) validateAndRegister(route *routev1.Route) error {
-	// validate
+	// validate (synchronous, throttled by semaphore)
 	if err := p.validate(route); err != nil {
 		return err
 	}
@@ -213,6 +226,7 @@ func (p *RouteSecretManager) validateAndRegister(route *routev1.Route) error {
 	if err := p.secretManager.RegisterRoute(context.TODO(), route.Namespace, route.Name, route.Spec.TLS.ExternalCertificate.Name, handler); err != nil {
 		return fmt.Errorf("failed to register router: %w", err)
 	}
+
 	// read referenced secret and update TLS certificate and key
 	if err := p.populateRouteTLSFromSecret(route); err != nil {
 		return err
@@ -260,6 +274,7 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 		AddFunc: func(obj interface{}) {
 			secret := obj.(*kapi.Secret)
 			log.V(4).Info("Secret added for route", "namespace", namespace, "secret", secret.Name, "route", routeName)
+			routeapihelpers.InvalidateAsyncSARCache(namespace, secret.Name)
 
 			// Secret re-creation scenario
 			// Check if the route key exists in the deletedSecrets map, indicating that the secret was previously deleted for this route.
@@ -289,6 +304,7 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 			secretNew := new.(*kapi.Secret)
 			key := generateKey(namespace, routeName)
 			log.V(4).Info("Secret updated for route", "namespace", namespace, "secret", secretNew.Name, "oldSecretVersion", secretOld.ResourceVersion, "newSecretVersion", secretNew.ResourceVersion, "route", routeName)
+			routeapihelpers.InvalidateAsyncSARCache(namespace, secretNew.Name)
 
 			// Ensure fetching the updated route
 			route, err := p.routelister.Routes(namespace).Get(routeName)
@@ -309,10 +325,23 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 		},
 
 		DeleteFunc: func(obj interface{}) {
-			secret := obj.(*kapi.Secret)
+			secret, ok := obj.(*kapi.Secret)
+			if !ok {
+				tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+				if !ok {
+					log.Error(nil, "Couldn't get object from tombstone", "type", fmt.Sprintf("%T", obj))
+					return
+				}
+				secret, ok = tombstone.Obj.(*kapi.Secret)
+				if !ok {
+					log.Error(nil, "Tombstone contained object that is not a secret", "type", fmt.Sprintf("%T", tombstone.Obj))
+					return
+				}
+			}
 			key := generateKey(namespace, routeName)
 			msg := fmt.Sprintf("secret %q deleted for route %q", secret.Name, key)
 			log.V(4).Info(msg)
+			routeapihelpers.InvalidateAsyncSARCache(namespace, secret.Name)
 
 			// keep the secret monitor active and mark the secret as deleted for this route.
 			p.deletedSecrets.Store(key, true)
@@ -334,10 +363,14 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 // If the validation fails, it records the route rejection and triggers
 // the deletion of the route by calling the HandleRoute method with a watch.Deleted event.
 //
+// This function is synchronous: it blocks until the SAR check completes.
+// Concurrency is throttled by the semaphore in ValidateTLSExternalCertificate.
+//
 // NOTE: TLS data validation and sanitization are handled by the next plugin `ExtendedValidator`,
 // by reading the "tls.crt" and "tls.key" added by populateRouteTLSFromSecret.
 func (p *RouteSecretManager) validate(route *routev1.Route) error {
 	fldPath := field.NewPath("spec").Child("tls").Child("externalCertificate")
+
 	if err := routeapihelpers.ValidateTLSExternalCertificate(route, fldPath, p.sarClient, p.secretsGetter).ToAggregate(); err != nil {
 		log.Error(err, "skipping route due to invalid externalCertificate configuration", "namespace", route.Namespace, "route", route.Name)
 		p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonValidationFailed, err.Error())
@@ -352,7 +385,9 @@ func (p *RouteSecretManager) validate(route *routev1.Route) error {
 // the deletion of the route by calling the HandleRoute method with a watch.Deleted event.
 // Note: This function performs an in-place update of the route. The caller should be aware that the route's TLS configuration will be modified directly.
 func (p *RouteSecretManager) populateRouteTLSFromSecret(route *routev1.Route) error {
-	// read referenced secret
+	// read referenced secret from the informer cache.
+	// GetSecret attempts to read from the cache and falls back to a direct API
+	// call if the cache is not synced or the secret is not found.
 	secret, err := p.secretManager.GetSecret(context.TODO(), route.Namespace, route.Name)
 	if err != nil {
 		log.Error(err, "failed to get referenced secret")