From 1b5b4b96de0231036b2dcba8f08dee88ff87b583 Mon Sep 17 00:00:00 2001 From: Pradeep Lakshmi Narasimha Date: Sat, 30 Aug 2025 18:18:31 +0530 Subject: [PATCH] fix: Resource tags don't propagate to frontend NLB #4279 Signed-off-by: Pradeep Lakshmi Narasimha --- docs/guide/ingress/annotations.md | 34 +++- pkg/annotations/constants.go | 1 + pkg/ingress/model_build_frontend_nlb.go | 41 ++++ pkg/ingress/model_build_frontend_nlb_test.go | 201 +++++++++++++++++++ test/e2e/ingress/vanilla_ingress_test.go | 69 +++++++ 5 files changed, 335 insertions(+), 11 deletions(-) diff --git a/docs/guide/ingress/annotations.md b/docs/guide/ingress/annotations.md index 98bb2efbd3..1af82f2715 100644 --- a/docs/guide/ingress/annotations.md +++ b/docs/guide/ingress/annotations.md @@ -82,6 +82,7 @@ You can add annotations to kubernetes Ingress and Service objects to customize t | [alb.ingress.kubernetes.io/frontend-nlb-healthcheck-healthy-threshold-count](#frontend-nlb-healthcheck-healthy-threshold-count) | integer |3| Ingress | N/A | | [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 | ## IngressGroup IngressGroup feature enables you to group multiple Ingress resources together. @@ -104,7 +105,7 @@ By default, Ingresses don't belong to any IngressGroup, and we treat it as a "im other Kubernetes users may create/modify their Ingresses to belong to the same IngressGroup, and can thus add more rules or overwrite existing rules with higher priority to the ALB for your Ingress. We'll add more fine-grained access-control in future versions. - + !!!note "Rename behavior" The ALB for an IngressGroup is found by searching for an AWS tag `ingress.k8s.aws/stack` tag with the name of the IngressGroup as its value. For an implicit IngressGroup, the value is `namespace/ingressname`. @@ -195,9 +196,9 @@ Traffic Routing can be controlled with following annotations: - Once defined on a single Ingress, it impacts every Ingress within the IngressGroup. !!!note "Annotation Behavior" - + - This annotation **takes effect only during the creation** of the Ingress. If the Ingress already exists, the change will not be applied until the Ingress is **deleted and recreated**. - + !!!example ``` alb.ingress.kubernetes.io/load-balancer-name: custom-name @@ -499,9 +500,9 @@ Traffic Routing can be controlled with following annotations: name: use-annotation ``` - !!!note + !!!note If you are using `alb.ingress.kubernetes.io/target-group-attributes` with `stickiness.enabled=true`, you should add `TargetGroupStickinessConfig` under `alb.ingress.kubernetes.io/actions.weighted-routing` - + !!!example ```yaml @@ -843,10 +844,10 @@ TLS support can be controlled with the following annotations: - `alb.ingress.kubernetes.io/mutual-authentication` specifies the mutual authentication configuration that should be assigned to the Application Load Balancer secure listener ports. See [Mutual authentication with TLS](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/mutual-authentication.html) in the AWS documentation for more details. - !!!note + !!!note - This annotation is not applicable for Outposts, Local Zones or Wavelength zones. - "Configuration Options" - - `port: listen port ` + - `port: listen port ` - Must be an HTTPS port specified by [listen-ports](#listen-ports). - `mode: "off" (default) | "passthrough" | "verify"` - `verify` mode requires an existing trust store resource. @@ -857,7 +858,7 @@ TLS support can be controlled with the following annotations: - `ignoreClientCertificateExpiry : true | false (default)` - `advertiseTrustStoreCaNames : "on" | "off" (default)` - Once the Mutual Authentication is set, to turn it off, you will have to explicitly pass in this annotation with `mode : "off"`. - + !!!example - [listen-ports](#listen-ports) specifies four HTTPS ports: `80, 443, 8080, 8443` - listener `HTTPS:80` will be set to `passthrough` mode @@ -910,7 +911,7 @@ Custom attributes to LoadBalancers and TargetGroups can be controlled with follo ``` - set client_keep_alive to 3600 seconds ``` - alb.ingress.kubernetes.io/load-balancer-attributes: client_keep_alive.seconds=3600 + alb.ingress.kubernetes.io/load-balancer-attributes: client_keep_alive.seconds=3600 ``` - enable [connection logs](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/load-balancer-connection-logs.html) ``` @@ -1077,11 +1078,11 @@ Load balancer capacity unit reservation can be configured via following annotati ## Enable frontend NLB When this option is set to true, the controller will automatically provision a Network Load Balancer and register the Application Load Balancer as its target. Additional annotations are available to customize the NLB configurations, including options for scheme, security groups, subnets, and health check. The ingress resource will have two status entries, one for the NLB DNS and one for the ALB DNS. This allows users to combine the benefits of NLB and ALB into a single solution, leveraging NLB features like static IP address and PrivateLink, while retaining the rich routing capabilities of ALB. -!!!warning +!!!warning - If you need to change the ALB [scheme](#scheme), make sure to disable this feature first. Changing the scheme will create a new ALB, which could interfere with the current configuration. - If you create ingress and enable the feature at once, provisioning the NLB and registering the ALB as target can take up to 3-4 mins to complete. -- `alb.ingress.kubernetes.io/enable-frontend-nlb` enables frontend Network Load Balancer functionality. +- `alb.ingress.kubernetes.io/enable-frontend-nlb` enables frontend Network Load Balancer functionality. !!!example - Enable frontend nlb @@ -1187,3 +1188,14 @@ When this option is set to true, the controller will automatically provision a N ``` alb.ingress.kubernetes.io/frontend-nlb-healthcheck-success-codes: '200' ``` + +- `alb.ingress.kubernetes.io/frontend-nlb-tags` specifies additional tags to be applied to the frontend NLB. If not specified, the tags from ALB (specified via `alb.ingress.kubernetes.io/tags`) will be propagated to the NLB. + + !!!note "Merge Behavior" + `frontend-nlb-tags` is exclusive across all Ingresses in IngressGroup. + If specified on multiple Ingresses within IngressGroup, the values must match. + + !!!example + ``` + alb.ingress.kubernetes.io/frontend-nlb-tags: Environment=prod,Team=platform + ``` diff --git a/pkg/annotations/constants.go b/pkg/annotations/constants.go index 0c9483086c..d93540cc0c 100644 --- a/pkg/annotations/constants.go +++ b/pkg/annotations/constants.go @@ -73,6 +73,7 @@ const ( IngressSuffixFrontendNlbHealthCheckHealthyThresholdCount = "frontend-nlb-healthcheck-healthy-threshold-count" IngressSuffixFrontendNlHealthCheckbUnhealthyThresholdCount = "frontend-nlb-healthcheck-unhealthy-threshold-count" IngressSuffixFrontendNlbHealthCheckSuccessCodes = "frontend-nlb-healthcheck-success-codes" + IngressSuffixFrontendNlbTags = "frontend-nlb-tags" // NLB annotation suffixes // prefixes service.beta.kubernetes.io, service.kubernetes.io diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index ae366b80dc..339ca26bf8 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -184,6 +184,11 @@ func (t *defaultModelBuildTask) buildFrontendNlbSpec(ctx context.Context, scheme return elbv2model.LoadBalancerSpec{}, err } + tags, err := t.buildFrontendNlbTags(ctx, alb) + if err != nil { + return elbv2model.LoadBalancerSpec{}, err + } + spec := elbv2model.LoadBalancerSpec{ Name: name, Type: elbv2model.LoadBalancerTypeNetwork, @@ -191,6 +196,7 @@ func (t *defaultModelBuildTask) buildFrontendNlbSpec(ctx context.Context, scheme IPAddressType: alb.Spec.IPAddressType, SecurityGroups: securityGroups, SubnetMappings: subnetMappings, + Tags: tags, } return spec, nil @@ -765,3 +771,38 @@ func mergeHealthCheckField[T comparable](fieldName string, finalValue **T, curre } return nil } + +func (t *defaultModelBuildTask) buildFrontendNlbTags(ctx context.Context, alb *elbv2model.LoadBalancer) (map[string]string, error) { + // First check for frontend-nlb specific tags + var frontendNlbTags map[string]string + for _, member := range t.ingGroup.Members { + rawFrontendNlbTags := make(map[string]string) + exists, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixFrontendNlbTags, &rawFrontendNlbTags, member.Ing.Annotations) + if err != nil { + return nil, err + } + if !exists { + continue + } + + if frontendNlbTags != nil { + // If we already found tags from another ingress in the group, they must match + if !cmp.Equal(frontendNlbTags, rawFrontendNlbTags) { + return nil, errors.Errorf("conflicting frontend NLB tags: %v | %v", frontendNlbTags, rawFrontendNlbTags) + } + } else { + frontendNlbTags = rawFrontendNlbTags + } + } + + if frontendNlbTags == nil { + // If no frontend-nlb specific tags are found, use the ALB tags + albTags, err := t.buildLoadBalancerTags(ctx) + if err != nil { + return nil, err + } + return albTags, nil + } + + return frontendNlbTags, nil +} diff --git a/pkg/ingress/model_build_frontend_nlb_test.go b/pkg/ingress/model_build_frontend_nlb_test.go index bbe9391613..af0d695cf7 100644 --- a/pkg/ingress/model_build_frontend_nlb_test.go +++ b/pkg/ingress/model_build_frontend_nlb_test.go @@ -662,6 +662,207 @@ func Test_buildEnableFrontendNlbViaAnnotation(t *testing.T) { } } +func Test_buildFrontendNlbTags(t *testing.T) { + tests := []struct { + name string + ingGroup Group + defaultTags map[string]string + wantTags map[string]string + wantErr bool + errMsg string + }{ + { + name: "no tags specified", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-1", + Annotations: map[string]string{}, + }, + }, + }, + }, + }, + defaultTags: nil, + wantTags: make(map[string]string), // Expect an empty map, not nil + wantErr: false, + }, + { + name: "frontend-nlb-specific tags", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/frontend-nlb-tags": "key1=value1,key2=value2", + }, + }, + }, + }, + }, + }, + defaultTags: map[string]string{ + "default": "value", + }, + wantTags: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + wantErr: false, + }, + { + name: "ALB tags propagation when no frontend-nlb-tags", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/tags": "key1=value1,key2=value2", + }, + }, + }, + }, + }, + }, + defaultTags: nil, + wantTags: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + wantErr: false, + }, + { + name: "frontend-nlb-tags take precedence over ALB tags", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/frontend-nlb-tags": "nlb-key=nlb-value", + "alb.ingress.kubernetes.io/tags": "alb-key=alb-value", + }, + }, + }, + }, + }, + }, + defaultTags: map[string]string{ + "default": "value", + }, + wantTags: map[string]string{ + "nlb-key": "nlb-value", + }, + wantErr: false, + }, + { + name: "conflicting frontend-nlb-tags", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/frontend-nlb-tags": "key1=value1", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-2", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/frontend-nlb-tags": "key1=value2", + }, + }, + }, + }, + }, + }, + defaultTags: nil, + wantTags: nil, + wantErr: true, + errMsg: "conflicting frontend NLB tags", + }, + { + name: "consistent frontend-nlb-tags across ingresses", + ingGroup: Group{ + Members: []ClassifiedIngress{ + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/frontend-nlb-tags": "key1=value1", + }, + }, + }, + }, + { + Ing: &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + Name: "ing-2", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/frontend-nlb-tags": "key1=value1", + }, + }, + }, + }, + }, + }, + defaultTags: nil, + wantTags: map[string]string{ + "key1": "value1", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock task that embeds defaultModelBuildTask and overrides buildLoadBalancerTags + task := &defaultModelBuildTask{ + ingGroup: tt.ingGroup, + annotationParser: annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io"), + // Default implementation will return an empty map when no tags are specified + defaultTags: tt.defaultTags, + } + + got, err := task.buildFrontendNlbTags(context.Background(), nil) + + if tt.wantErr { + assert.Error(t, err) + if tt.errMsg != "" { + assert.Contains(t, err.Error(), tt.errMsg) + } + } else { + assert.NoError(t, err) + assert.Equal(t, tt.wantTags, got) + if got == nil { + t.Error("got nil map, expected non-nil map") + } + } + }) + } +} + func Test_mergeFrontendNlbListenPortConfigs(t *testing.T) { tests := []struct { name string diff --git a/test/e2e/ingress/vanilla_ingress_test.go b/test/e2e/ingress/vanilla_ingress_test.go index ae01906dae..404e484703 100644 --- a/test/e2e/ingress/vanilla_ingress_test.go +++ b/test/e2e/ingress/vanilla_ingress_test.go @@ -8,6 +8,8 @@ import ( "time" awssdk "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2" + elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types" "github.com/gavv/httpexpect/v2" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -829,6 +831,73 @@ var _ = Describe("vanilla ingress tests", func() { }) }) + Context("When created with managed alb with tags provided via annotations", func() { + It("ALB will have correct tags", func() { + appBuilder := manifest.NewFixedResponseServiceBuilder() + ingBuilder := manifest.NewIngressBuilder() + dp, svc := appBuilder.Build(sandboxNS.Name, "app", tf.Options.TestImageRegistry) + ingBackend := networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: svc.Name, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + } + annotation := map[string]string{ + "kubernetes.io/ingress.class": "alb", + "alb.ingress.kubernetes.io/scheme": "internet-facing", + "alb.ingress.kubernetes.io/tags": "k1=v1,k2=v2", + } + if tf.Options.IPFamily == "IPv6" { + annotation["alb.ingress.kubernetes.io/ip-address-type"] = "dualstack" + annotation["alb.ingress.kubernetes.io/target-type"] = "ip" + } + ing := ingBuilder. + AddHTTPRoute("", networking.HTTPIngressPath{Path: "/path", PathType: &exact, Backend: ingBackend}). + WithAnnotations(annotation).Build(sandboxNS.Name, "ing") + resStack := fixture.NewK8SResourceStack(tf, dp, svc, ing) + err := resStack.Setup(ctx) + Expect(err).NotTo(HaveOccurred()) + + defer resStack.TearDown(ctx) + + lbARN, _ := ExpectOneLBProvisionedForIngress(ctx, tf, ing) + + // Verify tags + expectedTags := map[string]string{ + "k1": "v1", + "k2": "v2", + } + // Verify tags using DescribeTags + var tagDescriptions []elbv2types.TagDescription + Eventually(func() (bool, error) { + req := &elasticloadbalancingv2.DescribeTagsInput{ + ResourceArns: []string{lbARN}, + } + resp, err := tf.Cloud.ELBV2().DescribeTagsWithContext(ctx, req) + if err != nil { + return false, err + } + if len(resp.TagDescriptions) > 0 { + tagDescriptions = resp.TagDescriptions + return true, nil + } + return false, nil + }, utils.PollTimeoutShort, utils.PollIntervalMedium).Should(BeTrue()) + + // At this point we should have exactly one TagDescription since we only queried one LB + Expect(tagDescriptions).To(HaveLen(1), "Expected exactly one TagDescription") + foundTags := 0 + for _, tag := range tagDescriptions[0].Tags { + if val, ok := expectedTags[awssdk.ToString(tag.Key)]; ok && val == awssdk.ToString(tag.Value) { + foundTags++ + } + } + Expect(foundTags).To(Equal(len(expectedTags)), "Not all expected tags were found on the ALB") + }) + }) + Context("with frontend NLB enabled", func() { It("should create a frontend NLB and route traffic correctly", func() { appBuilder := manifest.NewFixedResponseServiceBuilder()