Correct h2c conditional check

Author: bjee19

internal/controller/state/graph/route_common.go

@@ -766,13 +766,13 @@ func bindL7RouteToListeners(
 			continue
 		}
 
-		for _, rule := range route.Spec.Rules {
-			for _, backendRef := range rule.BackendRefs {
+		for ruleIdx, rule := range route.Spec.Rules {
+			for backendRefIdx, backendRef := range rule.BackendRefs {
 				if cond, ok := backendRef.InvalidForGateways[gwNsName]; ok {
 					attachment.FailedConditions = append(attachment.FailedConditions, cond)
 				}
 
-				checkAppProtocolH2CConditional(backendRef, gw.EffectiveNginxProxy, route)
+				checkAppProtocolH2CConditional(backendRef, gw.EffectiveNginxProxy, route, ruleIdx, backendRefIdx)
 			}
 		}
 
@@ -797,15 +797,21 @@ func bindL7RouteToListeners(
 	}
 }
 
-func checkAppProtocolH2CConditional(backendRef BackendRef, npCfg *EffectiveNginxProxy, route *L7Route) {
+func checkAppProtocolH2CConditional(
+	backendRef BackendRef,
+	npCfg *EffectiveNginxProxy,
+	route *L7Route,
+	ruleIdx,
+	backendRefIdx int,
+) {
 	// For all backendRefs referring to a Service with appProtocol h2c, we need to also check if http2
 	// is enabled. Don't need to check for RouteTypeGRPC since there are checks before this which fail the
 	// route from connecting to the Gateway.
 	if backendRef.ServicePort.AppProtocol != nil &&
 		*backendRef.ServicePort.AppProtocol == AppProtocolTypeH2C &&
 		route.RouteType == RouteTypeHTTP &&
 		isHTTP2Disabled(npCfg) {
-		backendRef.Valid = false
+		route.Spec.Rules[ruleIdx].BackendRefs[backendRefIdx].Valid = false
 		route.Conditions = append(route.Conditions, conditions.NewRouteBackendRefUnsupportedProtocol(
 			fmt.Errorf("HTTP2 is disabled - cannot support appProtocol h2c on route type %s",
 				route.RouteType,

internal/controller/state/graph/route_common_test.go

@@ -403,6 +403,7 @@ func TestBindRouteToListeners(t *testing.T) {
 				{
 					BackendRefs: []BackendRef{
 						{
+							Valid: true,
 							ServicePort: v1.ServicePort{
 								AppProtocol: helpers.GetPointer(AppProtocolTypeH2C),
 							},
@@ -422,6 +423,9 @@ func TestBindRouteToListeners(t *testing.T) {
 		},
 	}
 
+	invalidBackendRefH2c := *normalHTTPRouteWithH2CBackendRef
+	invalidBackendRefH2c.Spec.Rules[0].BackendRefs[0].Valid = false
+
 	getLastNormalHTTPRoute := func() *L7Route {
 		return normalHTTPRoute
 	}
@@ -599,8 +603,9 @@ func TestBindRouteToListeners(t *testing.T) {
 	tests := []struct {
 		route                    *L7Route
 		gateway                  *Gateway
-		expectedGatewayListeners []*Listener
+		expectedModifiedRoute    *L7Route
 		name                     string
+		expectedGatewayListeners []*Listener
 		expectedSectionNameRefs  []ParentRef
 		expectedConditions       []conditions.Condition
 	}{
@@ -719,7 +724,8 @@ func TestBindRouteToListeners(t *testing.T) {
 				conditions.NewRouteBackendRefUnsupportedProtocol(
 					"HTTP2 is disabled - cannot support appProtocol h2c on route type http"),
 			},
-			name: "httpRoute with h2c service port protocol in backend and h2c is disabled",
+			expectedModifiedRoute: &invalidBackendRefH2c,
+			name:                  "httpRoute with h2c service port protocol in backend and h2c is disabled",
 		},
 		{
 			route: routeWithMissingSectionName,
@@ -1509,6 +1515,12 @@ func TestBindRouteToListeners(t *testing.T) {
 			g.Expect(test.route.ParentRefs).To(Equal(test.expectedSectionNameRefs))
 			g.Expect(helpers.Diff(test.gateway.Listeners, test.expectedGatewayListeners)).To(BeEmpty())
 			g.Expect(helpers.Diff(test.route.Conditions, test.expectedConditions)).To(BeEmpty())
+
+			// in situations where bindRouteToListeners modifies the route spec, for instance marking a backendRef
+			// as invalid
+			if test.expectedModifiedRoute != nil {
+				g.Expect(helpers.Diff(test.route.Spec, test.expectedModifiedRoute.Spec)).To(BeEmpty())
+			}
 		})
 	}
 }

tests/Makefile

@@ -12,7 +12,7 @@ GW_SERVICE_TYPE = NodePort## Service type to use for the gateway
 NGF_VERSION ?= edge## NGF version to be tested
 PULL_POLICY = Never## Pull policy for the images
 NGINX_CONF_DIR = internal/controller/nginx/conf
-SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolWebSocket,HTTPRouteBackendProtocolH2C
+SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteBackendProtocolWebSocket
 STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC
 EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS
 CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles.