Skip to content

Commit 2f5aed0

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4273 from zac-nixon/znixon/refactor-missing-backends
refactor how no backends are modeled in NLB / ALB Gateways
2 parents 011edcd + ebefb4f commit 2f5aed0

File tree

6 files changed

+339
-21
lines changed

6 files changed

+339
-21
lines changed

‎pkg/gateway/model/mock_tg_builder.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package model
2+
3+
import (
4+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5+
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
6+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
7+
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
8+
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
9+
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
10+
)
11+
12+
type MockTargetGroupBuilder struct {
13+
tgs []*elbv2model.TargetGroup
14+
buildErr error
15+
}
16+
17+
func (m *MockTargetGroupBuilder) buildTargetGroup(stack core.Stack, gw *gwv1.Gateway, lbConfig elbv2gw.LoadBalancerConfiguration, lbIPType elbv2model.IPAddressType, routeDescriptor routeutils.RouteDescriptor, backend routeutils.Backend, backendSGIDToken core.StringToken) (*elbv2model.TargetGroup, error) {
18+
var tg *elbv2model.TargetGroup
19+
20+
if len(m.tgs) > 0 {
21+
tg = m.tgs[0]
22+
m.tgs = m.tgs[1:]
23+
}
24+
25+
return tg, m.buildErr
26+
}
27+
28+
func (m *MockTargetGroupBuilder) buildTargetGroupSpec(gw *gwv1.Gateway, route routeutils.RouteDescriptor, lbConfig elbv2gw.LoadBalancerConfiguration, lbIPType elbv2model.IPAddressType, backend routeutils.Backend, targetGroupProps *elbv2gw.TargetGroupProps) (elbv2model.TargetGroupSpec, error) {
29+
//TODO implement me
30+
panic("implement me")
31+
}
32+
33+
func (m *MockTargetGroupBuilder) buildTargetGroupBindingSpec(gw *gwv1.Gateway, tgProps *elbv2gw.TargetGroupProps, tgSpec elbv2model.TargetGroupSpec, nodeSelector *metav1.LabelSelector, backend routeutils.Backend, backendSGIDToken core.StringToken) elbv2model.TargetGroupBindingResourceSpec {
34+
//TODO implement me
35+
panic("implement me")
36+
}
37+
38+
var _ targetGroupBuilder = &MockTargetGroupBuilder{}

‎pkg/gateway/model/model_build_listener.go‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func (l listenerBuilderImpl) buildListeners(ctx context.Context, stack core.Stac
6969

7070
// build rules only for L7 gateways
7171
if l.loadBalancerType == elbv2model.LoadBalancerTypeApplication {
72-
if err := l.buildListenerRules(stack, ls, lb, securityGroups, gw, port, lbCfg, routes); err != nil {
72+
if err := l.buildListenerRules(stack, ls, lb.Spec.IPAddressType, securityGroups, gw, port, lbCfg, routes); err != nil {
7373
return err
7474
}
7575
}
@@ -167,6 +167,12 @@ func (l listenerBuilderImpl) buildL4ListenerSpec(ctx context.Context, stack core
167167
return &elbv2model.ListenerSpec{}, errors.Errorf("multiple backend refs found for route %v for listener on port:protocol %v:%v for gateway %v , only one must be specified", routeDescriptor.GetRouteNamespacedName(), port, listenerSpec.Protocol, k8s.NamespacedName(gw))
168168
}
169169
backend := routeDescriptor.GetAttachedRules()[0].GetBackends()[0]
170+
171+
if backend.Weight == 0 {
172+
l.logger.Info("Ignoring NLB backend with 0 weight.", "route", routeDescriptor.GetRouteNamespacedName())
173+
return nil, nil
174+
}
175+
170176
targetGroup, tgErr := l.tgBuilder.buildTargetGroup(stack, gw, lbCfg, lb.Spec.IPAddressType, routeDescriptor, backend, securityGroups.backendSecurityGroupToken)
171177
if tgErr != nil {
172178
return &elbv2model.ListenerSpec{}, tgErr
@@ -176,7 +182,7 @@ func (l listenerBuilderImpl) buildL4ListenerSpec(ctx context.Context, stack core
176182
}
177183

178184
// this is only for L7 ALB
179-
func (l listenerBuilderImpl) buildListenerRules(stack core.Stack, ls *elbv2model.Listener, lb *elbv2model.LoadBalancer, securityGroups securityGroupOutput, gw *gwv1.Gateway, port int32, lbCfg elbv2gw.LoadBalancerConfiguration, routes map[int32][]routeutils.RouteDescriptor) error {
185+
func (l listenerBuilderImpl) buildListenerRules(stack core.Stack, ls *elbv2model.Listener, ipAddressType elbv2model.IPAddressType, securityGroups securityGroupOutput, gw *gwv1.Gateway, port int32, lbCfg elbv2gw.LoadBalancerConfiguration, routes map[int32][]routeutils.RouteDescriptor) error {
180186
// sort all rules based on precedence
181187
rulesWithPrecedenceOrder := routeutils.SortAllRulesByPrecedence(routes[port])
182188

@@ -201,7 +207,7 @@ func (l listenerBuilderImpl) buildListenerRules(stack core.Stack, ls *elbv2model
201207
}
202208
targetGroupTuples := make([]elbv2model.TargetGroupTuple, 0, len(rule.GetBackends()))
203209
for _, backend := range rule.GetBackends() {
204-
targetGroup, tgErr := l.tgBuilder.buildTargetGroup(stack, gw, lbCfg, lb.Spec.IPAddressType, route, backend, securityGroups.backendSecurityGroupToken)
210+
targetGroup, tgErr := l.tgBuilder.buildTargetGroup(stack, gw, lbCfg, ipAddressType, route, backend, securityGroups.backendSecurityGroupToken)
205211
if tgErr != nil {
206212
return tgErr
207213
}
@@ -215,7 +221,7 @@ func (l listenerBuilderImpl) buildListenerRules(stack core.Stack, ls *elbv2model
215221

216222
var actions []elbv2model.Action
217223

218-
if len(targetGroupTuples) > 0 {
224+
if shouldProvisionActions(targetGroupTuples) {
219225
actions = buildL7ListenerForwardActions(targetGroupTuples, nil)
220226
}
221227

@@ -545,3 +551,16 @@ func newListenerBuilder(ctx context.Context, loadBalancerType elbv2model.LoadBal
545551
logger: logger,
546552
}
547553
}
554+
555+
// shouldProvisionActions -- determine if the given target groups are acceptable for ELB Actions.
556+
// The criteria -
557+
// 1/ One or more target groups are present.
558+
// 2/ At least one target group has a weight greater than zero.
559+
func shouldProvisionActions(targetGroups []elbv2model.TargetGroupTuple) bool {
560+
for _, tg := range targetGroups {
561+
if tg.Weight == nil || *tg.Weight != 0 {
562+
return true
563+
}
564+
}
565+
return false
566+
}

‎pkg/gateway/model/model_build_listener_test.go‎

Lines changed: 195 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@ package model
22

33
import (
44
"context"
5+
"fmt"
56
awssdk "github.com/aws/aws-sdk-go-v2/aws"
67
elbv2sdk "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
78
elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
9+
"github.com/google/go-cmp/cmp"
10+
"github.com/google/go-cmp/cmp/cmpopts"
11+
corev1 "k8s.io/api/core/v1"
812
"reflect"
913
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
14+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
15+
coremodel "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
1016
"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
1117
"strings"
1218
"testing"
@@ -16,7 +22,7 @@ import (
1622
"github.com/pkg/errors"
1723
"github.com/stretchr/testify/assert"
1824
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
19-
certs "sigs.k8s.io/aws-load-balancer-controller/pkg/certs"
25+
"sigs.k8s.io/aws-load-balancer-controller/pkg/certs"
2026
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
2127
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
2228
)
@@ -1083,3 +1089,191 @@ func Test_buildMutualAuthenticationAttributes(t *testing.T) {
10831089
})
10841090
}
10851091
}
1092+
1093+
func TestBuildListenerRules(t *testing.T) {
1094+
testCases := []struct {
1095+
name string
1096+
sgOutput securityGroupOutput
1097+
ipAddressType elbv2model.IPAddressType
1098+
port int32
1099+
routes map[int32][]routeutils.RouteDescriptor
1100+
1101+
expectedRules []*elbv2model.ListenerRuleSpec
1102+
expectedTags map[string]string
1103+
tagErr error
1104+
}{
1105+
{
1106+
name: "no backends should result in 503 fixed response",
1107+
port: 80,
1108+
ipAddressType: elbv2model.IPAddressTypeIPV4,
1109+
sgOutput: securityGroupOutput{
1110+
backendSecurityGroupToken: coremodel.LiteralStringToken("sg-B"),
1111+
},
1112+
routes: map[int32][]routeutils.RouteDescriptor{
1113+
80: {
1114+
&routeutils.MockRoute{
1115+
Kind: routeutils.HTTPRouteKind,
1116+
Name: "my-route",
1117+
Namespace: "my-route-ns",
1118+
Rules: []routeutils.RouteRule{
1119+
&routeutils.MockRule{
1120+
RawRule: &gwv1.HTTPRouteRule{
1121+
Matches: []gwv1.HTTPRouteMatch{
1122+
{
1123+
Path: &gwv1.HTTPPathMatch{
1124+
Type: (*gwv1.PathMatchType)(awssdk.String("PathPrefix")),
1125+
Value: awssdk.String("/"),
1126+
},
1127+
},
1128+
},
1129+
},
1130+
},
1131+
},
1132+
},
1133+
},
1134+
},
1135+
expectedRules: []*elbv2model.ListenerRuleSpec{
1136+
{
1137+
Priority: 1,
1138+
Actions: []elbv2model.Action{
1139+
{
1140+
Type: "fixed-response",
1141+
FixedResponseConfig: &elbv2model.FixedResponseActionConfig{
1142+
ContentType: awssdk.String("text/plain"),
1143+
StatusCode: "503",
1144+
},
1145+
},
1146+
},
1147+
Conditions: []elbv2model.RuleCondition{
1148+
{
1149+
Field: "path-pattern",
1150+
PathPatternConfig: &elbv2model.PathPatternConditionConfig{
1151+
Values: []string{"/*"},
1152+
},
1153+
},
1154+
},
1155+
},
1156+
},
1157+
},
1158+
{
1159+
name: "backends should result in forward action generated",
1160+
port: 80,
1161+
ipAddressType: elbv2model.IPAddressTypeIPV4,
1162+
sgOutput: securityGroupOutput{
1163+
backendSecurityGroupToken: coremodel.LiteralStringToken("sg-B"),
1164+
},
1165+
routes: map[int32][]routeutils.RouteDescriptor{
1166+
80: {
1167+
&routeutils.MockRoute{
1168+
Kind: routeutils.HTTPRouteKind,
1169+
Name: "my-route",
1170+
Namespace: "my-route-ns",
1171+
Rules: []routeutils.RouteRule{
1172+
&routeutils.MockRule{
1173+
RawRule: &gwv1.HTTPRouteRule{
1174+
Matches: []gwv1.HTTPRouteMatch{
1175+
{
1176+
Path: &gwv1.HTTPPathMatch{
1177+
Type: (*gwv1.PathMatchType)(awssdk.String("PathPrefix")),
1178+
Value: awssdk.String("/"),
1179+
},
1180+
},
1181+
},
1182+
},
1183+
BackendRefs: []routeutils.Backend{
1184+
{
1185+
Service: &corev1.Service{},
1186+
ServicePort: &corev1.ServicePort{Name: "svcport"},
1187+
Weight: 1,
1188+
},
1189+
},
1190+
},
1191+
},
1192+
},
1193+
},
1194+
},
1195+
expectedRules: []*elbv2model.ListenerRuleSpec{
1196+
{
1197+
Priority: 1,
1198+
Actions: []elbv2model.Action{
1199+
{
1200+
Type: "forward",
1201+
ForwardConfig: &elbv2model.ForwardActionConfig{
1202+
// cmp can't compare the TG ARN, so don't inject it.
1203+
TargetGroups: []elbv2model.TargetGroupTuple{
1204+
{
1205+
Weight: awssdk.Int32(1),
1206+
},
1207+
},
1208+
},
1209+
},
1210+
},
1211+
Conditions: []elbv2model.RuleCondition{
1212+
{
1213+
Field: "path-pattern",
1214+
PathPatternConfig: &elbv2model.PathPatternConditionConfig{
1215+
Values: []string{"/*"},
1216+
},
1217+
},
1218+
},
1219+
},
1220+
},
1221+
},
1222+
}
1223+
1224+
for _, tc := range testCases {
1225+
t.Run(tc.name, func(t *testing.T) {
1226+
stack := coremodel.NewDefaultStack(coremodel.StackID{Namespace: "namespace", Name: "name"})
1227+
mockTagger := &mockTagHelper{
1228+
tags: tc.expectedTags,
1229+
err: tc.tagErr,
1230+
}
1231+
1232+
mockTgBuilder := &MockTargetGroupBuilder{
1233+
tgs: []*elbv2model.TargetGroup{
1234+
{
1235+
ResourceMeta: coremodel.NewResourceMeta(stack, "AWS::ElasticLoadBalancingV2::TargetGroup", "id-1"),
1236+
},
1237+
},
1238+
}
1239+
1240+
builder := &listenerBuilderImpl{
1241+
tagHelper: mockTagger,
1242+
tgBuilder: mockTgBuilder,
1243+
}
1244+
1245+
err := builder.buildListenerRules(stack, &elbv2model.Listener{}, tc.ipAddressType, tc.sgOutput, &gwv1.Gateway{}, tc.port, elbv2gw.LoadBalancerConfiguration{}, tc.routes)
1246+
assert.NoError(t, err)
1247+
1248+
var resLRs []*elbv2model.ListenerRule
1249+
assert.NoError(t, stack.ListResources(&resLRs))
1250+
1251+
assert.Equal(t, len(tc.expectedRules), len(resLRs))
1252+
1253+
// cmp absolutely barfs trying to validate the TargetGroupARN due to stack id semantics
1254+
opt := cmp.Options{
1255+
cmpopts.IgnoreFields(elbv2model.TargetGroupTuple{}, "TargetGroupARN"),
1256+
}
1257+
1258+
processedSet := make(map[*elbv2model.ListenerRule]bool)
1259+
for _, elr := range tc.expectedRules {
1260+
for _, alr := range resLRs {
1261+
conditionsEqual := cmp.Equal(elr.Conditions, alr.Spec.Conditions)
1262+
actionsEqual := cmp.Equal(elr.Actions, alr.Spec.Actions, opt)
1263+
priorityEqual := elr.Priority == alr.Spec.Priority
1264+
fmt.Printf("%+v,%+v,%+v\n", conditionsEqual, actionsEqual, priorityEqual)
1265+
if conditionsEqual && actionsEqual && priorityEqual {
1266+
processedSet[alr] = true
1267+
break
1268+
}
1269+
}
1270+
}
1271+
1272+
assert.Equal(t, len(tc.expectedRules), len(processedSet))
1273+
1274+
for _, lr := range resLRs {
1275+
assert.Equal(t, tc.expectedTags, lr.Spec.Tags)
1276+
}
1277+
})
1278+
}
1279+
}

‎pkg/gateway/routeutils/backend.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
const (
1919
serviceKind = "Service"
2020
referenceGrantNotExists = "No explicit ReferenceGrant exists to allow the reference."
21+
maxWeight = 999
2122
)
2223

2324
var (
@@ -91,10 +92,6 @@ func commonBackendLoader(ctx context.Context, k8sClient client.Client, typeSpeci
9192
return nil, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonInvalidKind, &wrappedGatewayErrorMessage, nil), nil
9293
}
9394

94-
if backendRef.Weight != nil && *backendRef.Weight == 0 {
95-
return nil, nil, nil
96-
}
97-
9895
if backendRef.Port == nil {
9996
initialErrorMessage := "Port is required"
10097
wrappedGatewayErrorMessage := generateInvalidMessageWithRouteDetails(initialErrorMessage, routeKind, routeIdentifier)
@@ -217,6 +214,10 @@ func commonBackendLoader(ctx context.Context, k8sClient client.Client, typeSpeci
217214
if backendRef.Weight != nil {
218215
weight = int(*backendRef.Weight)
219216
}
217+
218+
if weight > maxWeight {
219+
return nil, nil, errors.Errorf("Weight [%d] must be less than or equal to %d", weight, maxWeight)
220+
}
220221
return &Backend{
221222
Service: svc,
222223
ServicePort: servicePort,

0 commit comments

Comments
 (0)