Skip to content

Commit

Permalink
build backend sg name and cluster sg rule label based on controller c…
Browse files Browse the repository at this point in the history
…onfig
  • Loading branch information
oliviassss committed Oct 24, 2024
1 parent d694388 commit 96e66f6
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 66 deletions.
4 changes: 2 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ func main() {
tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(),
podInfoRepo, sgManager, sgReconciler, vpcInfoProvider, multiClusterManager,
cloud.VpcID(), controllerCFG.ClusterName, controllerCFG.FeatureGates.Enabled(config.EndpointsFailOpen), controllerCFG.EnableEndpointSlices, controllerCFG.DisableRestrictedSGRules,
controllerCFG.ServiceTargetENISGTags, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
controllerCFG.ServiceTargetENISGTags, controllerCFG.ResourcePrefix[config.ClusterSgRuleLabelPrefixKey], mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
cloud.VpcID(), cloud.EC2(), mgr.GetClient(), controllerCFG.DefaultTags, ctrl.Log.WithName("backend-sg-provider"))
cloud.VpcID(), cloud.EC2(), mgr.GetClient(), controllerCFG.ResourcePrefix[config.ClusterTagPrefixKey], controllerCFG.ResourcePrefix[config.BackendSGNamePrefixKey], controllerCFG.DefaultTags, ctrl.Log.WithName("backend-sg-provider"))
sgResolver := networking.NewDefaultSecurityGroupResolver(cloud.EC2(), cloud.VpcID())
elbv2TaggingManager := elbv2deploy.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), controllerCFG.FeatureGates, cloud.RGT(), ctrl.Log)
ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),
Expand Down
48 changes: 26 additions & 22 deletions pkg/networking/backend_sg_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ const (
defaultSGDeletionTimeout = 2 * time.Minute

resourceTypeSecurityGroup = "security-group"
tagKeyK8sCluster = "elbv2.k8s.aws/cluster"
tagKeyResource = "elbv2.k8s.aws/resource"
tagValueBackend = "backend-sg"

explicitGroupFinalizerPrefix = "group.ingress.k8s.aws/"
Expand All @@ -59,16 +57,19 @@ type BackendSGProvider interface {

// NewBackendSGProvider constructs a new defaultBackendSGProvider
func NewBackendSGProvider(clusterName string, backendSG string, vpcID string,
ec2Client services.EC2, k8sClient client.Client, defaultTags map[string]string, logger logr.Logger) *defaultBackendSGProvider {
ec2Client services.EC2, k8sClient client.Client, clusterTagPrefixKey string, backendSGNamePrefix string, defaultTags map[string]string, logger logr.Logger) *defaultBackendSGProvider {
return &defaultBackendSGProvider{
vpcID: vpcID,
clusterName: clusterName,
backendSG: backendSG,
defaultTags: defaultTags,
ec2Client: ec2Client,
k8sClient: k8sClient,
logger: logger,
mutex: sync.Mutex{},
vpcID: vpcID,
clusterName: clusterName,
backendSG: backendSG,
tagKeyK8sCluster: clusterTagPrefixKey + "/cluster",
tagKeyResource: clusterTagPrefixKey + "/resource",
backendSGNamePrefix: backendSGNamePrefix,
defaultTags: defaultTags,
ec2Client: ec2Client,
k8sClient: k8sClient,
logger: logger,
mutex: sync.Mutex{},

checkIngressFinalizersFunc: func(finalizers []string) bool {
for _, fin := range finalizers {
Expand Down Expand Up @@ -100,12 +101,15 @@ type defaultBackendSGProvider struct {
clusterName string
mutex sync.Mutex

backendSG string
autoGeneratedSG string
defaultTags map[string]string
ec2Client services.EC2
k8sClient client.Client
logger logr.Logger
backendSG string
autoGeneratedSG string
tagKeyK8sCluster string
tagKeyResource string
backendSGNamePrefix string
defaultTags map[string]string
ec2Client services.EC2
k8sClient client.Client
logger logr.Logger
// objectsMap keeps track of whether the backend SG is required for any tracked resources in the cluster.
// If any entry in the map is true, or there are resources with this controller specific finalizers which
// haven't been tracked in the map yet, controller doesn't delete the backend SG. If the controller has
Expand Down Expand Up @@ -269,11 +273,11 @@ func (p *defaultBackendSGProvider) buildBackendSGTags(_ context.Context) []ec2ty
ResourceType: resourceTypeSecurityGroup,
Tags: append(defaultTags, []ec2types.Tag{
{
Key: awssdk.String(tagKeyK8sCluster),
Key: awssdk.String(p.tagKeyK8sCluster),
Value: awssdk.String(p.clusterName),
},
{
Key: awssdk.String(tagKeyResource),
Key: awssdk.String(p.tagKeyResource),
Value: awssdk.String(tagValueBackend),
},
}...),
Expand All @@ -289,11 +293,11 @@ func (p *defaultBackendSGProvider) getBackendSGFromEC2(ctx context.Context, sgNa
Values: []string{vpcID},
},
{
Name: awssdk.String(fmt.Sprintf("tag:%v", tagKeyK8sCluster)),
Name: awssdk.String(fmt.Sprintf("tag:%v", p.tagKeyK8sCluster)),
Values: []string{p.clusterName},
},
{
Name: awssdk.String(fmt.Sprintf("tag:%v", tagKeyResource)),
Name: awssdk.String(fmt.Sprintf("tag:%v", p.tagKeyResource)),
Values: []string{tagValueBackend},
},
},
Expand Down Expand Up @@ -342,7 +346,7 @@ func (p *defaultBackendSGProvider) getBackendSGName() string {
_, _ = sgNameHash.Write([]byte(p.clusterName))
sgHash := hex.EncodeToString(sgNameHash.Sum(nil))
sanitizedClusterName := invalidSGNamePattern.ReplaceAllString(p.clusterName, "")
return fmt.Sprintf("k8s-traffic-%.232s-%.10s", sanitizedClusterName, sgHash)
return fmt.Sprintf("%v-%.232s-%.10s", p.backendSGNamePrefix, sanitizedClusterName, sgHash)
}

func isSecurityGroupDependencyViolationError(err error) bool {
Expand Down
45 changes: 31 additions & 14 deletions pkg/networking/backend_sg_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ import (
)

const (
defaultVPCID = "vpc-xxxyyy"
defaultClusterName = "testCluster"
defaultVPCID = "vpc-xxxyyy"
defaultClusterName = "testCluster"
defaultClusterTagPrefixKey = "elbv2.k8s.aws"
defaultBackendSGNamePrefix = "k8s-traffic"
defaultTagKeyK8sCluster = "elbv2.k8s.aws/cluster"
defaultTagKeyResource = "elbv2.k8s.aws/resource"
)

func Test_defaultBackendSGProvider_Get(t *testing.T) {
Expand All @@ -42,12 +46,14 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
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
clusterTagPrefixKey string
backendSGNamePrefix string
defaultTags map[string]string
describeSGCalls []describeSecurityGroupsAsListCall
createSGCalls []createSecurityGroupWithContexCall
}
defaultEC2Filters := []ec2types.Filter{
{
Expand Down Expand Up @@ -110,7 +116,8 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
},
},
},
ingResources: []*networking.Ingress{ing, ing1},
ingResources: []*networking.Ingress{ing, ing1},
clusterTagPrefixKey: defaultClusterTagPrefixKey,
},
want: "sg-autogen",
},
Expand Down Expand Up @@ -152,7 +159,9 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
},
},
},
ingResources: []*networking.Ingress{ing, ing1},
ingResources: []*networking.Ingress{ing, ing1},
clusterTagPrefixKey: defaultClusterTagPrefixKey,
backendSGNamePrefix: defaultBackendSGNamePrefix,
},
want: "sg-newauto",
},
Expand Down Expand Up @@ -206,6 +215,8 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
},
},
},
clusterTagPrefixKey: defaultClusterTagPrefixKey,
backendSGNamePrefix: defaultBackendSGNamePrefix,
defaultTags: map[string]string{
"zzzKey": "value",
"KubernetesCluster": defaultClusterName,
Expand All @@ -226,7 +237,9 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
err: &smithy.GenericAPIError{Code: "Some.Other.Error", Message: "describe security group as list error"},
},
},
ingResources: []*networking.Ingress{ing},
ingResources: []*networking.Ingress{ing},
clusterTagPrefixKey: defaultClusterTagPrefixKey,
backendSGNamePrefix: defaultBackendSGNamePrefix,
},
wantErr: errors.New("api error Some.Other.Error: describe security group as list error"),
},
Expand Down Expand Up @@ -266,7 +279,9 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
err: &smithy.GenericAPIError{Code: "Create.Error", Message: "unable to create security group"},
},
},
ingResources: []*networking.Ingress{ing1},
ingResources: []*networking.Ingress{ing1},
clusterTagPrefixKey: defaultClusterTagPrefixKey,
backendSGNamePrefix: defaultBackendSGNamePrefix,
},
wantErr: errors.New("api error Create.Error: unable to create security group"),
},
Expand All @@ -285,7 +300,7 @@ func Test_defaultBackendSGProvider_Get(t *testing.T) {
}
k8sClient := mock_client.NewMockClient(ctrl)
sgProvider := NewBackendSGProvider(defaultClusterName, tt.fields.backendSG,
defaultVPCID, ec2Client, k8sClient, tt.fields.defaultTags, logr.New(&log.NullLogSink{}))
defaultVPCID, ec2Client, k8sClient, tt.fields.clusterTagPrefixKey, tt.fields.backendSGNamePrefix, tt.fields.defaultTags, logr.New(&log.NullLogSink{}))

resourceType := ResourceTypeIngress
var activeResources []types.NamespacedName
Expand Down Expand Up @@ -329,6 +344,8 @@ func Test_defaultBackendSGProvider_Release(t *testing.T) {
type fields struct {
autogenSG string
backendSG string
clusterTagPrefixKey string
backendSGNamePrefix string
defaultTags map[string]string
listIngressCalls []listIngressCall
deleteSGCalls []deleteSecurityGroupWithContextCall
Expand Down Expand Up @@ -732,7 +749,7 @@ func Test_defaultBackendSGProvider_Release(t *testing.T) {
ec2Client := services.NewMockEC2(ctrl)
k8sClient := mock_client.NewMockClient(ctrl)
sgProvider := NewBackendSGProvider(defaultClusterName, tt.fields.backendSG,
defaultVPCID, ec2Client, k8sClient, tt.fields.defaultTags, logr.New(&log.NullLogSink{}))
defaultVPCID, ec2Client, k8sClient, tt.fields.clusterTagPrefixKey, tt.fields.backendSGNamePrefix, tt.fields.defaultTags, logr.New(&log.NullLogSink{}))
if len(tt.fields.autogenSG) > 0 {
sgProvider.backendSG = ""
sgProvider.autoGeneratedSG = tt.fields.autogenSG
Expand Down
49 changes: 25 additions & 24 deletions pkg/targetgroupbinding/networking_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
)

const (
tgbNetworkingIPPermissionLabelKey = "elbv2.k8s.aws/targetGroupBinding"
//tgbNetworkingIPPermissionLabelKey = "elbv2.k8s.aws/targetGroupBinding"
tgbNetworkingIPPermissionLabelValue = "shared"
defaultTgbMinPort = int32(0)
defaultTgbMaxPort = int32(65535)
Expand All @@ -47,18 +47,18 @@ type NetworkingManager interface {

// NewDefaultNetworkingManager constructs defaultNetworkingManager.
func NewDefaultNetworkingManager(k8sClient client.Client, podENIResolver networking.PodENIInfoResolver, nodeENIResolver networking.NodeENIInfoResolver,
sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler, vpcID string, clusterName string, serviceTargetENISGTags map[string]string, logger logr.Logger, disabledRestrictedSGRulesFlag bool) *defaultNetworkingManager {

sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler, vpcID string, clusterName string, serviceTargetENISGTags map[string]string, clusterSgRuleLabelPrefix string, logger logr.Logger, disabledRestrictedSGRulesFlag bool) *defaultNetworkingManager {
return &defaultNetworkingManager{
k8sClient: k8sClient,
podENIResolver: podENIResolver,
nodeENIResolver: nodeENIResolver,
sgManager: sgManager,
sgReconciler: sgReconciler,
vpcID: vpcID,
clusterName: clusterName,
serviceTargetENISGTags: serviceTargetENISGTags,
logger: logger,
k8sClient: k8sClient,
podENIResolver: podENIResolver,
nodeENIResolver: nodeENIResolver,
sgManager: sgManager,
sgReconciler: sgReconciler,
vpcID: vpcID,
clusterName: clusterName,
serviceTargetENISGTags: serviceTargetENISGTags,
tgbNetworkingIPPermissionLabelKey: clusterSgRuleLabelPrefix + "/targetGroupBinding",
logger: logger,

mutex: sync.Mutex{},
ingressPermissionsPerSGByTGB: make(map[types.NamespacedName]map[string][]networking.IPPermissionInfo),
Expand All @@ -70,15 +70,16 @@ func NewDefaultNetworkingManager(k8sClient client.Client, podENIResolver network

// default implementation for NetworkingManager.
type defaultNetworkingManager struct {
k8sClient client.Client
podENIResolver networking.PodENIInfoResolver
nodeENIResolver networking.NodeENIInfoResolver
sgManager networking.SecurityGroupManager
sgReconciler networking.SecurityGroupReconciler
vpcID string
clusterName string
serviceTargetENISGTags map[string]string
logger logr.Logger
k8sClient client.Client
podENIResolver networking.PodENIInfoResolver
nodeENIResolver networking.NodeENIInfoResolver
sgManager networking.SecurityGroupManager
sgReconciler networking.SecurityGroupReconciler
vpcID string
clusterName string
serviceTargetENISGTags map[string]string
tgbNetworkingIPPermissionLabelKey string
logger logr.Logger

// mutex will serialize our TargetGroup's networking reconcile requests.
mutex sync.Mutex
Expand Down Expand Up @@ -202,7 +203,7 @@ func (m *defaultNetworkingManager) reconcileWithIngressPermissionsPerSG(ctx cont
computedForAllTGBs := m.consolidateIngressPermissionsPerSGByTGB(ctx, tgbsWithNetworking)
aggregatedIngressPermissionsPerSG := m.computeAggregatedIngressPermissionsPerSG(ctx)

permissionSelector := labels.SelectorFromSet(labels.Set{tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue})
permissionSelector := labels.SelectorFromSet(labels.Set{m.tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue})
var sgReconciliationErrors []error
for sgID, permissions := range aggregatedIngressPermissionsPerSG {
if err := m.sgReconciler.ReconcileIngress(ctx, sgID, permissions,
Expand Down Expand Up @@ -421,7 +422,7 @@ func (m *defaultNetworkingManager) computePermissionsForPeerPort(ctx context.Con
})
}

permissionLabels := map[string]string{tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue}
permissionLabels := map[string]string{m.tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue}
if peer.SecurityGroup != nil {
groupID := peer.SecurityGroup.GroupID
permissions := make([]networking.IPPermissionInfo, 0, len(sdkFromToPortPairs))
Expand Down Expand Up @@ -484,7 +485,7 @@ func (m *defaultNetworkingManager) gcIngressPermissionsFromUnusedEndpointSGs(ctx
usedEndpointSGs := sets.StringKeySet(ingressPermissionsPerSG)
unusedEndpointSGs := endpointSGs.Difference(usedEndpointSGs)

permissionSelector := labels.SelectorFromSet(labels.Set{tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue})
permissionSelector := labels.SelectorFromSet(labels.Set{m.tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelValue})
for sgID := range unusedEndpointSGs {
err := m.sgReconciler.ReconcileIngress(ctx, sgID, nil,
networking.WithPermissionSelector(permissionSelector))
Expand Down
10 changes: 8 additions & 2 deletions pkg/targetgroupbinding/networking_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
)

const tgbNetworkingIPPermissionLabelKey = "elbv2.k8s.aws/targetGroupBinding"

func Test_defaultNetworkingManager_computeIngressPermissionsForTGBNetworking(t *testing.T) {
port8080 := intstr.FromInt(8080)
port8443 := intstr.FromInt(8443)
Expand Down Expand Up @@ -228,7 +230,9 @@ func Test_defaultNetworkingManager_computeIngressPermissionsForTGBNetworking(t *
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := &defaultNetworkingManager{}
m := &defaultNetworkingManager{
tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelKey,
}
got, err := m.computeIngressPermissionsForTGBNetworking(context.Background(), tt.args.tgbNetworking, tt.args.pods)
if tt.wantErr != nil {
assert.EqualError(t, err, tt.wantErr.Error())
Expand Down Expand Up @@ -476,7 +480,9 @@ func Test_defaultNetworkingManager_computePermissionsForPeerPort(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := &defaultNetworkingManager{}
m := &defaultNetworkingManager{
tgbNetworkingIPPermissionLabelKey: tgbNetworkingIPPermissionLabelKey,
}
got, err := m.computePermissionsForPeerPort(context.Background(), tt.args.peer, tt.args.port, tt.args.pods)
if tt.wantErr != nil {
assert.EqualError(t, err, tt.wantErr.Error())
Expand Down
4 changes: 2 additions & 2 deletions pkg/targetgroupbinding/resource_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELB
podInfoRepo k8s.PodInfoRepo, sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler,
vpcInfoProvider networking.VPCInfoProvider, multiClusterManager MultiClusterManager,
vpcID string, clusterName string, failOpenEnabled bool, endpointSliceEnabled bool, disabledRestrictedSGRulesFlag bool,
endpointSGTags map[string]string,
endpointSGTags map[string]string, clusterSgRuleLabelPrefix string,
eventRecorder record.EventRecorder, logger logr.Logger) *defaultResourceManager {
targetsManager := NewCachedTargetsManager(elbv2Client, logger)
endpointResolver := backend.NewDefaultEndpointResolver(k8sClient, podInfoRepo, failOpenEnabled, endpointSliceEnabled, logger)
Expand All @@ -48,7 +48,7 @@ func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELB
podENIResolver := networking.NewDefaultPodENIInfoResolver(k8sClient, ec2Client, nodeInfoProvider, vpcID, logger)
nodeENIResolver := networking.NewDefaultNodeENIInfoResolver(nodeInfoProvider, logger)

networkingManager := NewDefaultNetworkingManager(k8sClient, podENIResolver, nodeENIResolver, sgManager, sgReconciler, vpcID, clusterName, endpointSGTags, logger, disabledRestrictedSGRulesFlag)
networkingManager := NewDefaultNetworkingManager(k8sClient, podENIResolver, nodeENIResolver, sgManager, sgReconciler, vpcID, clusterName, endpointSGTags, clusterSgRuleLabelPrefix, logger, disabledRestrictedSGRulesFlag)
return &defaultResourceManager{
k8sClient: k8sClient,
targetsManager: targetsManager,
Expand Down

0 comments on commit 96e66f6

Please sign in to comment.