diff --git a/controllers/ingress/group_controller.go b/controllers/ingress/group_controller.go index 13eca518d..4f9ba7c56 100644 --- a/controllers/ingress/group_controller.go +++ b/controllers/ingress/group_controller.go @@ -57,7 +57,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder annotationParser, subnetsResolver, authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager, cloud.VpcID(), config.ClusterName, config.DefaultTags, config.ExternalManagedTags, - config.DefaultSSLPolicy, backendSGProvider, config.EnableBackendSecurityGroup, logger) + config.DefaultSSLPolicy, backendSGProvider, config.EnableBackendSecurityGroup, config.DisableRestrictedSGRules, logger) stackMarshaller := deploy.NewDefaultStackMarshaller() stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, config, ingressTagPrefix, logger) diff --git a/main.go b/main.go index 8cd3406c2..15301c521 100644 --- a/main.go +++ b/main.go @@ -107,7 +107,7 @@ func main() { subnetResolver := networking.NewDefaultSubnetsResolver(azInfoProvider, cloud.EC2(), cloud.VpcID(), controllerCFG.ClusterName, ctrl.Log.WithName("subnets-resolver")) vpcResolver := networking.NewDefaultVPCResolver(cloud.EC2(), cloud.VpcID(), ctrl.Log.WithName("vpc-resolver")) tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(), - podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log, controllerCFG.EnableEndpointSlices) + podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log, controllerCFG.EnableEndpointSlices, controllerCFG.DisableRestrictedSGRules) backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup, cloud.VpcID(), cloud.EC2(), mgr.GetClient(), ctrl.Log.WithName("backend-sg-provider")) ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"), diff --git a/pkg/config/controller_config.go b/pkg/config/controller_config.go index 008d76ddd..9c7699cb0 100644 --- a/pkg/config/controller_config.go +++ b/pkg/config/controller_config.go @@ -23,12 +23,14 @@ const ( flagEnableBackendSG = "enable-backend-security-group" flagBackendSecurityGroup = "backend-security-group" flagEnableEndpointSlices = "enable-endpoint-slices" + flagDisableRestrictedSGRules = "disable-restricted-sg-rules" defaultLogLevel = "info" defaultMaxConcurrentReconciles = 3 defaultMaxExponentialBackoffDelay = time.Second * 1000 defaultSSLPolicy = "ELBSecurityPolicy-2016-08" defaultEnableBackendSG = true defaultEnableEndpointSlices = false + defaultDisableRestrictedSGRules = false ) var ( @@ -84,6 +86,9 @@ type ControllerConfig struct { // BackendSecurityGroups specifies the configured backend security group to use // for optimized security group rules BackendSecurityGroup string + + // DisableRestrictedSGRules specifies whether to use restricted security group rules + DisableRestrictedSGRules bool } // BindFlags binds the command line flags to the fields in the config object @@ -109,6 +114,9 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) { "Backend security group id to use for the ingress rules on the worker node SG") fs.BoolVar(&cfg.EnableEndpointSlices, flagEnableEndpointSlices, defaultEnableEndpointSlices, "Enable EndpointSlices for IP targets instead of Endpoints") + fs.BoolVar(&cfg.DisableRestrictedSGRules, flagDisableRestrictedSGRules, defaultDisableRestrictedSGRules, + "Disable the usage of restricted security group rules") + cfg.AWSConfig.BindFlags(fs) cfg.RuntimeConfig.BindFlags(fs) diff --git a/pkg/ingress/model_build_target_group.go b/pkg/ingress/model_build_target_group.go index d4d986131..8769927f5 100644 --- a/pkg/ingress/model_build_target_group.go +++ b/pkg/ingress/model_build_target_group.go @@ -53,7 +53,7 @@ func (t *defaultModelBuildTask) buildTargetGroupBinding(ctx context.Context, tg func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context, tg *elbv2model.TargetGroup, svc *corev1.Service, port intstr.IntOrString, nodeSelector *metav1.LabelSelector) elbv2model.TargetGroupBindingResourceSpec { targetType := elbv2api.TargetType(tg.Spec.TargetType) - tgbNetworking := t.buildTargetGroupBindingNetworking(ctx) + tgbNetworking := t.buildTargetGroupBindingNetworking(ctx, tg.Spec.Port, *tg.Spec.HealthCheckConfig.Port) return elbv2model.TargetGroupBindingResourceSpec{ Template: elbv2model.TargetGroupBindingTemplate{ ObjectMeta: metav1.ObjectMeta{ @@ -74,29 +74,59 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context, } } -func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Context) *elbv2model.TargetGroupBindingNetworking { +func (t *defaultModelBuildTask) buildTargetGroupBindingNetworking(ctx context.Context, targetGroupPort int64, healthCheckPort intstr.IntOrString) *elbv2model.TargetGroupBindingNetworking { if t.backendSGIDToken == nil { return nil } protocolTCP := elbv2api.NetworkingProtocolTCP - return &elbv2model.TargetGroupBindingNetworking{ - Ingress: []elbv2model.NetworkingIngressRule{ - { - From: []elbv2model.NetworkingPeer{ - { - SecurityGroup: &elbv2model.SecurityGroup{ - GroupID: t.backendSGIDToken, + if t.disableRestrictedSGRules { + return &elbv2model.TargetGroupBindingNetworking{ + Ingress: []elbv2model.NetworkingIngressRule{ + { + From: []elbv2model.NetworkingPeer{ + { + SecurityGroup: &elbv2model.SecurityGroup{ + GroupID: t.backendSGIDToken, + }, + }, + }, + Ports: []elbv2api.NetworkingPort{ + { + Protocol: &protocolTCP, + Port: nil, }, }, }, - Ports: []elbv2api.NetworkingPort{ - { - Protocol: &protocolTCP, - Port: nil, + }, + } + } + var networkingPorts []elbv2api.NetworkingPort + var networkingRules []elbv2model.NetworkingIngressRule + tgPort := intstr.FromInt(int(targetGroupPort)) + networkingPorts = append(networkingPorts, elbv2api.NetworkingPort{ + Protocol: &protocolTCP, + Port: &tgPort, + }) + if healthCheckPort.String() != healthCheckPortTrafficPort { + networkingPorts = append(networkingPorts, elbv2api.NetworkingPort{ + Protocol: &protocolTCP, + Port: &healthCheckPort, + }) + } + for _, port := range networkingPorts { + networkingRules = append(networkingRules, elbv2model.NetworkingIngressRule{ + From: []elbv2model.NetworkingPeer{ + { + SecurityGroup: &elbv2model.SecurityGroup{ + GroupID: t.backendSGIDToken, }, }, }, - }, + Ports: []elbv2api.NetworkingPort{port}, + }) + } + return &elbv2model.TargetGroupBindingNetworking{ + Ingress: networkingRules, } } diff --git a/pkg/ingress/model_builder.go b/pkg/ingress/model_builder.go index dd56b1b4a..62ddf07d3 100644 --- a/pkg/ingress/model_builder.go +++ b/pkg/ingress/model_builder.go @@ -40,29 +40,30 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR authConfigBuilder AuthConfigBuilder, enhancedBackendBuilder EnhancedBackendBuilder, trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager, vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, - backendSGProvider networkingpkg.BackendSGProvider, enableBackendSG bool, logger logr.Logger) *defaultModelBuilder { + backendSGProvider networkingpkg.BackendSGProvider, enableBackendSG bool, disableRestrictedSGRules bool, logger logr.Logger) *defaultModelBuilder { certDiscovery := NewACMCertDiscovery(acmClient, logger) ruleOptimizer := NewDefaultRuleOptimizer(logger) return &defaultModelBuilder{ - k8sClient: k8sClient, - eventRecorder: eventRecorder, - ec2Client: ec2Client, - vpcID: vpcID, - clusterName: clusterName, - annotationParser: annotationParser, - subnetsResolver: subnetsResolver, - backendSGProvider: backendSGProvider, - certDiscovery: certDiscovery, - authConfigBuilder: authConfigBuilder, - enhancedBackendBuilder: enhancedBackendBuilder, - ruleOptimizer: ruleOptimizer, - trackingProvider: trackingProvider, - elbv2TaggingManager: elbv2TaggingManager, - defaultTags: defaultTags, - externalManagedTags: sets.NewString(externalManagedTags...), - defaultSSLPolicy: defaultSSLPolicy, - enableBackendSG: enableBackendSG, - logger: logger, + k8sClient: k8sClient, + eventRecorder: eventRecorder, + ec2Client: ec2Client, + vpcID: vpcID, + clusterName: clusterName, + annotationParser: annotationParser, + subnetsResolver: subnetsResolver, + backendSGProvider: backendSGProvider, + certDiscovery: certDiscovery, + authConfigBuilder: authConfigBuilder, + enhancedBackendBuilder: enhancedBackendBuilder, + ruleOptimizer: ruleOptimizer, + trackingProvider: trackingProvider, + elbv2TaggingManager: elbv2TaggingManager, + defaultTags: defaultTags, + externalManagedTags: sets.NewString(externalManagedTags...), + defaultSSLPolicy: defaultSSLPolicy, + enableBackendSG: enableBackendSG, + disableRestrictedSGRules: disableRestrictedSGRules, + logger: logger, } } @@ -77,19 +78,20 @@ type defaultModelBuilder struct { vpcID string clusterName string - annotationParser annotations.Parser - subnetsResolver networkingpkg.SubnetsResolver - backendSGProvider networkingpkg.BackendSGProvider - certDiscovery CertDiscovery - authConfigBuilder AuthConfigBuilder - enhancedBackendBuilder EnhancedBackendBuilder - ruleOptimizer RuleOptimizer - trackingProvider tracking.Provider - elbv2TaggingManager elbv2deploy.TaggingManager - defaultTags map[string]string - externalManagedTags sets.String - defaultSSLPolicy string - enableBackendSG bool + annotationParser annotations.Parser + subnetsResolver networkingpkg.SubnetsResolver + backendSGProvider networkingpkg.BackendSGProvider + certDiscovery CertDiscovery + authConfigBuilder AuthConfigBuilder + enhancedBackendBuilder EnhancedBackendBuilder + ruleOptimizer RuleOptimizer + trackingProvider tracking.Provider + elbv2TaggingManager elbv2deploy.TaggingManager + defaultTags map[string]string + externalManagedTags sets.String + defaultSSLPolicy string + enableBackendSG bool + disableRestrictedSGRules bool logger logr.Logger } @@ -98,22 +100,23 @@ type defaultModelBuilder struct { func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group) (core.Stack, *elbv2model.LoadBalancer, error) { stack := core.NewDefaultStack(core.StackID(ingGroup.ID)) task := &defaultModelBuildTask{ - k8sClient: b.k8sClient, - eventRecorder: b.eventRecorder, - ec2Client: b.ec2Client, - vpcID: b.vpcID, - clusterName: b.clusterName, - annotationParser: b.annotationParser, - subnetsResolver: b.subnetsResolver, - certDiscovery: b.certDiscovery, - authConfigBuilder: b.authConfigBuilder, - enhancedBackendBuilder: b.enhancedBackendBuilder, - ruleOptimizer: b.ruleOptimizer, - trackingProvider: b.trackingProvider, - elbv2TaggingManager: b.elbv2TaggingManager, - backendSGProvider: b.backendSGProvider, - logger: b.logger, - enableBackendSG: b.enableBackendSG, + k8sClient: b.k8sClient, + eventRecorder: b.eventRecorder, + ec2Client: b.ec2Client, + vpcID: b.vpcID, + clusterName: b.clusterName, + annotationParser: b.annotationParser, + subnetsResolver: b.subnetsResolver, + certDiscovery: b.certDiscovery, + authConfigBuilder: b.authConfigBuilder, + enhancedBackendBuilder: b.enhancedBackendBuilder, + ruleOptimizer: b.ruleOptimizer, + trackingProvider: b.trackingProvider, + elbv2TaggingManager: b.elbv2TaggingManager, + backendSGProvider: b.backendSGProvider, + logger: b.logger, + enableBackendSG: b.enableBackendSG, + disableRestrictedSGRules: b.disableRestrictedSGRules, ingGroup: ingGroup, stack: stack, @@ -163,11 +166,12 @@ type defaultModelBuildTask struct { elbv2TaggingManager elbv2deploy.TaggingManager logger logr.Logger - ingGroup Group - sslRedirectConfig *SSLRedirectConfig - stack core.Stack - backendSGIDToken core.StringToken - enableBackendSG bool + ingGroup Group + sslRedirectConfig *SSLRedirectConfig + stack core.Stack + backendSGIDToken core.StringToken + enableBackendSG bool + disableRestrictedSGRules bool defaultTags map[string]string externalManagedTags sets.String diff --git a/pkg/ingress/model_builder_test.go b/pkg/ingress/model_builder_test.go index 3325e339d..8560bc403 100644 --- a/pkg/ingress/model_builder_test.go +++ b/pkg/ingress/model_builder_test.go @@ -518,6 +518,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port":32768, "protocol":"TCP" } ] @@ -557,6 +558,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port":32768, "protocol":"TCP" } ] @@ -596,6 +598,22 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 8443, + "protocol":"TCP" + } + ] + }, + { + "from":[ + { + "securityGroup":{ + "groupID": "sg-auto" + } + } + ], + "ports":[ + { + "port": 9090, "protocol":"TCP" } ] @@ -956,13 +974,14 @@ func Test_defaultModelBuilder_Build(t *testing.T) { { "securityGroup":{ "groupID": { - "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" - } + "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" + } } } ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -997,13 +1016,14 @@ func Test_defaultModelBuilder_Build(t *testing.T) { { "securityGroup":{ "groupID": { - "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" - } + "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" + } } } ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -1038,13 +1058,31 @@ func Test_defaultModelBuilder_Build(t *testing.T) { { "securityGroup":{ "groupID": { - "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" - } + "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" + } } } ], "ports":[ { + "port": 8443, + "protocol":"TCP" + } + ] + }, + { + "from":[ + { + "securityGroup":{ + "groupID": { + "$ref":"#/resources/AWS::EC2::SecurityGroup/ManagedLBSecurityGroup/status/groupID" + } + } + } + ], + "ports":[ + { + "port": 9090, "protocol":"TCP" } ] @@ -1414,6 +1452,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { +"port": 32768, "protocol":"TCP" } ] @@ -1453,6 +1492,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { +"port": 32768, "protocol":"TCP" } ] @@ -1492,6 +1532,22 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 8443, + "protocol":"TCP" + } + ] + }, + { + "from":[ + { + "securityGroup":{ + "groupID": "sg-auto" + } + } + ], + "ports":[ + { + "port": 9090, "protocol":"TCP" } ] @@ -1874,6 +1930,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -1913,6 +1970,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -1952,6 +2010,22 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 8443, + "protocol":"TCP" + } + ] + }, +{ + "from":[ + { + "securityGroup":{ + "groupID": "sg-auto" + } + } + ], + "ports":[ + { + "port": 9090, "protocol":"TCP" } ] @@ -2241,6 +2315,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -2280,6 +2355,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -2701,6 +2777,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -2740,6 +2817,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 32768, "protocol":"TCP" } ] @@ -2779,6 +2857,22 @@ func Test_defaultModelBuilder_Build(t *testing.T) { ], "ports":[ { + "port": 8443, + "protocol":"TCP" + } + ] + }, + { + "from":[ + { + "securityGroup":{ + "groupID": "sg-auto" + } + } + ], + "ports":[ + { + "port": 9090, "protocol":"TCP" } ] diff --git a/pkg/targetgroupbinding/networking_manager.go b/pkg/targetgroupbinding/networking_manager.go index a4e6aca38..389087fc8 100644 --- a/pkg/targetgroupbinding/networking_manager.go +++ b/pkg/targetgroupbinding/networking_manager.go @@ -26,6 +26,8 @@ import ( const ( tgbNetworkingIPPermissionLabelKey = "elbv2.k8s.aws/targetGroupBinding" tgbNetworkingIPPermissionLabelValue = "shared" + defaultTgbMinPort = int64(0) + defaultTgbMaxPort = int64(65535) ) // NetworkingManager manages the networking for targetGroupBindings. @@ -42,7 +44,7 @@ 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, logger logr.Logger) *defaultNetworkingManager { + sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler, vpcID string, clusterName string, logger logr.Logger, disabledRestrictedSGRulesFlag bool) *defaultNetworkingManager { return &defaultNetworkingManager{ k8sClient: k8sClient, @@ -58,6 +60,7 @@ func NewDefaultNetworkingManager(k8sClient client.Client, podENIResolver network ingressPermissionsPerSGByTGB: make(map[types.NamespacedName]map[string][]networking.IPPermissionInfo), trackedEndpointSGs: sets.NewString(), trackedEndpointSGsInitialized: false, + disableRestrictedSGRules: disabledRestrictedSGRulesFlag, } } @@ -84,6 +87,8 @@ type defaultNetworkingManager struct { // we discovery endpointSGs from VPC using clusterTags once, so we can still GC rules if some SGs are no longer referenced. // a SG/nodeGroup might be removed from cluster while this controller is not running. trackedEndpointSGsInitialized bool + // disableRestrictedSGRules specifies whether to use restricted security group rules + disableRestrictedSGRules bool } func (m *defaultNetworkingManager) ReconcileForPodEndpoints(ctx context.Context, tgb *elbv2api.TargetGroupBinding, endpoints []backend.PodEndpoint) error { @@ -225,7 +230,94 @@ func (m *defaultNetworkingManager) consolidateIngressPermissionsPerSGByTGB(_ con } // computeAggregatedIngressPermissionsPerSG will aggregate ingress permissions by SG across all TGBs. -func (m *defaultNetworkingManager) computeAggregatedIngressPermissionsPerSG(_ context.Context) map[string][]networking.IPPermissionInfo { +func (m *defaultNetworkingManager) computeAggregatedIngressPermissionsPerSG(ctx context.Context) map[string][]networking.IPPermissionInfo { + if m.disableRestrictedSGRules { + return m.computeUnrestrictedIngressPermissionsPerSG(ctx) + } + return m.computeRestrictedIngressPermissionsPerSG(ctx) +} + +func (m *defaultNetworkingManager) groupIngressPermsBySourceAndProtocolPerSG(_ context.Context) (map[string][]networking.IPPermissionInfo, map[string]map[string]map[string][]networking.IPPermissionInfo) { + permsFromIPRangeRulesPerSG := make(map[string][]networking.IPPermissionInfo) + permsByProtocolAndSourcePerSG := make(map[string]map[string]map[string][]networking.IPPermissionInfo) + for _, ingressPermissionsPerSG := range m.ingressPermissionsPerSGByTGB { + for sgID, permissions := range ingressPermissionsPerSG { + if _, ok := permsByProtocolAndSourcePerSG[sgID]; !ok { + permsByProtocolAndSourcePerSG[sgID] = make(map[string]map[string][]networking.IPPermissionInfo) + } + for _, permission := range permissions { + if len(permission.Permission.UserIdGroupPairs) == 0 && (len(permission.Permission.IpRanges) == 1 || len(permission.Permission.Ipv6Ranges) == 1) { + if _, ok := permsFromIPRangeRulesPerSG[sgID]; !ok { + permsFromIPRangeRulesPerSG[sgID] = []networking.IPPermissionInfo{} + } + permsFromIPRangeRulesPerSG[sgID] = append(permsFromIPRangeRulesPerSG[sgID], permission) + } else { + protocol := awssdk.StringValue(permission.Permission.IpProtocol) + if _, ok := permsByProtocolAndSourcePerSG[sgID][protocol]; !ok { + permsByProtocolAndSourcePerSG[sgID][protocol] = make(map[string][]networking.IPPermissionInfo) + } + groupID := "" + if len(permission.Permission.UserIdGroupPairs) == 1 { + groupID = awssdk.StringValue(permission.Permission.UserIdGroupPairs[0].GroupId) + } + if _, ok := permsByProtocolAndSourcePerSG[sgID][protocol][groupID]; !ok { + permsByProtocolAndSourcePerSG[sgID][protocol][groupID] = []networking.IPPermissionInfo{} + } + permsByProtocolAndSourcePerSG[sgID][protocol][groupID] = append(permsByProtocolAndSourcePerSG[sgID][protocol][groupID], permission) + } + } + } + } + return permsFromIPRangeRulesPerSG, permsByProtocolAndSourcePerSG +} + +// computeRestrictedIngressPermissionsPerSG will compute restricted ingress permissions group by source and protocol per SG +func (m *defaultNetworkingManager) computeRestrictedIngressPermissionsPerSG(ctx context.Context) map[string][]networking.IPPermissionInfo { + permsFromIPRangeRulesPerSG, permsByProtocolAndSourcePerSG := m.groupIngressPermsBySourceAndProtocolPerSG(ctx) + + restrictedPermByProtocolPerSG := make(map[string][]networking.IPPermissionInfo) + for sgID, permsByProtocolAndSource := range permsByProtocolAndSourcePerSG { + if _, ok := restrictedPermByProtocolPerSG[sgID]; !ok { + restrictedPermByProtocolPerSG[sgID] = []networking.IPPermissionInfo{} + } + for _, permsBySource := range permsByProtocolAndSource { + for _, perms := range permsBySource { + minPort, maxPort := defaultTgbMaxPort, defaultTgbMinPort + if len(perms) == 0 { + continue + } + permForCurrGroup := perms[0] + for _, perm := range perms { + if awssdk.Int64Value(perm.Permission.FromPort) > 0 && awssdk.Int64Value(perm.Permission.FromPort) < minPort { + minPort = awssdk.Int64Value(perm.Permission.FromPort) + } + if awssdk.Int64Value(perm.Permission.ToPort) > maxPort { + maxPort = awssdk.Int64Value(perm.Permission.ToPort) + } + } + if minPort > maxPort { + minPort, maxPort = defaultTgbMinPort, defaultTgbMaxPort + } + permForCurrGroup.Permission.FromPort = awssdk.Int64(minPort) + permForCurrGroup.Permission.ToPort = awssdk.Int64(maxPort) + restrictedPermByProtocolPerSG[sgID] = append(restrictedPermByProtocolPerSG[sgID], permForCurrGroup) + } + } + } + + for sgID, permsFromIPRangeRules := range permsFromIPRangeRulesPerSG { + for _, perm := range permsFromIPRangeRules { + if _, ok := restrictedPermByProtocolPerSG[sgID]; !ok { + restrictedPermByProtocolPerSG[sgID] = []networking.IPPermissionInfo{} + } + restrictedPermByProtocolPerSG[sgID] = append(restrictedPermByProtocolPerSG[sgID], perm) + } + } + return restrictedPermByProtocolPerSG +} + +// computeUnrestrictedIngressPermissionsPerSG will compute unrestricted ingress permissions by SG across all TGBs. +func (m *defaultNetworkingManager) computeUnrestrictedIngressPermissionsPerSG(_ context.Context) map[string][]networking.IPPermissionInfo { permByHashCodePerSG := make(map[string]map[string]networking.IPPermissionInfo) for _, ingressPermissionsPerSG := range m.ingressPermissionsPerSGByTGB { for sgID, permissions := range ingressPermissionsPerSG { @@ -237,15 +329,15 @@ func (m *defaultNetworkingManager) computeAggregatedIngressPermissionsPerSG(_ co } } } - aggregatedPermsPerSG := make(map[string][]networking.IPPermissionInfo) + unrestrictedPermsPerSG := make(map[string][]networking.IPPermissionInfo) for sgID, permByHashCode := range permByHashCodePerSG { aggregatedPerms := make([]networking.IPPermissionInfo, 0, len(permByHashCode)) for _, hashCode := range sets.StringKeySet(permByHashCode).List() { aggregatedPerms = append(aggregatedPerms, permByHashCode[hashCode]) } - aggregatedPermsPerSG[sgID] = aggregatedPerms + unrestrictedPermsPerSG[sgID] = aggregatedPerms } - return aggregatedPermsPerSG + return unrestrictedPermsPerSG } // computeIngressPermissionsForTGBNetworking computes the needed Inbound IPPermissions for specified TargetGroupBinding. diff --git a/pkg/targetgroupbinding/networking_manager_test.go b/pkg/targetgroupbinding/networking_manager_test.go index 9536b7024..fd8bcdc7d 100644 --- a/pkg/targetgroupbinding/networking_manager_test.go +++ b/pkg/targetgroupbinding/networking_manager_test.go @@ -582,7 +582,7 @@ func Test_defaultNetworkingManager_computeNumericalPorts(t *testing.T) { } } -func Test_defaultNetworkingManager_computeAggregatedIngressPermissionsPerSG(t *testing.T) { +func Test_defaultNetworkingManager_computeUnrestrictedIngressPermissionsPerSG(t *testing.T) { type fields struct { ingressPermissionsPerSGByTGB map[types.NamespacedName]map[string][]networking.IPPermissionInfo } @@ -854,7 +854,362 @@ func Test_defaultNetworkingManager_computeAggregatedIngressPermissionsPerSG(t *t m := &defaultNetworkingManager{ ingressPermissionsPerSGByTGB: tt.fields.ingressPermissionsPerSGByTGB, } - got := m.computeAggregatedIngressPermissionsPerSG(context.Background()) + got := m.computeUnrestrictedIngressPermissionsPerSG(context.Background()) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_defaultNetworkingManager_computeRestrictedIngressPermissionsPerSG(t *testing.T) { + type fields struct { + ingressPermissionsPerSGByTGB map[types.NamespacedName]map[string][]networking.IPPermissionInfo + } + var tests = []struct { + name string + fields fields + want map[string][]networking.IPPermissionInfo + }{ + { + name: "single sg, single protocol", + fields: fields{ + ingressPermissionsPerSGByTGB: map[types.NamespacedName]map[string][]networking.IPPermissionInfo{ + types.NamespacedName{Namespace: "ns-1", Name: "tgb-1"}: { + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(30), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + }, + }, + }, + }, + want: map[string][]networking.IPPermissionInfo{ + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(30), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + Labels: map[string]string(nil), + }, + }, + }, + }, + { + name: "multiple sg, multiple protocols", + fields: fields{ + ingressPermissionsPerSGByTGB: map[types.NamespacedName]map[string][]networking.IPPermissionInfo{ + types.NamespacedName{Namespace: "ns-1", Name: "tgb-1"}: { + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(30), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + }, + "sg-b": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("udp"), + FromPort: awssdk.Int64(8443), + ToPort: awssdk.Int64(8443), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-2")}, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("udp"), + FromPort: awssdk.Int64(8080), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-2")}, + }, + }, + }, + }, + }, + }, + }, + want: map[string][]networking.IPPermissionInfo{ + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(30), + ToPort: awssdk.Int64(8080), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + Labels: map[string]string(nil), + }, + }, + "sg-b": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("udp"), + FromPort: awssdk.Int64(8080), + ToPort: awssdk.Int64(8443), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-2")}, + }, + }, + }, + }, + }, + }, + { + name: "test for CIDRs", + fields: fields{ + ingressPermissionsPerSGByTGB: map[types.NamespacedName]map[string][]networking.IPPermissionInfo{ + types.NamespacedName{Namespace: "ns-1", Name: "tgb-1"}: { + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(80), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.168.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8080), + ToPort: awssdk.Int64(8080), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.169.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8443), + ToPort: awssdk.Int64(8443), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.170.0.0/16"), + }, + }, + }, + }, + }, + }, + }, + }, + want: map[string][]networking.IPPermissionInfo{ + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(80), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.168.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8080), + ToPort: awssdk.Int64(8080), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.169.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8443), + ToPort: awssdk.Int64(8443), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.170.0.0/16"), + }, + }, + }, + }, + }, + }, + }, + { + name: "test for both sg and CIDRs", + fields: fields{ + ingressPermissionsPerSGByTGB: map[types.NamespacedName]map[string][]networking.IPPermissionInfo{ + types.NamespacedName{Namespace: "ns-1", Name: "tgb-1"}: { + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(80), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.168.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8080), + ToPort: awssdk.Int64(8080), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.169.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(8080), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.170.0.0/16"), + }, + }, + }, + }, + }, + "sg-b": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8443), + ToPort: awssdk.Int64(9090), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8443), + ToPort: awssdk.Int64(32768), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + }, + }, + }, + }, + want: map[string][]networking.IPPermissionInfo{ + "sg-a": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(80), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.168.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8080), + ToPort: awssdk.Int64(8080), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.169.0.0/16"), + }, + }, + }, + }, + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(80), + ToPort: awssdk.Int64(8080), + IpRanges: []*ec2sdk.IpRange{ + { + CidrIp: awssdk.String("192.170.0.0/16"), + }, + }, + }, + }, + }, + "sg-b": { + { + Permission: ec2sdk.IpPermission{ + IpProtocol: awssdk.String("tcp"), + FromPort: awssdk.Int64(8443), + ToPort: awssdk.Int64(32768), + UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{ + {GroupId: awssdk.String("group-1")}, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := &defaultNetworkingManager{ + ingressPermissionsPerSGByTGB: tt.fields.ingressPermissionsPerSGByTGB, + } + got := m.computeRestrictedIngressPermissionsPerSG(context.Background()) assert.Equal(t, tt.want, got) }) } diff --git a/pkg/targetgroupbinding/resource_manager.go b/pkg/targetgroupbinding/resource_manager.go index ee05d1d91..c406a08a4 100644 --- a/pkg/targetgroupbinding/resource_manager.go +++ b/pkg/targetgroupbinding/resource_manager.go @@ -39,7 +39,8 @@ type ResourceManager interface { // NewDefaultResourceManager constructs new defaultResourceManager. func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELBV2, ec2Client services.EC2, podInfoRepo k8s.PodInfoRepo, sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler, - vpcID string, clusterName string, eventRecorder record.EventRecorder, logger logr.Logger, useEndpointSlices bool) *defaultResourceManager { + vpcID string, clusterName string, eventRecorder record.EventRecorder, logger logr.Logger, useEndpointSlices bool, disabledRestrictedSGRulesFlag bool) *defaultResourceManager { + targetsManager := NewCachedTargetsManager(elbv2Client, logger) endpointResolver := backend.NewDefaultEndpointResolver(k8sClient, podInfoRepo, logger) @@ -47,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, logger) + networkingManager := NewDefaultNetworkingManager(k8sClient, podENIResolver, nodeENIResolver, sgManager, sgReconciler, vpcID, clusterName, logger, disabledRestrictedSGRulesFlag) return &defaultResourceManager{ k8sClient: k8sClient, targetsManager: targetsManager,