fix: never report a gateway with a dropped listener

config/e2e-downstream/issuer.yaml

@@ -39,3 +39,42 @@ metadata:
 spec:
   ca:
     secretName: e2e-extension-server-ca
+---
+# Self-signed root + CA-backed ClusterIssuer that listener certificates are
+# issued from, standing in for the ACME issuer production maps `auto` to
+# (clusterIssuerMap in apps/network-services-operator/control-plane/production
+# of datum-cloud/infra). Without it the operator-injected default-https listener
+# carries an issuer it cannot resolve, so no Certificate is minted and the
+# listener never reaches the edge.
+apiVersion: cert-manager.io/v1
+kind: ClusterIssuer
+metadata:
+  name: e2e-gateway-http-selfsigned
+spec:
+  selfSigned: {}
+---
+apiVersion: cert-manager.io/v1
+kind: Certificate
+metadata:
+  name: e2e-gateway-http-ca
+  namespace: cert-manager
+spec:
+  isCA: true
+  commonName: e2e-gateway-http-ca
+  secretName: e2e-gateway-http-ca
+  duration: 8760h
+  privateKey:
+    algorithm: ECDSA
+    size: 256
+  issuerRef:
+    name: e2e-gateway-http-selfsigned
+    kind: ClusterIssuer
+    group: cert-manager.io
+---
+apiVersion: cert-manager.io/v1
+kind: ClusterIssuer
+metadata:
+  name: e2e-gateway-http
+spec:
+  ca:
+    secretName: e2e-gateway-http-ca

config/e2e/config.yaml

@@ -14,5 +14,12 @@ gateway:
   enableDNSIntegration: true
   permittedTLSOptions:
     gateway.networking.datumapis.com/certificate-issuer: []
+  # Resolve the `auto` issuer the defaulting webhook stamps on default-https to
+  # the e2e CA (config/e2e-downstream/issuer.yaml), as production resolves it to
+  # its ACME issuer. A gateway that declares no issuer of its own would
+  # otherwise get no certificate for that listener, and the listener would be
+  # withheld from the downstream gateway for the life of the test.
+  clusterIssuerMap:
+    auto: e2e-gateway-http
 downstreamResourceManagement:
   kubeconfigPath: /etc/downstream-cluster/kubeconfig

internal/controller/gateway_controller.go

@@ -299,16 +299,15 @@ func (r *GatewayReconciler) ensureDownstreamGateway(
 		listenerCertHealth,
 	)
 
-	// Only a hostname we could not claim makes the gateway dishonest about being
-	// programmed. A listener held back by an unhealthy certificate is a normal
-	// transient state that reports itself through the certificate conditions.
-	listenersDropped := false
-	for _, l := range upstreamGateway.Spec.Listeners {
-		if l.Hostname != nil && !slices.Contains(claimedHostnames, string(*l.Hostname)) {
-			listenersDropped = true
-			break
-		}
-	}
+	// A listener the user asked for that never reaches the downstream gateway
+	// makes the gateway dishonest about being programmed, whatever held it back.
+	// Derived from the listener set actually built, so a new reason to withhold
+	// one is covered without being enumerated here.
+	droppedListeners := summarizeDroppedListeners(
+		upstreamGateway,
+		desiredDownstreamGateway,
+		listenerCertHealth,
+	)
 
 	if len(desiredDownstreamGateway.Spec.Listeners) == 0 {
 		// The Gateway API requires at least one listener, so writing this would
@@ -388,7 +387,7 @@ func (r *GatewayReconciler) ensureDownstreamGateway(
 		upstreamClient,
 		upstreamGateway,
 		downstreamGateway,
-		listenersDropped,
+		droppedListeners,
 	)
 	if gatewayStatusResult.Err != nil || gatewayStatusResult.StopProcessing {
 		return gatewayStatusResult.Merge(result), nil
@@ -724,52 +723,53 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway(
 				"upstream_listener_index", listenerIndex, "listener", l.Name)
 		}
 
+		if l.Hostname == nil {
+			logger.Info("skipping downstream gateway listener with unset hostname",
+				"upstream_listener_index", listenerIndex, "listener", l.Name)
+			continue
+		}
+
 		// Per-listener TLS decision: hostnames covered by the wildcard
 		// (*.targetDomain) reference the pre-provisioned shared secret;
 		// all others reference a per-listener secret populated by a
 		// Certificate resource created in ensureListenerCertificates.
-		hostnameUnderWildcard := false
-		if l.Hostname != nil {
-			h := string(*l.Hostname)
-			hostnameUnderWildcard = strings.HasSuffix(h, wildcardSuffix) || h == r.Config.Gateway.TargetDomain
-		}
+		hostname := string(*l.Hostname)
+		hostnameUnderWildcard := strings.HasSuffix(hostname, wildcardSuffix) || hostname == r.Config.Gateway.TargetDomain
 		useSharedTLS := hostnameUnderWildcard && r.Config.Gateway.HasDefaultListenerTLSSecret()
 
-		if l.Hostname != nil {
-			listenerCopy := l.DeepCopy()
-			if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" {
-				delete(listenerCopy.TLS.Options, certificateIssuerTLSOption)
-
-				tlsMode := gatewayv1.TLSModeTerminate
-				if useSharedTLS {
-					listenerCopy.TLS = &gatewayv1.ListenerTLSConfig{
-						Mode: &tlsMode,
-						CertificateRefs: []gatewayv1.SecretObjectReference{
-							{
-								Group: ptr.To(gatewayv1.Group("")),
-								Kind:  ptr.To(gatewayv1.Kind("Secret")),
-								Name:  gatewayv1.ObjectName(r.Config.Gateway.DefaultListenerTLSSecretName),
-							},
+		listenerCopy := l.DeepCopy()
+		if l.TLS != nil && l.TLS.Options[certificateIssuerTLSOption] != "" {
+			delete(listenerCopy.TLS.Options, certificateIssuerTLSOption)
+
+			tlsMode := gatewayv1.TLSModeTerminate
+			if useSharedTLS {
+				listenerCopy.TLS = &gatewayv1.ListenerTLSConfig{
+					Mode: &tlsMode,
+					CertificateRefs: []gatewayv1.SecretObjectReference{
+						{
+							Group: ptr.To(gatewayv1.Group("")),
+							Kind:  ptr.To(gatewayv1.Kind("Secret")),
+							Name:  gatewayv1.ObjectName(r.Config.Gateway.DefaultListenerTLSSecretName),
 						},
-					}
-				} else {
-					// Secret name must match the Certificate created by
-					// ensureListenerCertificates for this listener.
-					listenerCopy.TLS = &gatewayv1.ListenerTLSConfig{
-						Mode: &tlsMode,
-						CertificateRefs: []gatewayv1.SecretObjectReference{
-							{
-								Group: ptr.To(gatewayv1.Group("")),
-								Kind:  ptr.To(gatewayv1.Kind("Secret")),
-								Name:  gatewayv1.ObjectName(listenerCertificateSecretName(upstreamGateway.Name, l.Name)),
-							},
+					},
+				}
+			} else {
+				// Secret name must match the Certificate created by
+				// ensureListenerCertificates for this listener.
+				listenerCopy.TLS = &gatewayv1.ListenerTLSConfig{
+					Mode: &tlsMode,
+					CertificateRefs: []gatewayv1.SecretObjectReference{
+						{
+							Group: ptr.To(gatewayv1.Group("")),
+							Kind:  ptr.To(gatewayv1.Kind("Secret")),
+							Name:  gatewayv1.ObjectName(listenerCertificateSecretName(upstreamGateway.Name, l.Name)),
 						},
-					}
+					},
 				}
 			}
-
-			listeners = append(listeners, *listenerCopy)
 		}
+
+		listeners = append(listeners, *listenerCopy)
 	}
 
 	// TODO(jreese) get from "scheduler"
@@ -780,6 +780,48 @@ func (r *GatewayReconciler) getDesiredDownstreamGateway(
 	return &downstreamGateway
 }
 
+// listenerDropReport names the spec listeners that never reached the downstream
+// gateway, and records whether an unusable certificate held back every one of
+// them.
+type listenerDropReport struct {
+	names           []gatewayv1.SectionName
+	allCertWithheld bool
+}
+
+// summarizeDroppedListeners compares the listeners the user asked for against
+// the ones the downstream gateway will carry. Working from the built set rather
+// than re-deriving each reason keeps a listener withheld by a future condition
+// from going unreported.
+func summarizeDroppedListeners(
+	upstreamGateway *gatewayv1.Gateway,
+	desiredDownstreamGateway *gatewayv1.Gateway,
+	listenerCertHealth map[gatewayv1.SectionName]listenerCertStatus,
+) listenerDropReport {
+	programmed := make(map[gatewayv1.SectionName]struct{}, len(desiredDownstreamGateway.Spec.Listeners))
+	for _, l := range desiredDownstreamGateway.Spec.Listeners {
+		programmed[l.Name] = struct{}{}
+	}
+
+	report := listenerDropReport{allCertWithheld: true}
+	for _, l := range upstreamGateway.Spec.Listeners {
+		if _, ok := programmed[l.Name]; ok {
+			continue
+		}
+
+		report.names = append(report.names, l.Name)
+
+		if status, gated := listenerCertHealth[l.Name]; !gated || status.healthy {
+			report.allCertWithheld = false
+		}
+	}
+
+	if len(report.names) == 0 {
+		report.allCertWithheld = false
+	}
+
+	return report
+}
+
 // listenerCertificateSecretName returns the deterministic Secret name that a
 // Certificate resource will populate for a given gateway listener.
 func listenerCertificateSecretName(gatewayName string, listenerName gatewayv1.SectionName) string {
@@ -1143,7 +1185,7 @@ func (r *GatewayReconciler) reconcileGatewayStatus(
 	upstreamClient client.Client,
 	upstreamGateway *gatewayv1.Gateway,
 	downstreamGateway *gatewayv1.Gateway,
-	listenersDropped bool,
+	droppedListeners listenerDropReport,
 ) (result Result) {
 	logger := log.FromContext(ctx)
 
@@ -1177,10 +1219,19 @@ func (r *GatewayReconciler) reconcileGatewayStatus(
 			programmedReady = true
 		}
 
-		if listenersDropped {
+		if len(droppedListeners.names) > 0 {
 			message = "One or more listeners could not be programmed. See the listener status for the reason."
-			status = metav1.ConditionFalse
 			reason = string(gatewayv1.GatewayReasonListenersNotValid)
+
+			// A listener waiting on a usable certificate is still missing from the
+			// edge, so the gateway is not programmed. It is reported apart from a
+			// listener dropped for any other reason because it clears on its own.
+			if droppedListeners.allCertWithheld {
+				message = "One or more listeners are waiting on a usable certificate. See the listener status for the reason."
+				reason = string(gatewayv1.GatewayReasonPending)
+			}
+
+			status = metav1.ConditionFalse
 			programmedReady = false
 		}