Skip to content

Commit 6760e7f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4500 from zac-nixon/main
refactor app protocol, add support for app protocols that kubernetes + elb supports
2 parents f1dd7c8 + 82f349c commit 6760e7f

File tree

7 files changed

+194
-33
lines changed

7 files changed

+194
-33
lines changed

‎pkg/gateway/model/model_build_target_group.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ func (builder *targetGroupBuilderImpl) buildTargetGroupSpec(gw *gwv1.Gateway, ro
266266
if err != nil {
267267
return elbv2model.TargetGroupSpec{}, err
268268
}
269-
tgProtocolVersion := builder.buildTargetGroupProtocolVersion(targetGroupProps, route)
269+
tgProtocolVersion := builder.buildTargetGroupProtocolVersion(targetGroupProps, backendConfig, listenerProtocol, route)
270270

271271
healthCheckConfig, err := builder.buildTargetGroupHealthCheckConfig(targetGroupProps, tgProtocol, tgProtocolVersion, targetType, backendConfig)
272272
if err != nil {
@@ -423,13 +423,18 @@ var (
423423
grpc = elbv2model.ProtocolVersionGRPC
424424
)
425425

426-
func (builder *targetGroupBuilderImpl) buildTargetGroupProtocolVersion(targetGroupProps *elbv2gw.TargetGroupProps, route routeutils.RouteDescriptor) *elbv2model.ProtocolVersion {
426+
func (builder *targetGroupBuilderImpl) buildTargetGroupProtocolVersion(targetGroupProps *elbv2gw.TargetGroupProps, backendConfig routeutils.TargetGroupConfigurator, listenerProtocol elbv2model.Protocol, route routeutils.RouteDescriptor) *elbv2model.ProtocolVersion {
427427
// NLB doesn't support protocol version
428428
if builder.loadBalancerType == elbv2model.LoadBalancerTypeNetwork {
429429
return nil
430430
}
431+
432+
// As of writing, only HTTPS listeners support different Protocol Version
433+
if listenerProtocol != elbv2model.ProtocolHTTPS {
434+
return &http1
435+
}
436+
431437
if targetGroupProps != nil && targetGroupProps.ProtocolVersion != nil {
432-
// TODO - We can infer GRPC here from route
433438
pv := elbv2model.ProtocolVersion(*targetGroupProps.ProtocolVersion)
434439
return &pv
435440
}
@@ -438,6 +443,11 @@ func (builder *targetGroupBuilderImpl) buildTargetGroupProtocolVersion(targetGro
438443
return &grpc
439444
}
440445

446+
resolvedProtocolVersion := backendConfig.GetProtocolVersion()
447+
if resolvedProtocolVersion != nil {
448+
return resolvedProtocolVersion
449+
}
450+
441451
return &http1
442452
}
443453

‎pkg/gateway/model/model_build_target_group_test.go‎

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,52 +1172,165 @@ func Test_buildTargetGroupProtocolVersion(t *testing.T) {
11721172
testCases := []struct {
11731173
name string
11741174
loadBalancerType elbv2model.LoadBalancerType
1175+
tgConfigurator routeutils.TargetGroupConfigurator
1176+
listenerProtocol elbv2model.Protocol
11751177
route routeutils.RouteDescriptor
11761178
targetGroupProps *elbv2gw.TargetGroupProps
11771179
expected *elbv2model.ProtocolVersion
11781180
}{
11791181
{
11801182
name: "nlb - no props",
11811183
loadBalancerType: elbv2model.LoadBalancerTypeNetwork,
1182-
route: &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
1184+
listenerProtocol: elbv2model.ProtocolTCP,
1185+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1186+
Protocol: corev1.ProtocolTCP,
1187+
Port: 80,
1188+
TargetPort: intstr.IntOrString{
1189+
IntVal: 80,
1190+
Type: intstr.Int,
1191+
},
1192+
}),
1193+
route: &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
11831194
},
11841195
{
11851196
name: "nlb - with props",
11861197
loadBalancerType: elbv2model.LoadBalancerTypeNetwork,
1187-
route: &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
1198+
listenerProtocol: elbv2model.ProtocolTCP,
1199+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1200+
Protocol: corev1.ProtocolTCP,
1201+
Port: 80,
1202+
TargetPort: intstr.IntOrString{
1203+
IntVal: 80,
1204+
Type: intstr.Int,
1205+
},
1206+
}),
1207+
route: &routeutils.MockRoute{Kind: routeutils.TCPRouteKind},
11881208
targetGroupProps: &elbv2gw.TargetGroupProps{
11891209
ProtocolVersion: &http2Gw,
11901210
},
11911211
},
11921212
{
11931213
name: "alb - no props",
1214+
listenerProtocol: elbv2model.ProtocolHTTPS,
1215+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1216+
Protocol: corev1.ProtocolTCP,
1217+
Port: 80,
1218+
TargetPort: intstr.IntOrString{
1219+
IntVal: 80,
1220+
Type: intstr.Int,
1221+
},
1222+
}),
11941223
route: &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
11951224
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
11961225
expected: &http1Elb,
11971226
},
11981227
{
11991228
name: "alb - no props - grpc",
1229+
listenerProtocol: elbv2model.ProtocolHTTPS,
1230+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1231+
Protocol: corev1.ProtocolTCP,
1232+
Port: 80,
1233+
TargetPort: intstr.IntOrString{
1234+
IntVal: 80,
1235+
Type: intstr.Int,
1236+
},
1237+
}),
12001238
route: &routeutils.MockRoute{Kind: routeutils.GRPCRouteKind},
12011239
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
12021240
expected: &grpcElb,
12031241
},
12041242
{
12051243
name: "alb - with props",
1244+
listenerProtocol: elbv2model.ProtocolHTTPS,
1245+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1246+
Protocol: corev1.ProtocolTCP,
1247+
Port: 80,
1248+
TargetPort: intstr.IntOrString{
1249+
IntVal: 80,
1250+
Type: intstr.Int,
1251+
},
1252+
}),
12061253
route: &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
12071254
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
12081255
targetGroupProps: &elbv2gw.TargetGroupProps{
12091256
ProtocolVersion: &http2Gw,
12101257
},
12111258
expected: &http2Elb,
12121259
},
1260+
{
1261+
name: "alb - with props - http protocol",
1262+
listenerProtocol: elbv2model.ProtocolHTTP,
1263+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1264+
Protocol: corev1.ProtocolTCP,
1265+
Port: 80,
1266+
TargetPort: intstr.IntOrString{
1267+
IntVal: 80,
1268+
Type: intstr.Int,
1269+
},
1270+
}),
1271+
route: &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
1272+
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
1273+
targetGroupProps: &elbv2gw.TargetGroupProps{
1274+
ProtocolVersion: &http2Gw,
1275+
},
1276+
expected: &http1Elb,
1277+
},
1278+
{
1279+
name: "alb - pv found on svc port - http listener",
1280+
listenerProtocol: elbv2model.ProtocolHTTP,
1281+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1282+
Protocol: corev1.ProtocolTCP,
1283+
Port: 80,
1284+
AppProtocol: awssdk.String("kubernetes.io/h2c"),
1285+
TargetPort: intstr.IntOrString{
1286+
IntVal: 80,
1287+
Type: intstr.Int,
1288+
},
1289+
}),
1290+
route: &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
1291+
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
1292+
expected: &http1Elb,
1293+
},
1294+
{
1295+
name: "alb - pv found on svc port - https listener",
1296+
listenerProtocol: elbv2model.ProtocolHTTPS,
1297+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1298+
Protocol: corev1.ProtocolTCP,
1299+
Port: 80,
1300+
AppProtocol: awssdk.String("kubernetes.io/h2c"),
1301+
TargetPort: intstr.IntOrString{
1302+
IntVal: 80,
1303+
Type: intstr.Int,
1304+
},
1305+
}),
1306+
route: &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
1307+
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
1308+
expected: &http2Elb,
1309+
},
1310+
{
1311+
name: "alb - unknown pv found on svc port - https listener",
1312+
listenerProtocol: elbv2model.ProtocolHTTPS,
1313+
tgConfigurator: routeutils.NewServiceBackendConfig(nil, nil, &corev1.ServicePort{
1314+
Protocol: corev1.ProtocolTCP,
1315+
Port: 80,
1316+
AppProtocol: awssdk.String("foo"),
1317+
TargetPort: intstr.IntOrString{
1318+
IntVal: 80,
1319+
Type: intstr.Int,
1320+
},
1321+
}),
1322+
route: &routeutils.MockRoute{Kind: routeutils.HTTPRouteKind},
1323+
loadBalancerType: elbv2model.LoadBalancerTypeApplication,
1324+
expected: &http1Elb,
1325+
},
12131326
}
12141327

12151328
for _, tc := range testCases {
12161329
t.Run(tc.name, func(t *testing.T) {
12171330
builder := targetGroupBuilderImpl{
12181331
loadBalancerType: tc.loadBalancerType,
12191332
}
1220-
res := builder.buildTargetGroupProtocolVersion(tc.targetGroupProps, tc.route)
1333+
res := builder.buildTargetGroupProtocolVersion(tc.targetGroupProps, tc.tgConfigurator, tc.listenerProtocol, tc.route)
12211334
assert.Equal(t, tc.expected, res)
12221335
})
12231336
}

‎pkg/gateway/routeutils/backend.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ type TargetGroupConfigurator interface {
4848
GetTargetGroupPort(targetType elbv2model.TargetType) int32
4949
// GetHealthCheckPort returns the port to send health check traffic
5050
GetHealthCheckPort(targetType elbv2model.TargetType, isServiceExternalTrafficPolicyTypeLocal bool) (intstr.IntOrString, error)
51+
// GetProtocolVersion returns the protocol version to use for this target group
52+
GetProtocolVersion() *elbv2model.ProtocolVersion
5153
}
5254

5355
// 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‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ type GatewayBackendConfig struct {
2727
port int32
2828
}
2929

30+
func (g *GatewayBackendConfig) GetProtocolVersion() *elbv2model.ProtocolVersion {
31+
// Gateway backends don't support a protocol version.
32+
return nil
33+
}
34+
3035
func NewGatewayBackendConfig(gateway *gwv1.Gateway, targetGroupProps *elbv2gw.TargetGroupProps, arn string, port int32) *GatewayBackendConfig {
3136
return &GatewayBackendConfig{
3237
gateway: gateway,

‎pkg/gateway/routeutils/backend_gateway_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,3 +248,8 @@ func TestValidateGatewayARN(t *testing.T) {
248248
})
249249
}
250250
}
251+
252+
func TestGatewayBackendConfig_GetProtocolVersion(t *testing.T) {
253+
config := &GatewayBackendConfig{}
254+
assert.Nil(t, config.GetProtocolVersion())
255+
}

‎pkg/gateway/routeutils/backend_service.go‎

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package routeutils
33
import (
44
"context"
55
"fmt"
6-
"strings"
7-
86
"github.com/pkg/errors"
97
corev1 "k8s.io/api/core/v1"
108
"k8s.io/apimachinery/pkg/types"
@@ -121,6 +119,25 @@ func (s *ServiceBackendConfig) GetTargetGroupProps() *elbv2gw.TargetGroupProps {
121119
return s.targetGroupProps
122120
}
123121

122+
var (
123+
http2 = elbv2model.ProtocolVersionHTTP2
124+
http1 = elbv2model.ProtocolVersionHTTP1
125+
grpc = elbv2model.ProtocolVersionHTTP1
126+
)
127+
128+
func (s *ServiceBackendConfig) GetProtocolVersion() *elbv2model.ProtocolVersion {
129+
if s.servicePort.AppProtocol == nil {
130+
return nil
131+
}
132+
133+
switch *s.servicePort.AppProtocol {
134+
case "kubernetes.io/h2c":
135+
return &http2
136+
default:
137+
return nil
138+
}
139+
}
140+
124141
func serviceLoader(ctx context.Context, k8sClient client.Client, routeIdentifier types.NamespacedName, routeKind RouteKind, backendRef gwv1.BackendRef) (*ServiceBackendConfig, error, error) {
125142
if backendRef.Port == nil {
126143
initialErrorMessage := "Port is required"
@@ -203,30 +220,5 @@ func serviceLoader(ctx context.Context, k8sClient client.Client, routeIdentifier
203220
tgProps = tgConfigConstructor.ConstructTargetGroupConfigForRoute(tgConfig, routeIdentifier.Name, routeIdentifier.Namespace, string(routeKind))
204221
}
205222

206-
// validate if protocol version is compatible with appProtocol
207-
if tgProps != nil && servicePort.AppProtocol != nil {
208-
appProtocol := strings.ToLower(*servicePort.AppProtocol)
209-
if tgProps.ProtocolVersion != nil {
210-
isCompatible := true
211-
switch *tgProps.ProtocolVersion {
212-
case elbv2gw.ProtocolVersionGRPC:
213-
if appProtocol == "http" {
214-
isCompatible = false
215-
}
216-
case elbv2gw.ProtocolVersionHTTP1, elbv2gw.ProtocolVersionHTTP2:
217-
if appProtocol == "grpc" {
218-
isCompatible = false
219-
}
220-
}
221-
if !isCompatible {
222-
initialErrorMessage := fmt.Sprintf("Service port appProtocol %s is not compatible with target group protocolVersion %s", *servicePort.AppProtocol, *tgProps.ProtocolVersion)
223-
wrappedGatewayErrorMessage := generateInvalidMessageWithRouteDetails(initialErrorMessage, routeKind, routeIdentifier)
224-
225-
// This potentially could be fatal, but let's make the reconcile cycle as resilient as possible.
226-
return nil, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonUnsupportedProtocol, &wrappedGatewayErrorMessage, nil), nil
227-
}
228-
}
229-
}
230-
231223
return NewServiceBackendConfig(svc, tgProps, servicePort), nil, nil
232224
}

‎pkg/gateway/routeutils/backend_service_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,37 @@ func Test_buildTargetGroupTargetType(t *testing.T) {
264264
res = svcBackendWithProps.GetTargetType(elbv2model.TargetTypeIP)
265265
assert.Equal(t, elbv2model.TargetTypeInstance, res)
266266
}
267+
268+
func Test_GetProtocolVersion(t *testing.T) {
269+
testCases := []struct {
270+
name string
271+
svcPort *corev1.ServicePort
272+
expected *elbv2model.ProtocolVersion
273+
}{
274+
{
275+
name: "null app protocol version",
276+
svcPort: &corev1.ServicePort{},
277+
},
278+
{
279+
name: "unknown app protocol version",
280+
svcPort: &corev1.ServicePort{
281+
AppProtocol: awssdk.String("foo"),
282+
},
283+
},
284+
{
285+
name: "supported protocol version",
286+
svcPort: &corev1.ServicePort{
287+
AppProtocol: awssdk.String("kubernetes.io/h2c"),
288+
},
289+
expected: &http2,
290+
},
291+
}
292+
293+
for _, tc := range testCases {
294+
t.Run(tc.name, func(t *testing.T) {
295+
svcBackend := NewServiceBackendConfig(nil, nil, tc.svcPort)
296+
res := svcBackend.GetProtocolVersion()
297+
assert.Equal(t, res, tc.expected)
298+
})
299+
}
300+
}

0 commit comments

Comments
 (0)