add support for external-managed-tags & prefer defaultTags (#1970)

controllers/ingress/group_controller.go

@@ -46,7 +46,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
 		cloud.EC2(), cloud.ACM(),
 		annotationParser, subnetsResolver,
 		authConfigBuilder, enhancedBackendBuilder,
-		cloud.VpcID(), config.ClusterName, config.DefaultTags,
+		cloud.VpcID(), config.ClusterName, config.DefaultTags, config.ExternalManagedTags,
 		config.DefaultSSLPolicy, logger)
 	stackMarshaller := deploy.NewDefaultStackMarshaller()
 	stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler,

controllers/service/service_controller.go

@@ -42,7 +42,7 @@ func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorde
 	trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, config.ClusterName)
 	elbv2TaggingManager := elbv2.NewDefaultTaggingManager(cloud.ELBV2(), logger)
 	modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcResolver, trackingProvider,
-		elbv2TaggingManager, config.ClusterName, config.DefaultTags, config.DefaultSSLPolicy)
+		elbv2TaggingManager, config.ClusterName, config.DefaultTags, config.ExternalManagedTags, config.DefaultSSLPolicy)
 	stackMarshaller := deploy.NewDefaultStackMarshaller()
 	stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, config, serviceTagPrefix, logger)
 	return &serviceReconciler{

pkg/config/controller_config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/aws"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/inject"
 )
@@ -11,6 +12,7 @@ const (
 	flagLogLevel                                  = "log-level"
 	flagK8sClusterName                            = "cluster-name"
 	flagDefaultTags                               = "default-tags"
+	flagExternalManagedTags                       = "external-managed-tags"
 	flagServiceMaxConcurrentReconciles            = "service-max-concurrent-reconciles"
 	flagTargetGroupBindingMaxConcurrentReconciles = "targetgroupbinding-max-concurrent-reconciles"
 	flagDefaultSSLPolicy                          = "default-ssl-policy"
@@ -19,6 +21,16 @@ const (
 	defaultSSLPolicy                              = "ELBSecurityPolicy-2016-08"
 )
 
+var (
+	trackingTagKeys = sets.NewString(
+		"elbv2.k8s.aws/cluster",
+		"ingress.k8s.aws/stack",
+		"ingress.k8s.aws/resource",
+		"service.k8s.aws/stack",
+		"service.k8s.aws/resource",
+	)
+)
+
 // ControllerConfig contains the controller configuration
 type ControllerConfig struct {
 	// Log level for the controller logs
@@ -39,6 +51,9 @@ type ControllerConfig struct {
 	// Default AWS Tags that will be applied to all AWS resources managed by this controller.
 	DefaultTags map[string]string
 
+	// List of Tag keys on AWS resources that will be managed externally.
+	ExternalManagedTags []string
+
 	// Default SSL Policy that will be applied to all ingresses or services that do not have
 	// the SSL Policy annotation.
 	DefaultSSLPolicy string
@@ -56,6 +71,8 @@ func (cfg *ControllerConfig) BindFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&cfg.ClusterName, flagK8sClusterName, "", "Kubernetes cluster name")
 	fs.StringToStringVar(&cfg.DefaultTags, flagDefaultTags, nil,
 		"Default AWS Tags that will be applied to all AWS resources managed by this controller")
+	fs.StringSliceVar(&cfg.ExternalManagedTags, flagExternalManagedTags, nil,
+		"List of Tag keys on AWS resources that will be managed externally")
 	fs.IntVar(&cfg.ServiceMaxConcurrentReconciles, flagServiceMaxConcurrentReconciles, defaultMaxConcurrentReconciles,
 		"Maximum number of concurrently running reconcile loops for service")
 	fs.IntVar(&cfg.TargetGroupBindingMaxConcurrentReconciles, flagTargetGroupBindingMaxConcurrentReconciles, defaultMaxConcurrentReconciles,
@@ -76,5 +93,43 @@ func (cfg *ControllerConfig) Validate() error {
 	if len(cfg.ClusterName) == 0 {
 		return errors.New("kubernetes cluster name must be specified")
 	}
+
+	if err := cfg.validateDefaultTagsCollisionWithTrackingTags(); err != nil {
+		return err
+	}
+	if err := cfg.validateExternalManagedTagsCollisionWithTrackingTags(); err != nil {
+		return err
+	}
+	if err := cfg.validateExternalManagedTagsCollisionWithDefaultTags(); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (cfg *ControllerConfig) validateDefaultTagsCollisionWithTrackingTags() error {
+	for tagKey := range cfg.DefaultTags {
+		if trackingTagKeys.Has(tagKey) {
+			return errors.Errorf("tag key %v cannot be specified in %v flag", tagKey, flagDefaultTags)
+		}
+	}
+	return nil
+}
+
+func (cfg *ControllerConfig) validateExternalManagedTagsCollisionWithTrackingTags() error {
+	for _, tagKey := range cfg.ExternalManagedTags {
+		if trackingTagKeys.Has(tagKey) {
+			return errors.Errorf("tag key %v cannot be specified in %v flag", tagKey, flagExternalManagedTags)
+		}
+	}
+	return nil
+}
+
+func (cfg *ControllerConfig) validateExternalManagedTagsCollisionWithDefaultTags() error {
+	for _, tagKey := range cfg.ExternalManagedTags {
+		if _, ok := cfg.DefaultTags[tagKey]; ok {
+			return errors.Errorf("tag key %v cannot be specified in both %v and %v flag",
+				tagKey, flagDefaultTags, flagExternalManagedTags)
+		}
+	}
 	return nil
 }

pkg/config/controller_config_test.go

@@ -0,0 +1,166 @@
+package config
+
+import (
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestControllerConfig_validateDefaultTagsCollisionWithTrackingTags(t *testing.T) {
+	type fields struct {
+		DefaultTags map[string]string
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr error
+	}{
+		{
+			name: "default tags and tracking tags have no collision",
+			fields: fields{
+				DefaultTags: map[string]string{
+					"tag-a": "value-a",
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "default tags and tracking tags have collision",
+			fields: fields{
+				DefaultTags: map[string]string{
+					"elbv2.k8s.aws/cluster": "value-a",
+				},
+			},
+			wantErr: errors.New("tag key elbv2.k8s.aws/cluster cannot be specified in default-tags flag"),
+		},
+		{
+			name: "default tags is empty",
+			fields: fields{
+				DefaultTags: nil,
+			},
+			wantErr: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &ControllerConfig{
+				DefaultTags: tt.fields.DefaultTags,
+			}
+			err := cfg.validateDefaultTagsCollisionWithTrackingTags()
+			if tt.wantErr != nil {
+				assert.EqualError(t, err, tt.wantErr.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestControllerConfig_validateExternalManagedTagsCollisionWithTrackingTags(t *testing.T) {
+	type fields struct {
+		ExternalManagedTags []string
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr error
+	}{
+		{
+			name: "external managed tags and tracking tags have no collision",
+			fields: fields{
+				ExternalManagedTags: []string{"tag-a"},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "external managed tags and tracking tags have collision",
+			fields: fields{
+				ExternalManagedTags: []string{"elbv2.k8s.aws/cluster"},
+			},
+			wantErr: errors.New("tag key elbv2.k8s.aws/cluster cannot be specified in external-managed-tags flag"),
+		},
+		{
+			name: "external managed tags is empty",
+			fields: fields{
+				ExternalManagedTags: nil,
+			},
+			wantErr: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &ControllerConfig{
+				ExternalManagedTags: tt.fields.ExternalManagedTags,
+			}
+			err := cfg.validateExternalManagedTagsCollisionWithTrackingTags()
+			if tt.wantErr != nil {
+				assert.EqualError(t, err, tt.wantErr.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestControllerConfig_validateExternalManagedTagsCollisionWithDefaultTags(t *testing.T) {
+	type fields struct {
+		DefaultTags         map[string]string
+		ExternalManagedTags []string
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		wantErr error
+	}{
+		{
+			name: "default tags and external managed tags have no collision",
+			fields: fields{
+				DefaultTags: map[string]string{
+					"tag-a": "value-a",
+				},
+				ExternalManagedTags: []string{"tag-b"},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "default tags and external managed tags have collision",
+			fields: fields{
+				DefaultTags: map[string]string{
+					"tag-a": "value-a",
+				},
+				ExternalManagedTags: []string{"tag-a"},
+			},
+			wantErr: errors.New("tag key tag-a cannot be specified in both default-tags and external-managed-tags flag"),
+		},
+		{
+			name: "empty default tags and non-empty external managed tags",
+			fields: fields{
+				DefaultTags:         nil,
+				ExternalManagedTags: []string{"tag-a"},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "empty default tags and empty external managed tags",
+			fields: fields{
+				DefaultTags:         nil,
+				ExternalManagedTags: nil,
+			},
+			wantErr: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &ControllerConfig{
+				DefaultTags:         tt.fields.DefaultTags,
+				ExternalManagedTags: tt.fields.ExternalManagedTags,
+			}
+			err := cfg.validateExternalManagedTagsCollisionWithDefaultTags()
+			if tt.wantErr != nil {
+				assert.EqualError(t, err, tt.wantErr.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

pkg/deploy/ec2/security_group_manager.go

@@ -31,13 +31,14 @@ type SecurityGroupManager interface {
 
 // NewDefaultSecurityGroupManager constructs new defaultSecurityGroupManager.
 func NewDefaultSecurityGroupManager(ec2Client services.EC2, trackingProvider tracking.Provider, taggingManager TaggingManager,
-	networkingSGReconciler networking.SecurityGroupReconciler, vpcID string, logger logr.Logger) *defaultSecurityGroupManager {
+	networkingSGReconciler networking.SecurityGroupReconciler, vpcID string, externalManagedTags []string, logger logr.Logger) *defaultSecurityGroupManager {
 	return &defaultSecurityGroupManager{
 		ec2Client:              ec2Client,
 		trackingProvider:       trackingProvider,
 		taggingManager:         taggingManager,
 		networkingSGReconciler: networkingSGReconciler,
 		vpcID:                  vpcID,
+		externalManagedTags:    externalManagedTags,
 		logger:                 logger,
 
 		waitSGDeletionPollInterval: defaultWaitSGDeletionPollInterval,
@@ -52,6 +53,7 @@ type defaultSecurityGroupManager struct {
 	taggingManager         TaggingManager
 	networkingSGReconciler networking.SecurityGroupReconciler
 	vpcID                  string
+	externalManagedTags    []string
 	logger                 logr.Logger
 
 	waitSGDeletionPollInterval time.Duration
@@ -136,7 +138,8 @@ func (m *defaultSecurityGroupManager) updateSDKSecurityGroupGroupWithTags(ctx co
 	desiredSGTags := m.trackingProvider.ResourceTags(resSG.Stack(), resSG, resSG.Spec.Tags)
 	return m.taggingManager.ReconcileTags(ctx, sdkSG.SecurityGroupID, desiredSGTags,
 		WithCurrentTags(sdkSG.Tags),
-		WithIgnoredTagKeys(m.trackingProvider.LegacyTagKeys()))
+		WithIgnoredTagKeys(m.trackingProvider.LegacyTagKeys()),
+		WithIgnoredTagKeys(m.externalManagedTags))
 }
 
 func buildIPPermissionInfos(permissions []ec2model.IPPermission) ([]networking.IPPermissionInfo, error) {

pkg/deploy/ec2/tagging_manager.go

@@ -43,7 +43,7 @@ func WithCurrentTags(tags map[string]string) ReconcileTagsOption {
 // WithIgnoredTagKeys is a reconcile option that configures IgnoredTagKeys.
 func WithIgnoredTagKeys(ignoredTagKeys []string) ReconcileTagsOption {
 	return func(opts *ReconcileTagsOptions) {
-		opts.IgnoredTagKeys = ignoredTagKeys
+		opts.IgnoredTagKeys = append(opts.IgnoredTagKeys, ignoredTagKeys...)
 	}
 }
 

pkg/deploy/elbv2/listener_manager.go

@@ -27,11 +27,12 @@ type ListenerManager interface {
 }
 
 func NewDefaultListenerManager(elbv2Client services.ELBV2, trackingProvider tracking.Provider,
-	taggingManager TaggingManager, logger logr.Logger) *defaultListenerManager {
+	taggingManager TaggingManager, externalManagedTags []string, logger logr.Logger) *defaultListenerManager {
 	return &defaultListenerManager{
 		elbv2Client:                 elbv2Client,
 		trackingProvider:            trackingProvider,
 		taggingManager:              taggingManager,
+		externalManagedTags:         externalManagedTags,
 		logger:                      logger,
 		waitLSExistencePollInterval: defaultWaitLSExistencePollInterval,
 		waitLSExistenceTimeout:      defaultWaitLSExistenceTimeout,
@@ -42,10 +43,11 @@ var _ ListenerManager = &defaultListenerManager{}
 
 // default implementation for ListenerManager
 type defaultListenerManager struct {
-	elbv2Client      services.ELBV2
-	trackingProvider tracking.Provider
-	taggingManager   TaggingManager
-	logger           logr.Logger
+	elbv2Client         services.ELBV2
+	trackingProvider    tracking.Provider
+	taggingManager      TaggingManager
+	externalManagedTags []string
+	logger              logr.Logger
 
 	waitLSExistencePollInterval time.Duration
 	waitLSExistenceTimeout      time.Duration
@@ -113,7 +115,8 @@ func (m *defaultListenerManager) Delete(ctx context.Context, sdkLS ListenerWithT
 func (m *defaultListenerManager) updateSDKListenerWithTags(ctx context.Context, resLS *elbv2model.Listener, sdkLS ListenerWithTags) error {
 	desiredLSTags := m.trackingProvider.ResourceTags(resLS.Stack(), resLS, resLS.Spec.Tags)
 	return m.taggingManager.ReconcileTags(ctx, awssdk.StringValue(sdkLS.Listener.ListenerArn), desiredLSTags,
-		WithCurrentTags(sdkLS.Tags))
+		WithCurrentTags(sdkLS.Tags),
+		WithIgnoredTagKeys(m.externalManagedTags))
 }
 
 func (m *defaultListenerManager) updateSDKListenerWithSettings(ctx context.Context, resLS *elbv2model.Listener, sdkLS ListenerWithTags) error {