Skip to content

feat: add --default-subnets option #4083

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion controllers/ingress/group_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func NewGroupReconciler(cloud services.Cloud, k8sClient client.Client, eventReco
cloud.EC2(), cloud.ELBV2(), cloud.ACM(),
annotationParser, subnetsResolver,
authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager, controllerConfig.FeatureGates,
cloud.VpcID(), controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
cloud.VpcID(), controllerConfig.ClusterName, controllerConfig.DefaultSubnets, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, backendSGProvider, sgResolver,
controllerConfig.EnableBackendSecurityGroup, controllerConfig.EnableManageBackendSecurityGroupRules, controllerConfig.DisableRestrictedSGRules, controllerConfig.IngressConfig.AllowedCertificateAuthorityARNs, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), logger, metricsCollector)
stackMarshaller := deploy.NewDefaultStackMarshaller()
Expand Down
2 changes: 1 addition & 1 deletion controllers/service/service_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewServiceReconciler(cloud services.Cloud, k8sClient client.Client, eventRe
trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, controllerConfig.ClusterName)
serviceUtils := service.NewServiceUtils(annotationParser, shared_constants.ServiceFinalizer, controllerConfig.ServiceConfig.LoadBalancerClass, controllerConfig.FeatureGates)
modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, cloud.VpcID(), trackingProvider,
elbv2TaggingManager, cloud.EC2(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
elbv2TaggingManager, cloud.EC2(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultSubnets, controllerConfig.DefaultTags, controllerConfig.ExternalManagedTags,
controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, controllerConfig.FeatureGates.Enabled(config.EnableIPTargetType), serviceUtils,
backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.EnableManageBackendSecurityGroupRules, controllerConfig.DisableRestrictedSGRules, logger, metricsCollector)
stackMarshaller := deploy.NewDefaultStackMarshaller()
Expand Down
1 change: 1 addition & 0 deletions docs/deploy/configurations.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
| backend-security-group | string | | Backend security group id to use for the ingress rules on the worker node SG |
| cluster-name | string | | Kubernetes cluster name |
| default-ssl-policy | string | ELBSecurityPolicy-2016-08 | Default SSL Policy that will be applied to all Ingresses or Services that do not have the SSL Policy annotation |
| default-subnets | stringList | [] | Default subnets to be selected when not explicitly specified through annotations or other methods |
| default-tags | stringMap | | AWS Tags that will be applied to all AWS resources managed by this controller. Specified Tags takes highest priority |
| default-target-type | string | instance | Default target type for Ingresses and Services - ip, instance |
| default-load-balancer-scheme | string | internal | Default scheme for ELBs - internal, internet-facing |
Expand Down
3 changes: 2 additions & 1 deletion docs/deploy/subnet_discovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ A subnet is classified as public if its route table contains a route to an Inter
The controller selects one subnet per availability zone. When multiple subnets exist per Availability Zone, the following priority order applies:

1. Subnets with cluster tag for the current cluster (`kubernetes.io/cluster/<clusterName>`) are prioritized
2. Subnets with lower lexicographical order of subnet ID are prioritized
2. Subnets with the `--default-subnets` flag (prioritized in the order specified)
3. Subnets with lower lexicographical order of subnet ID are prioritized

## Minimum Subnet Requirements

Expand Down
31 changes: 30 additions & 1 deletion pkg/config/controller_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
const (
flagLogLevel = "log-level"
flagK8sClusterName = "cluster-name"
flagDefaultSubnets = "default-subnets"
flagDefaultTags = "default-tags"
flagDefaultTargetType = "default-target-type"
flagDefaultLoadBalancerScheme = "default-load-balancer-scheme"
Expand Down Expand Up @@ -78,6 +79,9 @@ type ControllerConfig struct {
// Configurations for the Service controller
ServiceConfig ServiceConfig

// Default subnets that will be used for all AWS resources managed by the networking controller.
DefaultSubnets []string

// Default AWS Tags that will be applied to all AWS resources managed by this controller.
DefaultTags map[string]string

Expand Down Expand Up @@ -137,6 +141,8 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
fs.StringVar(&cfg.LogLevel, flagLogLevel, defaultLogLevel,
"Set the controller log level - info(default), debug")
fs.StringVar(&cfg.ClusterName, flagK8sClusterName, "", "Kubernetes cluster name")
fs.StringSliceVar(&cfg.DefaultSubnets, flagDefaultSubnets, nil,
"Default subnets that will be used for all AWS resources managed by the networking controller")
fs.StringToStringVar(&cfg.DefaultTags, flagDefaultTags, nil,
"Default AWS Tags that will be applied to all AWS resources managed by this controller")
fs.StringVar(&cfg.DefaultTargetType, flagDefaultTargetType, string(elbv2.TargetTypeInstance),
Expand Down Expand Up @@ -186,7 +192,9 @@ func (cfg *ControllerConfig) Validate() error {
if len(cfg.ClusterName) == 0 {
return errors.New("kubernetes cluster name must be specified")
}

if err := cfg.validateDefaultSubnets(); err != nil {
return err
}
if err := cfg.validateDefaultTagsCollisionWithTrackingTags(); err != nil {
return err
}
Expand All @@ -211,6 +219,27 @@ func (cfg *ControllerConfig) Validate() error {
return nil
}

func (cfg *ControllerConfig) validateDefaultSubnets() error {
if len(cfg.DefaultSubnets) == 0 {
return nil
}
for _, subnetID := range cfg.DefaultSubnets {
if !strings.HasPrefix(subnetID, "subnet-") {
return errors.Errorf("invalid value %v for default subnet id", subnetID)
}
}

//validate duplicate subnet ids
seen := make(map[string]bool)
for _, str := range cfg.DefaultSubnets {
if seen[str] {
return errors.Errorf("duplicate subnet id %v is specified in the --default-subnets flag", str)
}
seen[str] = true
}
return nil
}

func (cfg *ControllerConfig) validateDefaultTagsCollisionWithTrackingTags() error {
for tagKey := range cfg.DefaultTags {
if trackingTagKeys.Has(tagKey) {
Expand Down
49 changes: 48 additions & 1 deletion pkg/config/controller_config_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package config

import (
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
"testing"
)

func TestControllerConfig_validateDefaultTagsCollisionWithTrackingTags(t *testing.T) {
Expand Down Expand Up @@ -219,3 +220,49 @@ func TestControllerConfig_validateManageBackendSecurityGroupRulesConfiguration(t
})
}
}

func TestControllerConfig_validateDefaultSubnets(t *testing.T) {
type fields struct {
DefaultSubnets []string
}
tests := []struct {
name string
fields fields
wantErr error
}{
{
name: "default subnets is empty",
fields: fields{
DefaultSubnets: nil,
},
wantErr: nil,
},
{
name: "default subnets is not empty",
fields: fields{
DefaultSubnets: []string{"subnet-1", "subnet-2"},
},
wantErr: nil,
},
{
name: "default subnets is not empty and duplicate subnets are specified",
fields: fields{
DefaultSubnets: []string{"subnet-1", "subnet-2", "subnet-1"},
},
wantErr: errors.New("duplicate subnet id subnet-1 is specified in the --default-subnets flag"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg := &ControllerConfig{
DefaultSubnets: tt.fields.DefaultSubnets,
}
err := cfg.validateDefaultSubnets()
if tt.wantErr != nil {
assert.EqualError(t, err, tt.wantErr.Error())
} else {
assert.NoError(t, err)
}
})
}
}
1 change: 1 addition & 0 deletions pkg/ingress/model_build_load_balancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerSubnetMappings(ctx context.Cont
chosenSubnets, err := t.subnetsResolver.ResolveViaDiscovery(ctx,
networking.WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication),
networking.WithSubnetsResolveLBScheme(scheme),
networking.WithDefaultSubnets(t.defaultSubnets),
)
if err != nil {
return nil, errors.Wrap(err, "couldn't auto-discover subnets")
Expand Down
9 changes: 7 additions & 2 deletions pkg/ingress/model_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package ingress
import (
"context"
"reflect"
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
"strconv"

"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"

elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"

awssdk "github.com/aws/aws-sdk-go-v2/aws"
Expand Down Expand Up @@ -46,7 +47,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
annotationParser annotations.Parser, subnetsResolver networkingpkg.SubnetsResolver,
authConfigBuilder AuthConfigBuilder, enhancedBackendBuilder EnhancedBackendBuilder,
trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager, featureGates config.FeatureGates,
vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, defaultLoadBalancerScheme string,
vpcID string, clusterName string, defaultSubnets []string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, defaultTargetType string, defaultLoadBalancerScheme string,
backendSGProvider networkingpkg.BackendSGProvider, sgResolver networkingpkg.SecurityGroupResolver,
enableBackendSG bool, defaultEnableManageBackendSGRules bool, disableRestrictedSGRules bool, allowedCAARNs []string, enableIPTargetType bool, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector) *defaultModelBuilder {
certDiscovery := NewACMCertDiscovery(acmClient, allowedCAARNs, logger)
Expand All @@ -69,6 +70,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
trackingProvider: trackingProvider,
elbv2TaggingManager: elbv2TaggingManager,
featureGates: featureGates,
defaultSubnets: defaultSubnets,
defaultTags: defaultTags,
externalManagedTags: sets.NewString(externalManagedTags...),
defaultSSLPolicy: defaultSSLPolicy,
Expand Down Expand Up @@ -106,6 +108,7 @@ type defaultModelBuilder struct {
trackingProvider tracking.Provider
elbv2TaggingManager elbv2deploy.TaggingManager
featureGates config.FeatureGates
defaultSubnets []string
defaultTags map[string]string
externalManagedTags sets.String
defaultSSLPolicy string
Expand Down Expand Up @@ -154,6 +157,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group, metrics
stack: stack,
frontendNlbTargetGroupDesiredState: frontendNlbTargetGroupDesiredState,

defaultSubnets: b.defaultSubnets,
defaultTags: b.defaultTags,
externalManagedTags: b.externalManagedTags,
defaultIPAddressType: elbv2model.IPAddressTypeIPV4,
Expand Down Expand Up @@ -213,6 +217,7 @@ type defaultModelBuildTask struct {
disableRestrictedSGRules bool
enableIPTargetType bool

defaultSubnets []string
defaultTags map[string]string
externalManagedTags sets.String
defaultIPAddressType elbv2model.IPAddressType
Expand Down
38 changes: 35 additions & 3 deletions pkg/networking/subnet_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ type SubnetsResolveOptions struct {
// The Load Balancer Scheme.
// By default, it's internet-facing.
LBScheme elbv2model.LoadBalancerScheme
// Subnets specified with --default-subnets
DefaultSubnets []string
}

// ApplyOptions applies slice of SubnetsResolveOption.
Expand Down Expand Up @@ -86,6 +88,13 @@ func WithSubnetsResolveLBScheme(lbScheme elbv2model.LoadBalancerScheme) SubnetsR
}
}

// WithDefaultSubnets generates an option that configures DefaultSubnets.
func WithDefaultSubnets(defaultSubnets []string) SubnetsResolveOption {
return func(opts *SubnetsResolveOptions) {
opts.DefaultSubnets = defaultSubnets
}
}

// SubnetsResolver is responsible for resolve EC2 Subnets for Load Balancers.
type SubnetsResolver interface {
// ResolveViaDiscovery resolve subnets by auto discover matching subnets.
Expand Down Expand Up @@ -412,7 +421,7 @@ func (r *defaultSubnetsResolver) validateSpecifiedSubnets(ctx context.Context, s
// chooseAndValidateSubnetsPerAZ will choose one subnet per AZ from eligible subnets and then validate against chosen subnets.
func (r *defaultSubnetsResolver) chooseAndValidateSubnetsPerAZ(ctx context.Context, subnets []ec2types.Subnet, resolveOpts SubnetsResolveOptions) ([]ec2types.Subnet, error) {
categorizedSubnets := r.categorizeSubnetsByEligibility(subnets)
chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible)
chosenSubnets := r.chooseSubnetsPerAZ(categorizedSubnets.eligible, resolveOpts.DefaultSubnets)
if len(chosenSubnets) == 0 {
return nil, fmt.Errorf("unable to resolve at least one subnet. Evaluated %d subnets: %d are tagged for other clusters, and %d have insufficient available IP addresses",
len(subnets), len(categorizedSubnets.ineligibleClusterTag), len(categorizedSubnets.insufficientIPs))
Expand Down Expand Up @@ -452,7 +461,15 @@ func (r *defaultSubnetsResolver) categorizeSubnetsByEligibility(subnets []ec2typ

// chooseSubnetsPerAZ will choose one subnet per AZ.
// * subnets with current cluster tag will be prioritized.
func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) []ec2types.Subnet {
func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet, defaultSubnets []string) []ec2types.Subnet {

prioritySubnetMap := make(map[string]int)

if len(defaultSubnets) > 0 {
for i, subnetID := range defaultSubnets {
prioritySubnetMap[subnetID] = i
}
}
subnetsByAZ := mapSDKSubnetsByAZ(subnets)
chosenSubnets := make([]ec2types.Subnet, 0, len(subnetsByAZ))
for az, azSubnets := range subnetsByAZ {
Expand All @@ -467,9 +484,24 @@ func (r *defaultSubnetsResolver) chooseSubnetsPerAZ(subnets []ec2types.Subnet) [
} else if (!subnetIHasCurrentClusterTag) && subnetJHasCurrentClusterTag {
return false
}

// When azSubnets are specified in --default-azSubnets, the azSubnets list will be sorted according to this order.
// Any azSubnets not specified in --default-azSubnets will be sorted in lexicographical order and placed after the prioritized azSubnets.
iVal, iExists := prioritySubnetMap[awssdk.ToString(azSubnets[i].SubnetId)]
jVal, jExists := prioritySubnetMap[awssdk.ToString(azSubnets[j].SubnetId)]

if iExists && jExists {
return iVal < jVal
}
if iExists {
return true
}
if jExists {
return false
}
return awssdk.ToString(azSubnets[i].SubnetId) < awssdk.ToString(azSubnets[j].SubnetId)
})
r.logger.V(1).Info("multiple subnets in the same AvailabilityZone", "AvailabilityZone", az,
r.logger.V(1).Info("multiple azSubnets in the same AvailabilityZone", "AvailabilityZone", az,
"chosen", azSubnets[0].SubnetId, "ignored", extractSubnetIDs(azSubnets[1:]))
chosenSubnets = append(chosenSubnets, azSubnets[0])
}
Expand Down
Loading