Skip to content

Commit 0b6edb2

Browse files
committed
Invalidate h2c support for http route
1 parent dfc6066 commit 0b6edb2

File tree

4 files changed

+16
-155
lines changed

4 files changed

+16
-155
lines changed

internal/controller/state/graph/backend_refs.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,10 +453,14 @@ func validateRouteBackendRefAppProtocol(
453453
// Currently we only support recognition of the Kubernetes Standard Application Protocols defined in KEP-3726.
454454
switch appProtocol {
455455
case AppProtocolTypeH2C:
456-
if routeType == RouteTypeHTTP || routeType == RouteTypeGRPC {
456+
if routeType == RouteTypeGRPC {
457457
return nil
458458
}
459459

460+
if routeType == RouteTypeHTTP {
461+
return fmt.Errorf("%w; nginx does not support proxying to upstreams with http2 or h2c", err)
462+
}
463+
460464
return err
461465
case AppProtocolTypeWS:
462466
if routeType == RouteTypeHTTP {

internal/controller/state/graph/backend_refs_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -700,14 +700,19 @@ func TestAddBackendRefsToRules(t *testing.T) {
700700
{
701701
SvcNsName: svcH2cNsName,
702702
ServicePort: svcH2c.Spec.Ports[0],
703-
Valid: true,
703+
Valid: false,
704704
Weight: 1,
705705
InvalidForGateways: map[types.NamespacedName]conditions.Condition{},
706706
},
707707
},
708-
expectedConditions: nil,
709-
policies: emptyPolicies,
710-
name: "valid backendRef with service port appProtocol h2c and route type http",
708+
expectedConditions: []conditions.Condition{
709+
conditions.NewRouteBackendRefUnsupportedProtocol(
710+
"route type http does not support service port appProtocol kubernetes.io/h2c;" +
711+
" nginx does not support proxying to upstreams with http2 or h2c",
712+
),
713+
},
714+
policies: emptyPolicies,
715+
name: "invalid backendRef with service port appProtocol h2c and route type http",
711716
},
712717
{
713718
route: createRoute("hr1", RouteTypeHTTP, "Service", 1, "svcWS"),

internal/controller/state/graph/route_common.go

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -766,13 +766,11 @@ func bindL7RouteToListeners(
766766
continue
767767
}
768768

769-
for ruleIdx, rule := range route.Spec.Rules {
770-
for backendRefIdx, backendRef := range rule.BackendRefs {
769+
for _, rule := range route.Spec.Rules {
770+
for _, backendRef := range rule.BackendRefs {
771771
if cond, ok := backendRef.InvalidForGateways[gwNsName]; ok {
772772
attachment.FailedConditions = append(attachment.FailedConditions, cond)
773773
}
774-
775-
checkAppProtocolH2CConditional(backendRef, gw.EffectiveNginxProxy, route, ruleIdx, backendRefIdx)
776774
}
777775
}
778776

@@ -797,29 +795,6 @@ func bindL7RouteToListeners(
797795
}
798796
}
799797

800-
func checkAppProtocolH2CConditional(
801-
backendRef BackendRef,
802-
npCfg *EffectiveNginxProxy,
803-
route *L7Route,
804-
ruleIdx,
805-
backendRefIdx int,
806-
) {
807-
// For all backendRefs referring to a Service with appProtocol h2c, we need to also check if http2
808-
// is enabled. Don't need to check for RouteTypeGRPC since there are checks before this which fail the
809-
// route from connecting to the Gateway.
810-
if backendRef.ServicePort.AppProtocol != nil &&
811-
*backendRef.ServicePort.AppProtocol == AppProtocolTypeH2C &&
812-
route.RouteType == RouteTypeHTTP &&
813-
isHTTP2Disabled(npCfg) {
814-
route.Spec.Rules[ruleIdx].BackendRefs[backendRefIdx].Valid = false
815-
route.Conditions = append(route.Conditions, conditions.NewRouteBackendRefUnsupportedProtocol(
816-
fmt.Errorf("HTTP2 is disabled - cannot support appProtocol h2c on route type %s",
817-
route.RouteType,
818-
).Error(),
819-
))
820-
}
821-
}
822-
823798
func isHTTP2Disabled(npCfg *EffectiveNginxProxy) bool {
824799
if npCfg == nil {
825800
return false

internal/controller/state/graph/route_common_test.go

Lines changed: 0 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -394,38 +394,6 @@ func TestBindRouteToListeners(t *testing.T) {
394394
return normalHTTPRoute
395395
}
396396

397-
normalHTTPRouteWithH2CBackendRef := &L7Route{
398-
RouteType: RouteTypeHTTP,
399-
Source: hr,
400-
Spec: L7RouteSpec{
401-
Hostnames: hr.Spec.Hostnames,
402-
Rules: []RouteRule{
403-
{
404-
BackendRefs: []BackendRef{
405-
{
406-
Valid: true,
407-
ServicePort: v1.ServicePort{
408-
AppProtocol: helpers.GetPointer(AppProtocolTypeH2C),
409-
},
410-
},
411-
},
412-
},
413-
},
414-
},
415-
Valid: true,
416-
Attachable: true,
417-
ParentRefs: []ParentRef{
418-
{
419-
Idx: 0,
420-
Gateway: &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
421-
SectionName: hr.Spec.ParentRefs[0].SectionName,
422-
},
423-
},
424-
}
425-
426-
invalidBackendRefH2c := *normalHTTPRouteWithH2CBackendRef
427-
invalidBackendRefH2c.Spec.Rules[0].BackendRefs[0].Valid = false
428-
429397
getLastNormalHTTPRoute := func() *L7Route {
430398
return normalHTTPRoute
431399
}
@@ -603,7 +571,6 @@ func TestBindRouteToListeners(t *testing.T) {
603571
tests := []struct {
604572
route *L7Route
605573
gateway *Gateway
606-
expectedModifiedRoute *L7Route
607574
name string
608575
expectedGatewayListeners []*Listener
609576
expectedSectionNameRefs []ParentRef
@@ -643,90 +610,6 @@ func TestBindRouteToListeners(t *testing.T) {
643610
},
644611
name: "normal case",
645612
},
646-
{
647-
route: normalHTTPRouteWithH2CBackendRef,
648-
gateway: &Gateway{
649-
Source: gw,
650-
Valid: true,
651-
Listeners: []*Listener{
652-
createListener("listener-80-1"),
653-
},
654-
EffectiveNginxProxy: &EffectiveNginxProxy{
655-
DisableHTTP2: helpers.GetPointer(false),
656-
},
657-
},
658-
expectedSectionNameRefs: []ParentRef{
659-
{
660-
Idx: 0,
661-
Gateway: &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
662-
SectionName: hr.Spec.ParentRefs[0].SectionName,
663-
Attachment: &ParentRefAttachmentStatus{
664-
Attached: true,
665-
AcceptedHostnames: map[string][]string{
666-
CreateGatewayListenerKey(
667-
client.ObjectKeyFromObject(gw),
668-
"listener-80-1",
669-
): {"foo.example.com"},
670-
},
671-
},
672-
},
673-
},
674-
expectedGatewayListeners: []*Listener{
675-
createModifiedListener("listener-80-1", func(l *Listener) {
676-
l.Routes = map[RouteKey]*L7Route{
677-
CreateRouteKey(hr): normalHTTPRouteWithH2CBackendRef,
678-
}
679-
}),
680-
},
681-
name: "httpRoute with h2c service port protocol in backend and h2c is enabled",
682-
},
683-
{
684-
route: normalHTTPRouteWithH2CBackendRef,
685-
gateway: &Gateway{
686-
Source: gw,
687-
Valid: true,
688-
Listeners: []*Listener{
689-
createListener("listener-80-1"),
690-
},
691-
EffectiveNginxProxy: &EffectiveNginxProxy{
692-
DisableHTTP2: helpers.GetPointer(true),
693-
},
694-
},
695-
expectedSectionNameRefs: []ParentRef{
696-
{
697-
Idx: 0,
698-
Gateway: &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
699-
SectionName: hr.Spec.ParentRefs[0].SectionName,
700-
Attachment: &ParentRefAttachmentStatus{
701-
Attached: true,
702-
AcceptedHostnames: map[string][]string{
703-
CreateGatewayListenerKey(
704-
client.ObjectKeyFromObject(gw),
705-
"listener-80-1",
706-
): {"foo.example.com"},
707-
},
708-
},
709-
},
710-
},
711-
expectedGatewayListeners: []*Listener{
712-
createModifiedListener("listener-80-1", func(l *Listener) {
713-
route := *normalHTTPRouteWithH2CBackendRef
714-
route.Conditions = []conditions.Condition{
715-
conditions.NewRouteBackendRefUnsupportedProtocol(
716-
"HTTP2 is disabled - cannot support appProtocol h2c on route type http"),
717-
}
718-
l.Routes = map[RouteKey]*L7Route{
719-
CreateRouteKey(hr): &route,
720-
}
721-
}),
722-
},
723-
expectedConditions: []conditions.Condition{
724-
conditions.NewRouteBackendRefUnsupportedProtocol(
725-
"HTTP2 is disabled - cannot support appProtocol h2c on route type http"),
726-
},
727-
expectedModifiedRoute: &invalidBackendRefH2c,
728-
name: "httpRoute with h2c service port protocol in backend and h2c is disabled",
729-
},
730613
{
731614
route: routeWithMissingSectionName,
732615
gateway: &Gateway{
@@ -1515,12 +1398,6 @@ func TestBindRouteToListeners(t *testing.T) {
15151398
g.Expect(test.route.ParentRefs).To(Equal(test.expectedSectionNameRefs))
15161399
g.Expect(helpers.Diff(test.gateway.Listeners, test.expectedGatewayListeners)).To(BeEmpty())
15171400
g.Expect(helpers.Diff(test.route.Conditions, test.expectedConditions)).To(BeEmpty())
1518-
1519-
// in situations where bindRouteToListeners modifies the route spec, for instance marking a backendRef
1520-
// as invalid
1521-
if test.expectedModifiedRoute != nil {
1522-
g.Expect(helpers.Diff(test.route.Spec, test.expectedModifiedRoute.Spec)).To(BeEmpty())
1523-
}
15241401
})
15251402
}
15261403
}

0 commit comments

Comments
 (0)