Skip to content

Commit eaf77ed

Browse files
zac-nixonniclask25
zac-nixon
authored and
niclask25
committed
add sg tests
1 parent d9f5406 commit eaf77ed

File tree

7 files changed

+452
-22
lines changed

7 files changed

+452
-22
lines changed

‎main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func main() {
167167
cloud.VpcID(), controllerCFG.ClusterName, controllerCFG.FeatureGates.Enabled(config.EndpointsFailOpen), controllerCFG.EnableEndpointSlices, controllerCFG.DisableRestrictedSGRules,
168168
controllerCFG.ServiceTargetENISGTags, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
169169
backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
170-
cloud.VpcID(), cloud.EC2(), mgr.GetClient(), controllerCFG.DefaultTags, nlbGatewayEnabled, albGatewayEnabled, ctrl.Log.WithName("backend-sg-provider"))
170+
cloud.VpcID(), cloud.EC2(), mgr.GetClient(), controllerCFG.DefaultTags, nlbGatewayEnabled || albGatewayEnabled, ctrl.Log.WithName("backend-sg-provider"))
171171
sgResolver := networking.NewDefaultSecurityGroupResolver(cloud.EC2(), cloud.VpcID())
172172
elbv2TaggingManager := elbv2deploy.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), controllerCFG.FeatureGates, cloud.RGT(), ctrl.Log)
173173
ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),

‎pkg/gateway/model/loadbalancer/mutator.go

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎pkg/gateway/model/mock_gateway_tag_helper.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package model
2+
3+
import elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
4+
5+
type mockTagHelper struct {
6+
tags map[string]string
7+
err error
8+
}
9+
10+
func (m *mockTagHelper) getGatewayTags(lbConf *elbv2gw.LoadBalancerConfiguration) (map[string]string, error) {
11+
return m.tags, m.err
12+
}
13+
14+
var _ tagHelper = &mockTagHelper{}

‎pkg/gateway/model/model_build_security_group.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (builder *securityGroupBuilderImpl) buildSecurityGroups(ctx context.Context
7575
return builder.handleManagedSecurityGroup(ctx, stack, lbConf, gw, routes, ipAddressType)
7676
}
7777

78-
return builder.handleCustomerSpecifiedSecurityGroups(ctx, lbConf, gw, sgNameOrIds)
78+
return builder.handleExplicitSecurityGroups(ctx, lbConf, gw, sgNameOrIds)
7979
}
8080

8181
func (builder *securityGroupBuilderImpl) handleManagedSecurityGroup(ctx context.Context, stack core.Stack, lbConf *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, routes map[int][]routeutils.RouteDescriptor, ipAddressType elbv2model.IPAddressType) (securityGroupOutput, error) {
@@ -105,7 +105,7 @@ func (builder *securityGroupBuilderImpl) handleManagedSecurityGroup(ctx context.
105105
}, nil
106106
}
107107

108-
func (builder *securityGroupBuilderImpl) handleCustomerSpecifiedSecurityGroups(ctx context.Context, lbConf *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, sgNameOrIds []string) (securityGroupOutput, error) {
108+
func (builder *securityGroupBuilderImpl) handleExplicitSecurityGroups(ctx context.Context, lbConf *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, sgNameOrIds []string) (securityGroupOutput, error) {
109109
var lbSGTokens []core.StringToken
110110
manageBackendSGRules := lbConf.Spec.ManageBackendSecurityGroupRules
111111
frontendSGIDs, err := builder.sgResolver.ResolveViaNameOrID(ctx, sgNameOrIds)

‎pkg/gateway/model/model_build_security_group_test.go

Lines changed: 311 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
1+
package model
2+
3+
import (
4+
"context"
5+
"github.com/go-logr/logr"
6+
"github.com/golang/mock/gomock"
7+
"github.com/pkg/errors"
8+
"github.com/stretchr/testify/assert"
9+
"k8s.io/apimachinery/pkg/types"
10+
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
11+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
12+
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
13+
coremodel "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
14+
ec2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/ec2"
15+
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
16+
"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
17+
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
18+
"testing"
19+
)
20+
21+
func Test_BuildSecurityGroups_Specified(t *testing.T) {
22+
const clusterName = "my-cluster"
23+
24+
type resolveSgCall struct {
25+
securityGroups []string
26+
err error
27+
}
28+
29+
type backendSgProviderCall struct {
30+
sgId string
31+
err error
32+
}
33+
34+
testCases := []struct {
35+
name string
36+
lbConf *elbv2gw.LoadBalancerConfiguration
37+
ipAddressType elbv2model.IPAddressType
38+
expectedTags map[string]string
39+
tagErr error
40+
enableBackendSg bool
41+
42+
resolveSg *resolveSgCall
43+
providerCall *backendSgProviderCall
44+
45+
expectErr bool
46+
expectedBackendSgToken coremodel.StringToken
47+
expectedSgTokens []coremodel.StringToken
48+
backendSgAllocated bool
49+
}{
50+
{
51+
name: "sg specified - no backend sg",
52+
lbConf: &elbv2gw.LoadBalancerConfiguration{
53+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
54+
SecurityGroups: &[]string{
55+
"sg1",
56+
"sg2",
57+
},
58+
},
59+
},
60+
resolveSg: &resolveSgCall{
61+
securityGroups: []string{
62+
"sg1",
63+
"sg2",
64+
},
65+
},
66+
expectedSgTokens: []coremodel.StringToken{
67+
coremodel.LiteralStringToken("sg1"),
68+
coremodel.LiteralStringToken("sg2"),
69+
},
70+
},
71+
{
72+
name: "sg specified - with backend sg",
73+
enableBackendSg: true,
74+
lbConf: &elbv2gw.LoadBalancerConfiguration{
75+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
76+
ManageBackendSecurityGroupRules: true,
77+
SecurityGroups: &[]string{
78+
"sg1",
79+
"sg2",
80+
},
81+
},
82+
},
83+
resolveSg: &resolveSgCall{
84+
securityGroups: []string{
85+
"sg1",
86+
"sg2",
87+
},
88+
},
89+
providerCall: &backendSgProviderCall{
90+
sgId: "auto-allocated",
91+
},
92+
expectedSgTokens: []coremodel.StringToken{
93+
coremodel.LiteralStringToken("sg1"),
94+
coremodel.LiteralStringToken("sg2"),
95+
coremodel.LiteralStringToken("auto-allocated"),
96+
},
97+
expectedBackendSgToken: coremodel.LiteralStringToken("auto-allocated"),
98+
backendSgAllocated: true,
99+
},
100+
{
101+
name: "sg specified - with backend sg - error - backendsg not enabled",
102+
lbConf: &elbv2gw.LoadBalancerConfiguration{
103+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
104+
ManageBackendSecurityGroupRules: true,
105+
SecurityGroups: &[]string{
106+
"sg1",
107+
"sg2",
108+
},
109+
},
110+
},
111+
resolveSg: &resolveSgCall{
112+
securityGroups: []string{
113+
"sg1",
114+
"sg2",
115+
},
116+
},
117+
expectErr: true,
118+
},
119+
{
120+
name: "sg specified - with backend sg - error - resolve sg error",
121+
enableBackendSg: true,
122+
lbConf: &elbv2gw.LoadBalancerConfiguration{
123+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
124+
ManageBackendSecurityGroupRules: true,
125+
SecurityGroups: &[]string{
126+
"sg1",
127+
"sg2",
128+
},
129+
},
130+
},
131+
resolveSg: &resolveSgCall{
132+
err: errors.New("bad thing"),
133+
},
134+
expectErr: true,
135+
},
136+
{
137+
name: "sg specified - with backend sg - error - resolve sg error",
138+
enableBackendSg: true,
139+
lbConf: &elbv2gw.LoadBalancerConfiguration{
140+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
141+
ManageBackendSecurityGroupRules: true,
142+
SecurityGroups: &[]string{
143+
"sg1",
144+
"sg2",
145+
},
146+
},
147+
},
148+
resolveSg: &resolveSgCall{
149+
securityGroups: []string{
150+
"sg1",
151+
"sg2",
152+
},
153+
},
154+
providerCall: &backendSgProviderCall{
155+
err: errors.New("bad thing"),
156+
},
157+
expectErr: true,
158+
},
159+
}
160+
161+
for _, tc := range testCases {
162+
t.Run(tc.name, func(t *testing.T) {
163+
ctrl := gomock.NewController(t)
164+
defer ctrl.Finish()
165+
166+
mockTagger := &mockTagHelper{
167+
tags: tc.expectedTags,
168+
err: tc.tagErr,
169+
}
170+
171+
gw := &gwv1.Gateway{}
172+
gw.Name = "my-gw"
173+
gw.Namespace = "my-namespace"
174+
175+
mockSgProvider := networking.NewMockBackendSGProvider(ctrl)
176+
mockSgResolver := networking.NewMockSecurityGroupResolver(ctrl)
177+
178+
if tc.resolveSg != nil {
179+
mockSgResolver.EXPECT().ResolveViaNameOrID(gomock.Any(), gomock.Eq(*tc.lbConf.Spec.SecurityGroups)).Return(tc.resolveSg.securityGroups, tc.resolveSg.err).Times(1)
180+
}
181+
182+
if tc.providerCall != nil {
183+
mockSgProvider.EXPECT().Get(gomock.Any(), gomock.Eq(networking.ResourceType(networking.ResourceTypeGateway)), gomock.Eq([]types.NamespacedName{k8s.NamespacedName(gw)})).Return(tc.providerCall.sgId, tc.providerCall.err).Times(1)
184+
}
185+
186+
stack := coremodel.NewDefaultStack(coremodel.StackID{Namespace: "namespace", Name: "name"})
187+
builder := newSecurityGroupBuilder(mockTagger, clusterName, tc.enableBackendSg, mockSgResolver, mockSgProvider, logr.Discard())
188+
189+
out, err := builder.buildSecurityGroups(context.Background(), stack, tc.lbConf, gw, make(map[int][]routeutils.RouteDescriptor), tc.ipAddressType)
190+
191+
if tc.expectErr {
192+
assert.Error(t, err)
193+
return
194+
}
195+
assert.NoError(t, err)
196+
assert.Equal(t, tc.expectedBackendSgToken, out.backendSecurityGroupToken)
197+
assert.Equal(t, tc.expectedSgTokens, out.securityGroupTokens)
198+
assert.Equal(t, tc.backendSgAllocated, out.backendSecurityGroupAllocated)
199+
})
200+
}
201+
}
202+
203+
func Test_BuildSecurityGroups_Allocate(t *testing.T) {
204+
const clusterName = "my-cluster"
205+
206+
type backendSgProviderCall struct {
207+
sgId string
208+
err error
209+
}
210+
211+
testCases := []struct {
212+
name string
213+
lbConf *elbv2gw.LoadBalancerConfiguration
214+
ipAddressType elbv2model.IPAddressType
215+
expectedTags map[string]string
216+
tagErr error
217+
enableBackendSg bool
218+
219+
providerCall *backendSgProviderCall
220+
221+
expectErr bool
222+
hasBackendSg bool
223+
backendSgAllocated bool
224+
expectedStackResources int
225+
}{
226+
{
227+
name: "sg allocate - no backend sg",
228+
lbConf: &elbv2gw.LoadBalancerConfiguration{
229+
Spec: elbv2gw.LoadBalancerConfigurationSpec{},
230+
},
231+
expectedStackResources: 1,
232+
},
233+
{
234+
name: "sg allocate - with backend sg",
235+
enableBackendSg: true,
236+
lbConf: &elbv2gw.LoadBalancerConfiguration{
237+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
238+
ManageBackendSecurityGroupRules: true,
239+
},
240+
},
241+
providerCall: &backendSgProviderCall{
242+
sgId: "auto-allocated",
243+
},
244+
backendSgAllocated: true,
245+
expectedStackResources: 1,
246+
},
247+
{
248+
name: "sg allocate - provider error",
249+
enableBackendSg: true,
250+
lbConf: &elbv2gw.LoadBalancerConfiguration{
251+
Spec: elbv2gw.LoadBalancerConfigurationSpec{
252+
ManageBackendSecurityGroupRules: true,
253+
},
254+
},
255+
providerCall: &backendSgProviderCall{
256+
err: errors.New("bad thing"),
257+
},
258+
expectErr: true,
259+
},
260+
{
261+
name: "sg allocate - tag error",
262+
lbConf: &elbv2gw.LoadBalancerConfiguration{
263+
Spec: elbv2gw.LoadBalancerConfigurationSpec{},
264+
},
265+
expectErr: true,
266+
tagErr: errors.New("bad thing"),
267+
},
268+
}
269+
270+
for _, tc := range testCases {
271+
t.Run(tc.name, func(t *testing.T) {
272+
ctrl := gomock.NewController(t)
273+
defer ctrl.Finish()
274+
275+
mockTagger := &mockTagHelper{
276+
tags: tc.expectedTags,
277+
err: tc.tagErr,
278+
}
279+
280+
gw := &gwv1.Gateway{}
281+
gw.Name = "my-gw"
282+
gw.Namespace = "my-namespace"
283+
284+
mockSgProvider := networking.NewMockBackendSGProvider(ctrl)
285+
mockSgResolver := networking.NewMockSecurityGroupResolver(ctrl)
286+
287+
if tc.providerCall != nil {
288+
mockSgProvider.EXPECT().Get(gomock.Any(), gomock.Eq(networking.ResourceType(networking.ResourceTypeGateway)), gomock.Eq([]types.NamespacedName{k8s.NamespacedName(gw)})).Return(tc.providerCall.sgId, tc.providerCall.err).Times(1)
289+
}
290+
291+
stack := coremodel.NewDefaultStack(coremodel.StackID{Namespace: "namespace", Name: "name"})
292+
builder := newSecurityGroupBuilder(mockTagger, clusterName, tc.enableBackendSg, mockSgResolver, mockSgProvider, logr.Discard())
293+
294+
out, err := builder.buildSecurityGroups(context.Background(), stack, tc.lbConf, gw, make(map[int][]routeutils.RouteDescriptor), tc.ipAddressType)
295+
296+
if tc.expectErr {
297+
assert.Error(t, err)
298+
return
299+
}
300+
assert.NoError(t, err)
301+
assert.Equal(t, tc.backendSgAllocated, out.backendSecurityGroupAllocated)
302+
var resSGs []*ec2model.SecurityGroup
303+
listErr := stack.ListResources(&resSGs)
304+
assert.NoError(t, listErr)
305+
assert.Equal(t, tc.expectedStackResources, len(resSGs))
306+
if tc.hasBackendSg {
307+
assert.NotNil(t, out.backendSecurityGroupToken)
308+
}
309+
})
310+
}
311+
}

‎pkg/networking/backend_sg_provider.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ type BackendSGProvider interface {
5656

5757
// NewBackendSGProvider constructs a new defaultBackendSGProvider
5858
func NewBackendSGProvider(clusterName string, backendSG string, vpcID string,
59-
ec2Client services.EC2, k8sClient client.Client, defaultTags map[string]string, enableNLBGateway bool, enableALBGateway bool, logger logr.Logger) *defaultBackendSGProvider {
59+
ec2Client services.EC2, k8sClient client.Client, defaultTags map[string]string, enableGatewayCheck bool, logger logr.Logger) *defaultBackendSGProvider {
6060
return &defaultBackendSGProvider{
6161
vpcID: vpcID,
6262
clusterName: clusterName,
@@ -67,7 +67,7 @@ func NewBackendSGProvider(clusterName string, backendSG string, vpcID string,
6767
logger: logger,
6868
mutex: sync.Mutex{},
6969

70-
enableGatewayCheck: enableALBGateway || enableNLBGateway,
70+
enableGatewayCheck: enableGatewayCheck,
7171

7272
checkIngressFinalizersFunc: func(finalizers []string) bool {
7373
for _, fin := range finalizers {
@@ -186,7 +186,7 @@ func (p *defaultBackendSGProvider) isBackendSGRequired(ctx context.Context) (boo
186186
if required, err := p.checkServiceListForUnmapped(ctx); required || err != nil {
187187
return required, err
188188
}
189-
if required, err := p.checkServiceListForUnmapped(ctx); required || err != nil {
189+
if required, err := p.checkGatewayListForUnmapped(ctx); required || err != nil {
190190
return required, err
191191
}
192192
return false, nil
@@ -332,7 +332,7 @@ func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgNa
332332
},
333333
},
334334
}
335-
p.logger.V(1).Info("Queriying existing SG", "vpc-id", vpcID, "name", sgName)
335+
p.logger.V(1).Info("Querying existing SG", "vpc-id", vpcID, "name", sgName)
336336
sgs, err := p.ec2Client.DescribeSecurityGroupsAsList(ctx, req)
337337
if err != nil && !isEC2SecurityGroupNotFoundError(err) {
338338
return "", err

0 commit comments

Comments
 (0)