From df6d2da9af7af342d9968165119d84d4352fc51d Mon Sep 17 00:00:00 2001 From: M00nF1sh Date: Mon, 3 May 2021 15:34:20 -0700 Subject: [PATCH] add support for external-managed-tags & prefer defaultTags (#1970) --- controllers/ingress/group_controller.go | 2 +- controllers/service/service_controller.go | 2 +- pkg/config/controller_config.go | 55 ++++++ pkg/config/controller_config_test.go | 166 ++++++++++++++++++ pkg/deploy/ec2/security_group_manager.go | 7 +- pkg/deploy/ec2/tagging_manager.go | 2 +- pkg/deploy/elbv2/listener_manager.go | 15 +- pkg/deploy/elbv2/listener_rule_manager.go | 15 +- pkg/deploy/elbv2/load_balancer_manager.go | 7 +- pkg/deploy/elbv2/tagging_manager.go | 2 +- pkg/deploy/elbv2/target_group_manager.go | 7 +- pkg/deploy/stack_deployer.go | 10 +- pkg/ingress/model_build_listener.go | 19 +- pkg/ingress/model_build_listener_rules.go | 16 +- pkg/ingress/model_build_load_balancer.go | 19 +- pkg/ingress/model_build_load_balancer_test.go | 84 ++++++++- pkg/ingress/model_build_managed_sg.go | 17 +- pkg/ingress/model_build_managed_sg_test.go | 84 ++++++++- pkg/ingress/model_build_target_group.go | 18 +- pkg/ingress/model_build_target_group_test.go | 38 +++- pkg/ingress/model_builder.go | 6 +- pkg/service/model_build_load_balancer.go | 13 +- pkg/service/model_build_load_balancer_test.go | 47 ++++- pkg/service/model_builder.go | 13 +- pkg/service/model_builder_test.go | 2 +- 25 files changed, 564 insertions(+), 102 deletions(-) create mode 100644 pkg/config/controller_config_test.go diff --git a/controllers/ingress/group_controller.go b/controllers/ingress/group_controller.go index 1bf5eda4f..855295097 100644 --- a/controllers/ingress/group_controller.go +++ b/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, diff --git a/controllers/service/service_controller.go b/controllers/service/service_controller.go index 45c8373da..0e1d00ce4 100644 --- a/controllers/service/service_controller.go +++ b/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{ diff --git a/pkg/config/controller_config.go b/pkg/config/controller_config.go index df474cc20..3e8e3979f 100644 --- a/pkg/config/controller_config.go +++ b/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 } diff --git a/pkg/config/controller_config_test.go b/pkg/config/controller_config_test.go new file mode 100644 index 000000000..5b5fc8dbb --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/deploy/ec2/security_group_manager.go b/pkg/deploy/ec2/security_group_manager.go index bc811f543..9fc84ceeb 100644 --- a/pkg/deploy/ec2/security_group_manager.go +++ b/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) { diff --git a/pkg/deploy/ec2/tagging_manager.go b/pkg/deploy/ec2/tagging_manager.go index 53597f82e..e972c2029 100644 --- a/pkg/deploy/ec2/tagging_manager.go +++ b/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...) } } diff --git a/pkg/deploy/elbv2/listener_manager.go b/pkg/deploy/elbv2/listener_manager.go index 20614496f..4beb85f3b 100644 --- a/pkg/deploy/elbv2/listener_manager.go +++ b/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 { diff --git a/pkg/deploy/elbv2/listener_rule_manager.go b/pkg/deploy/elbv2/listener_rule_manager.go index 06218c594..85e6da819 100644 --- a/pkg/deploy/elbv2/listener_rule_manager.go +++ b/pkg/deploy/elbv2/listener_rule_manager.go @@ -26,11 +26,12 @@ type ListenerRuleManager interface { // NewDefaultListenerRuleManager constructs new defaultListenerRuleManager. func NewDefaultListenerRuleManager(elbv2Client services.ELBV2, trackingProvider tracking.Provider, - taggingManager TaggingManager, logger logr.Logger) *defaultListenerRuleManager { + taggingManager TaggingManager, externalManagedTags []string, logger logr.Logger) *defaultListenerRuleManager { return &defaultListenerRuleManager{ elbv2Client: elbv2Client, trackingProvider: trackingProvider, taggingManager: taggingManager, + externalManagedTags: externalManagedTags, logger: logger, waitLSExistencePollInterval: defaultWaitLSExistencePollInterval, waitLSExistenceTimeout: defaultWaitLSExistenceTimeout, @@ -39,10 +40,11 @@ func NewDefaultListenerRuleManager(elbv2Client services.ELBV2, trackingProvider // default implementation for ListenerRuleManager. type defaultListenerRuleManager 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 @@ -134,7 +136,8 @@ func (m *defaultListenerRuleManager) updateSDKListenerRuleWithSettings(ctx conte func (m *defaultListenerRuleManager) updateSDKListenerRuleWithTags(ctx context.Context, resLR *elbv2model.ListenerRule, sdkLR ListenerRuleWithTags) error { desiredTags := m.trackingProvider.ResourceTags(resLR.Stack(), resLR, resLR.Spec.Tags) return m.taggingManager.ReconcileTags(ctx, awssdk.StringValue(sdkLR.ListenerRule.RuleArn), desiredTags, - WithCurrentTags(sdkLR.Tags)) + WithCurrentTags(sdkLR.Tags), + WithIgnoredTagKeys(m.externalManagedTags)) } func isSDKListenerRuleSettingsDrifted(lrSpec elbv2model.ListenerRuleSpec, sdkLR ListenerRuleWithTags, diff --git a/pkg/deploy/elbv2/load_balancer_manager.go b/pkg/deploy/elbv2/load_balancer_manager.go index 495810584..345e61cb1 100644 --- a/pkg/deploy/elbv2/load_balancer_manager.go +++ b/pkg/deploy/elbv2/load_balancer_manager.go @@ -25,12 +25,13 @@ type LoadBalancerManager interface { // NewDefaultLoadBalancerManager constructs new defaultLoadBalancerManager. func NewDefaultLoadBalancerManager(elbv2Client services.ELBV2, trackingProvider tracking.Provider, - taggingManager TaggingManager, logger logr.Logger) *defaultLoadBalancerManager { + taggingManager TaggingManager, externalManagedTags []string, logger logr.Logger) *defaultLoadBalancerManager { return &defaultLoadBalancerManager{ elbv2Client: elbv2Client, trackingProvider: trackingProvider, taggingManager: taggingManager, attributesReconciler: NewDefaultLoadBalancerAttributeReconciler(elbv2Client, logger), + externalManagedTags: externalManagedTags, logger: logger, } } @@ -43,6 +44,7 @@ type defaultLoadBalancerManager struct { trackingProvider tracking.Provider taggingManager TaggingManager attributesReconciler LoadBalancerAttributeReconciler + externalManagedTags []string logger logr.Logger } @@ -221,7 +223,8 @@ func (m *defaultLoadBalancerManager) updateSDKLoadBalancerWithTags(ctx context.C desiredLBTags := m.trackingProvider.ResourceTags(resLB.Stack(), resLB, resLB.Spec.Tags) return m.taggingManager.ReconcileTags(ctx, awssdk.StringValue(sdkLB.LoadBalancer.LoadBalancerArn), desiredLBTags, WithCurrentTags(sdkLB.Tags), - WithIgnoredTagKeys(m.trackingProvider.LegacyTagKeys())) + WithIgnoredTagKeys(m.trackingProvider.LegacyTagKeys()), + WithIgnoredTagKeys(m.externalManagedTags)) } func buildSDKCreateLoadBalancerInput(lbSpec elbv2model.LoadBalancerSpec) (*elbv2sdk.CreateLoadBalancerInput, error) { diff --git a/pkg/deploy/elbv2/tagging_manager.go b/pkg/deploy/elbv2/tagging_manager.go index 06131d1b7..12f6785a9 100644 --- a/pkg/deploy/elbv2/tagging_manager.go +++ b/pkg/deploy/elbv2/tagging_manager.go @@ -69,7 +69,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...) } } diff --git a/pkg/deploy/elbv2/target_group_manager.go b/pkg/deploy/elbv2/target_group_manager.go index a1c617c88..1999dabf9 100644 --- a/pkg/deploy/elbv2/target_group_manager.go +++ b/pkg/deploy/elbv2/target_group_manager.go @@ -30,13 +30,14 @@ type TargetGroupManager interface { // NewDefaultTargetGroupManager constructs new defaultTargetGroupManager. func NewDefaultTargetGroupManager(elbv2Client services.ELBV2, trackingProvider tracking.Provider, - taggingManager TaggingManager, vpcID string, logger logr.Logger) *defaultTargetGroupManager { + taggingManager TaggingManager, vpcID string, externalManagedTags []string, logger logr.Logger) *defaultTargetGroupManager { return &defaultTargetGroupManager{ elbv2Client: elbv2Client, trackingProvider: trackingProvider, taggingManager: taggingManager, attributesReconciler: NewDefaultTargetGroupAttributesReconciler(elbv2Client, logger), vpcID: vpcID, + externalManagedTags: externalManagedTags, logger: logger, waitTGDeletionPollInterval: defaultWaitTGDeletionPollInterval, @@ -53,6 +54,7 @@ type defaultTargetGroupManager struct { taggingManager TaggingManager attributesReconciler TargetGroupAttributesReconciler vpcID string + externalManagedTags []string logger logr.Logger @@ -147,7 +149,8 @@ func (m *defaultTargetGroupManager) updateSDKTargetGroupWithTags(ctx context.Con desiredTGTags := m.trackingProvider.ResourceTags(resTG.Stack(), resTG, resTG.Spec.Tags) return m.taggingManager.ReconcileTags(ctx, awssdk.StringValue(sdkTG.TargetGroup.TargetGroupArn), desiredTGTags, WithCurrentTags(sdkTG.Tags), - WithIgnoredTagKeys(m.trackingProvider.LegacyTagKeys())) + WithIgnoredTagKeys(m.trackingProvider.LegacyTagKeys()), + WithIgnoredTagKeys(m.externalManagedTags)) } func isSDKTargetGroupHealthCheckDrifted(tgSpec elbv2model.TargetGroupSpec, sdkTG TargetGroupWithTags) bool { diff --git a/pkg/deploy/stack_deployer.go b/pkg/deploy/stack_deployer.go index c6aff0961..7bdec54e9 100644 --- a/pkg/deploy/stack_deployer.go +++ b/pkg/deploy/stack_deployer.go @@ -37,12 +37,12 @@ func NewDefaultStackDeployer(cloud aws.Cloud, k8sClient client.Client, addonsConfig: config.AddonsConfig, trackingProvider: trackingProvider, ec2TaggingManager: ec2TaggingManager, - ec2SGManager: ec2.NewDefaultSecurityGroupManager(cloud.EC2(), trackingProvider, ec2TaggingManager, networkingSGReconciler, cloud.VpcID(), logger), + ec2SGManager: ec2.NewDefaultSecurityGroupManager(cloud.EC2(), trackingProvider, ec2TaggingManager, networkingSGReconciler, cloud.VpcID(), config.ExternalManagedTags, logger), elbv2TaggingManager: elbv2TaggingManager, - elbv2LBManager: elbv2.NewDefaultLoadBalancerManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, logger), - elbv2LSManager: elbv2.NewDefaultListenerManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, logger), - elbv2LRManager: elbv2.NewDefaultListenerRuleManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, logger), - elbv2TGManager: elbv2.NewDefaultTargetGroupManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, cloud.VpcID(), logger), + elbv2LBManager: elbv2.NewDefaultLoadBalancerManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, config.ExternalManagedTags, logger), + elbv2LSManager: elbv2.NewDefaultListenerManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, config.ExternalManagedTags, logger), + elbv2LRManager: elbv2.NewDefaultListenerRuleManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, config.ExternalManagedTags, logger), + elbv2TGManager: elbv2.NewDefaultTargetGroupManager(cloud.ELBV2(), trackingProvider, elbv2TaggingManager, cloud.VpcID(), config.ExternalManagedTags, logger), elbv2TGBManager: elbv2.NewDefaultTargetGroupBindingManager(k8sClient, trackingProvider, logger), wafv2WebACLAssociationManager: wafv2.NewDefaultWebACLAssociationManager(cloud.WAFv2(), logger), wafRegionalWebACLAssociationManager: wafregional.NewDefaultWebACLAssociationManager(cloud.WAFRegional(), logger), diff --git a/pkg/ingress/model_build_listener.go b/pkg/ingress/model_build_listener.go index fbe9a6f23..eeb5ef2f0 100644 --- a/pkg/ingress/model_build_listener.go +++ b/pkg/ingress/model_build_listener.go @@ -4,17 +4,19 @@ import ( "context" "encoding/json" "fmt" + "net" + "strings" + awssdk "github.com/aws/aws-sdk-go/aws" "github.com/pkg/errors" networking "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - "net" + "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" - "strings" ) func (t *defaultModelBuildTask) buildListener(ctx context.Context, lbARN core.StringToken, port int64, config listenPortConfig, ingList []*networking.Ingress) (*elbv2model.Listener, error) { @@ -234,18 +236,17 @@ func (t *defaultModelBuildTask) modelBuildListenerTags(_ context.Context, ingLis return nil, err } for tagKey, tagValue := range rawTags { + if t.externalManagedTags.Has(tagKey) { + return nil, errors.Errorf("external managed tag key %v cannot be specified on Ingress %v", + tagKey, k8s.NamespacedName(ing).String()) + } + if existingTagValue, exists := annotationTags[tagKey]; exists && existingTagValue != tagValue { return nil, errors.Errorf("conflicting tag %v: %v | %v", tagKey, existingTagValue, tagValue) } annotationTags[tagKey] = tagValue } } - mergedTags := make(map[string]string) - for k, v := range t.defaultTags { - mergedTags[k] = v - } - for k, v := range annotationTags { - mergedTags[k] = v - } + mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags) return mergedTags, nil } diff --git a/pkg/ingress/model_build_listener_rules.go b/pkg/ingress/model_build_listener_rules.go index 460473c83..f831e5bf9 100644 --- a/pkg/ingress/model_build_listener_rules.go +++ b/pkg/ingress/model_build_listener_rules.go @@ -3,13 +3,15 @@ package ingress import ( "context" "fmt" + "strings" + "github.com/pkg/errors" networking "k8s.io/api/networking/v1beta1" + "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" - "strings" ) func (t *defaultModelBuildTask) buildListenerRules(ctx context.Context, lsARN core.StringToken, port int64, protocol elbv2model.Protocol, ingList []*networking.Ingress) error { @@ -264,12 +266,12 @@ func (t *defaultModelBuildTask) modelBuildListenerRuleTags(_ context.Context, in if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &rawTags, ing.Annotations); err != nil { return nil, err } - mergedTags := make(map[string]string) - for k, v := range t.defaultTags { - mergedTags[k] = v - } - for k, v := range rawTags { - mergedTags[k] = v + for tagKey := range rawTags { + if t.externalManagedTags.Has(tagKey) { + return nil, errors.Errorf("external managed tag key %v cannot be specified", tagKey) + } } + + mergedTags := algorithm.MergeStringMap(t.defaultTags, rawTags) return mergedTags, nil } diff --git a/pkg/ingress/model_build_load_balancer.go b/pkg/ingress/model_build_load_balancer.go index da7777214..1a504df76 100644 --- a/pkg/ingress/model_build_load_balancer.go +++ b/pkg/ingress/model_build_load_balancer.go @@ -5,19 +5,21 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "regexp" + "strings" + awssdk "github.com/aws/aws-sdk-go/aws" ec2sdk "github.com/aws/aws-sdk-go/service/ec2" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/sets" - "regexp" + "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/equality" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" "sigs.k8s.io/aws-load-balancer-controller/pkg/networking" - "strings" ) const ( @@ -307,19 +309,18 @@ func (t *defaultModelBuildTask) buildLoadBalancerTags(_ context.Context) (map[st return nil, err } for tagKey, tagValue := range rawTags { + if t.externalManagedTags.Has(tagKey) { + return nil, errors.Errorf("external managed tag key %v cannot be specified on Ingress %v", + tagKey, k8s.NamespacedName(member.Ing).String()) + } + if existingTagValue, exists := annotationTags[tagKey]; exists && existingTagValue != tagValue { return nil, errors.Errorf("conflicting tag %v: %v | %v", tagKey, existingTagValue, tagValue) } annotationTags[tagKey] = tagValue } } - mergedTags := make(map[string]string) - for k, v := range t.defaultTags { - mergedTags[k] = v - } - for k, v := range annotationTags { - mergedTags[k] = v - } + mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags) return mergedTags, nil } diff --git a/pkg/ingress/model_build_load_balancer_test.go b/pkg/ingress/model_build_load_balancer_test.go index 096197e58..7b44804dd 100644 --- a/pkg/ingress/model_build_load_balancer_test.go +++ b/pkg/ingress/model_build_load_balancer_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" "testing" @@ -226,8 +227,9 @@ func Test_defaultModelBuildTask_buildLoadBalancerCOIPv4Pool(t *testing.T) { func Test_defaultModelBuildTask_buildLoadBalancerTags(t *testing.T) { type fields struct { - ingGroup Group - defaultTags map[string]string + ingGroup Group + defaultTags map[string]string + externalManagedTags sets.String } tests := []struct { name string @@ -356,7 +358,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerTags(t *testing.T) { want: map[string]string{ "k1": "v1", "k2": "v2", - "k3": "v3a", + "k3": "v3", "k4": "v4", }, }, @@ -389,13 +391,83 @@ func Test_defaultModelBuildTask_buildLoadBalancerTags(t *testing.T) { }, wantErr: errors.New("conflicting tag k3: v3a | v3b"), }, + { + name: "non empty external managed tags, no conflicts", + fields: fields{ + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k1=v1", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-2", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k2=v2", + }, + }, + }, + }, + }, + }, + externalManagedTags: sets.NewString("k3"), + }, + want: map[string]string{ + "k1": "v1", + "k2": "v2", + }, + }, + { + name: "non empty external managed tags, has conflicts", + fields: fields{ + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k1=v1", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-2", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k2=v2", + }, + }, + }, + }, + }, + }, + externalManagedTags: sets.NewString("k2"), + }, + wantErr: errors.New("external managed tag key k2 cannot be specified on Ingress awesome-ns/ing-2"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { task := &defaultModelBuildTask{ - ingGroup: tt.fields.ingGroup, - defaultTags: tt.fields.defaultTags, - annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), + ingGroup: tt.fields.ingGroup, + defaultTags: tt.fields.defaultTags, + externalManagedTags: tt.fields.externalManagedTags, + annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), } got, err := task.buildLoadBalancerTags(context.Background()) if tt.wantErr != nil { diff --git a/pkg/ingress/model_build_managed_sg.go b/pkg/ingress/model_build_managed_sg.go index 339dcd3ac..76300da54 100644 --- a/pkg/ingress/model_build_managed_sg.go +++ b/pkg/ingress/model_build_managed_sg.go @@ -5,10 +5,13 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "regexp" + awssdk "github.com/aws/aws-sdk-go/aws" "github.com/pkg/errors" - "regexp" + "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" + "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" ec2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/ec2" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" ) @@ -69,19 +72,17 @@ func (t *defaultModelBuildTask) buildManagedSecurityGroupTags(_ context.Context) return nil, err } for tagKey, tagValue := range rawTags { + if t.externalManagedTags.Has(tagKey) { + return nil, errors.Errorf("external managed tag key %v cannot be specified on Ingress %v", + tagKey, k8s.NamespacedName(member.Ing).String()) + } if existingTagValue, exists := annotationTags[tagKey]; exists && existingTagValue != tagValue { return nil, errors.Errorf("conflicting tag %v: %v | %v", tagKey, existingTagValue, tagValue) } annotationTags[tagKey] = tagValue } } - mergedTags := make(map[string]string) - for k, v := range t.defaultTags { - mergedTags[k] = v - } - for k, v := range annotationTags { - mergedTags[k] = v - } + mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags) return mergedTags, nil } diff --git a/pkg/ingress/model_build_managed_sg_test.go b/pkg/ingress/model_build_managed_sg_test.go index a2d85369f..3629c1451 100644 --- a/pkg/ingress/model_build_managed_sg_test.go +++ b/pkg/ingress/model_build_managed_sg_test.go @@ -6,14 +6,16 @@ import ( "github.com/stretchr/testify/assert" networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "testing" ) func Test_defaultModelBuildTask_buildManagedSecurityGroupTags(t *testing.T) { type fields struct { - ingGroup Group - defaultTags map[string]string + ingGroup Group + defaultTags map[string]string + externalManagedTags sets.String } tests := []struct { name string @@ -142,7 +144,7 @@ func Test_defaultModelBuildTask_buildManagedSecurityGroupTags(t *testing.T) { want: map[string]string{ "k1": "v1", "k2": "v2", - "k3": "v3a", + "k3": "v3", "k4": "v4", }, }, @@ -175,13 +177,83 @@ func Test_defaultModelBuildTask_buildManagedSecurityGroupTags(t *testing.T) { }, wantErr: errors.New("conflicting tag k3: v3a | v3b"), }, + { + name: "non empty external managed tags, no conflicts", + fields: fields{ + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k1=v1", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-2", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k2=v2", + }, + }, + }, + }, + }, + }, + externalManagedTags: sets.NewString("k3"), + }, + want: map[string]string{ + "k1": "v1", + "k2": "v2", + }, + }, + { + name: "non empty external managed tags, has conflicts", + fields: fields{ + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k1=v1", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "awesome-ns", + Name: "ing-2", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k2=v2", + }, + }, + }, + }, + }, + }, + externalManagedTags: sets.NewString("k2"), + }, + wantErr: errors.New("external managed tag key k2 cannot be specified on Ingress awesome-ns/ing-2"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { task := &defaultModelBuildTask{ - ingGroup: tt.fields.ingGroup, - defaultTags: tt.fields.defaultTags, - annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), + ingGroup: tt.fields.ingGroup, + defaultTags: tt.fields.defaultTags, + externalManagedTags: tt.fields.externalManagedTags, + annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), } got, err := task.buildManagedSecurityGroupTags(context.Background()) if tt.wantErr != nil { diff --git a/pkg/ingress/model_build_target_group.go b/pkg/ingress/model_build_target_group.go index 7cadcd843..e349ff6d3 100644 --- a/pkg/ingress/model_build_target_group.go +++ b/pkg/ingress/model_build_target_group.go @@ -5,19 +5,20 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "regexp" + "strconv" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" - "regexp" elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" - "strconv" ) const ( @@ -392,13 +393,14 @@ func (t *defaultModelBuildTask) buildTargetGroupTags(_ context.Context, svcAndIn if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &annotationTags, svcAndIngAnnotations); err != nil { return nil, err } - mergedTags := make(map[string]string) - for k, v := range t.defaultTags { - mergedTags[k] = v - } - for k, v := range annotationTags { - mergedTags[k] = v + + for tagKey := range annotationTags { + if t.externalManagedTags.Has(tagKey) { + return nil, errors.Errorf("external managed tag key %v cannot be specified", tagKey) + } } + + mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags) return mergedTags, nil } diff --git a/pkg/ingress/model_build_target_group_test.go b/pkg/ingress/model_build_target_group_test.go index 1e02dc9c6..c35ed74de 100644 --- a/pkg/ingress/model_build_target_group_test.go +++ b/pkg/ingress/model_build_target_group_test.go @@ -10,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" "testing" @@ -196,7 +197,8 @@ func Test_defaultModelBuildTask_buildTargetGroupPort(t *testing.T) { func Test_defaultModelBuildTask_buildTargetGroupTags(t *testing.T) { type fields struct { - defaultTags map[string]string + defaultTags map[string]string + externalManagedTags sets.String } type args struct { svcAndIngAnnotations map[string]string @@ -265,16 +267,44 @@ func Test_defaultModelBuildTask_buildTargetGroupTags(t *testing.T) { want: map[string]string{ "k1": "v1", "k2": "v2", - "k3": "v3a", + "k3": "v3", "k4": "v4", }, }, + { + name: "non empty external managed tags, no conflicts", + fields: fields{ + externalManagedTags: sets.NewString("k3"), + }, + args: args{ + svcAndIngAnnotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k1=v1,k2=v2", + }, + }, + want: map[string]string{ + "k1": "v1", + "k2": "v2", + }, + }, + { + name: "non empty external managed tags, has conflicts", + fields: fields{ + externalManagedTags: sets.NewString("k2"), + }, + args: args{ + svcAndIngAnnotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "k1=v1,k2=v2", + }, + }, + wantErr: errors.New("external managed tag key k2 cannot be specified"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { task := &defaultModelBuildTask{ - defaultTags: tt.fields.defaultTags, - annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), + defaultTags: tt.fields.defaultTags, + externalManagedTags: tt.fields.externalManagedTags, + annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), } got, err := task.buildTargetGroupTags(context.Background(), tt.args.svcAndIngAnnotations) if tt.wantErr != nil { diff --git a/pkg/ingress/model_builder.go b/pkg/ingress/model_builder.go index e0b5832f9..11292ee6d 100644 --- a/pkg/ingress/model_builder.go +++ b/pkg/ingress/model_builder.go @@ -36,7 +36,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR ec2Client services.EC2, acmClient services.ACM, annotationParser annotations.Parser, subnetsResolver networkingpkg.SubnetsResolver, authConfigBuilder AuthConfigBuilder, enhancedBackendBuilder EnhancedBackendBuilder, - vpcID string, clusterName string, defaultTags map[string]string, defaultSSLPolicy string, + vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string, logger logr.Logger) *defaultModelBuilder { certDiscovery := NewACMCertDiscovery(acmClient, logger) ruleOptimizer := NewDefaultRuleOptimizer(logger) @@ -53,6 +53,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR enhancedBackendBuilder: enhancedBackendBuilder, ruleOptimizer: ruleOptimizer, defaultTags: defaultTags, + externalManagedTags: sets.NewString(externalManagedTags...), defaultSSLPolicy: defaultSSLPolicy, logger: logger, } @@ -76,6 +77,7 @@ type defaultModelBuilder struct { enhancedBackendBuilder EnhancedBackendBuilder ruleOptimizer RuleOptimizer defaultTags map[string]string + externalManagedTags sets.String defaultSSLPolicy string logger logr.Logger @@ -102,6 +104,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group) (core.S stack: stack, defaultTags: b.defaultTags, + externalManagedTags: b.externalManagedTags, defaultIPAddressType: elbv2model.IPAddressTypeIPV4, defaultScheme: elbv2model.LoadBalancerSchemeInternal, defaultSSLPolicy: b.defaultSSLPolicy, @@ -147,6 +150,7 @@ type defaultModelBuildTask struct { stack core.Stack defaultTags map[string]string + externalManagedTags sets.String defaultIPAddressType elbv2model.IPAddressType defaultScheme elbv2model.LoadBalancerScheme defaultSSLPolicy string diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index 4a15a9b12..b86338372 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "regexp" + "sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm" "strconv" "github.com/aws/aws-sdk-go/aws" @@ -138,13 +139,13 @@ func (t *defaultModelBuildTask) buildAdditionalResourceTags(_ context.Context) ( if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.SvcLBSuffixAdditionalTags, &annotationTags, t.service.Annotations); err != nil { return nil, err } - mergedTags := make(map[string]string) - for k, v := range t.defaultTags { - mergedTags[k] = v - } - for k, v := range annotationTags { - mergedTags[k] = v + for tagKey := range annotationTags { + if t.externalManagedTags.Has(tagKey) { + return nil, errors.Errorf("external managed tag key %v cannot be specified on Service", tagKey) + } } + + mergedTags := algorithm.MergeStringMap(t.defaultTags, annotationTags) return mergedTags, nil } diff --git a/pkg/service/model_build_load_balancer_test.go b/pkg/service/model_build_load_balancer_test.go index fa440aebe..3a3abe3bb 100644 --- a/pkg/service/model_build_load_balancer_test.go +++ b/pkg/service/model_build_load_balancer_test.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/aws-load-balancer-controller/pkg/networking" "testing" @@ -612,8 +613,9 @@ func Test_defaultModelBuildTask_buildLoadBalancerIPAddressType(t *testing.T) { func Test_defaultModelBuildTask_buildAdditionalResourceTags(t *testing.T) { type fields struct { - service *corev1.Service - defaultTags map[string]string + service *corev1.Service + defaultTags map[string]string + externalManagedTags sets.String } tests := []struct { name string @@ -686,17 +688,50 @@ func Test_defaultModelBuildTask_buildAdditionalResourceTags(t *testing.T) { want: map[string]string{ "k1": "v1", "k2": "v2", - "k3": "v3a", + "k3": "v3", "k4": "v4", }, }, + { + name: "non-empty external tags, non-empty tags annotation - no collision", + fields: fields{ + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags": "k1=v1,k2=v2,k3=v3a", + }, + }, + }, + externalManagedTags: sets.NewString("k4"), + }, + want: map[string]string{ + "k1": "v1", + "k2": "v2", + "k3": "v3a", + }, + }, + { + name: "non-empty external tags, non-empty tags annotation - has collision", + fields: fields{ + service: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags": "k1=v1,k2=v2,k3=v3a", + }, + }, + }, + externalManagedTags: sets.NewString("k3", "k4"), + }, + wantErr: errors.New("external managed tag key k3 cannot be specified on Service"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { task := &defaultModelBuildTask{ - service: tt.fields.service, - defaultTags: tt.fields.defaultTags, - annotationParser: annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io"), + service: tt.fields.service, + defaultTags: tt.fields.defaultTags, + externalManagedTags: tt.fields.externalManagedTags, + annotationParser: annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io"), } got, err := task.buildAdditionalResourceTags(context.Background()) if tt.wantErr != nil { diff --git a/pkg/service/model_builder.go b/pkg/service/model_builder.go index 12df7eff0..d3de2a4c6 100644 --- a/pkg/service/model_builder.go +++ b/pkg/service/model_builder.go @@ -2,6 +2,7 @@ package service import ( "context" + "k8s.io/apimachinery/pkg/util/sets" "strconv" "github.com/aws/aws-sdk-go/service/ec2" @@ -31,7 +32,7 @@ type ModelBuilder interface { // NewDefaultModelBuilder construct a new defaultModelBuilder func NewDefaultModelBuilder(annotationParser annotations.Parser, subnetsResolver networking.SubnetsResolver, vpcResolver networking.VPCResolver, trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager, - clusterName string, defaultTags map[string]string, defaultSSLPolicy string) *defaultModelBuilder { + clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string) *defaultModelBuilder { return &defaultModelBuilder{ annotationParser: annotationParser, subnetsResolver: subnetsResolver, @@ -40,6 +41,7 @@ func NewDefaultModelBuilder(annotationParser annotations.Parser, subnetsResolver elbv2TaggingManager: elbv2TaggingManager, clusterName: clusterName, defaultTags: defaultTags, + externalManagedTags: sets.NewString(externalManagedTags...), defaultSSLPolicy: defaultSSLPolicy, } } @@ -53,9 +55,10 @@ type defaultModelBuilder struct { trackingProvider tracking.Provider elbv2TaggingManager elbv2deploy.TaggingManager - clusterName string - defaultTags map[string]string - defaultSSLPolicy string + clusterName string + defaultTags map[string]string + externalManagedTags sets.String + defaultSSLPolicy string } func (b *defaultModelBuilder) Build(ctx context.Context, service *corev1.Service) (core.Stack, *elbv2model.LoadBalancer, error) { @@ -73,6 +76,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, service *corev1.Service tgByResID: make(map[string]*elbv2model.TargetGroup), defaultTags: b.defaultTags, + externalManagedTags: b.externalManagedTags, defaultSSLPolicy: b.defaultSSLPolicy, defaultAccessLogS3Enabled: false, defaultAccessLogsS3Bucket: "", @@ -119,6 +123,7 @@ type defaultModelBuildTask struct { ec2Subnets []*ec2.Subnet defaultTags map[string]string + externalManagedTags sets.String defaultSSLPolicy string defaultAccessLogS3Enabled bool defaultAccessLogsS3Bucket string diff --git a/pkg/service/model_builder_test.go b/pkg/service/model_builder_test.go index b793af139..0c3dd2a07 100644 --- a/pkg/service/model_builder_test.go +++ b/pkg/service/model_builder_test.go @@ -2153,7 +2153,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { vpcResolver.EXPECT().ResolveCIDRs(gomock.Any()).Return(call.cidrs, call.err).AnyTimes() } builder := NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcResolver, trackingProvider, elbv2TaggingManager, - "my-cluster", nil, "ELBSecurityPolicy-2016-08") + "my-cluster", nil, nil, "ELBSecurityPolicy-2016-08") ctx := context.Background() stack, _, err := builder.Build(ctx, tt.svc) if tt.wantError {