Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: sync created Backend SG tags #3990

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: sync created Backend SG tags #3990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c679bfd
init
phuhung273 Dec 19, 2024
3f312a0
unit test
phuhung273 Dec 20, 2024
597eb70
cleanup
phuhung273 Dec 20, 2024
e175472
improve reconcile tag logic
phuhung273 Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 84 additions & 6 deletions pkg/networking/backend_sg_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
corev1 "k8s.io/api/core/v1"
networking "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm"
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
"sigs.k8s.io/aws-load-balancer-controller/pkg/runtime"
Expand Down Expand Up @@ -227,13 +229,20 @@ func (p *defaultBackendSGProvider) allocateBackendSG(ctx context.Context, resour
}

sgName := p.getBackendSGName()
sgID, err := p.getBackendSGFromEC2(ctx, sgName, p.vpcID)
sg, err := p.getBackendSGFromEC2(ctx, sgName, p.vpcID)
if err != nil {
return err
}
if len(sgID) > 1 {
if sg != nil {
sgID := awssdk.ToString(sg.GroupId)
p.logger.V(1).Info("Existing SG found", "id", sgID)
p.autoGeneratedSG = sgID

if err = p.reconcileTags(ctx, *sg); err != nil {
p.logger.Error(err, "failed to synchronize tags of existing securityGroup", sgID)
return err
}
p.logger.Info("added resource tags", "resourceID", sgID)
return nil
}

Expand All @@ -253,6 +262,59 @@ func (p *defaultBackendSGProvider) allocateBackendSG(ctx context.Context, resour
return nil
}

func (p *defaultBackendSGProvider) reconcileTags(ctx context.Context, sg ec2types.SecurityGroup) error {
desiredTags := p.buildBackendSGTagsMap()
currentTags := make(map[string]string)
for _, tag := range sg.Tags {
currentTags[awssdk.ToString(tag.Key)] = awssdk.ToString(tag.Value)
}

tagsToUpdate, tagsToRemove := algorithm.DiffStringMap(desiredTags, currentTags)

if len(tagsToUpdate) > 0 {
req := &ec2sdk.CreateTagsInput{
Resources: []string{*sg.GroupId},
Tags: convertTagsToSDKTags(tagsToUpdate),
}

p.logger.Info("adding resource tags",
"resourceID", sg.GroupId,
"change", tagsToUpdate)
if _, err := p.ec2Client.CreateTagsWithContext(ctx, req); err != nil {
return err
}
p.logger.Info("added resource tags",
"resourceID", sg.GroupId)
}

if len(tagsToRemove) > 0 {
req := &ec2sdk.DeleteTagsInput{
Resources: []string{*sg.GroupId},
Tags: convertTagsToSDKTags(tagsToRemove),
}

p.logger.Info("removing resource tags",
"resourceID", sg.GroupId,
"change", tagsToRemove)
if _, err := p.ec2Client.DeleteTagsWithContext(ctx, req); err != nil {
return err
}
p.logger.Info("removed resource tags",
"resourceID", sg.GroupId)
}
return nil
}

func (p *defaultBackendSGProvider) buildBackendSGTagsMap() map[string]string {
defaultTags := make(map[string]string)
for key, val := range p.defaultTags {
defaultTags[key] = val
}
defaultTags[tagKeyK8sCluster] = p.clusterName
defaultTags[tagKeyResource] = tagValueBackend
return defaultTags
}

func (p *defaultBackendSGProvider) buildBackendSGTags(_ context.Context) []ec2types.TagSpecification {
var defaultTags []ec2types.Tag
for key, val := range p.defaultTags {
Expand Down Expand Up @@ -281,7 +343,7 @@ func (p *defaultBackendSGProvider) buildBackendSGTags(_ context.Context) []ec2ty
}
}

func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgName string, vpcID string) (string, error) {
func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgName string, vpcID string) (*ec2types.SecurityGroup, error) {
req := &ec2sdk.DescribeSecurityGroupsInput{
Filters: []ec2types.Filter{
{
Expand All @@ -301,12 +363,12 @@ func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgNa
p.logger.V(1).Info("Queriying existing SG", "vpc-id", vpcID, "name", sgName)
sgs, err := p.ec2Client.DescribeSecurityGroupsAsList(ctx, req)
if err != nil && !isEC2SecurityGroupNotFoundError(err) {
return "", err
return nil, err
}
if len(sgs) > 0 {
return awssdk.ToString(sgs[0].GroupId), nil
return &sgs[0], nil
}
return "", nil
return nil, nil
}

func (p *defaultBackendSGProvider) releaseSG(ctx context.Context) error {
Expand Down Expand Up @@ -364,3 +426,19 @@ func isEC2SecurityGroupNotFoundError(err error) bool {
func getObjectKey(resourceType ResourceType, resource types.NamespacedName) string {
return string(resourceType) + "/" + resource.String()
}

// convert tags into AWS SDK tag presentation.
func convertTagsToSDKTags(tags map[string]string) []ec2types.Tag {
if len(tags) == 0 {
return nil
}
sdkTags := make([]ec2types.Tag, 0, len(tags))

for _, key := range sets.StringKeySet(tags).List() {
sdkTags = append(sdkTags, ec2types.Tag{
Key: awssdk.String(key),
Value: awssdk.String(tags[key]),
})
}
return sdkTags
}
173 changes: 167 additions & 6 deletions pkg/networking/backend_sg_provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,25 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
resp *ec2sdk.CreateSecurityGroupOutput
err error
}
type createTagsWithContextCall struct {
req *ec2sdk.CreateTagsInput
resp *ec2sdk.CreateTagsOutput
err error
}
type deleteTagsWithContextCall struct {
req *ec2sdk.DeleteTagsInput
resp *ec2sdk.DeleteTagsOutput
err error
}
type fields struct {
backendSG string
ingResources []*networking.Ingress
svcResource *corev1.Service
defaultTags map[string]string
describeSGCalls []describeSecurityGroupsAsListCall
createSGCalls []createSecurityGroupWithContexCall
backendSG string
ingResources []*networking.Ingress
svcResource *corev1.Service
defaultTags map[string]string
describeSGCalls []describeSecurityGroupsAsListCall
createSGCalls []createSecurityGroupWithContexCall
createTagsWithContextCalls []createTagsWithContextCall
deleteTagsWithContextCalls []deleteTagsWithContextCall
}
defaultEC2Filters := []ec2types.Filter{
{
Expand Down Expand Up @@ -110,10 +122,153 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
},
},
},
createTagsWithContextCalls: []createTagsWithContextCall{
{
req: &ec2sdk.CreateTagsInput{
Resources: []string{"sg-autogen"},
Tags: []ec2types.Tag{
{
Key: awssdk.String("elbv2.k8s.aws/cluster"),
Value: awssdk.String(defaultClusterName),
},
{
Key: awssdk.String("elbv2.k8s.aws/resource"),
Value: awssdk.String("backend-sg"),
},
},
},
},
},
ingResources: []*networking.Ingress{ing, ing1},
},
want: "sg-autogen",
},
{
name: "backend sg enabled, auto-gen, SG exists, try to sync tags",
fields: fields{
describeSGCalls: []describeSecurityGroupsAsListCall{
{
req: &ec2sdk.DescribeSecurityGroupsInput{
Filters: defaultEC2Filters,
},
resp: []ec2types.SecurityGroup{
{
GroupId: awssdk.String("sg-autogen"),
Tags: []ec2types.Tag{
{
Key: awssdk.String("tag-to-be-deleted"),
Value: awssdk.String("delete-me"),
},
},
},
},
},
},
createTagsWithContextCalls: []createTagsWithContextCall{
{
req: &ec2sdk.CreateTagsInput{
Resources: []string{"sg-autogen"},
Tags: []ec2types.Tag{
{
Key: awssdk.String("KubernetesCluster"),
Value: awssdk.String(defaultClusterName),
},
{
Key: awssdk.String("defaultTag"),
Value: awssdk.String("specified"),
},
{
Key: awssdk.String("elbv2.k8s.aws/cluster"),
Value: awssdk.String(defaultClusterName),
},
{
Key: awssdk.String("elbv2.k8s.aws/resource"),
Value: awssdk.String("backend-sg"),
},
{
Key: awssdk.String("zzzKey"),
Value: awssdk.String("value"),
},
},
},
},
},
deleteTagsWithContextCalls: []deleteTagsWithContextCall{
{
req: &ec2sdk.DeleteTagsInput{
Resources: []string{"sg-autogen"},
Tags: []ec2types.Tag{
{
Key: awssdk.String("tag-to-be-deleted"),
Value: awssdk.String("delete-me"),
},
},
},
},
},
defaultTags: map[string]string{
"zzzKey": "value",
"KubernetesCluster": defaultClusterName,
"defaultTag": "specified",
},
ingResources: []*networking.Ingress{ing, ing1},
},
want: "sg-autogen",
},
{
name: "backend sg enabled, auto-gen, SG exists, tags sync error",
fields: fields{
describeSGCalls: []describeSecurityGroupsAsListCall{
{
req: &ec2sdk.DescribeSecurityGroupsInput{
Filters: defaultEC2Filters,
},
resp: []ec2types.SecurityGroup{
{
GroupId: awssdk.String("sg-autogen"),
},
},
},
},
createTagsWithContextCalls: []createTagsWithContextCall{
{
req: &ec2sdk.CreateTagsInput{
Resources: []string{"sg-autogen"},
Tags: []ec2types.Tag{
{
Key: awssdk.String("KubernetesCluster"),
Value: awssdk.String(defaultClusterName),
},
{
Key: awssdk.String("defaultTag"),
Value: awssdk.String("specified"),
},
{
Key: awssdk.String("elbv2.k8s.aws/cluster"),
Value: awssdk.String(defaultClusterName),
},
{
Key: awssdk.String("elbv2.k8s.aws/resource"),
Value: awssdk.String("backend-sg"),
},
{
Key: awssdk.String("zzzKey"),
Value: awssdk.String("value"),
},
},
},
err: &smithy.GenericAPIError{Code: "Some.Other.Error", Message: "unable to tag security group"},
},
},
defaultTags: map[string]string{
"zzzKey": "value",
"KubernetesCluster": defaultClusterName,
"defaultTag": "specified",
},
svcResource: svc,
},
wantErr: errors.New("api error Some.Other.Error: unable to tag security group"),
},
{
name: "backend sg enabled, auto-gen new SG",
fields: fields{
Expand Down Expand Up @@ -283,6 +438,12 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
for _, call := range tt.fields.createSGCalls {
ec2Client.EXPECT().CreateSecurityGroupWithContext(context.Background(), call.req).Return(call.resp, call.err)
}
for _, call := range tt.fields.createTagsWithContextCalls {
ec2Client.EXPECT().CreateTagsWithContext(context.Background(), call.req).Return(call.resp, call.err)
}
for _, call := range tt.fields.deleteTagsWithContextCalls {
ec2Client.EXPECT().DeleteTagsWithContext(gomock.Any(), call.req).Return(call.resp, call.err)
}
k8sClient := mock_client.NewMockClient(ctrl)
sgProvider := NewBackendSGProvider(defaultClusterName, tt.fields.backendSG,
defaultVPCID, ec2Client, k8sClient, tt.fields.defaultTags, logr.New(&log.NullLogSink{}))
Expand Down