Skip to content

Commit 4749148

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2205 from kishorj/optimized-sg
Support optimized security group rules for ALB
2 parents 7b3d25d + cf095ae commit 4749148

13 files changed

+1637
-105
lines changed

‎controllers/ingress/group_controller.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const (
4444
func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
4545
finalizerManager k8s.FinalizerManager, networkingSGManager networkingpkg.SecurityGroupManager,
4646
networkingSGReconciler networkingpkg.SecurityGroupReconciler, subnetsResolver networkingpkg.SubnetsResolver,
47-
config config.ControllerConfig, logger logr.Logger) *groupReconciler {
47+
config config.ControllerConfig, backendSGProvider networkingpkg.BackendSGProvider, logger logr.Logger) *groupReconciler {
4848

4949
annotationParser := annotations.NewSuffixAnnotationParser(annotations.AnnotationPrefixIngress)
5050
authConfigBuilder := ingress.NewDefaultAuthConfigBuilder(annotationParser)
@@ -57,7 +57,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
5757
annotationParser, subnetsResolver,
5858
authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager,
5959
cloud.VpcID(), config.ClusterName, config.DefaultTags, config.ExternalManagedTags,
60-
config.DefaultSSLPolicy, logger)
60+
config.DefaultSSLPolicy, backendSGProvider, config.EnableBackendSecurityGroup, logger)
6161
stackMarshaller := deploy.NewDefaultStackMarshaller()
6262
stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler,
6363
config, ingressTagPrefix, logger)
@@ -68,12 +68,13 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
6868
groupFinalizerManager := ingress.NewDefaultFinalizerManager(finalizerManager)
6969

7070
return &groupReconciler{
71-
k8sClient: k8sClient,
72-
eventRecorder: eventRecorder,
73-
referenceIndexer: referenceIndexer,
74-
modelBuilder: modelBuilder,
75-
stackMarshaller: stackMarshaller,
76-
stackDeployer: stackDeployer,
71+
k8sClient: k8sClient,
72+
eventRecorder: eventRecorder,
73+
referenceIndexer: referenceIndexer,
74+
modelBuilder: modelBuilder,
75+
stackMarshaller: stackMarshaller,
76+
stackDeployer: stackDeployer,
77+
backendSGProvider: backendSGProvider,
7778

7879
groupLoader: groupLoader,
7980
groupFinalizerManager: groupFinalizerManager,
@@ -85,12 +86,13 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
8586

8687
// GroupReconciler reconciles a IngressGroup
8788
type groupReconciler struct {
88-
k8sClient client.Client
89-
eventRecorder record.EventRecorder
90-
referenceIndexer ingress.ReferenceIndexer
91-
modelBuilder ingress.ModelBuilder
92-
stackMarshaller deploy.StackMarshaller
93-
stackDeployer deploy.StackDeployer
89+
k8sClient client.Client
90+
eventRecorder record.EventRecorder
91+
referenceIndexer ingress.ReferenceIndexer
92+
modelBuilder ingress.ModelBuilder
93+
stackMarshaller deploy.StackMarshaller
94+
stackDeployer deploy.StackDeployer
95+
backendSGProvider networkingpkg.BackendSGProvider
9496

9597
groupLoader ingress.GroupLoader
9698
groupFinalizerManager ingress.FinalizerManager
@@ -124,7 +126,6 @@ func (r *groupReconciler) reconcile(ctx context.Context, req ctrl.Request) error
124126
r.recordIngressGroupEvent(ctx, ingGroup, corev1.EventTypeWarning, k8s.IngressEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
125127
return err
126128
}
127-
128129
_, lb, err := r.buildAndDeployModel(ctx, ingGroup)
129130
if err != nil {
130131
return err
@@ -141,6 +142,12 @@ func (r *groupReconciler) reconcile(ctx context.Context, req ctrl.Request) error
141142
}
142143
}
143144

145+
if len(ingGroup.Members) == 0 {
146+
if err := r.backendSGProvider.Release(ctx); err != nil {
147+
return err
148+
}
149+
}
150+
144151
if len(ingGroup.InactiveMembers) > 0 {
145152
if err := r.groupFinalizerManager.RemoveGroupFinalizer(ctx, ingGroupID, ingGroup.InactiveMembers); err != nil {
146153
r.recordIngressGroupEvent(ctx, ingGroup, corev1.EventTypeWarning, k8s.IngressEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove finalizer due to %v", err))

‎main.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ func main() {
106106
vpcResolver := networking.NewDefaultVPCResolver(cloud.EC2(), cloud.VpcID(), ctrl.Log.WithName("vpc-resolver"))
107107
tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(),
108108
podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
109+
backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
110+
cloud.VpcID(), cloud.EC2(), mgr.GetClient(), ctrl.Log.WithName("backend-sg-provider"))
109111
ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),
110112
finalizerManager, sgManager, sgReconciler, subnetResolver,
111-
controllerCFG, ctrl.Log.WithName("controllers").WithName("ingress"))
113+
controllerCFG, backendSGProvider, ctrl.Log.WithName("controllers").WithName("ingress"))
112114
svcReconciler := service.NewServiceReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("service"),
113115
finalizerManager, sgManager, sgReconciler, subnetResolver, vpcResolver,
114116
controllerCFG, ctrl.Log.WithName("controllers").WithName("service"))

‎pkg/annotations/constants.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ const (
4545
IngressSuffixAuthSessionCookie = "auth-session-cookie"
4646
IngressSuffixAuthSessionTimeout = "auth-session-timeout"
4747
IngressSuffixTargetNodeLabels = "target-node-labels"
48+
IngressSuffixManageSecurityGroupRules = "manage-backend-security-group-rules"
4849

4950
// NLB annotation suffixes
5051
// prefixes service.beta.kubernetes.io, service.kubernetes.io

‎pkg/config/controller_config.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package config
22

33
import (
4+
"strings"
45
"time"
56

67
"github.com/pkg/errors"
@@ -19,10 +20,13 @@ const (
1920
flagTargetGroupBindingMaxConcurrentReconciles = "targetgroupbinding-max-concurrent-reconciles"
2021
flagTargetGroupBindingMaxExponentialBackoffDelay = "targetgroupbinding-max-exponential-backoff-delay"
2122
flagDefaultSSLPolicy = "default-ssl-policy"
23+
flagEnableBackendSG = "enable-backend-security-group"
24+
flagBackendSecurityGroup = "backend-security-group"
2225
defaultLogLevel = "info"
2326
defaultMaxConcurrentReconciles = 3
2427
defaultMaxExponentialBackoffDelay = time.Second * 1000
2528
defaultSSLPolicy = "ELBSecurityPolicy-2016-08"
29+
defaultEnableBackendSG = true
2630
)
2731

2832
var (
@@ -68,6 +72,13 @@ type ControllerConfig struct {
6872
TargetGroupBindingMaxConcurrentReconciles int
6973
// Max exponential backoff delay for reconcile failures of TargetGroupBinding
7074
TargetGroupBindingMaxExponentialBackoffDelay time.Duration
75+
76+
// EnableBackendSecurityGroup specifies whether to use optimized security group rules
77+
EnableBackendSecurityGroup bool
78+
79+
// BackendSecurityGroups specifies the configured backend security group to use
80+
// for optimized security group rules
81+
BackendSecurityGroup string
7182
}
7283

7384
// BindFlags binds the command line flags to the fields in the config object
@@ -87,6 +98,10 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
8798
"Maximum duration of exponential backoff for targetGroupBinding reconcile failures")
8899
fs.StringVar(&cfg.DefaultSSLPolicy, flagDefaultSSLPolicy, defaultSSLPolicy,
89100
"Default SSL policy for load balancers listeners")
101+
fs.BoolVar(&cfg.EnableBackendSecurityGroup, flagEnableBackendSG, defaultEnableBackendSG,
102+
"Enable sharing of security groups for backend traffic")
103+
fs.StringVar(&cfg.BackendSecurityGroup, flagBackendSecurityGroup, "",
104+
"Backend security group id to use for the ingress rules on the worker node SG")
90105

91106
cfg.AWSConfig.BindFlags(fs)
92107
cfg.RuntimeConfig.BindFlags(fs)
@@ -111,6 +126,9 @@ func (cfg *ControllerConfig) Validate() error {
111126
if err := cfg.validateExternalManagedTagsCollisionWithDefaultTags(); err != nil {
112127
return err
113128
}
129+
if err := cfg.validateBackendSecurityGroupConfiguration(); err != nil {
130+
return err
131+
}
114132
return nil
115133
}
116134

@@ -141,3 +159,13 @@ func (cfg *ControllerConfig) validateExternalManagedTagsCollisionWithDefaultTags
141159
}
142160
return nil
143161
}
162+
163+
func (cfg *ControllerConfig) validateBackendSecurityGroupConfiguration() error {
164+
if len(cfg.BackendSecurityGroup) == 0 {
165+
return nil
166+
}
167+
if !strings.HasPrefix(cfg.BackendSecurityGroup, "sg-") {
168+
return errors.Errorf("invalid value %v for backend security group id", cfg.BackendSecurityGroup)
169+
}
170+
return nil
171+
}

‎pkg/ingress/model_build_load_balancer.go

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,58 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
242242
}
243243

244244
func (t *defaultModelBuildTask) buildLoadBalancerSecurityGroups(ctx context.Context, listenPortConfigByPort map[int64]listenPortConfig, ipAddressType elbv2model.IPAddressType) ([]core.StringToken, error) {
245+
sgNameOrIDsViaAnnotation, err := t.buildFrontendSGNameOrIDsFromAnnotation(ctx)
246+
if err != nil {
247+
return nil, err
248+
}
249+
var lbSGTokens []core.StringToken
250+
if len(sgNameOrIDsViaAnnotation) == 0 {
251+
managedSG, err := t.buildManagedSecurityGroup(ctx, listenPortConfigByPort, ipAddressType)
252+
if err != nil {
253+
return nil, err
254+
}
255+
lbSGTokens = append(lbSGTokens, managedSG.GroupID())
256+
if !t.enableBackendSG {
257+
t.backendSGIDToken = managedSG.GroupID()
258+
} else {
259+
backendSGID, err := t.backendSGProvider.Get(ctx)
260+
if err != nil {
261+
return nil, err
262+
}
263+
t.backendSGIDToken = core.LiteralStringToken((backendSGID))
264+
lbSGTokens = append(lbSGTokens, t.backendSGIDToken)
265+
}
266+
t.logger.Info("Auto Create SG", "LB SGs", lbSGTokens, "backend SG", t.backendSGIDToken)
267+
} else {
268+
manageBackendSGRules, err := t.buildManageSecurityGroupRulesFlag(ctx)
269+
if err != nil {
270+
return nil, err
271+
}
272+
frontendSGIDs, err := t.resolveSecurityGroupIDsViaNameOrIDSlice(ctx, sgNameOrIDsViaAnnotation)
273+
if err != nil {
274+
return nil, err
275+
}
276+
for _, sgID := range frontendSGIDs {
277+
lbSGTokens = append(lbSGTokens, core.LiteralStringToken(sgID))
278+
}
279+
280+
if manageBackendSGRules {
281+
if !t.enableBackendSG {
282+
return nil, errors.New("backendSG feature is required to manage worker node SG rules when frontendSG manually specified")
283+
}
284+
backendSGID, err := t.backendSGProvider.Get(ctx)
285+
if err != nil {
286+
return nil, err
287+
}
288+
t.backendSGIDToken = core.LiteralStringToken(backendSGID)
289+
lbSGTokens = append(lbSGTokens, t.backendSGIDToken)
290+
}
291+
t.logger.Info("SG configured via annotation", "LB SGs", lbSGTokens, "backend SG", t.backendSGIDToken)
292+
}
293+
return lbSGTokens, nil
294+
}
295+
296+
func (t *defaultModelBuildTask) buildFrontendSGNameOrIDsFromAnnotation(ctx context.Context) ([]string, error) {
245297
var explicitSGNameOrIDsList [][]string
246298
for _, member := range t.ingGroup.Members {
247299
var rawSGNameOrIDs []string
@@ -251,29 +303,15 @@ func (t *defaultModelBuildTask) buildLoadBalancerSecurityGroups(ctx context.Cont
251303
explicitSGNameOrIDsList = append(explicitSGNameOrIDsList, rawSGNameOrIDs)
252304
}
253305
if len(explicitSGNameOrIDsList) == 0 {
254-
sg, err := t.buildManagedSecurityGroup(ctx, listenPortConfigByPort, ipAddressType)
255-
if err != nil {
256-
return nil, err
257-
}
258-
return []core.StringToken{sg.GroupID()}, nil
306+
return nil, nil
259307
}
260-
261308
chosenSGNameOrIDs := explicitSGNameOrIDsList[0]
262309
for _, sgNameOrIDs := range explicitSGNameOrIDsList[1:] {
263-
// securityGroups order might matters in the future(e.g. use the first securityGroup for traffic to nodeGroups)
264310
if !cmp.Equal(chosenSGNameOrIDs, sgNameOrIDs) {
265311
return nil, errors.Errorf("conflicting securityGroups: %v | %v", chosenSGNameOrIDs, sgNameOrIDs)
266312
}
267313
}
268-
chosenSGIDs, err := t.resolveSecurityGroupIDsViaNameOrIDSlice(ctx, chosenSGNameOrIDs)
269-
if err != nil {
270-
return nil, err
271-
}
272-
sgIDTokens := make([]core.StringToken, 0, len(chosenSGIDs))
273-
for _, sgID := range chosenSGIDs {
274-
sgIDTokens = append(sgIDTokens, core.LiteralStringToken(sgID))
275-
}
276-
return sgIDTokens, nil
314+
return chosenSGNameOrIDs, nil
277315
}
278316

279317
func (t *defaultModelBuildTask) buildLoadBalancerCOIPv4Pool(_ context.Context) (*string, error) {
@@ -369,7 +407,7 @@ func (t *defaultModelBuildTask) resolveSecurityGroupIDsViaNameOrIDSlice(ctx cont
369407
resolvedSGIDs = append(resolvedSGIDs, awssdk.StringValue(sg.GroupId))
370408
}
371409
if len(resolvedSGIDs) != len(sgNameOrIDs) {
372-
return nil, errors.Errorf("couldn't found all securityGroups, nameOrIDs: %v, found: %v", sgNameOrIDs, resolvedSGIDs)
410+
return nil, errors.Errorf("couldn't find all securityGroups, nameOrIDs: %v, found: %v", sgNameOrIDs, resolvedSGIDs)
373411
}
374412
return resolvedSGIDs, nil
375413
}

‎pkg/ingress/model_build_managed_sg.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroup(ctx context.Context, l
2424
}
2525

2626
sg := ec2model.NewSecurityGroup(t.stack, resourceIDManagedSecurityGroup, sgSpec)
27-
t.managedSG = sg
2827
return sg, nil
2928
}
3029

‎pkg/ingress/model_build_target_group.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context,
7474
}
7575
}
7676

77-
func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Context) *elbv2model.TargetGroupBindingNetworking {
78-
if t.managedSG == nil {
77+
func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Context) *elbv2model.TargetGroupBindingNetworking {
78+
if t.backendSGIDToken == nil {
7979
return nil
8080
}
8181
protocolTCP := elbv2api.NetworkingProtocolTCP
@@ -85,7 +85,7 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(_ context.Cont
8585
From: []elbv2model.NetworkingPeer{
8686
{
8787
SecurityGroup: &elbv2model.SecurityGroup{
88-
GroupID: t.managedSG.GroupID(),
88+
GroupID: t.backendSGIDToken,
8989
},
9090
},
9191
},

0 commit comments

Comments
 (0)