refactor app protocol, add support for app protocols that kubernetes + elb supports

pkg/gateway/model/model_build_target_group.go

@@ -266,7 +266,7 @@ func (builder *targetGroupBuilderImpl) buildTargetGroupSpec(gw *gwv1.Gateway, ro
 	if err != nil {
 		return elbv2model.TargetGroupSpec{}, err
 	}
-	tgProtocolVersion := builder.buildTargetGroupProtocolVersion(targetGroupProps, route)
+	tgProtocolVersion := builder.buildTargetGroupProtocolVersion(targetGroupProps, backendConfig, listenerProtocol, route)
 
 	healthCheckConfig, err := builder.buildTargetGroupHealthCheckConfig(targetGroupProps, tgProtocol, tgProtocolVersion, targetType, backendConfig)
 	if err != nil {
@@ -423,13 +423,18 @@ var (
 	grpc  = elbv2model.ProtocolVersionGRPC
 )
 
-func (builder *targetGroupBuilderImpl) buildTargetGroupProtocolVersion(targetGroupProps *elbv2gw.TargetGroupProps, route routeutils.RouteDescriptor) *elbv2model.ProtocolVersion {
+func (builder *targetGroupBuilderImpl) buildTargetGroupProtocolVersion(targetGroupProps *elbv2gw.TargetGroupProps, backendConfig routeutils.TargetGroupConfigurator, listenerProtocol elbv2model.Protocol, route routeutils.RouteDescriptor) *elbv2model.ProtocolVersion {
 	// NLB doesn't support protocol version
 	if builder.loadBalancerType == elbv2model.LoadBalancerTypeNetwork {
 		return nil
 	}
+
+	// As of writing, only HTTPS listeners support different Protocol Version
+	if listenerProtocol != elbv2model.ProtocolHTTPS {
+		return &http1
+	}
+
 	if targetGroupProps != nil && targetGroupProps.ProtocolVersion != nil {
-		// TODO - We can infer GRPC here from route
 		pv := elbv2model.ProtocolVersion(*targetGroupProps.ProtocolVersion)
 		return &pv
 	}
@@ -438,6 +443,11 @@ func (builder *targetGroupBuilderImpl) buildTargetGroupProtocolVersion(targetGro
 		return &grpc
 	}
 
+	resolvedProtocolVersion := backendConfig.GetProtocolVersion()
+	if resolvedProtocolVersion != nil {
+		return resolvedProtocolVersion
+	}
+
 	return &http1
 }
 

pkg/gateway/model/model_build_target_group_test.go

@@ -1172,52 +1172,165 @@ func Test_buildTargetGroupProtocolVersion(t *testing.T) {
 	testCases := []struct {
 		name             string
 		loadBalancerType elbv2model.LoadBalancerType
+		tgConfigurator   routeutils.TargetGroupConfigurator
+		listenerProtocol elbv2model.Protocol
 		route            routeutils.RouteDescriptor
 		targetGroupProps *elbv2gw.TargetGroupProps
 		expected         *elbv2model.ProtocolVersion
 	}{
 		{
 			name:             "nlb - no props",
 			loadBalancerType: elbv2model.LoadBalancerTypeNetwork,
-			route:            &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
+			listenerProtocol: elbv2model.ProtocolTCP,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol: corev1.ProtocolTCP,
+				Port:     80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
+			route: &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
 		},
 		{
 			name:             "nlb - with props",
 			loadBalancerType: elbv2model.LoadBalancerTypeNetwork,
-			route:            &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
+			listenerProtocol: elbv2model.ProtocolTCP,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol: corev1.ProtocolTCP,
+				Port:     80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
+			route: &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
 			targetGroupProps: &elbv2gw.TargetGroupProps{
 				ProtocolVersion: &http2Gw,
 			},
 		},
 		{
 			name:             "alb - no props",
+			listenerProtocol: elbv2model.ProtocolHTTPS,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol: corev1.ProtocolTCP,
+				Port:     80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
 			route:            &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
 			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
 			expected:         &http1Elb,
 		},
 		{
 			name:             "alb - no props - grpc",
+			listenerProtocol: elbv2model.ProtocolHTTPS,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol: corev1.ProtocolTCP,
+				Port:     80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
 			route:            &routeutils.MockRoute{Kind: routeutils.GRPCRouteKind},
 			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
 			expected:         &grpcElb,
 		},
 		{
 			name:             "alb - with props",
+			listenerProtocol: elbv2model.ProtocolHTTPS,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol: corev1.ProtocolTCP,
+				Port:     80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
 			route:            &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
 			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
 			targetGroupProps: &elbv2gw.TargetGroupProps{
 				ProtocolVersion: &http2Gw,
 			},
 			expected: &http2Elb,
 		},
+		{
+			name:             "alb - with props - http protocol",
+			listenerProtocol: elbv2model.ProtocolHTTP,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol: corev1.ProtocolTCP,
+				Port:     80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
+			route:            &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
+			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
+			targetGroupProps: &elbv2gw.TargetGroupProps{
+				ProtocolVersion: &http2Gw,
+			},
+			expected: &http1Elb,
+		},
+		{
+			name:             "alb - pv found on svc port - http listener",
+			listenerProtocol: elbv2model.ProtocolHTTP,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol:    corev1.ProtocolTCP,
+				Port:        80,
+				AppProtocol: awssdk.String("kubernetes.io/h2c"),
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
+			route:            &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
+			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
+			expected:         &http1Elb,
+		},
+		{
+			name:             "alb - pv found on svc port - https listener",
+			listenerProtocol: elbv2model.ProtocolHTTPS,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol:    corev1.ProtocolTCP,
+				Port:        80,
+				AppProtocol: awssdk.String("kubernetes.io/h2c"),
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
+			route:            &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
+			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
+			expected:         &http2Elb,
+		},
+		{
+			name:             "alb - unknown pv found on svc port - https listener",
+			listenerProtocol: elbv2model.ProtocolHTTPS,
+			tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
+				Protocol:    corev1.ProtocolTCP,
+				Port:        80,
+				AppProtocol: awssdk.String("foo"),
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+					Type:   intstr.Int,
+				},
+			}),
+			route:            &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
+			loadBalancerType: elbv2model.LoadBalancerTypeApplication,
+			expected:         &http1Elb,
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			builder := targetGroupBuilderImpl{
 				loadBalancerType: tc.loadBalancerType,
 			}
-			res := builder.buildTargetGroupProtocolVersion(tc.targetGroupProps, tc.route)
+			res := builder.buildTargetGroupProtocolVersion(tc.targetGroupProps, tc.tgConfigurator, tc.listenerProtocol, tc.route)
 			assert.Equal(t, tc.expected, res)
 		})
 	}

pkg/gateway/routeutils/backend.go

@@ -48,6 +48,8 @@ type TargetGroupConfigurator interface {
 	GetTargetGroupPort(targetType elbv2model.TargetType) int32
 	// GetHealthCheckPort returns the port to send health check traffic
 	GetHealthCheckPort(targetType elbv2model.TargetType, isServiceExternalTrafficPolicyTypeLocal bool) (intstr.IntOrString, error)
+	// GetProtocolVersion returns the protocol version to use for this target group
+	GetProtocolVersion() *elbv2model.ProtocolVersion
 }
 
 // Backend an abstraction on the Gateway Backend, meant to hide the underlying backend type from consumers (unless they really want to see it :))

pkg/gateway/routeutils/backend_gateway.go

@@ -27,6 +27,11 @@ type GatewayBackendConfig struct {
 	port             int32
 }
 
+func (g *GatewayBackendConfig) GetProtocolVersion() *elbv2model.ProtocolVersion {
+	// Gateway backends don't support a protocol version.
+	return nil
+}
+
 func NewGatewayBackendConfig(gateway *gwv1.Gateway, targetGroupProps *elbv2gw.TargetGroupProps, arn string, port int32) *GatewayBackendConfig {
 	return &GatewayBackendConfig{
 		gateway:          gateway,

pkg/gateway/routeutils/backend_gateway_test.go

@@ -248,3 +248,8 @@ func TestValidateGatewayARN(t *testing.T) {
 		})
 	}
 }
+
+func TestGatewayBackendConfig_GetProtocolVersion(t *testing.T) {
+	config := &GatewayBackendConfig{}
+	assert.Nil(t, config.GetProtocolVersion())
+}

pkg/gateway/routeutils/backend_service.go

@@ -3,8 +3,6 @@ package routeutils
 import (
 	"context"
 	"fmt"
-	"strings"
-
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -121,6 +119,25 @@ func (s *ServiceBackendConfig) GetTargetGroupProps() *elbv2gw.TargetGroupProps {
 	return s.targetGroupProps
 }
 
+var (
+	http2 = elbv2model.ProtocolVersionHTTP2
+	http1 = elbv2model.ProtocolVersionHTTP1
+	grpc  = elbv2model.ProtocolVersionHTTP1
+)
+
+func (s *ServiceBackendConfig) GetProtocolVersion() *elbv2model.ProtocolVersion {
+	if s.servicePort.AppProtocol == nil {
+		return nil
+	}
+
+	switch *s.servicePort.AppProtocol {
+	case "kubernetes.io/h2c":
+		return &http2
+	default:
+		return nil
+	}
+}
+
 func serviceLoader(ctx context.Context, k8sClient client.Client, routeIdentifier types.NamespacedName, routeKind RouteKind, backendRef gwv1.BackendRef) (*ServiceBackendConfig, error, error) {
 	if backendRef.Port == nil {
 		initialErrorMessage := "Port is required"
@@ -203,30 +220,5 @@ func serviceLoader(ctx context.Context, k8sClient client.Client, routeIdentifier
 		tgProps = tgConfigConstructor.ConstructTargetGroupConfigForRoute(tgConfig, routeIdentifier.Name, routeIdentifier.Namespace, string(routeKind))
 	}
 
-	// validate if protocol version is compatible with appProtocol
-	if tgProps != nil && servicePort.AppProtocol != nil {
-		appProtocol := strings.ToLower(*servicePort.AppProtocol)
-		if tgProps.ProtocolVersion != nil {
-			isCompatible := true
-			switch *tgProps.ProtocolVersion {
-			case elbv2gw.ProtocolVersionGRPC:
-				if appProtocol == "http" {
-					isCompatible = false
-				}
-			case elbv2gw.ProtocolVersionHTTP1, elbv2gw.ProtocolVersionHTTP2:
-				if appProtocol == "grpc" {
-					isCompatible = false
-				}
-			}
-			if !isCompatible {
-				initialErrorMessage := fmt.Sprintf("Service port appProtocol %s is not compatible with target group protocolVersion %s", *servicePort.AppProtocol, *tgProps.ProtocolVersion)
-				wrappedGatewayErrorMessage := generateInvalidMessageWithRouteDetails(initialErrorMessage, routeKind, routeIdentifier)
-
-				// This potentially could be fatal, but let's make the reconcile cycle as resilient as possible.
-				return nil, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonUnsupportedProtocol, &wrappedGatewayErrorMessage, nil), nil
-			}
-		}
-	}
-
 	return NewServiceBackendConfig(svc, tgProps, servicePort), nil, nil
 }

pkg/gateway/routeutils/backend_service_test.go

@@ -264,3 +264,37 @@ func Test_buildTargetGroupTargetType(t *testing.T) {
 	res = svcBackendWithProps.GetTargetType(elbv2model.TargetTypeIP)
 	assert.Equal(t, elbv2model.TargetTypeInstance, res)
 }
+
+func Test_GetProtocolVersion(t *testing.T) {
+	testCases := []struct {
+		name     string
+		svcPort  *corev1.ServicePort
+		expected *elbv2model.ProtocolVersion
+	}{
+		{
+			name:    "null app protocol version",
+			svcPort: &corev1.ServicePort{},
+		},
+		{
+			name: "unknown app protocol version",
+			svcPort: &corev1.ServicePort{
+				AppProtocol: awssdk.String("foo"),
+			},
+		},
+		{
+			name: "supported protocol version",
+			svcPort: &corev1.ServicePort{
+				AppProtocol: awssdk.String("kubernetes.io/h2c"),
+			},
+			expected: &http2,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			svcBackend := NewServiceBackendConfig(nil, nil, tc.svcPort)
+			res := svcBackend.GetProtocolVersion()
+			assert.Equal(t, res, tc.expected)
+		})
+	}
+}