Skip to content

Commit 7dd802c

Browse files
mwhittington21zac-nixon
mwhittington21
and
zac-nixon
authored
add feature gate to allow default tags to be overridden by annotation tags (#4384)
Co-authored-by: Zachary Nixon <nixozach@amazon.com>
1 parent acc95aa commit 7dd802c

25 files changed

+1828
-10
lines changed

‎docs/deploy/configurations.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,3 +189,4 @@ There are a set of key=value pairs that describe AWS load balancer controller fe
189189
| LBCapacityReservation | string | true | Enable or disable the capacity reservation feature on ALB and NLB |
190190
| EnableTCPUDPListenerType | string | false | Enable or disable creation of TCP_UDP type listeners. This value can be overriden at the Service level by the annotation `service.beta.kubernetes.io/aws-load-balancer-enable-tcp-udp-listener` |
191191
| EnhancedDefaultBehavior | string | false | Enable this feature to allow the controller to remove Provisioned Capacity or mTLS settings by removing the corresponding annotation. |
192+
| EnableDefaultTagsLowPriority | string | false | If enabled, tags supplied via `--default-tags` will be overridden by tags specified in other manners, like via annotations. |

‎helm/aws-load-balancer-controller/values.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ controllerConfig:
377377
# LBCapacityReservation: true
378378
# AGAController: true
379379
# EnhancedDefaultBehavior: false
380+
# EnableDefaultTagsLowPriority: false
380381

381382
certDiscovery:
382383
allowedCertificateAuthorityARNs: "" # empty means all CAs are in scope

‎main.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"k8s.io/apimachinery/pkg/util/sets"
2323
"os"
24+
2425
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
2526
"sigs.k8s.io/aws-load-balancer-controller/controllers/gateway"
2627
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
@@ -123,6 +124,7 @@ func main() {
123124
infoLogger.Error(err, "unable to load controller config")
124125
os.Exit(1)
125126
}
127+
126128
appLogger := getLoggerWithLogLevel(controllerCFG.LogLevel)
127129
ctrl.SetLogger(appLogger)
128130
klog.SetLoggerWithOptions(appLogger, klog.ContextualLogger(true))

‎pkg/config/feature_gates.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const (
2929
ALBGatewayAPI Feature = "ALBGatewayAPI"
3030
AGAController Feature = "AGAController"
3131
EnhancedDefaultBehavior Feature = "EnhancedDefaultBehavior"
32+
EnableDefaultTagsLowPriority Feature = "EnableDefaultTagsLowPriority"
3233
)
3334

3435
type FeatureGates interface {
@@ -74,6 +75,7 @@ func NewFeatureGates() FeatureGates {
7475
AGAController: true,
7576
EnableTCPUDPListenerType: false,
7677
EnhancedDefaultBehavior: false,
78+
EnableDefaultTagsLowPriority: false,
7779
},
7880
}
7981
}

‎pkg/gateway/model/base_model_builder.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package model
22

33
import (
44
"context"
5+
56
"k8s.io/apimachinery/pkg/types"
67
"sigs.k8s.io/aws-load-balancer-controller/pkg/addon"
78
config2 "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway"
@@ -42,7 +43,7 @@ func NewModelBuilder(subnetsResolver networking.SubnetsResolver,
4243
backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, enableBackendSG bool,
4344
disableRestrictedSGRules bool, allowedCAARNs []string, supportedAddons []addon.Addon, logger logr.Logger) Builder {
4445

45-
gwTagHelper := newTagHelper(sets.New(lbcConfig.ExternalManagedTags...), lbcConfig.DefaultTags)
46+
gwTagHelper := newTagHelper(sets.New(lbcConfig.ExternalManagedTags...), lbcConfig.DefaultTags, featureGates.Enabled(config.EnableDefaultTagsLowPriority))
4647
subnetBuilder := newSubnetModelBuilder(loadBalancerType, trackingProvider, subnetsResolver, elbv2TaggingManager)
4748
sgBuilder := newSecurityGroupBuilder(gwTagHelper, clusterName, enableBackendSG, sgResolver, backendSGProvider, logger)
4849
lbBuilder := newLoadBalancerBuilder(loadBalancerType, gwTagHelper, clusterName)

‎pkg/gateway/model/gateway_tag_helper.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ type tagHelper interface {
1212
}
1313

1414
type tagHelperImpl struct {
15-
externalManagedTags sets.Set[string]
16-
defaultTags map[string]string
15+
externalManagedTags sets.Set[string]
16+
defaultTags map[string]string
17+
additionalTagsOverrideDefaultTags bool
1718
}
1819

19-
func newTagHelper(externalManagedTags sets.Set[string], defaultTags map[string]string) tagHelper {
20+
func newTagHelper(externalManagedTags sets.Set[string], defaultTags map[string]string, additionalTagsOverrideDefaultTags bool) tagHelper {
2021
return &tagHelperImpl{
21-
externalManagedTags: externalManagedTags,
22-
defaultTags: defaultTags,
22+
externalManagedTags: externalManagedTags,
23+
defaultTags: defaultTags,
24+
additionalTagsOverrideDefaultTags: additionalTagsOverrideDefaultTags,
2325
}
2426
}
2527

@@ -36,6 +38,9 @@ func (t *tagHelperImpl) getGatewayTags(lbConf elbv2gw.LoadBalancerConfiguration)
3638
return nil, err
3739
}
3840

41+
if t.additionalTagsOverrideDefaultTags {
42+
return algorithm.MergeStringMap(annotationTags, t.defaultTags), nil
43+
}
3944
return algorithm.MergeStringMap(t.defaultTags, annotationTags), nil
4045
}
4146

‎pkg/gateway/model/gateway_tag_helper_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package model
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"k8s.io/apimachinery/pkg/util/sets"
8+
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
9+
)
10+
11+
func Test_tagHelperImpl_getGatewayTags(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
defaultTags map[string]string
15+
specTags map[string]string
16+
defaultTagsLowPriority bool
17+
want map[string]string
18+
wantErr bool
19+
}{
20+
{
21+
name: "when defaultTagsLowPriority is false, default tags override spec tags",
22+
defaultTags: map[string]string{
23+
"env": "prod",
24+
"team": "platform",
25+
},
26+
specTags: map[string]string{
27+
"env": "dev",
28+
"app": "web",
29+
},
30+
defaultTagsLowPriority: false,
31+
want: map[string]string{
32+
"env": "prod",
33+
"team": "platform",
34+
"app": "web",
35+
},
36+
},
37+
{
38+
name: "when defaultTagsLowPriority is true, spec tags override default tags",
39+
defaultTags: map[string]string{
40+
"env": "prod",
41+
"team": "platform",
42+
},
43+
specTags: map[string]string{
44+
"env": "dev",
45+
"app": "web",
46+
},
47+
defaultTagsLowPriority: true,
48+
want: map[string]string{
49+
"env": "dev",
50+
"team": "platform",
51+
"app": "web",
52+
},
53+
},
54+
{
55+
name: "when no overlapping tags, order doesn't matter",
56+
defaultTags: map[string]string{
57+
"team": "platform",
58+
"cost-center": "123",
59+
},
60+
specTags: map[string]string{
61+
"app": "web",
62+
"env": "dev",
63+
},
64+
defaultTagsLowPriority: false,
65+
want: map[string]string{
66+
"team": "platform",
67+
"cost-center": "123",
68+
"app": "web",
69+
"env": "dev",
70+
},
71+
},
72+
{
73+
name: "when defaultTags is empty, all spec tags are used",
74+
defaultTags: map[string]string{},
75+
specTags: map[string]string{
76+
"app": "web",
77+
"env": "dev",
78+
},
79+
defaultTagsLowPriority: false,
80+
want: map[string]string{
81+
"app": "web",
82+
"env": "dev",
83+
},
84+
},
85+
{
86+
name: "when specTags is empty, all default tags are used",
87+
defaultTags: map[string]string{
88+
"team": "platform",
89+
"cost-center": "123",
90+
},
91+
specTags: map[string]string{},
92+
defaultTagsLowPriority: false,
93+
want: map[string]string{
94+
"team": "platform",
95+
"cost-center": "123",
96+
},
97+
},
98+
{
99+
name: "when specTags contains external managed tag, returns error",
100+
defaultTags: map[string]string{
101+
"team": "platform",
102+
},
103+
specTags: map[string]string{
104+
"external-tag": "value",
105+
},
106+
defaultTagsLowPriority: false,
107+
want: nil,
108+
wantErr: true,
109+
},
110+
}
111+
112+
for _, tt := range tests {
113+
t.Run(tt.name, func(t *testing.T) {
114+
h := &tagHelperImpl{
115+
externalManagedTags: sets.New("external-tag"),
116+
defaultTags: tt.defaultTags,
117+
additionalTagsOverrideDefaultTags: tt.defaultTagsLowPriority,
118+
}
119+
120+
lbConf := &elbv2gw.LoadBalancerConfiguration{}
121+
if len(tt.specTags) > 0 {
122+
lbConf.Spec.Tags = &tt.specTags
123+
}
124+
125+
got, err := h.getGatewayTags(*lbConf)
126+
if tt.wantErr {
127+
assert.Error(t, err)
128+
} else {
129+
assert.NoError(t, err)
130+
assert.Equal(t, tt.want, got)
131+
}
132+
})
133+
}
134+
}

‎pkg/ingress/model_build_frontend_nlb_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"k8s.io/apimachinery/pkg/util/intstr"
1616
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
1717
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
18+
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
1819
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
1920
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
2021
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
@@ -987,7 +988,8 @@ func Test_buildFrontendNlbTags(t *testing.T) {
987988
ingGroup: tt.ingGroup,
988989
annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"),
989990
// Default implementation will return an empty map when no tags are specified
990-
defaultTags: tt.defaultTags,
991+
defaultTags: tt.defaultTags,
992+
featureGates: config.NewFeatureGates(),
991993
}
992994

993995
got, err := task.buildFrontendNlbTags(context.Background(), nil)

‎pkg/ingress/model_build_listener.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"k8s.io/apimachinery/pkg/util/sets"
1919
"sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm"
2020
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
21+
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
2122
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
2223
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
2324
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
@@ -102,6 +103,9 @@ func (t *defaultModelBuildTask) buildListenerTags(_ context.Context, ingList []C
102103
if err != nil {
103104
return nil, err
104105
}
106+
if t.featureGates.Enabled(config.EnableDefaultTagsLowPriority) {
107+
return algorithm.MergeStringMap(ingGroupTags, t.defaultTags), nil
108+
}
105109
return algorithm.MergeStringMap(t.defaultTags, ingGroupTags), nil
106110
}
107111

‎pkg/ingress/model_build_listener_rules.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/pkg/errors"
1111
networking "k8s.io/api/networking/v1"
1212
"sigs.k8s.io/aws-load-balancer-controller/pkg/algorithm"
13+
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
1314
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
1415
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
1516
elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
@@ -385,6 +386,9 @@ func (t *defaultModelBuildTask) buildListenerRuleTags(_ context.Context, ing Cla
385386
return nil, err
386387
}
387388

389+
if t.featureGates.Enabled(config.EnableDefaultTagsLowPriority) {
390+
return algorithm.MergeStringMap(ingTags, t.defaultTags), nil
391+
}
388392
return algorithm.MergeStringMap(t.defaultTags, ingTags), nil
389393
}
390394

0 commit comments

Comments
 (0)