From c61b20fc51afdaa32429d13861a2af4ad31233ff Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 16 Sep 2025 18:43:30 +1000 Subject: [PATCH 01/12] refactor(service): move attribute slice func to shared utils --- pkg/ingress/model_build_frontend_nlb.go | 20 ++++--- pkg/service/model_build_load_balancer.go | 20 +------ pkg/service/model_build_target_group.go | 2 + pkg/shared_utils/attribute_utils_test.go | 70 ++++++++++++++++++++++++ pkg/shared_utils/model_build_utils.go | 20 +++++++ 5 files changed, 108 insertions(+), 24 deletions(-) create mode 100644 pkg/shared_utils/attribute_utils_test.go create mode 100644 pkg/shared_utils/model_build_utils.go diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index 1ef2a9aba..f312cfdfa 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -238,14 +238,20 @@ func (t *defaultModelBuildTask) buildFrontendNlbSpec(ctx context.Context, scheme return elbv2model.LoadBalancerSpec{}, err } + lbAttributes, err := t.buildFrontendNlbAttributes(ctx) + if err != nil { + return elbv2model.LoadBalancerSpec{}, err + } + spec := elbv2model.LoadBalancerSpec{ - Name: name, - Type: elbv2model.LoadBalancerTypeNetwork, - Scheme: scheme, - IPAddressType: alb.Spec.IPAddressType, - SecurityGroups: securityGroups, - SubnetMappings: subnetMappings, - Tags: tags, + Name: name, + Type: elbv2model.LoadBalancerTypeNetwork, + Scheme: scheme, + IPAddressType: alb.Spec.IPAddressType, + LoadBalancerAttributes: lbAttributes, + SecurityGroups: securityGroups, + SubnetMappings: subnetMappings, + Tags: tags, } return spec, nil diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index af2ea703a..454b607e5 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -7,8 +7,6 @@ import ( "fmt" "net/netip" "regexp" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" - "sort" "strconv" awssdk "github.com/aws/aws-sdk-go-v2/aws" @@ -24,6 +22,8 @@ import ( "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" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_utils" ) const ( @@ -497,7 +497,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerAttributes(_ context.Context) ( return []elbv2model.LoadBalancerAttribute{}, err } mergedAttributes := algorithm.MergeStringMap(specificAttributes, loadBalancerAttributes) - return makeAttributesSliceFromMap(mergedAttributes), nil + return shared_utils.MakeAttributesSliceFromMap(mergedAttributes), nil } func (t *defaultModelBuildTask) buildLoadBalancerMinimumCapacity(_ context.Context) (*elbv2model.MinimumLoadBalancerCapacity, error) { @@ -527,20 +527,6 @@ func (t *defaultModelBuildTask) buildLoadBalancerMinimumCapacity(_ context.Conte return minimumLoadBalancerCapacity, nil } -func makeAttributesSliceFromMap(loadBalancerAttributesMap map[string]string) []elbv2model.LoadBalancerAttribute { - attributes := make([]elbv2model.LoadBalancerAttribute, 0, len(loadBalancerAttributesMap)) - for attrKey, attrValue := range loadBalancerAttributesMap { - attributes = append(attributes, elbv2model.LoadBalancerAttribute{ - Key: attrKey, - Value: attrValue, - }) - } - sort.Slice(attributes, func(i, j int) bool { - return attributes[i].Key < attributes[j].Key - }) - return attributes -} - func (t *defaultModelBuildTask) getLoadBalancerAttributes() (map[string]string, error) { var attributes map[string]string if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.SvcLBSuffixLoadBalancerAttributes, &attributes, t.service.Annotations); err != nil { diff --git a/pkg/service/model_build_target_group.go b/pkg/service/model_build_target_group.go index bb330493a..95272229e 100644 --- a/pkg/service/model_build_target_group.go +++ b/pkg/service/model_build_target_group.go @@ -10,6 +10,8 @@ import ( "strconv" "strings" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" + awssdk "github.com/aws/aws-sdk-go-v2/aws" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" diff --git a/pkg/shared_utils/attribute_utils_test.go b/pkg/shared_utils/attribute_utils_test.go new file mode 100644 index 000000000..6f362202e --- /dev/null +++ b/pkg/shared_utils/attribute_utils_test.go @@ -0,0 +1,70 @@ +package shared_utils + +import ( + "testing" + + "github.com/stretchr/testify/assert" + elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" +) + +func TestMakeLoadBalancerAttributeSliceFromMap(t *testing.T) { + tests := []struct { + name string + attributeMap map[string]string + expected []elbv2model.LoadBalancerAttribute + }{ + { + name: "empty map", + attributeMap: map[string]string{}, + expected: []elbv2model.LoadBalancerAttribute{}, + }, + { + name: "single attribute", + attributeMap: map[string]string{"key1": "value1"}, + expected: []elbv2model.LoadBalancerAttribute{{Key: "key1", Value: "value1"}}, + }, + { + name: "multiple attributes", + attributeMap: map[string]string{"key2": "value2", "key1": "value1", "key3": "value3"}, + expected: []elbv2model.LoadBalancerAttribute{ + {Key: "key1", Value: "value1"}, + {Key: "key2", Value: "value2"}, + {Key: "key3", Value: "value3"}, + }, + }, + { + name: "attributes with special characters", + attributeMap: map[string]string{"key-with-dash": "value with spaces", "key_with_underscore": "value=with=equals"}, + expected: []elbv2model.LoadBalancerAttribute{ + {Key: "key-with-dash", Value: "value with spaces"}, + {Key: "key_with_underscore", Value: "value=with=equals"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := MakeLoadBalancerAttributeSliceFromMap(tt.attributeMap) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMakeLoadBalancerAttributeSliceFromMap_Sorting(t *testing.T) { + // Test that attributes are properly sorted by key + attributeMap := map[string]string{ + "zebra": "last", + "apple": "first", + "banana": "middle", + } + + result := MakeLoadBalancerAttributeSliceFromMap(attributeMap) + + expected := []elbv2model.LoadBalancerAttribute{ + {Key: "apple", Value: "first"}, + {Key: "banana", Value: "middle"}, + {Key: "zebra", Value: "last"}, + } + + assert.Equal(t, expected, result) +} diff --git a/pkg/shared_utils/model_build_utils.go b/pkg/shared_utils/model_build_utils.go new file mode 100644 index 000000000..8adc57e60 --- /dev/null +++ b/pkg/shared_utils/model_build_utils.go @@ -0,0 +1,20 @@ +package shared_utils + +import ( + "sort" + elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" +) + +func MakeAttributesSliceFromMap(loadBalancerAttributesMap map[string]string) []elbv2model.LoadBalancerAttribute { + attributes := make([]elbv2model.LoadBalancerAttribute, 0, len(loadBalancerAttributesMap)) + for attrKey, attrValue := range loadBalancerAttributesMap { + attributes = append(attributes, elbv2model.LoadBalancerAttribute{ + Key: attrKey, + Value: attrValue, + }) + } + sort.Slice(attributes, func(i, j int) bool { + return attributes[i].Key < attributes[j].Key + }) + return attributes +} From d4d84282eacfabbf40599fe09d34482c89392d22 Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 16 Sep 2025 19:25:50 +1000 Subject: [PATCH 02/12] refactor(nlb): move constants to shared constants file --- pkg/ingress/model_build_frontend_nlb.go | 40 +++++++++++++++++++ pkg/service/model_build_load_balancer.go | 27 +++++-------- pkg/service/model_build_load_balancer_test.go | 28 ++++++------- pkg/shared_constants/attributes.go | 10 ++++- .../{model_build_utils.go => nlb_utils.go} | 0 5 files changed, 73 insertions(+), 32 deletions(-) rename pkg/shared_utils/{model_build_utils.go => nlb_utils.go} (100%) diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index f312cfdfa..dabc46205 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -8,6 +8,7 @@ import ( "strconv" "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_utils" awssdk "github.com/aws/aws-sdk-go-v2/aws" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" @@ -211,6 +212,45 @@ func (t *defaultModelBuildTask) buildFrontendNlb(ctx context.Context, scheme elb return nil } +func (t *defaultModelBuildTask) buildFrontendNlbAttributes(ctx context.Context) ([]elbv2model.LoadBalancerAttribute, error) { + loadBalancerAttributes, err := t.getFrontendNlbAttributes() + if err != nil { + return []elbv2model.LoadBalancerAttribute{}, err + } + return shared_utils.MakeAttributesSliceFromMap(loadBalancerAttributes), nil +} + +func (t *defaultModelBuildTask) getFrontendNlbAttributes() (any, any) { + var chosenAttributes map[string]string + for _, member := range t.ingGroup.Members { + var attributes map[string]string + if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.SvcLBSuffixLoadBalancerAttributes, &attributes, member.Ing.Annotations); err != nil { + return nil, err + } + if chosenAttributes == nil { + chosenAttributes = attributes + } else { + if !cmp.Equal(chosenAttributes, attributes) { + return nil, errors.Errorf("conflicting frontend NLB attributes: %v | %v", chosenAttributes, attributes) + } + } + } + + dnsRecordClientRoutingPolicy, exists := chosenAttributes[lbAttrsLoadBalancingDnsClientRoutingPolicy] + if exists { + switch dnsRecordClientRoutingPolicy { + case availabilityZoneAffinity: + case partialAvailabilityZoneAffinity: + case anyAvailabilityZone: + default: + return nil, errors.Errorf("invalid dns_record.client_routing_policy set in annotation %s: got '%s' expected one of ['%s', '%s', '%s']", + annotations.SvcLBSuffixLoadBalancerAttributes, dnsRecordClientRoutingPolicy, + anyAvailabilityZone, partialAvailabilityZoneAffinity, availabilityZoneAffinity) + } + } + return chosenAttributes, nil +} + func (t *defaultModelBuildTask) buildFrontendNlbSpec(ctx context.Context, scheme elbv2model.LoadBalancerScheme, alb *elbv2model.LoadBalancer) (elbv2model.LoadBalancerSpec, error) { securityGroups, err := t.buildFrontendNlbSecurityGroups(ctx) diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index 454b607e5..b6ba37ed9 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -27,14 +27,7 @@ import ( ) const ( - lbAttrsAccessLogsS3Enabled = "access_logs.s3.enabled" - lbAttrsAccessLogsS3Bucket = "access_logs.s3.bucket" - lbAttrsAccessLogsS3Prefix = "access_logs.s3.prefix" - lbAttrsLoadBalancingCrossZoneEnabled = "load_balancing.cross_zone.enabled" - lbAttrsLoadBalancingDnsClientRoutingPolicy = "dns_record.client_routing_policy" - availabilityZoneAffinity = "availability_zone_affinity" - partialAvailabilityZoneAffinity = "partial_availability_zone_affinity" - anyAvailabilityZone = "any_availability_zone" + resourceIDLoadBalancer = "LoadBalancer" ) func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error { @@ -532,16 +525,16 @@ func (t *defaultModelBuildTask) getLoadBalancerAttributes() (map[string]string, if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.SvcLBSuffixLoadBalancerAttributes, &attributes, t.service.Annotations); err != nil { return nil, err } - dnsRecordClientRoutingPolicy, exists := attributes[lbAttrsLoadBalancingDnsClientRoutingPolicy] + dnsRecordClientRoutingPolicy, exists := attributes[shared_constants.LBAttributeLoadBalancingDnsClientRoutingPolicy] if exists { switch dnsRecordClientRoutingPolicy { - case availabilityZoneAffinity: - case partialAvailabilityZoneAffinity: - case anyAvailabilityZone: + case shared_constants.LBAttributeAvailabilityZoneAffinity: + case shared_constants.LBAttributePartialAvailabilityZoneAffinity: + case shared_constants.LBAttributeAnyAvailabilityZone: default: return nil, errors.Errorf("invalid dns_record.client_routing_policy set in annotation %s: got '%s' expected one of ['%s', '%s', '%s']", annotations.SvcLBSuffixLoadBalancerAttributes, dnsRecordClientRoutingPolicy, - anyAvailabilityZone, partialAvailabilityZoneAffinity, availabilityZoneAffinity) + shared_constants.LBAttributeAnyAvailabilityZone, shared_constants.LBAttributePartialAvailabilityZoneAffinity, shared_constants.LBAttributeAvailabilityZoneAffinity) } } return attributes, nil @@ -559,12 +552,12 @@ func (t *defaultModelBuildTask) getAnnotationSpecificLbAttributes() (map[string] return nil, err } if exists && accessLogEnabled { - annotationSpecificAttrs[lbAttrsAccessLogsS3Enabled] = strconv.FormatBool(accessLogEnabled) + annotationSpecificAttrs[shared_constants.LBAttributeAccessLogsS3Enabled] = strconv.FormatBool(accessLogEnabled) if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixAccessLogS3BucketName, &bucketName, t.service.Annotations); exists { - annotationSpecificAttrs[lbAttrsAccessLogsS3Bucket] = bucketName + annotationSpecificAttrs[shared_constants.LBAttributeAccessLogsS3Bucket] = bucketName } if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixAccessLogS3BucketPrefix, &bucketPrefix, t.service.Annotations); exists { - annotationSpecificAttrs[lbAttrsAccessLogsS3Prefix] = bucketPrefix + annotationSpecificAttrs[shared_constants.LBAttributeAccessLogsS3Prefix] = bucketPrefix } } exists, err = t.annotationParser.ParseBoolAnnotation(annotations.SvcLBSuffixCrossZoneLoadBalancingEnabled, &crossZoneEnabled, t.service.Annotations) @@ -572,7 +565,7 @@ func (t *defaultModelBuildTask) getAnnotationSpecificLbAttributes() (map[string] return nil, err } if exists { - annotationSpecificAttrs[lbAttrsLoadBalancingCrossZoneEnabled] = strconv.FormatBool(crossZoneEnabled) + annotationSpecificAttrs[shared_constants.LBAttributeLoadBalancingCrossZoneEnabled] = strconv.FormatBool(crossZoneEnabled) } return annotationSpecificAttrs, nil } diff --git a/pkg/service/model_build_load_balancer_test.go b/pkg/service/model_build_load_balancer_test.go index 022182602..2271e1807 100644 --- a/pkg/service/model_build_load_balancer_test.go +++ b/pkg/service/model_build_load_balancer_test.go @@ -61,19 +61,19 @@ func Test_defaultModelBuilderTask_buildLBAttributes(t *testing.T) { wantError: false, wantValue: []elbv2.LoadBalancerAttribute{ { - Key: lbAttrsAccessLogsS3Enabled, + Key: shared_constants.LBAttributeAccessLogsS3Enabled, Value: "true", }, { - Key: lbAttrsAccessLogsS3Bucket, + Key: shared_constants.LBAttributeAccessLogsS3Bucket, Value: "nlb-bucket", }, { - Key: lbAttrsAccessLogsS3Prefix, + Key: shared_constants.LBAttributeAccessLogsS3Prefix, Value: "bkt-pfx", }, { - Key: lbAttrsLoadBalancingCrossZoneEnabled, + Key: shared_constants.LBAttributeLoadBalancingCrossZoneEnabled, Value: "true", }, { @@ -95,19 +95,19 @@ func Test_defaultModelBuilderTask_buildLBAttributes(t *testing.T) { wantError: false, wantValue: []elbv2.LoadBalancerAttribute{ { - Key: lbAttrsAccessLogsS3Enabled, + Key: shared_constants.LBAttributeAccessLogsS3Enabled, Value: "true", }, { - Key: lbAttrsAccessLogsS3Bucket, + Key: shared_constants.LBAttributeAccessLogsS3Bucket, Value: "nlb-bucket", }, { - Key: lbAttrsAccessLogsS3Prefix, + Key: shared_constants.LBAttributeAccessLogsS3Prefix, Value: "bkt-pfx", }, { - Key: lbAttrsLoadBalancingCrossZoneEnabled, + Key: shared_constants.LBAttributeLoadBalancingCrossZoneEnabled, Value: "true", }, { @@ -115,8 +115,8 @@ func Test_defaultModelBuilderTask_buildLBAttributes(t *testing.T) { Value: "true", }, { - Key: lbAttrsLoadBalancingDnsClientRoutingPolicy, - Value: availabilityZoneAffinity, + Key: shared_constants.LBAttributeLoadBalancingDnsClientRoutingPolicy, + Value: shared_constants.LBAttributeAvailabilityZoneAffinity, }, }, }, @@ -137,19 +137,19 @@ func Test_defaultModelBuilderTask_buildLBAttributes(t *testing.T) { wantError: false, wantValue: []elbv2.LoadBalancerAttribute{ { - Key: lbAttrsAccessLogsS3Enabled, + Key: shared_constants.LBAttributeAccessLogsS3Enabled, Value: "true", }, { - Key: lbAttrsAccessLogsS3Bucket, + Key: shared_constants.LBAttributeAccessLogsS3Bucket, Value: "overridden-nlb-bucket", }, { - Key: lbAttrsAccessLogsS3Prefix, + Key: shared_constants.LBAttributeAccessLogsS3Prefix, Value: "overridden-bkt-pfx", }, { - Key: lbAttrsLoadBalancingCrossZoneEnabled, + Key: shared_constants.LBAttributeLoadBalancingCrossZoneEnabled, Value: "false", }, }, diff --git a/pkg/shared_constants/attributes.go b/pkg/shared_constants/attributes.go index ffcc177d3..c21665437 100644 --- a/pkg/shared_constants/attributes.go +++ b/pkg/shared_constants/attributes.go @@ -2,7 +2,15 @@ package shared_constants const ( // LBAttributeDeletionProtection deletion protection attribute name - LBAttributeDeletionProtection = "deletion_protection.enabled" + LBAttributeDeletionProtection = "deletion_protection.enabled" + LBAttributeAccessLogsS3Enabled = "access_logs.s3.enabled" + LBAttributeAccessLogsS3Bucket = "access_logs.s3.bucket" + LBAttributeAccessLogsS3Prefix = "access_logs.s3.prefix" + LBAttributeLoadBalancingCrossZoneEnabled = "load_balancing.cross_zone.enabled" + LBAttributeLoadBalancingDnsClientRoutingPolicy = "dns_record.client_routing_policy" + LBAttributeAvailabilityZoneAffinity = "availability_zone_affinity" + LBAttributePartialAvailabilityZoneAffinity = "partial_availability_zone_affinity" + LBAttributeAnyAvailabilityZone = "any_availability_zone" ) const ( diff --git a/pkg/shared_utils/model_build_utils.go b/pkg/shared_utils/nlb_utils.go similarity index 100% rename from pkg/shared_utils/model_build_utils.go rename to pkg/shared_utils/nlb_utils.go From 9a218d9cc020abaeacf7020f57505b3a1ae1a666 Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 30 Sep 2025 17:48:21 +1000 Subject: [PATCH 03/12] fix(ingress): correct types for getFrontendNlbAttributes --- pkg/ingress/model_build_frontend_nlb.go | 12 ++-- pkg/shared_utils/attribute_utils_test.go | 70 ------------------------ 2 files changed, 6 insertions(+), 76 deletions(-) delete mode 100644 pkg/shared_utils/attribute_utils_test.go diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index dabc46205..145931f1e 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -220,7 +220,7 @@ func (t *defaultModelBuildTask) buildFrontendNlbAttributes(ctx context.Context) return shared_utils.MakeAttributesSliceFromMap(loadBalancerAttributes), nil } -func (t *defaultModelBuildTask) getFrontendNlbAttributes() (any, any) { +func (t *defaultModelBuildTask) getFrontendNlbAttributes() (map[string]string, error) { var chosenAttributes map[string]string for _, member := range t.ingGroup.Members { var attributes map[string]string @@ -236,16 +236,16 @@ func (t *defaultModelBuildTask) getFrontendNlbAttributes() (any, any) { } } - dnsRecordClientRoutingPolicy, exists := chosenAttributes[lbAttrsLoadBalancingDnsClientRoutingPolicy] + dnsRecordClientRoutingPolicy, exists := chosenAttributes[shared_constants.LBAttributeLoadBalancingDnsClientRoutingPolicy] if exists { switch dnsRecordClientRoutingPolicy { - case availabilityZoneAffinity: - case partialAvailabilityZoneAffinity: - case anyAvailabilityZone: + case shared_constants.LBAttributeAvailabilityZoneAffinity: + case shared_constants.LBAttributePartialAvailabilityZoneAffinity: + case shared_constants.LBAttributeAnyAvailabilityZone: default: return nil, errors.Errorf("invalid dns_record.client_routing_policy set in annotation %s: got '%s' expected one of ['%s', '%s', '%s']", annotations.SvcLBSuffixLoadBalancerAttributes, dnsRecordClientRoutingPolicy, - anyAvailabilityZone, partialAvailabilityZoneAffinity, availabilityZoneAffinity) + shared_constants.LBAttributeAnyAvailabilityZone, shared_constants.LBAttributePartialAvailabilityZoneAffinity, shared_constants.LBAttributeAvailabilityZoneAffinity) } } return chosenAttributes, nil diff --git a/pkg/shared_utils/attribute_utils_test.go b/pkg/shared_utils/attribute_utils_test.go deleted file mode 100644 index 6f362202e..000000000 --- a/pkg/shared_utils/attribute_utils_test.go +++ /dev/null @@ -1,70 +0,0 @@ -package shared_utils - -import ( - "testing" - - "github.com/stretchr/testify/assert" - elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" -) - -func TestMakeLoadBalancerAttributeSliceFromMap(t *testing.T) { - tests := []struct { - name string - attributeMap map[string]string - expected []elbv2model.LoadBalancerAttribute - }{ - { - name: "empty map", - attributeMap: map[string]string{}, - expected: []elbv2model.LoadBalancerAttribute{}, - }, - { - name: "single attribute", - attributeMap: map[string]string{"key1": "value1"}, - expected: []elbv2model.LoadBalancerAttribute{{Key: "key1", Value: "value1"}}, - }, - { - name: "multiple attributes", - attributeMap: map[string]string{"key2": "value2", "key1": "value1", "key3": "value3"}, - expected: []elbv2model.LoadBalancerAttribute{ - {Key: "key1", Value: "value1"}, - {Key: "key2", Value: "value2"}, - {Key: "key3", Value: "value3"}, - }, - }, - { - name: "attributes with special characters", - attributeMap: map[string]string{"key-with-dash": "value with spaces", "key_with_underscore": "value=with=equals"}, - expected: []elbv2model.LoadBalancerAttribute{ - {Key: "key-with-dash", Value: "value with spaces"}, - {Key: "key_with_underscore", Value: "value=with=equals"}, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := MakeLoadBalancerAttributeSliceFromMap(tt.attributeMap) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestMakeLoadBalancerAttributeSliceFromMap_Sorting(t *testing.T) { - // Test that attributes are properly sorted by key - attributeMap := map[string]string{ - "zebra": "last", - "apple": "first", - "banana": "middle", - } - - result := MakeLoadBalancerAttributeSliceFromMap(attributeMap) - - expected := []elbv2model.LoadBalancerAttribute{ - {Key: "apple", Value: "first"}, - {Key: "banana", Value: "middle"}, - {Key: "zebra", Value: "last"}, - } - - assert.Equal(t, expected, result) -} From e5b68b7e25c6340e82cffdbc01a8045814aeacbd Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 30 Sep 2025 18:03:38 +1000 Subject: [PATCH 04/12] fix(ingress): existing tests pass --- pkg/ingress/model_build_frontend_nlb.go | 6 +++--- pkg/service/model_build_target_group.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index 145931f1e..de014d033 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -212,7 +212,7 @@ func (t *defaultModelBuildTask) buildFrontendNlb(ctx context.Context, scheme elb return nil } -func (t *defaultModelBuildTask) buildFrontendNlbAttributes(ctx context.Context) ([]elbv2model.LoadBalancerAttribute, error) { +func (t *defaultModelBuildTask) buildFrontendNlbAttributes() ([]elbv2model.LoadBalancerAttribute, error) { loadBalancerAttributes, err := t.getFrontendNlbAttributes() if err != nil { return []elbv2model.LoadBalancerAttribute{}, err @@ -278,7 +278,7 @@ func (t *defaultModelBuildTask) buildFrontendNlbSpec(ctx context.Context, scheme return elbv2model.LoadBalancerSpec{}, err } - lbAttributes, err := t.buildFrontendNlbAttributes(ctx) + lbAttributes, err := t.buildFrontendNlbAttributes() if err != nil { return elbv2model.LoadBalancerSpec{}, err } @@ -854,7 +854,7 @@ func buildFrontendNlbResourceID(resourceType string, protocol elbv2model.Protoco if port != nil && protocol != "" { return fmt.Sprintf("FrontendNlb-%s-%v-%v", resourceType, protocol, *port) } - return fmt.Sprintf("FrontendNlb") + return "FrontendNlb" } func mergeHealthCheckField[T comparable](fieldName string, finalValue **T, currentValue *T, explicit map[string]bool, explicitFields map[string]bool, configIndex int) error { diff --git a/pkg/service/model_build_target_group.go b/pkg/service/model_build_target_group.go index 95272229e..4b9fe633d 100644 --- a/pkg/service/model_build_target_group.go +++ b/pkg/service/model_build_target_group.go @@ -25,7 +25,6 @@ import ( elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" elbv2modelk8s "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2/k8s" "sigs.k8s.io/aws-load-balancer-controller/pkg/networking" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" ) func (t *defaultModelBuildTask) buildTargetGroup(ctx context.Context, port corev1.ServicePort, tgProtocol elbv2model.Protocol, scheme elbv2model.LoadBalancerScheme) (*elbv2model.TargetGroup, error) { From 9ed727d808935fd3aea1cdad17cce35431f1897c Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 30 Sep 2025 18:16:02 +1000 Subject: [PATCH 05/12] test(ingress): first draft for all attributes --- pkg/ingress/model_build_frontend_nlb_test.go | 356 +++++++++++++++++++ pkg/service/model_build_load_balancer.go | 1 + 2 files changed, 357 insertions(+) diff --git a/pkg/ingress/model_build_frontend_nlb_test.go b/pkg/ingress/model_build_frontend_nlb_test.go index c7bde65ae..3baf7ca66 100644 --- a/pkg/ingress/model_build_frontend_nlb_test.go +++ b/pkg/ingress/model_build_frontend_nlb_test.go @@ -2,6 +2,7 @@ package ingress import ( "context" + "sort" "testing" awssdk "github.com/aws/aws-sdk-go-v2/aws" @@ -1347,3 +1348,358 @@ func Test_defaultModelBuildTask_buildFrontendNlbListeners(t *testing.T) { }) } } + +func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { + tests := []struct { + name string + ingGroup Group + wantAttributes map[string]string + wantErr bool + expectedErrMsg string + }{ + { + name: "no attributes specified", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + }, + }, + }, + }, + }, + wantAttributes: nil, + wantErr: false, + }, + { + name: "valid DNS client routing policy - availability_zone_affinity", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + }, + }, + }, + }, + }, + }, + wantAttributes: map[string]string{ + "dns_record.client_routing_policy": "availability_zone_affinity", + }, + wantErr: false, + }, + { + name: "valid DNS client routing policy - partial_availability_zone_affinity", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=partial_availability_zone_affinity", + }, + }, + }, + }, + }, + }, + wantAttributes: map[string]string{ + "dns_record.client_routing_policy": "partial_availability_zone_affinity", + }, + wantErr: false, + }, + { + name: "valid DNS client routing policy - any_availability_zone", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=any_availability_zone", + }, + }, + }, + }, + }, + }, + wantAttributes: map[string]string{ + "dns_record.client_routing_policy": "any_availability_zone", + }, + wantErr: false, + }, + { + name: "invalid DNS client routing policy", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=invalid_policy", + }, + }, + }, + }, + }, + }, + wantAttributes: nil, + wantErr: true, + expectedErrMsg: "invalid dns_record.client_routing_policy set in annotation", + }, + { + name: "multiple attributes with valid DNS policy", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", + }, + }, + }, + }, + }, + }, + wantAttributes: map[string]string{ + "dns_record.client_routing_policy": "availability_zone_affinity", + "cross_zone.enabled": "true", + }, + wantErr: false, + }, + { + name: "consistent attributes across multiple ingresses", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress2", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + }, + }, + }, + }, + }, + }, + wantAttributes: map[string]string{ + "dns_record.client_routing_policy": "availability_zone_affinity", + }, + wantErr: false, + }, + { + name: "conflicting attributes across multiple ingresses", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress2", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=any_availability_zone", + }, + }, + }, + }, + }, + }, + wantAttributes: nil, + wantErr: true, + expectedErrMsg: "conflicting frontend NLB attributes", + }, + { + name: "mixed ingresses - some with attributes, some without", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress2", + Namespace: "default", + }, + }, + }, + }, + }, + wantAttributes: nil, + wantErr: true, + expectedErrMsg: "conflicting frontend NLB attributes", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io") + task := &defaultModelBuildTask{ + annotationParser: parser, + ingGroup: tt.ingGroup, + } + + gotAttributes, err := task.getFrontendNlbAttributes() + + if tt.wantErr { + assert.Error(t, err) + if tt.expectedErrMsg != "" { + assert.Contains(t, err.Error(), tt.expectedErrMsg) + } + } else { + assert.NoError(t, err) + assert.Equal(t, tt.wantAttributes, gotAttributes) + } + }) + } +} + +func Test_defaultModelBuildTask_buildFrontendNlbAttributes(t *testing.T) { + tests := []struct { + name string + ingGroup Group + wantAttributes []elbv2model.LoadBalancerAttribute + wantErr bool + expectedErrMsg string + }{ + { + name: "no attributes specified", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + }, + }, + }, + }, + }, + wantAttributes: []elbv2model.LoadBalancerAttribute{}, + wantErr: false, + }, + { + name: "valid attributes conversion", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", + }, + }, + }, + }, + }, + }, + wantAttributes: []elbv2model.LoadBalancerAttribute{ + {Key: "cross_zone.enabled", Value: "true"}, + {Key: "dns_record.client_routing_policy", Value: "availability_zone_affinity"}, + }, + wantErr: false, + }, + { + name: "invalid DNS policy should cause error", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress1", + Namespace: "default", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=invalid_policy", + }, + }, + }, + }, + }, + }, + wantAttributes: []elbv2model.LoadBalancerAttribute{}, + wantErr: true, + expectedErrMsg: "invalid dns_record.client_routing_policy set in annotation", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io") + task := &defaultModelBuildTask{ + annotationParser: parser, + ingGroup: tt.ingGroup, + } + + gotAttributes, err := task.buildFrontendNlbAttributes() + + if tt.wantErr { + assert.Error(t, err) + if tt.expectedErrMsg != "" { + assert.Contains(t, err.Error(), tt.expectedErrMsg) + } + } else { + assert.NoError(t, err) + // Sort both slices to ensure stable comparison + sort.Slice(gotAttributes, func(i, j int) bool { + return gotAttributes[i].Key < gotAttributes[j].Key + }) + sort.Slice(tt.wantAttributes, func(i, j int) bool { + return tt.wantAttributes[i].Key < tt.wantAttributes[j].Key + }) + assert.Equal(t, tt.wantAttributes, gotAttributes) + } + }) + } +} diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index b6ba37ed9..de03d9e6c 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_utils" ) +// Abstract this out?? const ( resourceIDLoadBalancer = "LoadBalancer" ) From 1b7ff39488b7dab132727156314a5fa1b6740c4a Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 30 Sep 2025 18:59:42 +1000 Subject: [PATCH 06/12] feat(ingress): add required annotation --- pkg/annotations/constants.go | 3 ++- pkg/ingress/model_build_frontend_nlb.go | 4 ++-- pkg/shared_utils/nlb_utils.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/annotations/constants.go b/pkg/annotations/constants.go index 09863b028..55c18012a 100644 --- a/pkg/annotations/constants.go +++ b/pkg/annotations/constants.go @@ -65,7 +65,7 @@ const ( IngressSuffixFrontendNlbSubnets = "frontend-nlb-subnets" IngressSuffixFrontendNlbSecurityGroups = "frontend-nlb-security-groups" IngressSuffixFrontendNlbListenerPortMapping = "frontend-nlb-listener-port-mapping" - IngressSuffixFrontendNlbEipAlloactions = "frontend-nlb-eip-allocations" + IngressSuffixFrontendNlbEipAllocations = "frontend-nlb-eip-allocations" IngressSuffixFrontendNlbHealthCheckPort = "frontend-nlb-healthcheck-port" IngressSuffixFrontendNlbHealthCheckProtocol = "frontend-nlb-healthcheck-protocol" IngressSuffixFrontendNlbHealthCheckPath = "frontend-nlb-healthcheck-path" @@ -74,6 +74,7 @@ const ( IngressSuffixFrontendNlbHealthCheckHealthyThresholdCount = "frontend-nlb-healthcheck-healthy-threshold-count" IngressSuffixFrontendNlHealthCheckbUnhealthyThresholdCount = "frontend-nlb-healthcheck-unhealthy-threshold-count" IngressSuffixFrontendNlbHealthCheckSuccessCodes = "frontend-nlb-healthcheck-success-codes" + IngressSuffixFrontendNlbAttributes = "frontend-nlb-attributes" IngressSuffixFrontendNlbTags = "frontend-nlb-tags" IngressSuffixUseRegexPathMatch = "use-regex-path-match" diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index de014d033..aea82214f 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -162,7 +162,7 @@ func (t *defaultModelBuildTask) buildFrontendNlbSubnetMappings(ctx context.Conte explicitSubnetNameOrIDsList = append(explicitSubnetNameOrIDsList, rawSubnetNameOrIDs) } var rawEIP []string - if exists := t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixFrontendNlbEipAlloactions, &rawEIP, member.Ing.Annotations); exists { + if exists := t.annotationParser.ParseStringSliceAnnotation(annotations.IngressSuffixFrontendNlbEipAllocations, &rawEIP, member.Ing.Annotations); exists { eipAllocationsList = append(eipAllocationsList, rawEIP) } } @@ -224,7 +224,7 @@ func (t *defaultModelBuildTask) getFrontendNlbAttributes() (map[string]string, e var chosenAttributes map[string]string for _, member := range t.ingGroup.Members { var attributes map[string]string - if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.SvcLBSuffixLoadBalancerAttributes, &attributes, member.Ing.Annotations); err != nil { + if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixFrontendNlbAttributes, &attributes, member.Ing.Annotations); err != nil { return nil, err } if chosenAttributes == nil { diff --git a/pkg/shared_utils/nlb_utils.go b/pkg/shared_utils/nlb_utils.go index 8adc57e60..5971fc50a 100644 --- a/pkg/shared_utils/nlb_utils.go +++ b/pkg/shared_utils/nlb_utils.go @@ -1,8 +1,8 @@ package shared_utils import ( - "sort" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" + "sort" ) func MakeAttributesSliceFromMap(loadBalancerAttributesMap map[string]string) []elbv2model.LoadBalancerAttribute { From 0b47a419b6ab362d19ad794d3c0fd65a33e94771 Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 30 Sep 2025 19:36:51 +1000 Subject: [PATCH 07/12] docs(*): start updating for fnlb attributes --- docs/guide/ingress/annotations.md | 3 ++- pkg/shared_constants/attributes.go | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/guide/ingress/annotations.md b/docs/guide/ingress/annotations.md index 54c26079b..d769817b8 100644 --- a/docs/guide/ingress/annotations.md +++ b/docs/guide/ingress/annotations.md @@ -85,7 +85,8 @@ You can add annotations to kubernetes Ingress and Service objects to customize t | [alb.ingress.kubernetes.io/frontend-nlb-healthcheck-unhealthy-threshold-count](#frontend-nlb-healthcheck-unhealthy-threshold-count) | integer |3| Ingress | N/A | | [alb.ingress.kubernetes.io/frontend-nlb-healthcheck-success-codes](#frontend-nlb-healthcheck-success-codes) | string |200| Ingress | N/A | | [alb.ingress.kubernetes.io/frontend-nlb-tags](#frontend-nlb-tags) | stringMap | N/A | Ingress | Exclusive | -| [alb.ingress.kubernetes.io/frontend-nlb-eip-allocations](#frontend-nlb-eip-allocations) | stringList |200| Ingress | N/A | +| [alb.ingress.kubernetes.io/frontend-nlb-eip-allocation](#frontend-nlb-eip-allocation) | stringList |N/A| Ingress | N/A | +| [alb.ingress.kubernetes.io/frontend-nlb-attributes](#frontend-nlb-attributes) | stringList |N/A| Ingress | N/A | ## IngressGroup IngressGroup feature enables you to group multiple Ingress resources together. diff --git a/pkg/shared_constants/attributes.go b/pkg/shared_constants/attributes.go index c21665437..351a88128 100644 --- a/pkg/shared_constants/attributes.go +++ b/pkg/shared_constants/attributes.go @@ -1,16 +1,16 @@ package shared_constants const ( - // LBAttributeDeletionProtection deletion protection attribute name LBAttributeDeletionProtection = "deletion_protection.enabled" LBAttributeAccessLogsS3Enabled = "access_logs.s3.enabled" LBAttributeAccessLogsS3Bucket = "access_logs.s3.bucket" LBAttributeAccessLogsS3Prefix = "access_logs.s3.prefix" LBAttributeLoadBalancingCrossZoneEnabled = "load_balancing.cross_zone.enabled" LBAttributeLoadBalancingDnsClientRoutingPolicy = "dns_record.client_routing_policy" - LBAttributeAvailabilityZoneAffinity = "availability_zone_affinity" - LBAttributePartialAvailabilityZoneAffinity = "partial_availability_zone_affinity" - LBAttributeAnyAvailabilityZone = "any_availability_zone" + + LBAttributeAvailabilityZoneAffinity = "availability_zone_affinity" + LBAttributePartialAvailabilityZoneAffinity = "partial_availability_zone_affinity" + LBAttributeAnyAvailabilityZone = "any_availability_zone" ) const ( From 16b70b5cf2f273e87159999e73108d7f9ee433bf Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 7 Oct 2025 20:14:29 +1100 Subject: [PATCH 08/12] test(ingress): wrong annotation name --- pkg/ingress/model_build_frontend_nlb_test.go | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/ingress/model_build_frontend_nlb_test.go b/pkg/ingress/model_build_frontend_nlb_test.go index 3baf7ca66..39e37e298 100644 --- a/pkg/ingress/model_build_frontend_nlb_test.go +++ b/pkg/ingress/model_build_frontend_nlb_test.go @@ -1384,7 +1384,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity", }, }, }, @@ -1406,7 +1406,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=partial_availability_zone_affinity", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=partial_availability_zone_affinity", }, }, }, @@ -1428,7 +1428,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=any_availability_zone", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=any_availability_zone", }, }, }, @@ -1450,7 +1450,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=invalid_policy", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=invalid_policy", }, }, }, @@ -1471,7 +1471,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", }, }, }, @@ -1494,7 +1494,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity", }, }, }, @@ -1505,7 +1505,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress2", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity", }, }, }, @@ -1527,7 +1527,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity", }, }, }, @@ -1538,7 +1538,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress2", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=any_availability_zone", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=any_availability_zone", }, }, }, @@ -1559,7 +1559,7 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity", }, }, }, @@ -1638,7 +1638,7 @@ func Test_defaultModelBuildTask_buildFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", }, }, }, @@ -1661,7 +1661,7 @@ func Test_defaultModelBuildTask_buildFrontendNlbAttributes(t *testing.T) { Name: "ingress1", Namespace: "default", Annotations: map[string]string{ - "alb.ingress.kubernetes.io/aws-load-balancer-attributes": "dns_record.client_routing_policy=invalid_policy", + "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=invalid_policy", }, }, }, From fa19a3c6b62e803de49a640841959c910aee1ef6 Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 7 Oct 2025 20:21:50 +1100 Subject: [PATCH 09/12] test(shared utils): write for nlb utils --- pkg/shared_utils/nlb_utils_test.go | 53 ++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 pkg/shared_utils/nlb_utils_test.go diff --git a/pkg/shared_utils/nlb_utils_test.go b/pkg/shared_utils/nlb_utils_test.go new file mode 100644 index 000000000..0c81af79d --- /dev/null +++ b/pkg/shared_utils/nlb_utils_test.go @@ -0,0 +1,53 @@ +package shared_utils + +import ( + "reflect" + "testing" + + elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" +) + +func TestMakeAttributesSliceFromMap(t *testing.T) { + tests := []struct { + name string + input map[string]string + want []elbv2model.LoadBalancerAttribute + }{ + { + name: "empty map", + input: map[string]string{}, + want: []elbv2model.LoadBalancerAttribute{}, + }, + { + name: "single attribute", + input: map[string]string{ + "key1": "value1", + }, + want: []elbv2model.LoadBalancerAttribute{ + {Key: "key1", Value: "value1"}, + }, + }, + { + name: "multiple attributes, unordered input", + input: map[string]string{ + "zeta": "last", + "alpha": "first", + "mid": "middle", + }, + want: []elbv2model.LoadBalancerAttribute{ + {Key: "alpha", Value: "first"}, + {Key: "mid", Value: "middle"}, + {Key: "zeta", Value: "last"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MakeAttributesSliceFromMap(tt.input) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("MakeAttributesSliceFromMap() = %v, want %v", got, tt.want) + } + }) + } +} From 7e75d7bdea7be7eecd59a1d3f38e9248de37e05b Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 7 Oct 2025 20:38:38 +1100 Subject: [PATCH 10/12] docs(ingress): add section for frontend NLB attributes --- docs/guide/ingress/annotations.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/docs/guide/ingress/annotations.md b/docs/guide/ingress/annotations.md index d769817b8..768d773fe 100644 --- a/docs/guide/ingress/annotations.md +++ b/docs/guide/ingress/annotations.md @@ -1356,3 +1356,30 @@ When this option is set to true, the controller will automatically provision a N ``` alb.ingress.kubernetes.io/frontend-nlb-eip-allocations: eipalloc-xyz, eipalloc-zzz ``` + +- `alb.ingress.kubernetes.io/frontend-nlb-attributes` specifies [Load Balancer Attributes](http://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_LoadBalancerAttribute.html) that should be applied to the ALB. + + !!!warning "" + Only attributes defined in the annotation will be updated. To unset any AWS defaults(e.g. Disabling access logs after having them enabled once), the values need to be explicitly set to the original values(`access_logs.s3.enabled=false`) and omitting them is not sufficient. + + !!!note "" + - If `deletion_protection.enabled=true` is in annotation, the controller will not be able to delete the ALB during reconciliation. Once the attribute gets edited to `deletion_protection.enabled=false` during reconciliation, the deployer will force delete the resource. + - Please note, if the deletion protection is not enabled via annotation (e.g. via AWS console), the controller still deletes the underlying resource. + + !!!example + - enable access log to s3 + ``` + service.beta.kubernetes.io/aws-load-balancer-attributes: access_logs.s3.enabled=true,access_logs.s3.bucket=my-access-log-bucket,access_logs.s3.prefix=my-app + ``` + - enable NLB deletion protection + ``` + service.beta.kubernetes.io/aws-load-balancer-attributes: deletion_protection.enabled=true + ``` + - enable cross zone load balancing + ``` + service.beta.kubernetes.io/aws-load-balancer-attributes: load_balancing.cross_zone.enabled=true + ``` + - enable client availability zone affinity + ``` + service.beta.kubernetes.io/aws-load-balancer-attributes: dns_record.client_routing_policy=availability_zone_affinity + ``` \ No newline at end of file From be400b1c622e4fdfddb5eb755acea264e787d2d5 Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 7 Oct 2025 20:58:40 +1100 Subject: [PATCH 11/12] tests(ingress): remove envoloping tests --- pkg/ingress/model_build_frontend_nlb_test.go | 102 ------------------- pkg/service/model_build_load_balancer.go | 5 - 2 files changed, 107 deletions(-) diff --git a/pkg/ingress/model_build_frontend_nlb_test.go b/pkg/ingress/model_build_frontend_nlb_test.go index 39e37e298..409e94b79 100644 --- a/pkg/ingress/model_build_frontend_nlb_test.go +++ b/pkg/ingress/model_build_frontend_nlb_test.go @@ -2,7 +2,6 @@ package ingress import ( "context" - "sort" "testing" awssdk "github.com/aws/aws-sdk-go-v2/aws" @@ -1602,104 +1601,3 @@ func Test_defaultModelBuildTask_getFrontendNlbAttributes(t *testing.T) { }) } } - -func Test_defaultModelBuildTask_buildFrontendNlbAttributes(t *testing.T) { - tests := []struct { - name string - ingGroup Group - wantAttributes []elbv2model.LoadBalancerAttribute - wantErr bool - expectedErrMsg string - }{ - { - name: "no attributes specified", - ingGroup: Group{ - Members: []ClassifiedIngress{ - { - Ing: &networking.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress1", - Namespace: "default", - }, - }, - }, - }, - }, - wantAttributes: []elbv2model.LoadBalancerAttribute{}, - wantErr: false, - }, - { - name: "valid attributes conversion", - ingGroup: Group{ - Members: []ClassifiedIngress{ - { - Ing: &networking.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress1", - Namespace: "default", - Annotations: map[string]string{ - "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=availability_zone_affinity,cross_zone.enabled=true", - }, - }, - }, - }, - }, - }, - wantAttributes: []elbv2model.LoadBalancerAttribute{ - {Key: "cross_zone.enabled", Value: "true"}, - {Key: "dns_record.client_routing_policy", Value: "availability_zone_affinity"}, - }, - wantErr: false, - }, - { - name: "invalid DNS policy should cause error", - ingGroup: Group{ - Members: []ClassifiedIngress{ - { - Ing: &networking.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress1", - Namespace: "default", - Annotations: map[string]string{ - "alb.ingress.kubernetes.io/frontend-nlb-attributes": "dns_record.client_routing_policy=invalid_policy", - }, - }, - }, - }, - }, - }, - wantAttributes: []elbv2model.LoadBalancerAttribute{}, - wantErr: true, - expectedErrMsg: "invalid dns_record.client_routing_policy set in annotation", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - parser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io") - task := &defaultModelBuildTask{ - annotationParser: parser, - ingGroup: tt.ingGroup, - } - - gotAttributes, err := task.buildFrontendNlbAttributes() - - if tt.wantErr { - assert.Error(t, err) - if tt.expectedErrMsg != "" { - assert.Contains(t, err.Error(), tt.expectedErrMsg) - } - } else { - assert.NoError(t, err) - // Sort both slices to ensure stable comparison - sort.Slice(gotAttributes, func(i, j int) bool { - return gotAttributes[i].Key < gotAttributes[j].Key - }) - sort.Slice(tt.wantAttributes, func(i, j int) bool { - return tt.wantAttributes[i].Key < tt.wantAttributes[j].Key - }) - assert.Equal(t, tt.wantAttributes, gotAttributes) - } - }) - } -} diff --git a/pkg/service/model_build_load_balancer.go b/pkg/service/model_build_load_balancer.go index de03d9e6c..21b798f09 100644 --- a/pkg/service/model_build_load_balancer.go +++ b/pkg/service/model_build_load_balancer.go @@ -26,11 +26,6 @@ import ( "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_utils" ) -// Abstract this out?? -const ( - resourceIDLoadBalancer = "LoadBalancer" -) - func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error { existingLB, err := t.fetchExistingLoadBalancer(ctx) if err != nil { From bc73b9cfb9e6b815680e3f9dd08d01897f57c121 Mon Sep 17 00:00:00 2001 From: Sophie Warner Date: Tue, 7 Oct 2025 21:02:23 +1100 Subject: [PATCH 12/12] chore(service): clean up for pr --- pkg/service/model_build_target_group.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/service/model_build_target_group.go b/pkg/service/model_build_target_group.go index 4b9fe633d..bb330493a 100644 --- a/pkg/service/model_build_target_group.go +++ b/pkg/service/model_build_target_group.go @@ -10,8 +10,6 @@ import ( "strconv" "strings" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" - awssdk "github.com/aws/aws-sdk-go-v2/aws" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -25,6 +23,7 @@ import ( elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" elbv2modelk8s "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2/k8s" "sigs.k8s.io/aws-load-balancer-controller/pkg/networking" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" ) func (t *defaultModelBuildTask) buildTargetGroup(ctx context.Context, port corev1.ServicePort, tgProtocol elbv2model.Protocol, scheme elbv2model.LoadBalancerScheme) (*elbv2model.TargetGroup, error) {