Ambiguity when checking for existence of Backend SGs

[visit1985]

When searching for the existence of backend SGs, the controller searches by tags and does not consider the case where multiple SGs with that tags are returned.

To avoid ambiguity, the below function should either search by SG name (which is already present as an input parameter), or return an error when multiple SGs with the given tags are found.

aws-load-balancer-controller/pkg/networking/backend_sg_provider.go

Lines 284 to 310 in 8ba34e2

	func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgName string, vpcID string) (string, error) {
	req := &ec2sdk.DescribeSecurityGroupsInput{
	Filters: []ec2types.Filter{
	{
	Name: awssdk.String("vpc-id"),
	Values: []string{vpcID},
	},
	{
	Name: awssdk.String(fmt.Sprintf("tag:%v", tagKeyK8sCluster)),
	Values: []string{p.clusterName},
	},
	{
	Name: awssdk.String(fmt.Sprintf("tag:%v", tagKeyResource)),
	Values: []string{tagValueBackend},
	},
	},
	}
	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
	}
	if len(sgs) > 0 {
	return awssdk.ToString(sgs[0].GroupId), nil
	}
	return "", nil
	}

[shraddhabang]

Hey @visit1985 , While configuring the backend sg, the LBC uses a shared backend SG for all the resources which it creates once and applies these tags. So the customer should avoid to use the same tag for other sgs.
If you want to configure your own backend sg, you can disable this automanagement by setting flag --enable-backend-security-group=false. Then in that case the LBC will configure the SG that you provide.

More info on SG management : https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/deploy/security_groups/#backend-security-groups

[visit1985]

Yes, right. But even the it’s better to fail instead if using a random SG via sgs[0].

[M00nF1sh]

@visit1985
We should avoid dependencies on the name(which allows us to change naming patterns)
I'm curious why you would run into this issue? since the backendSG is already scoped by the clusterTag & resourceTag and vpcID, there shouldn't be more than one such backendSG exists. Did you created a SG with same set of tags out of the controller?

[visit1985]

Yes, someone created it during preparation to migrate to command line flag --backend-security-group. For some deployments this resulted in a scenario where both SGs are in use.

Adding a raise condition for len(sgs) > 1 should be backwards compatible, does not create a dependency on name and would inform the customer that he should avoid to use the same tag for other SGs.