OCPBUGS-77056: make external cert validation asynchronous

Signed-off-by: Brett Tofel <btofel@redhat.com>

Author: bentito

pkg/router/controller/route_secret_manager.go

@@ -281,7 +281,20 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 				// by all the plugins (including this plugin). Once passes, the route will become active again.
 				msg := fmt.Sprintf("secret %q recreated for route %q", secret.Name, key)
 				p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonSecretRecreated, msg)
+				return
+			}
+
+			// Async secret load scenario
+			// When the secret completes its initial cache sync, we need to trigger the router
+			// controller to evaluate this route again. We use RecordRouteRejection to keep the route
+			// pending until the full plugin chain evaluates and admits it.
+			route, err := p.routelister.Routes(namespace).Get(routeName)
+			if err != nil {
+				log.Error(err, "failed to get route", "namespace", namespace, "route", routeName)
+				return
 			}
+			msg := fmt.Sprintf("secret %q loaded for route %q", secret.Name, key)
+			p.recorder.RecordRouteRejection(route, "ExternalCertificateSecretLoaded", msg)
 		},
 
 		UpdateFunc: func(old interface{}, new interface{}) {

pkg/router/routeapihelpers/validation.go

@@ -9,6 +9,11 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
+<<<<<<< Updated upstream
+=======
+	"strings"
+	"sync"
+>>>>>>> Stashed changes
 
 	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/client-go/util/cert"
@@ -481,57 +486,69 @@ func UpgradeRouteValidation(route *routev1.Route) field.ErrorList {
 	return nil
 }
 
+var asyncSARCache sync.Map // key: namespace/secretName, value: field.ErrorList
+
 // ValidateTLSExternalCertificate tests different pre-conditions required for
 // using externalCertificate.
 func ValidateTLSExternalCertificate(route *routev1.Route, fldPath *field.Path, sarc authorizationclient.SubjectAccessReviewInterface, secretsGetter corev1client.SecretsGetter) field.ErrorList {
 	tls := route.Spec.TLS
-
-	errs := field.ErrorList{}
-	// The router serviceaccount must have permission to get/list/watch the referenced secret.
-	// The role and rolebinding to provide this access must be provided by the user.
-	if err := authorizationutil.Authorize(sarc, &user.DefaultInfo{Name: routerServiceAccount},
-		&authorizationv1.ResourceAttributes{
-			Namespace: route.Namespace,
-			Verb:      "get",
-			Resource:  "secrets",
-			Name:      tls.ExternalCertificate.Name,
-		}); err != nil {
-		errs = append(errs, field.Forbidden(fldPath, "router serviceaccount does not have permission to get this secret"))
+	if tls == nil || tls.ExternalCertificate == nil || tls.ExternalCertificate.Name == "" {
+		return nil
 	}
 
-	if err := authorizationutil.Authorize(sarc, &user.DefaultInfo{Name: routerServiceAccount},
-		&authorizationv1.ResourceAttributes{
-			Namespace: route.Namespace,
-			Verb:      "watch",
-			Resource:  "secrets",
-			Name:      tls.ExternalCertificate.Name,
-		}); err != nil {
-		errs = append(errs, field.Forbidden(fldPath, "router serviceaccount does not have permission to watch this secret"))
+	secretName := tls.ExternalCertificate.Name
+	cacheKey := route.Namespace + "/" + secretName
+
+	if cached, ok := asyncSARCache.Load(cacheKey); ok {
+		return cached.(field.ErrorList)
 	}
 
-	if err := authorizationutil.Authorize(sarc, &user.DefaultInfo{Name: routerServiceAccount},
-		&authorizationv1.ResourceAttributes{
-			Namespace: route.Namespace,
-			Verb:      "list",
-			Resource:  "secrets",
-			Name:      tls.ExternalCertificate.Name,
-		}); err != nil {
-		errs = append(errs, field.Forbidden(fldPath, "router serviceaccount does not have permission to list this secret"))
+	// For tests where dependencies might be mocked/nil, avoid panic
+	if sarc == nil || secretsGetter == nil {
+		return nil
 	}
 
-	// The secret should be in the same namespace as that of the route.
-	secret, err := secretsGetter.Secrets(route.Namespace).Get(context.TODO(), tls.ExternalCertificate.Name, metav1.GetOptions{})
-	if err != nil {
-		if apierrors.IsNotFound(err) {
-			return append(errs, field.NotFound(fldPath, err.Error()))
+	// Store empty error list temporarily so we only launch one goroutine per secret
+	asyncSARCache.Store(cacheKey, field.ErrorList{})
+
+	// Perform checks asynchronously per secret
+	go func() {
+		errs := field.ErrorList{}
+
+		if err := authorizationutil.Authorize(sarc, &user.DefaultInfo{Name: routerServiceAccount},
+			&authorizationv1.ResourceAttributes{
+				Namespace: route.Namespace, Verb: "get", Resource: "secrets", Name: secretName,
+			}); err != nil {
+			errs = append(errs, field.Forbidden(fldPath, "router serviceaccount does not have permission to get this secret"))
 		}
-		return append(errs, field.InternalError(fldPath, err))
-	}
 
-	// The secret should be of type kubernetes.io/tls
-	if secret.Type != kapi.SecretTypeTLS {
-		errs = append(errs, field.Invalid(fldPath, tls.ExternalCertificate.Name, fmt.Sprintf("secret of type %q required", kapi.SecretTypeTLS)))
-	}
+		if err := authorizationutil.Authorize(sarc, &user.DefaultInfo{Name: routerServiceAccount},
+			&authorizationv1.ResourceAttributes{
+				Namespace: route.Namespace, Verb: "watch", Resource: "secrets", Name: secretName,
+			}); err != nil {
+			errs = append(errs, field.Forbidden(fldPath, "router serviceaccount does not have permission to watch this secret"))
+		}
+
+		if err := authorizationutil.Authorize(sarc, &user.DefaultInfo{Name: routerServiceAccount},
+			&authorizationv1.ResourceAttributes{
+				Namespace: route.Namespace, Verb: "list", Resource: "secrets", Name: secretName,
+			}); err != nil {
+			errs = append(errs, field.Forbidden(fldPath, "router serviceaccount does not have permission to list this secret"))
+		}
+
+		secret, err := secretsGetter.Secrets(route.Namespace).Get(context.TODO(), secretName, metav1.GetOptions{})
+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				errs = append(errs, field.NotFound(fldPath, err.Error()))
+			} else {
+				errs = append(errs, field.InternalError(fldPath, err))
+			}
+		} else if secret.Type != kapi.SecretTypeTLS {
+			errs = append(errs, field.Invalid(fldPath, secretName, fmt.Sprintf("secret of type %q required", kapi.SecretTypeTLS)))
+		}
 
-	return errs
+		asyncSARCache.Store(cacheKey, errs)
+	}()
+
+	return nil
 }