Adding supports for unweighted target group through feature gate and checking SSL policy availability. (kubernetes-sigs#2380)

haouc · Timothy-Dougherty · commit 114bbc131f94 · 2023-11-09T11:29:07.000+11:00
* Adding supports for air gapped regions through feature gate.

* Updated changes according to comments and changed feature gate to feature gates

* Updates for configuration doc

* Updated feature gates doc and key name
diff --git a/controllers/ingress/group_controller.go b/controllers/ingress/group_controller.go
@@ -51,7 +51,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
 	enhancedBackendBuilder := ingress.NewDefaultEnhancedBackendBuilder(k8sClient, annotationParser, authConfigBuilder)
 	referenceIndexer := ingress.NewDefaultReferenceIndexer(enhancedBackendBuilder, authConfigBuilder, logger)
 	trackingProvider := tracking.NewDefaultProvider(ingressTagPrefix, config.ClusterName)
-	elbv2TaggingManager := elbv2deploy.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), config.FeatureGate, logger)
+	elbv2TaggingManager := elbv2deploy.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), config.FeatureGates, logger)
 	modelBuilder := ingress.NewDefaultModelBuilder(k8sClient, eventRecorder,
 		cloud.EC2(), cloud.ACM(),
 		annotationParser, subnetsResolver,
diff --git a/controllers/service/service_controller.go b/controllers/service/service_controller.go
@@ -40,7 +40,7 @@ func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorde
 
 	annotationParser := annotations.NewSuffixAnnotationParser(serviceAnnotationPrefix)
 	trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, config.ClusterName)
-	elbv2TaggingManager := elbv2.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), config.FeatureGate, logger)
+	elbv2TaggingManager := elbv2.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), config.FeatureGates, logger)
 	modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcResolver, trackingProvider,
 		elbv2TaggingManager, config.ClusterName, config.DefaultTags, config.ExternalManagedTags, config.DefaultSSLPolicy)
 	stackMarshaller := deploy.NewDefaultStackMarshaller()
diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md
@@ -83,7 +83,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
 |enable-waf                             | boolean                         | true            | Enable WAF addon for ALB |
 |enable-wafv2                           | boolean                         | true            | Enable WAF V2 addon for ALB |
 |external-managed-tags                  | stringList                      |                 | AWS Tag keys that will be managed externally. Specified Tags are ignored during reconciliation |
-|feature-gate                         | string                          | true             | A set of key=value pairs to enable or disable features |
+|[feature-gates](#feature-gates)        | stringMap                       |                 | A set of key=value pairs to enable or disable features |
 |ingress-class                          | string                          | alb             | Name of the ingress class this controller satisfies |
 |ingress-max-concurrent-reconciles      | int                             | 3               | Maximum number of concurrently running reconcile loops for ingress |
 |kubeconfig                             | string                          | in-cluster config | Path to the kubeconfig file containing authorization and API server information |
@@ -102,6 +102,7 @@ Currently, you can set only 1 namespace to watch in this flag. See [this Kuberne
 |webhook-cert-file                      | string                          | tls.crt | The server certificate name |
 |webhook-key-file                       | string                          | tls.key | The server key name |
 
+
 ### disable-ingress-class-annotation
 `--disable-ingress-class-annotation` controls whether to disable new usage of the `kubernetes.io/ingress.class` annotation.
 
@@ -129,3 +130,12 @@ WAF Regional:^AssociateWebACL|DisassociateWebACL=0.5:1,WAF Regional:^GetWebACLFo
 
 ### Instance metadata
 If running on EC2, the default values are obtained from the instance metadata service.
+
+
+### Feature Gates
+They are a set of kye=value pairs that describe AWS load balance controller features. You can use it as flags `--feature-gates=key1=value1,key2=value2`
+
+|Features-gate Supported Key            | Type                            | Default Value   | Description |
+|---------------------------------------|---------------------------------|-----------------|-------------|
+|ListenerRulesTagging                   | string                          | true            | Enable or disable tagging AWS load balancer listeners and rules |
+|WeightedTargetGroups                   | string                          | true            | Enable or disable weighted target groups |
diff --git a/main.go b/main.go
@@ -174,7 +174,7 @@ func loadControllerConfig() (config.ControllerConfig, error) {
 		AWSConfig: aws.CloudConfig{
 			ThrottleConfig: defaultAWSThrottleCFG,
 		},
-		FeatureGate: config.NewFeatureGate(),
+		FeatureGates: config.NewFeatureGates(),
 	}
 
 	fs := pflag.NewFlagSet("", pflag.ExitOnError)
diff --git a/pkg/config/controller_config.go b/pkg/config/controller_config.go
@@ -90,7 +90,7 @@ type ControllerConfig struct {
 	// DisableRestrictedSGRules specifies whether to use restricted security group rules
 	DisableRestrictedSGRules bool
 
-	FeatureGate FeatureGate
+	FeatureGates FeatureGates
 }
 
 // BindFlags binds the command line flags to the fields in the config object
@@ -119,7 +119,7 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&cfg.DisableRestrictedSGRules, flagDisableRestrictedSGRules, defaultDisableRestrictedSGRules,
 		"Disable the usage of restricted security group rules")
 
-	cfg.FeatureGate.BindFlags(fs)
+	cfg.FeatureGates.BindFlags(fs)
 	cfg.AWSConfig.BindFlags(fs)
 	cfg.RuntimeConfig.BindFlags(fs)
 
diff --git a/pkg/config/feature_gates.go b/pkg/config/feature_gates.go
@@ -10,10 +10,11 @@ import (
 type Feature string
 
 const (
-	EnableListenerRulesTagging Feature = "enable-listener-rules-tagging"
+	ListenerRulesTagging Feature = "ListenerRulesTagging"
+	WeightedTargetGroups Feature = "WeightedTargetGroups"
 )
 
-type FeatureGate interface {
+type FeatureGates interface {
 	// Enabled returns whether a feature is enabled
 	Enabled(feature Feature) bool
 
@@ -23,43 +24,44 @@ type FeatureGate interface {
 	// Disable will disable a feature
 	Disable(feature Feature)
 
-	// BindFlags bind featureGate flags
+	// BindFlags bind featureGates flags
 	BindFlags(fs *pflag.FlagSet)
 }
 
-var _ FeatureGate = (*defaultFeatureGate)(nil)
-var _ pflag.Value = (*defaultFeatureGate)(nil)
+var _ FeatureGates = (*defaultFeatureGates)(nil)
+var _ pflag.Value = (*defaultFeatureGates)(nil)
 
-type defaultFeatureGate struct {
+type defaultFeatureGates struct {
 	featureState map[Feature]bool
 }
 
-// NewFeatureGate constructs new featureGate
-func NewFeatureGate() FeatureGate {
-	return &defaultFeatureGate{
+// NewFeatureGates constructs new featureGates
+func NewFeatureGates() FeatureGates {
+	return &defaultFeatureGates{
 		featureState: map[Feature]bool{
-			EnableListenerRulesTagging: true,
+			ListenerRulesTagging: true,
+			WeightedTargetGroups: true,
 		},
 	}
 }
 
-func (f *defaultFeatureGate) BindFlags(fs *pflag.FlagSet) {
-	fs.Var(f, "feature-gate", "A set of key=bool pairs enable/disable features")
+func (f *defaultFeatureGates) BindFlags(fs *pflag.FlagSet) {
+	fs.Var(f, "feature-gates", "A set of key=bool pairs enable/disable features")
 }
 
-func (f *defaultFeatureGate) Enabled(feature Feature) bool {
+func (f *defaultFeatureGates) Enabled(feature Feature) bool {
 	return f.featureState[feature]
 }
 
-func (f *defaultFeatureGate) Enable(feature Feature) {
+func (f *defaultFeatureGates) Enable(feature Feature) {
 	f.featureState[feature] = true
 }
 
-func (f *defaultFeatureGate) Disable(feature Feature) {
+func (f *defaultFeatureGates) Disable(feature Feature) {
 	f.featureState[feature] = false
 }
 
-func (f *defaultFeatureGate) String() string {
+func (f *defaultFeatureGates) String() string {
 	var featureSettings []string
 	for feature, enabled := range f.featureState {
 		featureSettings = append(featureSettings, fmt.Sprintf("%v=%v", feature, enabled))
@@ -68,7 +70,7 @@ func (f *defaultFeatureGate) String() string {
 }
 
 // SplitMapStringBool parse comma-separated string of key1=value1,key2=value2. value is either true or false
-func (f *defaultFeatureGate) SplitMapStringBool(str string) (map[string]bool, error) {
+func (f *defaultFeatureGates) SplitMapStringBool(str string) (map[string]bool, error) {
 	result := make(map[string]bool)
 	for _, s := range strings.Split(str, ",") {
 		if len(s) == 0 {
@@ -88,10 +90,10 @@ func (f *defaultFeatureGate) SplitMapStringBool(str string) (map[string]bool, er
 	return result, nil
 }
 
-func (f *defaultFeatureGate) Set(value string) error {
+func (f *defaultFeatureGates) Set(value string) error {
 	settings, err := f.SplitMapStringBool(value)
 	if err != nil {
-		return fmt.Errorf("failed to parse feature-gate settings due to %v", err)
+		return fmt.Errorf("failed to parse feature-gates settings due to %v", err)
 	}
 	for k, v := range settings {
 		_, ok := f.featureState[Feature(k)]
@@ -103,6 +105,6 @@ func (f *defaultFeatureGate) Set(value string) error {
 	return nil
 }
 
-func (f *defaultFeatureGate) Type() string {
+func (f *defaultFeatureGates) Type() string {
 	return "mapStringBool"
 }
diff --git a/pkg/deploy/elbv2/listener_manager.go b/pkg/deploy/elbv2/listener_manager.go
@@ -28,13 +28,13 @@ type ListenerManager interface {
 }
 
 func NewDefaultListenerManager(elbv2Client services.ELBV2, trackingProvider tracking.Provider,
-	taggingManager TaggingManager, externalManagedTags []string, featureGate config.FeatureGate, logger logr.Logger) *defaultListenerManager {
+	taggingManager TaggingManager, externalManagedTags []string, featureGates config.FeatureGates, logger logr.Logger) *defaultListenerManager {
 	return &defaultListenerManager{
 		elbv2Client:                 elbv2Client,
 		trackingProvider:            trackingProvider,
 		taggingManager:              taggingManager,
 		externalManagedTags:         externalManagedTags,
-		featureGate:                 featureGate,
+		featureGates:                featureGates,
 		logger:                      logger,
 		waitLSExistencePollInterval: defaultWaitLSExistencePollInterval,
 		waitLSExistenceTimeout:      defaultWaitLSExistenceTimeout,
@@ -49,20 +49,20 @@ type defaultListenerManager struct {
 	trackingProvider    tracking.Provider
 	taggingManager      TaggingManager
 	externalManagedTags []string
-	featureGate         config.FeatureGate
+	featureGates        config.FeatureGates
 	logger              logr.Logger
 
 	waitLSExistencePollInterval time.Duration
 	waitLSExistenceTimeout      time.Duration
 }
 
 func (m *defaultListenerManager) Create(ctx context.Context, resLS *elbv2model.Listener) (elbv2model.ListenerStatus, error) {
-	req, err := buildSDKCreateListenerInput(resLS.Spec)
+	req, err := buildSDKCreateListenerInput(resLS.Spec, m.featureGates)
 	if err != nil {
 		return elbv2model.ListenerStatus{}, err
 	}
 	var lsTags map[string]string
-	if m.featureGate.Enabled(config.EnableListenerRulesTagging) {
+	if m.featureGates.Enabled(config.ListenerRulesTagging) {
 		lsTags = m.trackingProvider.ResourceTags(resLS.Stack(), resLS, resLS.Spec.Tags)
 	}
 	req.Tags = convertTagsToSDKTags(lsTags)
@@ -92,7 +92,7 @@ func (m *defaultListenerManager) Create(ctx context.Context, resLS *elbv2model.L
 }
 
 func (m *defaultListenerManager) Update(ctx context.Context, resLS *elbv2model.Listener, sdkLS ListenerWithTags) (elbv2model.ListenerStatus, error) {
-	if m.featureGate.Enabled(config.EnableListenerRulesTagging) {
+	if m.featureGates.Enabled(config.ListenerRulesTagging) {
 		if err := m.updateSDKListenerWithTags(ctx, resLS, sdkLS); err != nil {
 			return elbv2model.ListenerStatus{}, err
 		}
@@ -128,7 +128,7 @@ func (m *defaultListenerManager) updateSDKListenerWithTags(ctx context.Context,
 }
 
 func (m *defaultListenerManager) updateSDKListenerWithSettings(ctx context.Context, resLS *elbv2model.Listener, sdkLS ListenerWithTags) error {
-	desiredDefaultActions, err := buildSDKActions(resLS.Spec.DefaultActions)
+	desiredDefaultActions, err := buildSDKActions(resLS.Spec.DefaultActions, m.featureGates)
 	if err != nil {
 		return err
 	}
@@ -156,6 +156,12 @@ func (m *defaultListenerManager) updateSDKListenerWithSettings(ctx context.Conte
 // currentExtraCertificates is the current extra certificates, if it's nil, the current extra certificates will be fetched from AWS.
 func (m *defaultListenerManager) updateSDKListenerWithExtraCertificates(ctx context.Context, resLS *elbv2model.Listener,
 	sdkLS ListenerWithTags, isNewSDKListener bool) error {
+	// if TLS is not supported, we shouldn't update
+	if sdkLS.Listener.SslPolicy == nil {
+		m.logger.V(1).Info("SDK Listner doesn't have SSL Policy set, we skip updating extra certs for non-TLS listener.")
+		return nil
+	}
+
 	desiredExtraCertARNs := sets.NewString()
 	_, desiredExtraCerts := buildSDKCertificates(resLS.Spec.Certificates)
 	for _, cert := range desiredExtraCerts {
@@ -262,7 +268,7 @@ func isSDKListenerSettingsDrifted(lsSpec elbv2model.ListenerSpec, sdkLS Listener
 	return false
 }
 
-func buildSDKCreateListenerInput(lsSpec elbv2model.ListenerSpec) (*elbv2sdk.CreateListenerInput, error) {
+func buildSDKCreateListenerInput(lsSpec elbv2model.ListenerSpec, featureGates config.FeatureGates) (*elbv2sdk.CreateListenerInput, error) {
 	ctx := context.Background()
 	lbARN, err := lsSpec.LoadBalancerARN.Resolve(ctx)
 	if err != nil {
@@ -272,7 +278,7 @@ func buildSDKCreateListenerInput(lsSpec elbv2model.ListenerSpec) (*elbv2sdk.Crea
 	sdkObj.LoadBalancerArn = awssdk.String(lbARN)
 	sdkObj.Port = awssdk.Int64(lsSpec.Port)
 	sdkObj.Protocol = awssdk.String(string(lsSpec.Protocol))
-	defaultActions, err := buildSDKActions(lsSpec.DefaultActions)
+	defaultActions, err := buildSDKActions(lsSpec.DefaultActions, featureGates)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/deploy/elbv2/listener_rule_manager.go b/pkg/deploy/elbv2/listener_rule_manager.go
@@ -27,13 +27,13 @@ type ListenerRuleManager interface {
 
 // NewDefaultListenerRuleManager constructs new defaultListenerRuleManager.
 func NewDefaultListenerRuleManager(elbv2Client services.ELBV2, trackingProvider tracking.Provider,
-	taggingManager TaggingManager, externalManagedTags []string, featureGate config.FeatureGate, logger logr.Logger) *defaultListenerRuleManager {
+	taggingManager TaggingManager, externalManagedTags []string, featureGates config.FeatureGates, logger logr.Logger) *defaultListenerRuleManager {
 	return &defaultListenerRuleManager{
 		elbv2Client:                 elbv2Client,
 		trackingProvider:            trackingProvider,
 		taggingManager:              taggingManager,
 		externalManagedTags:         externalManagedTags,
-		featureGate:                 featureGate,
+		featureGates:                featureGates,
 		logger:                      logger,
 		waitLSExistencePollInterval: defaultWaitLSExistencePollInterval,
 		waitLSExistenceTimeout:      defaultWaitLSExistenceTimeout,
@@ -46,20 +46,20 @@ type defaultListenerRuleManager struct {
 	trackingProvider    tracking.Provider
 	taggingManager      TaggingManager
 	externalManagedTags []string
-	featureGate         config.FeatureGate
+	featureGates        config.FeatureGates
 	logger              logr.Logger
 
 	waitLSExistencePollInterval time.Duration
 	waitLSExistenceTimeout      time.Duration
 }
 
 func (m *defaultListenerRuleManager) Create(ctx context.Context, resLR *elbv2model.ListenerRule) (elbv2model.ListenerRuleStatus, error) {
-	req, err := buildSDKCreateListenerRuleInput(resLR.Spec)
+	req, err := buildSDKCreateListenerRuleInput(resLR.Spec, m.featureGates)
 	if err != nil {
 		return elbv2model.ListenerRuleStatus{}, err
 	}
 	var ruleTags map[string]string
-	if m.featureGate.Enabled(config.EnableListenerRulesTagging) {
+	if m.featureGates.Enabled(config.ListenerRulesTagging) {
 		ruleTags = m.trackingProvider.ResourceTags(resLR.Stack(), resLR, resLR.Spec.Tags)
 	}
 	req.Tags = convertTagsToSDKTags(ruleTags)
@@ -90,7 +90,7 @@ func (m *defaultListenerRuleManager) Create(ctx context.Context, resLR *elbv2mod
 }
 
 func (m *defaultListenerRuleManager) Update(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) (elbv2model.ListenerRuleStatus, error) {
-	if m.featureGate.Enabled(config.EnableListenerRulesTagging) {
+	if m.featureGates.Enabled(config.ListenerRulesTagging) {
 		if err := m.updateSDKListenerRuleWithTags(ctx, resLR, sdkLR); err != nil {
 			return elbv2model.ListenerRuleStatus{}, err
 		}
@@ -116,7 +116,7 @@ func (m *defaultListenerRuleManager) Delete(ctx context.Context, sdkLR ListenerR
 }
 
 func (m *defaultListenerRuleManager) updateSDKListenerRuleWithSettings(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) error {
-	desiredActions, err := buildSDKActions(resLR.Spec.Actions)
+	desiredActions, err := buildSDKActions(resLR.Spec.Actions, m.featureGates)
 	if err != nil {
 		return err
 	}
@@ -161,7 +161,7 @@ func isSDKListenerRuleSettingsDrifted(lrSpec elbv2model.ListenerRuleSpec, sdkLR
 	return false
 }
 
-func buildSDKCreateListenerRuleInput(lrSpec elbv2model.ListenerRuleSpec) (*elbv2sdk.CreateRuleInput, error) {
+func buildSDKCreateListenerRuleInput(lrSpec elbv2model.ListenerRuleSpec, featureGates config.FeatureGates) (*elbv2sdk.CreateRuleInput, error) {
 	ctx := context.Background()
 	lsARN, err := lrSpec.ListenerARN.Resolve(ctx)
 	if err != nil {
@@ -170,7 +170,7 @@ func buildSDKCreateListenerRuleInput(lrSpec elbv2model.ListenerRuleSpec) (*elbv2
 	sdkObj := &elbv2sdk.CreateRuleInput{}
 	sdkObj.ListenerArn = awssdk.String(lsARN)
 	sdkObj.Priority = awssdk.Int64(lrSpec.Priority)
-	actions, err := buildSDKActions(lrSpec.Actions)
+	actions, err := buildSDKActions(lrSpec.Actions, featureGates)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/deploy/elbv2/listener_utils.go b/pkg/deploy/elbv2/listener_utils.go
@@ -6,6 +6,7 @@ import (
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	elbv2sdk "github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/pkg/errors"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 	"time"
 )
@@ -15,12 +16,12 @@ const (
 	defaultWaitLSExistenceTimeout      = 20 * time.Second
 )
 
-func buildSDKActions(modelActions []elbv2model.Action) ([]*elbv2sdk.Action, error) {
+func buildSDKActions(modelActions []elbv2model.Action, featureGates config.FeatureGates) ([]*elbv2sdk.Action, error) {
 	var sdkActions []*elbv2sdk.Action
 	if len(modelActions) != 0 {
 		sdkActions = make([]*elbv2sdk.Action, 0, len(modelActions))
 		for index, modelAction := range modelActions {
-			sdkAction, err := buildSDKAction(modelAction)
+			sdkAction, err := buildSDKAction(modelAction, featureGates)
 			sdkAction.Order = awssdk.Int64(int64(index) + 1)
 			if err != nil {
 				return nil, err
@@ -31,7 +32,7 @@ func buildSDKActions(modelActions []elbv2model.Action) ([]*elbv2sdk.Action, erro
 	return sdkActions, nil
 }
 
-func buildSDKAction(modelAction elbv2model.Action) (*elbv2sdk.Action, error) {
+func buildSDKAction(modelAction elbv2model.Action, featureGates config.FeatureGates) (*elbv2sdk.Action, error) {
 	sdkObj := &elbv2sdk.Action{}
 	sdkObj.Type = awssdk.String(string(modelAction.Type))
 	if modelAction.AuthenticateCognitoConfig != nil {
@@ -51,7 +52,15 @@ func buildSDKAction(modelAction elbv2model.Action) (*elbv2sdk.Action, error) {
 		if err != nil {
 			return nil, err
 		}
-		sdkObj.ForwardConfig = forwardConfig
+		if !featureGates.Enabled(config.WeightedTargetGroups) {
+			if len(forwardConfig.TargetGroups) == 1 {
+				sdkObj.TargetGroupArn = forwardConfig.TargetGroups[0].TargetGroupArn
+			} else {
+				return nil, errors.New("Weighted target groups feature is disabled.")
+			}
+		} else {
+			sdkObj.ForwardConfig = forwardConfig
+		}
 	}
 	return sdkObj, nil
 }
diff --git a/pkg/deploy/elbv2/tagging_manager.go b/pkg/deploy/elbv2/tagging_manager.go
diff --git a/pkg/deploy/stack_deployer.go b/pkg/deploy/stack_deployer.go

Original file line number	Diff line number	Diff line change
`@@ -174,7 +174,7 @@ func loadControllerConfig() (config.ControllerConfig, error) {`
`174`	`174`	`AWSConfig: aws.CloudConfig{`
`175`	`175`	`ThrottleConfig: defaultAWSThrottleCFG,`
`176`	`176`	`},`
`177`		`- FeatureGate: config.NewFeatureGate(),`
	`177`	`+ FeatureGates: config.NewFeatureGates(),`
`178`	`178`	`}`
`179`	`179`
`180`	`180`	`fs := pflag.NewFlagSet("", pflag.ExitOnError)`