fix: Show the offline page when a backend or tunnel is down

internal/extensionserver/metrics/metrics.go

@@ -163,15 +163,23 @@ var (
 	)
 
 	// ConnectorOfflineRoutesTotal counts total user-facing forwarding routes
-	// rewritten to a tunnel-offline 503 direct_response across all hook
-	// invocations. Each increment = one route that previously targeted an
-	// endpoint-less offline-connector cluster and now returns a deterministic
-	// 503 (instead of Envoy's generic 503 no_healthy_upstream). Use rate()/sum()
-	// to derive per-build averages.
+	// moved off an offline connector's own cluster across all hook invocations.
+	// Use rate()/sum() to derive per-build averages.
 	ConnectorOfflineRoutesTotal = promauto.NewCounter(
 		prometheus.CounterOpts{
 			Name: "nso_extension_connector_offline_routes_total",
-			Help: "Total user-facing forwarding routes rewritten to a tunnel-offline 503 direct_response across all hook invocations.",
+			Help: "Total user-facing forwarding routes moved off an offline connector cluster across all hook invocations.",
+		},
+	)
+
+	// EmptyBackendRoutesTotal counts total routes with no ready endpoints
+	// pointed at the shared offline backend cluster across all hook
+	// invocations. Each increment is one route that now answers with the
+	// branded offline page instead of the generic error page.
+	EmptyBackendRoutesTotal = promauto.NewCounter(
+		prometheus.CounterOpts{
+			Name: "nso_extension_empty_backend_routes_total",
+			Help: "Total routes with no ready endpoints rewritten to the shared offline backend cluster across all hook invocations.",
 		},
 	)
 

internal/extensionserver/mutate/connector.go

@@ -75,15 +75,21 @@ func ReplaceConnectorClusters(
 //   - Online (replaced) connector: prepend a CONNECT upgrade route targeting the
 //     replaced cluster and append a unique per-connector domain to VH domains.
 //   - Offline connector: prepend a CONNECT direct_response 503 (tunnel-control
-//     clients) and rewrite the user-facing forwarding routes to a 503
-//     direct_response (see the offline branch for why).
+//     clients) and rewrite the user-facing forwarding routes (see the offline
+//     branch for why).
 //
-// Returns the number of VirtualHosts mutated and the number of forwarding
-// routes converted to a tunnel-offline direct_response.
+// brandedOffline selects what a user reaching an offline tunnel gets: the
+// shared endpoint-less cluster when there is a branded error page to serve,
+// otherwise the deterministic 503 they have always returned. The CONNECT route
+// is unaffected either way, since it answers the connector agent, not a browser.
+//
+// Returns the number of VirtualHosts mutated and the number of user-facing
+// forwarding routes rewritten.
 func ApplyConnectorRoutes(
 	rc *routev3.RouteConfiguration,
 	idx *extcache.PolicyIndex,
 	replaced, offline map[string]*extcache.ConnectorInfo,
+	brandedOffline bool,
 ) (mutated, converted int, err error) {
 	for _, vh := range rc.GetVirtualHosts() {
 		// Find any connector cluster referenced by routes in this VH.
@@ -129,16 +135,24 @@ func ApplyConnectorRoutes(
 			}
 			vh.Routes = append([]*routev3.Route{newRoute}, vh.Routes...)
 
-			// Route user traffic to a deterministic 503 instead of the
-			// endpoint-less offline cluster, which would yield a generic
-			// no_healthy_upstream plus retry/cluster-stat noise. Replacing only
-			// the Action oneof preserves each route's match/metadata; idempotent
-			// because direct_responses carry no cluster to re-match.
+			// Move user traffic off the connector's own endpoint-less
+			// cluster, which would otherwise report retry and connect
+			// failures against a per-connector cluster. Replacing only the
+			// Action oneof preserves each route's match and metadata, and
+			// neither replacement leaves the connector cluster to re-match.
 			for _, rt := range vh.GetRoutes() {
 				if routeCluster(rt) != connectorCluster {
 					continue
 				}
-				if derr := setRouteDirectResponse(rt, 503, offlineResponseBody); derr != nil {
+				if brandedOffline {
+					rt.Action = &routev3.Route_Route{
+						Route: &routev3.RouteAction{
+							ClusterSpecifier: &routev3.RouteAction_Cluster{
+								Cluster: OfflineTunnelClusterName,
+							},
+						},
+					}
+				} else if derr := setRouteDirectResponse(rt, 503, offlineResponseBody); derr != nil {
 					return mutated, converted, fmt.Errorf("convert offline forward route for %q: %w", vh.GetName(), derr)
 				}
 				converted++

internal/extensionserver/mutate/connector_test.go

@@ -207,7 +207,7 @@ func TestApplyConnectorRoutes_Online_PrependsCONNECTRouteAndAppendsUniqueDomain(
 		},
 	}
 
-	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 1, n, "one VH should be mutated")
 	assert.Equal(t, 0, converted, "online connector must not convert any forwarding routes")
@@ -257,7 +257,7 @@ func TestApplyConnectorRoutes_Online_DomainAppendIsIdempotent(t *testing.T) {
 		},
 	}
 
-	_, _, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	_, _, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 
 	// Synthetic domain must not be duplicated.
@@ -295,7 +295,7 @@ func TestApplyConnectorRoutes_Online_NoDomainCollisionAcrossConnectors(t *testin
 		},
 	}
 
-	_, _, err := ApplyConnectorRoutes(rc, &extcache.PolicyIndex{}, replaced, offline)
+	_, _, err := ApplyConnectorRoutes(rc, &extcache.PolicyIndex{}, replaced, offline, false)
 	require.NoError(t, err)
 
 	// No domain may appear more than once across the whole route config.
@@ -333,7 +333,7 @@ func TestApplyConnectorRoutes_Offline_Prepends503Route_NoDomain(t *testing.T) {
 		},
 	}
 
-	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 1, n, "offline VH must be mutated (503 route prepended)")
 	assert.Equal(t, 1, converted, "the user-facing forwarding route must be converted to a direct_response")
@@ -404,7 +404,7 @@ func TestApplyConnectorRoutes_Offline_PreservesMatchAndUntouchedRoute(t *testing
 		},
 	}
 
-	_, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	_, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 1, converted, "only the connector forwarding route must be converted")
 
@@ -447,12 +447,12 @@ func TestApplyConnectorRoutes_Offline_Idempotent(t *testing.T) {
 		},
 	}
 
-	_, converted1, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	_, converted1, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 1, converted1, "first pass converts the forwarding route")
 
 	// Second pass: the cluster is gone from all routes, so nothing converts.
-	_, converted2, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	_, converted2, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 0, converted2, "second pass must not re-convert any route")
 
@@ -483,7 +483,7 @@ func TestApplyConnectorRoutes_NoConnector_VHUntouched(t *testing.T) {
 		},
 	}
 
-	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 0, n, "VH with non-connector cluster must not be mutated")
 	assert.Equal(t, 0, converted, "no forwarding routes converted when no connector present")
@@ -498,8 +498,55 @@ func TestApplyConnectorRoutes_EmptyRouteConfiguration_NoOp(t *testing.T) {
 
 	rc := &routev3.RouteConfiguration{Name: "empty"}
 
-	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline)
+	n, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline, false)
 	require.NoError(t, err)
 	assert.Equal(t, 0, n)
 	assert.Equal(t, 0, converted)
 }
+
+// Only a forwarded request carries the UH flag the offline page selects on, so
+// with the branded page configured the user-facing routes must forward.
+func TestApplyConnectorRoutes_Offline_BrandedRoutesToTunnelCluster(t *testing.T) {
+	idx := connectorPolicyIndex(false)
+	clusterName := testClusterName()
+
+	offlineInfo := &extcache.ConnectorInfo{Online: false, TargetHost: testTargetHost, TargetPort: testTargetPort}
+	replaced := map[string]*extcache.ConnectorInfo{}
+	offline := map[string]*extcache.ConnectorInfo{clusterName: offlineInfo}
+
+	connRoute := routeTargeting(clusterName)
+	connRoute.Match = &routev3.RouteMatch{PathSpecifier: &routev3.RouteMatch_Prefix{Prefix: "/"}}
+	connRoute.TypedPerFilterConfig = map[string]*anypb.Any{
+		"envoy.filters.http.cors": {TypeUrl: "type.googleapis.com/example.Cfg"},
+	}
+
+	rc := &routev3.RouteConfiguration{
+		VirtualHosts: []*routev3.VirtualHost{
+			{Name: "vh", Domains: []string{"app.local.test"}, Routes: []*routev3.Route{connRoute}},
+		},
+	}
+
+	_, converted, err := ApplyConnectorRoutes(rc, idx, replaced, offline, true)
+	require.NoError(t, err)
+	assert.Equal(t, 1, converted)
+
+	vh := rc.VirtualHosts[0]
+	require.Len(t, vh.Routes, 2, "connect_matcher route prepended to the original")
+
+	// The CONNECT route answers the connector agent, not a browser, so it keeps
+	// its terse direct_response either way.
+	gotConnect := vh.Routes[0]
+	require.NotNil(t, gotConnect.GetDirectResponse(), "CONNECT route must stay a direct_response")
+	assert.Equal(t, offlineResponseBody,
+		gotConnect.GetDirectResponse().GetBody().GetInlineString())
+
+	gotUser := vh.Routes[1]
+	assert.Nil(t, gotUser.GetDirectResponse(), "user route must not be a direct_response")
+	assert.Equal(t, OfflineTunnelClusterName, routeCluster(gotUser),
+		"user route must forward to the shared offline-tunnel cluster")
+	assert.NotEqual(t, OfflineBackendClusterName, routeCluster(gotUser),
+		"an offline tunnel must not share the empty-backend sink")
+	assert.Equal(t, "/", gotUser.GetMatch().GetPrefix(), "prefix match must be preserved")
+	assert.Contains(t, gotUser.GetTypedPerFilterConfig(), "envoy.filters.http.cors",
+		"typed_per_filter_config must be preserved on the rewritten route")
+}

internal/extensionserver/mutate/emptybackend.go

@@ -0,0 +1,86 @@
+package mutate
+
+import (
+	"fmt"
+
+	clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
+	routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
+	"google.golang.org/protobuf/encoding/protojson"
+)
+
+const (
+	// OfflineBackendClusterName serves a backend with no ready endpoints.
+	OfflineBackendClusterName = "datum-offline-backend"
+	// OfflineTunnelClusterName serves an offline connector tunnel.
+	OfflineTunnelClusterName = "datum-offline-tunnel"
+)
+
+// emptyBackendStatus is the status Envoy Gateway collapses a route to when the
+// backend Service exists but has no ready endpoints. It is the only collapse
+// case EG answers with 503; every other one uses 500.
+const emptyBackendStatus = 503
+
+// EnsureOfflineCluster appends the named endpoint-less cluster if it is absent,
+// returning the cluster set and whether it added one. A request routed there
+// fails at host selection, so Envoy answers with the UH response flag the
+// branded offline page selects on.
+func EnsureOfflineCluster(clusters []*clusterv3.Cluster, name string) ([]*clusterv3.Cluster, bool, error) {
+	for _, c := range clusters {
+		if c.GetName() == name {
+			return clusters, false, nil
+		}
+	}
+
+	j := fmt.Sprintf(`{
+  "name": %q,
+  "type": "STATIC",
+  "connect_timeout": "1s",
+  "load_assignment": { "cluster_name": %q, "endpoints": [] }
+}`, name, name)
+
+	c := &clusterv3.Cluster{}
+	if err := protojson.Unmarshal([]byte(j), c); err != nil {
+		return clusters, false, fmt.Errorf("unmarshal offline cluster %q JSON: %w", name, err)
+	}
+	return append(clusters, c), true, nil
+}
+
+// RouteEmptyBackendsToOfflineCluster points every route Envoy Gateway collapsed
+// to a bodiless 503 at the shared endpoint-less cluster, returning how many it
+// rewrote. A direct_response short-circuits before the router filter, so it
+// carries neither the UH flag nor any route-supplied header the offline page
+// could select on, and its body is overridden by local_reply_config. Forwarding
+// restores the flag.
+//
+// Replacing only the Action oneof preserves each route's match, metadata and
+// per-filter config, and makes a second pass a no-op.
+func RouteEmptyBackendsToOfflineCluster(rc *routev3.RouteConfiguration) int {
+	rewritten := 0
+	for _, vh := range rc.GetVirtualHosts() {
+		for _, rt := range vh.GetRoutes() {
+			if !isEmptyBackendDirectResponse(rt) {
+				continue
+			}
+			rt.Action = &routev3.Route_Route{
+				Route: &routev3.RouteAction{
+					ClusterSpecifier: &routev3.RouteAction_Cluster{
+						Cluster: OfflineBackendClusterName,
+					},
+				},
+			}
+			rewritten++
+		}
+	}
+	return rewritten
+}
+
+// isEmptyBackendDirectResponse reports whether a route is EG's "no ready
+// endpoints" collapse. The absent body is what separates it from NSO's own
+// connector-offline routes and from a Gateway API filter's direct response.
+func isEmptyBackendDirectResponse(rt *routev3.Route) bool {
+	dr := rt.GetDirectResponse()
+	if dr == nil || dr.GetStatus() != emptyBackendStatus {
+		return false
+	}
+	return dr.GetBody() == nil
+}