Skip to content

Commit 7cba312

Browse files
phuhung273phuhung273
authored
feat: sync created Backend SG tags (#3990)
* init * unit test * cleanup * improve reconcile tag logic * fix conflict
1 parent c1e8c52 commit 7cba312

File tree

2 files changed

+255
-15
lines changed

2 files changed

+255
-15
lines changed

‎pkg/networking/backend_sg_provider.go‎

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
corev1 "k8s.io/api/core/v1"
2222
networking "k8s.io/api/networking/v1"
2323
"k8s.io/apimachinery/pkg/types"
24+
"k8s.io/apimachinery/pkg/util/sets"
25+
"sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm"
2426
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
2527
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
2628
"sigs.k8s.io/aws-load-balancer-controller/pkg/runtime"
@@ -261,13 +263,20 @@ func (p *defaultBackendSGProvider) allocateBackendSG(ctx context.Context, resour
261263
}
262264

263265
sgName := p.getBackendSGName()
264-
sgID, err := p.getBackendSGFromEC2(ctx, sgName, p.vpcID)
266+
sg, err := p.getBackendSGFromEC2(ctx, sgName, p.vpcID)
265267
if err != nil {
266268
return err
267269
}
268-
if len(sgID) > 1 {
270+
if sg != nil {
271+
sgID := awssdk.ToString(sg.GroupId)
269272
p.logger.V(1).Info("Existing SG found", "id", sgID)
270273
p.autoGeneratedSG = sgID
274+
275+
if err = p.reconcileTags(ctx, *sg); err != nil {
276+
p.logger.Error(err, "failed to synchronize tags of existing securityGroup", sgID)
277+
return err
278+
}
279+
p.logger.Info("added resource tags", "resourceID", sgID)
271280
return nil
272281
}
273282

@@ -287,6 +296,59 @@ func (p *defaultBackendSGProvider) allocateBackendSG(ctx context.Context, resour
287296
return nil
288297
}
289298

299+
func (p *defaultBackendSGProvider) reconcileTags(ctx context.Context, sg ec2types.SecurityGroup) error {
300+
desiredTags := p.buildBackendSGTagsMap()
301+
currentTags := make(map[string]string)
302+
for _, tag := range sg.Tags {
303+
currentTags[awssdk.ToString(tag.Key)] = awssdk.ToString(tag.Value)
304+
}
305+
306+
tagsToUpdate, tagsToRemove := algorithm.DiffStringMap(desiredTags, currentTags)
307+
308+
if len(tagsToUpdate) > 0 {
309+
req := &ec2sdk.CreateTagsInput{
310+
Resources: []string{*sg.GroupId},
311+
Tags: convertTagsToSDKTags(tagsToUpdate),
312+
}
313+
314+
p.logger.Info("adding resource tags",
315+
"resourceID", sg.GroupId,
316+
"change", tagsToUpdate)
317+
if _, err := p.ec2Client.CreateTagsWithContext(ctx, req); err != nil {
318+
return err
319+
}
320+
p.logger.Info("added resource tags",
321+
"resourceID", sg.GroupId)
322+
}
323+
324+
if len(tagsToRemove) > 0 {
325+
req := &ec2sdk.DeleteTagsInput{
326+
Resources: []string{*sg.GroupId},
327+
Tags: convertTagsToSDKTags(tagsToRemove),
328+
}
329+
330+
p.logger.Info("removing resource tags",
331+
"resourceID", sg.GroupId,
332+
"change", tagsToRemove)
333+
if _, err := p.ec2Client.DeleteTagsWithContext(ctx, req); err != nil {
334+
return err
335+
}
336+
p.logger.Info("removed resource tags",
337+
"resourceID", sg.GroupId)
338+
}
339+
return nil
340+
}
341+
342+
func (p *defaultBackendSGProvider) buildBackendSGTagsMap() map[string]string {
343+
defaultTags := make(map[string]string)
344+
for key, val := range p.defaultTags {
345+
defaultTags[key] = val
346+
}
347+
defaultTags[shared_constants.TagKeyK8sCluster] = p.clusterName
348+
defaultTags[shared_constants.TagKeyResource] = tagValueBackend
349+
return defaultTags
350+
}
351+
290352
func (p *defaultBackendSGProvider) buildBackendSGTags(_ context.Context) []ec2types.TagSpecification {
291353
var defaultTags []ec2types.Tag
292354
for key, val := range p.defaultTags {
@@ -315,7 +377,7 @@ func (p *defaultBackendSGProvider) buildBackendSGTags(_ context.Context) []ec2ty
315377
}
316378
}
317379

318-
func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgName string, vpcID string) (string, error) {
380+
func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgName string, vpcID string) (*ec2types.SecurityGroup, error) {
319381
req := &ec2sdk.DescribeSecurityGroupsInput{
320382
Filters: []ec2types.Filter{
321383
{
@@ -335,12 +397,12 @@ func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgNa
335397
p.logger.V(1).Info("Querying existing SG", "vpc-id", vpcID, "name", sgName)
336398
sgs, err := p.ec2Client.DescribeSecurityGroupsAsList(ctx, req)
337399
if err != nil && !isEC2SecurityGroupNotFoundError(err) {
338-
return "", err
400+
return nil, err
339401
}
340402
if len(sgs) > 0 {
341-
return awssdk.ToString(sgs[0].GroupId), nil
403+
return &sgs[0], nil
342404
}
343-
return "", nil
405+
return nil, nil
344406
}
345407

346408
func (p *defaultBackendSGProvider) releaseSG(ctx context.Context) error {
@@ -398,3 +460,19 @@ func isEC2SecurityGroupNotFoundError(err error) bool {
398460
func getObjectKey(resourceType ResourceType, resource types.NamespacedName) string {
399461
return string(resourceType) + "/" + resource.String()
400462
}
463+
464+
// convert tags into AWS SDK tag presentation.
465+
func convertTagsToSDKTags(tags map[string]string) []ec2types.Tag {
466+
if len(tags) == 0 {
467+
return nil
468+
}
469+
sdkTags := make([]ec2types.Tag, 0, len(tags))
470+
471+
for _, key := range sets.StringKeySet(tags).List() {
472+
sdkTags = append(sdkTags, ec2types.Tag{
473+
Key: awssdk.String(key),
474+
Value: awssdk.String(tags[key]),
475+
})
476+
}
477+
return sdkTags
478+
}

‎pkg/networking/backend_sg_provider_test.go‎

Lines changed: 171 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ package networking
22

33
import (
44
"context"
5+
"reflect"
6+
"testing"
7+
58
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
69
"github.com/aws/smithy-go"
710
"k8s.io/apimachinery/pkg/types"
8-
"reflect"
911
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
1012
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
11-
"testing"
1213

1314
"github.com/go-logr/logr"
1415
corev1 "k8s.io/api/core/v1"
@@ -42,14 +43,26 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
4243
resp *ec2sdk.CreateSecurityGroupOutput
4344
err error
4445
}
46+
type createTagsWithContextCall struct {
47+
req *ec2sdk.CreateTagsInput
48+
resp *ec2sdk.CreateTagsOutput
49+
err error
50+
}
51+
type deleteTagsWithContextCall struct {
52+
req *ec2sdk.DeleteTagsInput
53+
resp *ec2sdk.DeleteTagsOutput
54+
err error
55+
}
4556
type fields struct {
46-
backendSG string
47-
ingResources []*networking.Ingress
48-
svcResource *corev1.Service
49-
enableGatewayCheck bool
50-
defaultTags map[string]string
51-
describeSGCalls []describeSecurityGroupsAsListCall
52-
createSGCalls []createSecurityGroupWithContexCall
57+
backendSG string
58+
ingResources []*networking.Ingress
59+
svcResource *corev1.Service
60+
enableGatewayCheck bool
61+
defaultTags map[string]string
62+
describeSGCalls []describeSecurityGroupsAsListCall
63+
createSGCalls []createSecurityGroupWithContexCall
64+
createTagsWithContextCalls []createTagsWithContextCall
65+
deleteTagsWithContextCalls []deleteTagsWithContextCall
5366
}
5467
defaultEC2Filters := []ec2types.Filter{
5568
{
@@ -112,10 +125,153 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
112125
},
113126
},
114127
},
128+
createTagsWithContextCalls: []createTagsWithContextCall{
129+
{
130+
req: &ec2sdk.CreateTagsInput{
131+
Resources: []string{"sg-autogen"},
132+
Tags: []ec2types.Tag{
133+
{
134+
Key: awssdk.String("elbv2.k8s.aws/cluster"),
135+
Value: awssdk.String(defaultClusterName),
136+
},
137+
{
138+
Key: awssdk.String("elbv2.k8s.aws/resource"),
139+
Value: awssdk.String("backend-sg"),
140+
},
141+
},
142+
},
143+
},
144+
},
145+
ingResources: []*networking.Ingress{ing, ing1},
146+
},
147+
want: "sg-autogen",
148+
},
149+
{
150+
name: "backend sg enabled, auto-gen, SG exists, try to sync tags",
151+
fields: fields{
152+
describeSGCalls: []describeSecurityGroupsAsListCall{
153+
{
154+
req: &ec2sdk.DescribeSecurityGroupsInput{
155+
Filters: defaultEC2Filters,
156+
},
157+
resp: []ec2types.SecurityGroup{
158+
{
159+
GroupId: awssdk.String("sg-autogen"),
160+
Tags: []ec2types.Tag{
161+
{
162+
Key: awssdk.String("tag-to-be-deleted"),
163+
Value: awssdk.String("delete-me"),
164+
},
165+
},
166+
},
167+
},
168+
},
169+
},
170+
createTagsWithContextCalls: []createTagsWithContextCall{
171+
{
172+
req: &ec2sdk.CreateTagsInput{
173+
Resources: []string{"sg-autogen"},
174+
Tags: []ec2types.Tag{
175+
{
176+
Key: awssdk.String("KubernetesCluster"),
177+
Value: awssdk.String(defaultClusterName),
178+
},
179+
{
180+
Key: awssdk.String("defaultTag"),
181+
Value: awssdk.String("specified"),
182+
},
183+
{
184+
Key: awssdk.String("elbv2.k8s.aws/cluster"),
185+
Value: awssdk.String(defaultClusterName),
186+
},
187+
{
188+
Key: awssdk.String("elbv2.k8s.aws/resource"),
189+
Value: awssdk.String("backend-sg"),
190+
},
191+
{
192+
Key: awssdk.String("zzzKey"),
193+
Value: awssdk.String("value"),
194+
},
195+
},
196+
},
197+
},
198+
},
199+
deleteTagsWithContextCalls: []deleteTagsWithContextCall{
200+
{
201+
req: &ec2sdk.DeleteTagsInput{
202+
Resources: []string{"sg-autogen"},
203+
Tags: []ec2types.Tag{
204+
{
205+
Key: awssdk.String("tag-to-be-deleted"),
206+
Value: awssdk.String("delete-me"),
207+
},
208+
},
209+
},
210+
},
211+
},
212+
defaultTags: map[string]string{
213+
"zzzKey": "value",
214+
"KubernetesCluster": defaultClusterName,
215+
"defaultTag": "specified",
216+
},
115217
ingResources: []*networking.Ingress{ing, ing1},
116218
},
117219
want: "sg-autogen",
118220
},
221+
{
222+
name: "backend sg enabled, auto-gen, SG exists, tags sync error",
223+
fields: fields{
224+
describeSGCalls: []describeSecurityGroupsAsListCall{
225+
{
226+
req: &ec2sdk.DescribeSecurityGroupsInput{
227+
Filters: defaultEC2Filters,
228+
},
229+
resp: []ec2types.SecurityGroup{
230+
{
231+
GroupId: awssdk.String("sg-autogen"),
232+
},
233+
},
234+
},
235+
},
236+
createTagsWithContextCalls: []createTagsWithContextCall{
237+
{
238+
req: &ec2sdk.CreateTagsInput{
239+
Resources: []string{"sg-autogen"},
240+
Tags: []ec2types.Tag{
241+
{
242+
Key: awssdk.String("KubernetesCluster"),
243+
Value: awssdk.String(defaultClusterName),
244+
},
245+
{
246+
Key: awssdk.String("defaultTag"),
247+
Value: awssdk.String("specified"),
248+
},
249+
{
250+
Key: awssdk.String("elbv2.k8s.aws/cluster"),
251+
Value: awssdk.String(defaultClusterName),
252+
},
253+
{
254+
Key: awssdk.String("elbv2.k8s.aws/resource"),
255+
Value: awssdk.String("backend-sg"),
256+
},
257+
{
258+
Key: awssdk.String("zzzKey"),
259+
Value: awssdk.String("value"),
260+
},
261+
},
262+
},
263+
err: &smithy.GenericAPIError{Code: "Some.Other.Error", Message: "unable to tag security group"},
264+
},
265+
},
266+
defaultTags: map[string]string{
267+
"zzzKey": "value",
268+
"KubernetesCluster": defaultClusterName,
269+
"defaultTag": "specified",
270+
},
271+
svcResource: svc,
272+
},
273+
wantErr: errors.New("api error Some.Other.Error: unable to tag security group"),
274+
},
119275
{
120276
name: "backend sg enabled, auto-gen new SG",
121277
fields: fields{
@@ -285,6 +441,12 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
285441
for _, call := range tt.fields.createSGCalls {
286442
ec2Client.EXPECT().CreateSecurityGroupWithContext(context.Background(), call.req).Return(call.resp, call.err)
287443
}
444+
for _, call := range tt.fields.createTagsWithContextCalls {
445+
ec2Client.EXPECT().CreateTagsWithContext(context.Background(), call.req).Return(call.resp, call.err)
446+
}
447+
for _, call := range tt.fields.deleteTagsWithContextCalls {
448+
ec2Client.EXPECT().DeleteTagsWithContext(gomock.Any(), call.req).Return(call.resp, call.err)
449+
}
288450
k8sClient := mock_client.NewMockClient(ctrl)
289451
sgProvider := NewBackendSGProvider(defaultClusterName, tt.fields.backendSG,
290452
defaultVPCID, ec2Client, k8sClient, tt.fields.defaultTags, tt.fields.enableGatewayCheck, logr.New(&log.NullLogSink{}))

0 commit comments

Comments
 (0)