diff --git a/pkg/gateway/model/model_build_target_group.go b/pkg/gateway/model/model_build_target_group.go index fb51e6adb..4058787f2 100644 --- a/pkg/gateway/model/model_build_target_group.go +++ b/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 } diff --git a/pkg/gateway/model/model_build_target_group_test.go b/pkg/gateway/model/model_build_target_group_test.go index 01156765d..b78e1681f 100644 --- a/pkg/gateway/model/model_build_target_group_test.go +++ b/pkg/gateway/model/model_build_target_group_test.go @@ -1172,6 +1172,8 @@ 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 @@ -1179,30 +1181,75 @@ func Test_buildTargetGroupProtocolVersion(t *testing.T) { { 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{ @@ -1210,6 +1257,72 @@ func Test_buildTargetGroupProtocolVersion(t *testing.T) { }, 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 { @@ -1217,7 +1330,7 @@ func Test_buildTargetGroupProtocolVersion(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) }) } diff --git a/pkg/gateway/routeutils/backend.go b/pkg/gateway/routeutils/backend.go index 6b5f6d9e2..9736c8064 100644 --- a/pkg/gateway/routeutils/backend.go +++ b/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 :)) diff --git a/pkg/gateway/routeutils/backend_gateway.go b/pkg/gateway/routeutils/backend_gateway.go index 2fb77be55..b5b331638 100644 --- a/pkg/gateway/routeutils/backend_gateway.go +++ b/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, diff --git a/pkg/gateway/routeutils/backend_gateway_test.go b/pkg/gateway/routeutils/backend_gateway_test.go index 1c32eccd7..25639b097 100644 --- a/pkg/gateway/routeutils/backend_gateway_test.go +++ b/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()) +} diff --git a/pkg/gateway/routeutils/backend_service.go b/pkg/gateway/routeutils/backend_service.go index 9043c3e6a..5dc82b2e9 100644 --- a/pkg/gateway/routeutils/backend_service.go +++ b/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 } diff --git a/pkg/gateway/routeutils/backend_service_test.go b/pkg/gateway/routeutils/backend_service_test.go index f3c91d93a..37acf83c3 100644 --- a/pkg/gateway/routeutils/backend_service_test.go +++ b/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) + }) + } +}