From 91d9707b3e0d736ebdf84e55a6a2a10ac2130ad3 Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Mon, 11 Aug 2025 19:18:12 -0300 Subject: [PATCH 1/4] doc/service: describe supported target group attributes Document the new annotation for NLB to handle target group attributes, with examples and restrictions. --- docs/service_controller.md | 53 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/docs/service_controller.md b/docs/service_controller.md index 2e868a955f..d855793b18 100644 --- a/docs/service_controller.md +++ b/docs/service_controller.md @@ -33,3 +33,56 @@ The service controller is responsible for watch for service and node object chan | service.beta.kubernetes.io/aws-load-balancer-healthcheck-protocol | [tcp\|http\|https] | tcp | NLB | Specifies the protocol to use for the target group health check. | | service.beta.kubernetes.io/aws-load-balancer-subnets | Comma-separated list | - | ELB,NLB | Specifies the Availability Zone configuration for the load balancer. The values are comma separated list of subnetID or subnetName from different AZs. | | service.beta.kubernetes.io/aws-load-balancer-target-node-labels | Comma-separated list of key=value | - | ELB,NLB | Specifies a comma-separated list of key-value pairs which will be used to select the target nodes for the load balancer. | +| service.beta.kubernetes.io/aws-load-balancer-target-group-attributes | Comma-separated list of key=value | - | NLB | Specifies a comma-separated list of key-value pairs which will be applied as target group attributes. For example: "preserve_client_ip.enabled=false". The list of supported values is available [here](#tg-supported-attributes). | + + +## Target group attributes for Service type-loadBalancer NLB + +The following target group attributes are supported by the controller using the annotation `service.beta.kubernetes.io/aws-load-balancer-target-group-attributes`: + +| Attribute | Values | Description | +| -- | -- | -- | +| preserve_client_ip.enabled | [true\|false] | Whether to preserve client IP addresses when terminating connections at the target group level | +| proxy_protocol_v2.enabled | [true\|false] | Whether to enable proxy protocol v2 on the target group | + +**Format:** Attributes are specified as `key=value` pairs, separated by commas. + +**Example:** +```yaml +service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=true,proxy_protocol_v2.enabled=false +``` + +### Usage Example 1 - working with hairpin connection on internal NLB + +The following Service example changes the Target Group Traffic Control attribute "Preserve client IP addresses" from the default (`true`, when target type is instance) to `false`: + +```yaml +apiVersion: v1 +kind: Service +metadata: + name: $SVC_NAME + namespace: ${APP_NAMESPACE} + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb + service.beta.kubernetes.io/aws-load-balancer-internal: true + service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=false +[...] +``` + +### Usage Example 2 - working with hairpin connection on internal NLB tracking source IP address + +The following example allow users to fine tune the Services for a backend which requires tracking the original source IP address of internal Load Balancers NLB with support of hairpin connections: + + +```yaml +apiVersion: v1 +kind: Service +metadata: + name: $SVC_NAME + namespace: ${APP_NAMESPACE} + annotations: + service.beta.kubernetes.io/aws-load-balancer-type: nlb + service.beta.kubernetes.io/aws-load-balancer-internal: true + service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: preserve_client_ip.enabled=false,proxy_protocol_v2.enabled=true +[...] +``` \ No newline at end of file From 800cc8417bd2c79e12bcca4122e72d3f916abc3a Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Mon, 11 Aug 2025 19:17:07 -0300 Subject: [PATCH 2/4] e2e/loadbalancer: implement hairpin connection cases Implementing the hairpin connection test cases, and exposing an issue on NLB with internal scheme which fails when the client is trying to access a service loadbalancer which is hosted in the same node. The hairpin connection is caused by the client IP preservation attribute is set to true (default), and the service does not provide an interface to prevent the issue. The e2e is expecting to pass to prevent permanent failures in CI, but it is tracked by an issue https://github.com/kubernetes/cloud-provider-aws/issues/1160. --- tests/e2e/loadbalancer.go | 138 ++++++++++++++++++++++++++++++++------ 1 file changed, 117 insertions(+), 21 deletions(-) diff --git a/tests/e2e/loadbalancer.go b/tests/e2e/loadbalancer.go index 01d422a9d8..c5971004f8 100644 --- a/tests/e2e/loadbalancer.go +++ b/tests/e2e/loadbalancer.go @@ -38,9 +38,10 @@ import ( ) const ( - annotationLBType = "service.beta.kubernetes.io/aws-load-balancer-type" - annotationLBInternal = "service.beta.kubernetes.io/aws-load-balancer-internal" - annotationLBTargetNodeLabels = "service.beta.kubernetes.io/aws-load-balancer-target-node-labels" + annotationLBType = "service.beta.kubernetes.io/aws-load-balancer-type" + annotationLBInternal = "service.beta.kubernetes.io/aws-load-balancer-internal" + annotationLBTargetNodeLabels = "service.beta.kubernetes.io/aws-load-balancer-target-node-labels" + annotationLBTargetGroupAttributes = "service.beta.kubernetes.io/aws-load-balancer-target-group-attributes" ) var ( @@ -142,19 +143,20 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { requireAffinity: true, }, // Hairpining traffic test for NLB. - // Hairpin connection work with target type as instance only when preserve client IP is disabled. - // Currently CCM does not provide an interface to create a service with that setup, making an internal - // Service to fail. - // FIXME: https://github.com/kubernetes/cloud-provider-aws/issues/1160 - // Once issue 1160 is fixed, the skipTestFailure must be unset/false. + // The target type instance (default) sets the preserve client IP attribute to true, + // the NLB target group attributes are set to preserve_client_ip.enabled=false to allow hairpining traffic. + // The test also validates the target group attributes are set correctly to AWS resource. { name: "NLB internal should be reachable with hairpinning traffic", resourceSuffix: "hp-nlb-int", extraAnnotations: map[string]string{ - annotationLBType: "nlb", - annotationLBInternal: "true", + annotationLBType: "nlb", + annotationLBInternal: "true", + annotationLBTargetGroupAttributes: "preserve_client_ip.enabled=false", }, - listenerCount: 1, + listenerCount: 1, + overrideTestRunInClusterReachableHTTP: true, + requireAffinity: true, hookPostServiceConfig: func(cfg *e2eTestConfig) { framework.Logf("running hook post-service-config patching service annotations to enforce LB pins/selects target to a single node: kubernetes.io/hostname=%s", cfg.nodeSingleSample) if cfg.svc.Annotations == nil { @@ -162,9 +164,85 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { } cfg.svc.Annotations[annotationLBTargetNodeLabels] = fmt.Sprintf("kubernetes.io/hostname=%s", cfg.nodeSingleSample) }, - overrideTestRunInClusterReachableHTTP: true, - requireAffinity: true, - skipTestFailure: true, + hookPreTest: func(e2e *e2eTestConfig) { + framework.Logf("running hook pre-test: verify target group attributes are set correctly to AWS resource") + + if e2e.svc.Status.LoadBalancer.Ingress[0].Hostname == "" && e2e.svc.Status.LoadBalancer.Ingress[0].IP == "" { + framework.Failf("LoadBalancer ingress is empty (no hostname or IP) for service %s/%s", e2e.svc.Namespace, e2e.svc.Name) + } + + hostAddr := e2eservice.GetIngressPoint(&e2e.svc.Status.LoadBalancer.Ingress[0]) + framework.Logf("Load balancer's ingress address: %s", hostAddr) + + if hostAddr == "" { + framework.Failf("Unable to get LoadBalancer ingress address for service %s/%s", e2e.svc.Namespace, e2e.svc.Name) + } + + elbClient, err := getAWSClientLoadBalancer(e2e.ctx) + framework.ExpectNoError(err, "failed to create AWS ELB client") + + // DescribeLoadBalancers API doesn't support filtering by DNS name directly + // Use AWS SDK paginator to search through all load balancers + foundLB, err := getAWSLoadBalancerFromDNSName(e2e.ctx, elbClient, hostAddr) + framework.ExpectNoError(err, "failed to find load balancer with DNS name %s", hostAddr) + if foundLB == nil { + framework.Failf("Found load balancer is nil for DNS name %s", hostAddr) + } + + lbARN := aws.ToString(foundLB.LoadBalancerArn) + if lbARN == "" { + framework.Failf("Load balancer ARN is empty for DNS name %s", hostAddr) + } + framework.Logf("Found load balancer: %s with ARN: %s", aws.ToString(foundLB.LoadBalancerName), lbARN) + + // lookup target group ARN from load balancer ARN + targetGroups, err := elbClient.DescribeTargetGroups(e2e.ctx, &elbv2.DescribeTargetGroupsInput{ + LoadBalancerArn: aws.String(lbARN), + }) + framework.ExpectNoError(err, "failed to describe target groups") + framework.ExpectEqual(len(targetGroups.TargetGroups), 1) + + targetGroupAttributes, err := elbClient.DescribeTargetGroupAttributes(e2e.ctx, &elbv2.DescribeTargetGroupAttributesInput{ + TargetGroupArn: aws.String(aws.ToString(targetGroups.TargetGroups[0].TargetGroupArn)), + }) + framework.ExpectNoError(err, "failed to describe target group attributes") + + // verify if the target group attributes are set correctly + + annotationToDict := map[string]string{} + for _, v := range strings.Split(e2e.svc.Annotations[annotationLBTargetGroupAttributes], ",") { + parts := strings.Split(v, "=") + annotationToDict[parts[0]] = parts[1] + } + framework.Logf("TG attribute Annotation to dict: %v", annotationToDict) + + framework.Logf("=== All Target Group Attributes from AWS ===") + for _, attr := range targetGroupAttributes.Attributes { + framework.Logf(" %s=%s", aws.ToString(attr.Key), aws.ToString(attr.Value)) + } + + framework.Logf("=== Expected Target Group Attributes from Annotation ===") + for key, value := range annotationToDict { + framework.Logf(" %s=%s", key, value) + } + + // Check if our expected attributes are present and match + framework.Logf("=== Verifying Target Group Attributes ===") + for _, attr := range targetGroupAttributes.Attributes { + if expectedValue, ok := annotationToDict[aws.ToString(attr.Key)]; ok { + actualValue := aws.ToString(attr.Value) + framework.Logf("Checking attribute: %s", aws.ToString(attr.Key)) + framework.Logf(" Expected: %s", expectedValue) + framework.Logf(" Actual: %s", actualValue) + + if actualValue != expectedValue { + framework.Failf("Target group attribute mismatch for %s: expected %s, got %s", aws.ToString(attr.Key), expectedValue, actualValue) + } else { + framework.Logf("✓ Target group attribute %s matches expected value %s", aws.ToString(attr.Key), expectedValue) + } + } + } + }, }, } @@ -208,7 +286,23 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { By("waiting for AWS load balancer provisioning") var err error e2e.svc, err = e2e.LBJig.WaitForLoadBalancer(loadBalancerCreateTimeout) - framework.ExpectNoError(err) + // Collect comprehensive debugging information when LoadBalancer provisioning fails + if err != nil { + serviceName := e2e.LBJig.Name + if e2e.svc != nil { + serviceName = e2e.svc.Name + } + framework.Logf("ERROR: LoadBalancer provisioning failed for service %q: %v", serviceName, err) + framework.Logf("ERROR: LoadBalancer provisioning timeout reached after %v", loadBalancerCreateTimeout) + + // Ensure we have detailed debugging information before failing + framework.Logf("=== LoadBalancer Provisioning Failure Debug Information ===") + gatherEventosOnFailure(e2e.ctx, e2e.kubeClient, e2e.LBJig.Namespace, e2e.LBJig.Name) + framework.Logf("=== End of LoadBalancer Provisioning Failure Debug Information ===") + + // Fail the test immediately to prevent further execution + framework.ExpectNoError(err, "LoadBalancer provisioning failed - check debug information above") + } framework.Logf("[AWS] Load balancer provisioned successfully") By("creating backend server pods") @@ -244,7 +338,6 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { framework.Logf("=== End of Service Validation Error Debug Information ===") framework.Failf("Service is nil after LoadBalancer provisioning for service %s", e2e.LBJig.Name) } - if len(e2e.svc.Spec.Ports) == 0 { framework.Logf("=== Service Ports Error Debug Information ===") framework.Logf("Service spec: %+v", e2e.svc.Spec) @@ -259,6 +352,7 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { framework.Logf("=== End of LoadBalancer Ingress Error Debug Information ===") framework.Failf("No ingress found in LoadBalancer status for service %s/%s", e2e.svc.Namespace, e2e.svc.Name) } + svcPort := int(e2e.svc.Spec.Ports[0].Port) ingressAddress := e2eservice.GetIngressPoint(&e2e.svc.Status.LoadBalancer.Ingress[0]) framework.Logf("[LB-INFO] Ingress address: %s, port: %d", ingressAddress, svcPort) @@ -278,7 +372,7 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { // overrideTestRunInClusterReachableHTTP changes the default test function to run the client in the cluster. if tc.overrideTestRunInClusterReachableHTTP { - By("testing HTTP connectivity from internal network") + By("testing HTTP connectivity for internal load balancer") framework.Logf("[TEST] Running internal connectivity test from node: %s", e2e.nodeSingleSample) err := inClusterTestReachableHTTP(cs, ns.Name, e2e.nodeSingleSample, ingressAddress, svcPort) if err != nil && tc.skipTestFailure { @@ -286,7 +380,7 @@ var _ = Describe("[cloud-provider-aws-e2e] loadbalancer", func() { } framework.ExpectNoError(err) } else { - By("testing HTTP connectivity from external client") + By("testing HTTP connectivity for external/internet-facing load balancer") framework.Logf("[TEST] Running external connectivity test to %s:%d", ingressAddress, svcPort) e2eservice.TestReachableHTTP(ingressAddress, svcPort, e2eservice.LoadBalancerLagTimeoutAWS) } @@ -570,9 +664,11 @@ func getAWSLoadBalancerFromDNSName(ctx context.Context, elbClient *elbv2.Client, paginator := elbv2.NewDescribeLoadBalancersPaginator(elbClient, &elbv2.DescribeLoadBalancersInput{}) for paginator.HasMorePages() { page, err := paginator.NextPage(ctx) - framework.ExpectNoError(err) + if err != nil { + return nil, fmt.Errorf("failed to describe load balancers: %v", err) + } - framework.Logf("found %d load balancers", len(page.LoadBalancers)) + framework.Logf("found %d load balancers in page", len(page.LoadBalancers)) // Search for the load balancer with matching DNS name in this page for i := range page.LoadBalancers { if aws.ToString(page.LoadBalancers[i].DNSName) == lbDNSName { @@ -587,7 +683,7 @@ func getAWSLoadBalancerFromDNSName(ctx context.Context, elbClient *elbv2.Client, } if foundLB == nil { - framework.Failf("No load balancer found with DNS name: %s", lbDNSName) + return nil, fmt.Errorf("no load balancer found with DNS name: %s", lbDNSName) } return foundLB, nil From 36deb69863ae06093f6ec4252d477f7ab4fa141b Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Wed, 20 Aug 2025 09:19:39 -0300 Subject: [PATCH 3/4] aws/validations: introduce pre-flight validations for EnsureLoadBalancer Introduce pre-flight validations adding pre-flight checks for EnsureLoadBalancer with tasks to validate Service object constraints prior making calls to the provider. This aims to prevent changes to the resources when invalid configuration is provided. Currently only NLB target group attributes validations is added as part of this change. feat/tg-attr: support target group attrib annotation on NLB --- pkg/providers/v1/aws_validations.go | 116 ++++++++++ pkg/providers/v1/aws_validations_test.go | 268 +++++++++++++++++++++++ 2 files changed, 384 insertions(+) create mode 100644 pkg/providers/v1/aws_validations.go create mode 100644 pkg/providers/v1/aws_validations_test.go diff --git a/pkg/providers/v1/aws_validations.go b/pkg/providers/v1/aws_validations.go new file mode 100644 index 0000000000..51e26e2a3d --- /dev/null +++ b/pkg/providers/v1/aws_validations.go @@ -0,0 +1,116 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package aws + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" +) + +// validationInput is the input parameters for validations. +// TODO: ensure validations receive copy of values preventing mutation. +type awsValidationInput struct { + apiService *v1.Service + annotations map[string]string +} + +// ensureLoadBalancerValidation validates the Service configuration early on EnsureLoadBalancer. +// It validates the Service annotations and other constraints provided by the user are valid and supported by the controller. +// It does not validate the AWS constraints. +// +// input: +// v: awsValidationInput containing the required configuration to validate the Service object. +// +// returns: +// - error: validation errors. +func ensureLoadBalancerValidation(v *awsValidationInput) error { + // Validate Service annotations. + if err := validateServiceAnnotations(v); err != nil { + return err + } + + // TODO: migrate other validations from EnsureLoadBalancer to this function. + return nil +} + +// validateServiceAnnotations validates the service annotations constraints provided by the user +// are valid and supported by the controller. +func validateServiceAnnotations(v *awsValidationInput) error { + isNLB := isNLB(v.annotations) + + // ServiceAnnotationLoadBalancerTargetGroupAttributes + if _, present := v.annotations[ServiceAnnotationLoadBalancerTargetGroupAttributes]; present { + if !isNLB { + return fmt.Errorf("target group annotations attribute is only supported for NLB") + } + if err := validateServiceAnnotationTargetGroupAttributes(v); err != nil { + return err + } + } + return nil +} + +// validateServiceAnnotationTargetGroupAttributes validates the target group attributes set through annotation: +// Annotation: service.beta.kubernetes.io/aws-load-balancer-target-group-attributes +// +// input: +// v: awsValidationInput containing the required configuration to validate the Service object. +// +// returns: +// - error: validation errors. +func validateServiceAnnotationTargetGroupAttributes(v *awsValidationInput) error { + errPrefix := "error validating target group attributes" + + // Attributes are in format key=value separated by comma. + annotationGroupAttributes := getKeyValuePropertiesFromAnnotation(v.annotations, ServiceAnnotationLoadBalancerTargetGroupAttributes) + targetGroupAttributes := make(map[string]string, len(annotationGroupAttributes)) + + for attrKey, attrValue := range annotationGroupAttributes { + if _, ok := targetGroupAttributes[attrKey]; ok { + return fmt.Errorf("%s: %q is set twice in the annotation", errPrefix, attrKey) + } + if len(attrValue) == 0 { + return fmt.Errorf("%s: attribute value is empty for %q", errPrefix, attrKey) + } + + switch attrKey { + case targetGroupAttributePreserveClientIPEnabled: + if attrValue != "true" && attrValue != "false" { + return fmt.Errorf("%s: invalid attribute value for %q: %s", errPrefix, attrKey, attrValue) + } + // AWS restriction: Client IP preservation can't be disabled for UDP and TCP_UDP target groups. + for _, port := range v.apiService.Spec.Ports { + if (port.Protocol == v1.ProtocolUDP || port.Protocol == "TCP_UDP") && attrValue == "false" { + return fmt.Errorf("%s: client IP preservation can't be disabled for UDP ports", errPrefix) + } + } + targetGroupAttributes[attrKey] = attrValue + + case targetGroupAttributeProxyProtocolV2Enabled: + if attrValue != "true" && attrValue != "false" { + return fmt.Errorf("%s: invalid attribute value for %q: %s", errPrefix, attrKey, attrValue) + } + targetGroupAttributes[attrKey] = attrValue + + default: + return fmt.Errorf("%s: the attribute %q is not supported by the controller or is invalid", errPrefix, attrKey) + } + } + + return nil +} diff --git a/pkg/providers/v1/aws_validations_test.go b/pkg/providers/v1/aws_validations_test.go new file mode 100644 index 0000000000..3f33774219 --- /dev/null +++ b/pkg/providers/v1/aws_validations_test.go @@ -0,0 +1,268 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package aws + +import ( + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestValidateServiceAnnotationTargetGroupAttributes(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + servicePorts []v1.ServicePort + expectedError string + }{ + { + name: "no target group attributes annotation", + annotations: map[string]string{}, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "empty target group attributes annotation", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "valid preserve_client_ip.enabled=true", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "valid preserve_client_ip.enabled=false", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "valid proxy_protocol_v2.enabled=true", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "proxy_protocol_v2.enabled=true", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "valid proxy_protocol_v2.enabled=false", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "proxy_protocol_v2.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "valid multiple attributes", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true,proxy_protocol_v2.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", + }, + { + name: "duplicate attribute in annotation (last one wins - no error expected)", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true,preserve_client_ip.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "", // getKeyValuePropertiesFromAnnotation overwrites, so no duplicate detection + }, + { + name: "empty attribute value", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "attribute value is empty for \"preserve_client_ip.enabled\"", + }, + { + name: "invalid preserve_client_ip.enabled value", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=invalid", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "invalid attribute value for \"preserve_client_ip.enabled\": invalid", + }, + { + name: "invalid proxy_protocol_v2.enabled value", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "proxy_protocol_v2.enabled=yes", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "invalid attribute value for \"proxy_protocol_v2.enabled\": yes", + }, + { + name: "unsupported attribute", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "unsupported_attribute=value", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "the attribute \"unsupported_attribute\" is not supported by the controller or is invalid", + }, + { + name: "preserve_client_ip.enabled=false with UDP port should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 53, Protocol: v1.ProtocolUDP}, + }, + expectedError: "client IP preservation can't be disabled for UDP ports", + }, + { + name: "preserve_client_ip.enabled=false with TCP_UDP port should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 53, Protocol: "TCP_UDP"}, + }, + expectedError: "client IP preservation can't be disabled for UDP ports", + }, + { + name: "preserve_client_ip.enabled=true with UDP port should succeed", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true", + }, + servicePorts: []v1.ServicePort{ + {Port: 53, Protocol: v1.ProtocolUDP}, + }, + expectedError: "", + }, + { + name: "preserve_client_ip.enabled=false with mixed TCP and UDP ports should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + {Port: 53, Protocol: v1.ProtocolUDP}, + }, + expectedError: "client IP preservation can't be disabled for UDP ports", + }, + { + name: "multiple attributes with preserve_client_ip.enabled=false and UDP port should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false,proxy_protocol_v2.enabled=true", + }, + servicePorts: []v1.ServicePort{ + {Port: 53, Protocol: v1.ProtocolUDP}, + }, + expectedError: "client IP preservation can't be disabled for UDP ports", + }, + { + name: "case sensitivity - preserve_client_ip.enabled with True should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=True", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "invalid attribute value for \"preserve_client_ip.enabled\": True", + }, + { + name: "case sensitivity - proxy_protocol_v2.enabled with FALSE should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "proxy_protocol_v2.enabled=FALSE", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "invalid attribute value for \"proxy_protocol_v2.enabled\": FALSE", + }, + { + name: "whitespace in attribute values should fail", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled= true ", + }, + servicePorts: []v1.ServicePort{ + {Port: 80, Protocol: v1.ProtocolTCP}, + }, + expectedError: "invalid attribute value for \"preserve_client_ip.enabled\":", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a test service with the specified ports + service := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: "test-namespace", + Annotations: tt.annotations, + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeLoadBalancer, + Ports: tt.servicePorts, + }, + } + + // Create validation input + input := &awsValidationInput{ + apiService: service, + annotations: tt.annotations, + } + + // Execute the validation + err := validateServiceAnnotationTargetGroupAttributes(input) + + // Verify the result + if tt.expectedError == "" { + assert.NoError(t, err, "Expected no error for test case: %s", tt.name) + } else { + assert.Error(t, err, "Expected error for test case: %s", tt.name) + assert.Contains(t, err.Error(), tt.expectedError, "Error message should contain expected text for test case: %s", tt.name) + } + }) + } +} From 396ab55a2e15d40b9c3b034274c7c31f737d74a1 Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Wed, 20 Aug 2025 09:19:05 -0300 Subject: [PATCH 4/4] feat/tg-attr: support target group attrib annotation on NLB Introduce the target group annotation[1] for all listeners on a Service type-loadBalancer NLB. [1] Annotation service.beta.kubernetes.io/aws-load-balancer-target-group-attributes The annotation provides a interface for users to opt into non-default configurations of a target group when creating or updating a Service. This change also provides a fix for a critical hairpin bug impacting NLB default configuration (using target type instance), which disables the 'preserve source ip configuration' attribute, leading to timeouts in such scenario. --- pkg/providers/v1/aws.go | 28 + pkg/providers/v1/aws_loadbalancer.go | 155 +++++ pkg/providers/v1/aws_loadbalancer_test.go | 737 ++++++++++++++++++++++ 3 files changed, 920 insertions(+) diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index e1070eecd4..024dab2ada 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -216,6 +216,12 @@ const ServiceAnnotationLoadBalancerHCTimeout = "service.beta.kubernetes.io/aws-l // service to specify, in seconds, the interval between health checks. const ServiceAnnotationLoadBalancerHCInterval = "service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval" +// ServiceAnnotationLoadBalancerTargetGroupAttributes is the annotation used on the +// service to specify a comma-separated list of key-value pairs which will be applied as +// target group attributes. +// For example: "preserve_client_ip.enabled=false,proxy_protocol_v2.enabled=true" +const ServiceAnnotationLoadBalancerTargetGroupAttributes = "service.beta.kubernetes.io/aws-load-balancer-target-group-attributes" + // ServiceAnnotationLoadBalancerEIPAllocations is the annotation used on the // service to specify a comma separated list of EIP allocations to use as // static IP addresses for the NLB. Only supported on elbv2 (NLB) @@ -267,6 +273,20 @@ const ( regularAvailabilityZoneType = "availability-zone" ) +// Target Group Attributes +// https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2@main/types#TargetGroupAttribute +const ( + // targetGroupAttributePreserveClientIPEnabled is the target group attribute preserve_client_ip.enabled. + // Indicates whether client IP preservation is enabled. + // Valid values are true or false. + targetGroupAttributePreserveClientIPEnabled = "preserve_client_ip.enabled" + + // targetGroupAttributeProxyProtocolV2Enabled is the target group attribute proxy_protocol_v2.enabled. + // Indicates whether Proxy Protocol version 2 is enabled. + // Valid values are true or false. + targetGroupAttributeProxyProtocolV2Enabled = "proxy_protocol_v2.enabled" +) + // awsTagNameMasterRoles is a set of well-known AWS tag names that indicate the instance is a master var awsTagNameMasterRoles = sets.NewString("kubernetes.io/role/master", "k8s.io/role/master") @@ -2147,6 +2167,14 @@ func (c *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, apiS klog.V(2).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v, %v)", clusterName, apiService.Namespace, apiService.Name, c.region, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, annotations) + // pre-flight validations for EnsureLoadBalancer. + if err := ensureLoadBalancerValidation(&awsValidationInput{ + apiService: apiService, + annotations: annotations, + }); err != nil { + return nil, err + } + if apiService.Spec.SessionAffinity != v1.ServiceAffinityNone { // ELB supports sticky sessions, but only when configured for HTTP/HTTPS return nil, fmt.Errorf("unsupported load balancer affinity: %v", apiService.Spec.SessionAffinity) diff --git a/pkg/providers/v1/aws_loadbalancer.go b/pkg/providers/v1/aws_loadbalancer.go index ca95a3ab68..d45c21bd2d 100644 --- a/pkg/providers/v1/aws_loadbalancer.go +++ b/pkg/providers/v1/aws_loadbalancer.go @@ -400,6 +400,14 @@ func (c *Cloud) ensureLoadBalancerv2(ctx context.Context, namespacedName types.N loadBalancer = &loadBalancers.LoadBalancers[0] } } + + // Reconcile target group attributes. + if _, present := annotations[ServiceAnnotationLoadBalancerTargetGroupAttributes]; present { + if err := c.reconcileTargetGroupsAttributes(ctx, aws.ToString(loadBalancer.LoadBalancerArn), annotations); err != nil { + return nil, fmt.Errorf("error reconciling target group attributes: %q", err) + } + } + return loadBalancer, nil } @@ -493,9 +501,156 @@ func (c *Cloud) reconcileLBAttributes(ctx context.Context, loadBalancerArn strin return fmt.Errorf("unable to update load balancer attributes during attribute sync: %q", err) } } + + return nil +} + +// reconcileTargetGroupsAttributes reconciles the target group attributes for all target groups +// associated with a load balancer to match the desired state specified in service annotations. +// Only supported attributes by controller are reconciled. +// +// Parameters: +// - ctx: context for AWS API calls with timeout and cancellation support +// - lbARN: AWS load balancer ARN to identify which target groups to process +// - annotations: service annotations containing desired target group attribute configuration +// +// Returns: +// - error: validation errors, AWS API errors, or target group attribute update failures +// +// Documentation generated by Cursor AI +func (c *Cloud) reconcileTargetGroupsAttributes(ctx context.Context, lbARN string, annotations map[string]string) error { + if len(lbARN) == 0 { + return fmt.Errorf("error updating target groups attributes: load balancer ARN is empty") + } + + describeTargetGroupsOutput, err := c.elbv2.DescribeTargetGroups(ctx, &elbv2.DescribeTargetGroupsInput{ + LoadBalancerArn: aws.String(lbARN), + }) + if err != nil { + return fmt.Errorf("error updating target groups attributes from load balancer %q: %w", lbARN, err) + } + + var errs []error + for _, tg := range describeTargetGroupsOutput.TargetGroups { + err := c.ensureTargetGroupAttributes(ctx, &tg, annotations) + if err != nil { + errs = append(errs, fmt.Errorf("error updating target group attributes for target group %q: %w", aws.ToString(tg.TargetGroupArn), err)) + } + } + if len(errs) > 0 { + return fmt.Errorf("one or more errors occurred while updating target group attributes: %v", errs) + } + return nil +} + +// ensureTargetGroupAttributes ensures that the target group attributes for a specific +// target group match the desired state specified in service annotations. +// +// Parameters: +// - ctx: context for AWS API calls and cancellation +// - tg: target group object containing ARN, protocol, and type information +// - annotations: service annotations containing desired target group attributes +// +// Returns: +// - error: validation errors, AWS API errors, or attribute building errors +// +// Documentation generated by Cursor AI +func (c *Cloud) ensureTargetGroupAttributes(ctx context.Context, tg *elbv2types.TargetGroup, annotations map[string]string) error { + if tg == nil { + return fmt.Errorf("unable to reconcile target group attributes: target group is required") + } + + tgAttributes, err := c.elbv2.DescribeTargetGroupAttributes(ctx, &elbv2.DescribeTargetGroupAttributesInput{ + TargetGroupArn: tg.TargetGroupArn, + }) + if err != nil { + return fmt.Errorf("unable to retrieve target group attributes during attribute sync: %w", err) + } + + desiredTargetGroupAttributes, err := c.buildTargetGroupAttributes(tg, tgAttributes.Attributes, annotations) + if err != nil { + return fmt.Errorf("unable to build target group attributes: %w", err) + } + + if len(desiredTargetGroupAttributes) == 0 { + return nil + } + klog.Infof("Updating attributes for target group %q", aws.ToString(tg.TargetGroupArn)) + + if _, err = c.elbv2.ModifyTargetGroupAttributes(ctx, &elbv2.ModifyTargetGroupAttributesInput{ + TargetGroupArn: tg.TargetGroupArn, + Attributes: desiredTargetGroupAttributes, + }); err != nil { + return fmt.Errorf("unable to modify target group attributes during attribute sync: %w", err) + } + klog.Infof("Successfully updated target group attributes for %q", aws.ToString(tg.TargetGroupArn)) + return nil } +// buildTargetGroupAttributes builds the list of target group attributes that need to be modified +// based on the Service annotation, and current attribute values, calculating only the attributes +// to be changed. +// +// Supported values to annotation ServiceAnnotationLoadBalancerTargetGroupAttributes: +// - preserve_client_ip.enabled=true|false - whether to preserve client IP addresses +// - proxy_protocol_v2.enabled=true|false - whether to enable proxy protocol v2 + +// Behavior when no annotations provided or removed: +// - Target groups preserves the last set values, and skips any changes. +// +// Parameters: +// - tg: target group object +// - tgAttributes: current target group attributes from AWS resource +// - annotations: service annotations containing desired attribute values +// +// Returns: +// - []elbv2types.TargetGroupAttribute: list of attributes that need to be modified +// - error: validation errors, parsing errors, or AWS restrictions +// +// Documentation generated by Cursor AI +func (c *Cloud) buildTargetGroupAttributes(tg *elbv2types.TargetGroup, tgAttributes []elbv2types.TargetGroupAttribute, annotations map[string]string) ([]elbv2types.TargetGroupAttribute, error) { + errPrefix := "error building target group attributes" + if tg == nil { + return nil, fmt.Errorf("%s: target group is nil", errPrefix) + } + if tgAttributes == nil { + return nil, fmt.Errorf("%s: target group attributes are nil", errPrefix) + } + + // existingAttributes are current target group attributes from AWS. + existingAttributes := make(map[string]string, len(tgAttributes)) + for _, attr := range tgAttributes { + existingAttributes[aws.ToString(attr.Key)] = aws.ToString(attr.Value) + } + + // annotationAttributes are the user-defined attributes set through annotations. + annotationAttributes := getKeyValuePropertiesFromAnnotation(annotations, ServiceAnnotationLoadBalancerTargetGroupAttributes) + + // Calculate attribute difference between current and desired state. + var diff []elbv2types.TargetGroupAttribute + for attrKey, attrValue := range annotationAttributes { + // Skip non-supported attributes by controller. + if _, ok := existingAttributes[attrKey]; !ok { + klog.V(2).Infof("Skipping non-supported target group attribute %q", attrKey) + continue + } + + // Calculate the target value: annotation override > current value. + if attrValue == existingAttributes[attrKey] { + klog.V(2).Infof("Skipping changes to target group attribute %q, values are the same: %q", attrKey, attrValue) + continue + } + klog.V(2).Infof("Setting from annotation the target group attribute %q value from %q to %q", attrKey, existingAttributes[attrKey], attrValue) + + diff = append(diff, elbv2types.TargetGroupAttribute{ + Key: aws.String(attrKey), + Value: aws.String(attrValue), + }) + } + return diff, nil +} + var invalidELBV2NameRegex = regexp.MustCompile("[^[:alnum:]]") // buildTargetGroupName will build unique name for targetGroup of service & port. diff --git a/pkg/providers/v1/aws_loadbalancer_test.go b/pkg/providers/v1/aws_loadbalancer_test.go index 0fab6469b0..569c4434db 100644 --- a/pkg/providers/v1/aws_loadbalancer_test.go +++ b/pkg/providers/v1/aws_loadbalancer_test.go @@ -30,6 +30,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" elbtypes "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing/types" + elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2" elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types" "github.com/stretchr/testify/assert" @@ -1120,3 +1121,739 @@ func TestEnsureSSLNegotiationPolicyErrorHandling(t *testing.T) { }) } } + +// Unit test generated by Cursor AI, reviewed by Human +func TestCloud_buildTargetGroupAttributes(t *testing.T) { + tests := []struct { + name string + targetGroup *elbv2types.TargetGroup + existingAttributes []elbv2types.TargetGroupAttribute + annotations map[string]string + expectedAttributes []elbv2types.TargetGroupAttribute + expectedError string + }{ + // Invalid AWS constraints are validated by pre-flight (validateServiceAnnotationTargetGroupAttributes). + // Examples: + // - preserve_client_ip.enabled=false for UDP target + // - preserve_client_ip.enabled=false for TCP_UDP target + // Unsupported attributes by controller are validated by pre-flight. + // Examples: + // - unsupported_attribute=value + // - different attribute names than supported by controller: + // - preserve_client_ip.enabled + // - proxy_protocol_v2.enabled + // Malformed annotations are validated by pre-flight. + // Duplicate attributes are validated by pre-flight. + { + name: "nil target group should return error", + targetGroup: nil, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + }, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + expectedError: "error building target group attributes: target group is nil", + }, + { + name: "nil existing attributes should return error", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: nil, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + expectedError: "error building target group attributes: target group attributes are nil", + }, + { + name: "no target group attributes annotation", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("some_key"), Value: aws.String("some_value")}, + }, + annotations: map[string]string{}, + expectedAttributes: []elbv2types.TargetGroupAttribute{}, + }, + { + name: "annotation parsing - empty annotation should return empty diff", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + }, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "", + }, + expectedAttributes: []elbv2types.TargetGroupAttribute{}, + }, + { + name: "valid preserve_client_ip.enabled=true for instance target", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("false")}, + }, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true", + }, + expectedAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + }, + }, + { + name: "valid preserve_client_ip.enabled=false for IP target with TCP", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumIp, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + }, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false", + }, + expectedAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("false")}, + }, + }, + { + name: "valid proxy_protocol_v2.enabled=true", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "proxy_protocol_v2.enabled=true", + }, + expectedAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("true")}, + }, + }, + { + name: "multiple attributes", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("false")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true,proxy_protocol_v2.enabled=true", + }, + expectedAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("true")}, + }, + }, + { + name: "no changes needed - attributes match defaults", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + existingAttributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + annotations: map[string]string{}, + expectedAttributes: []elbv2types.TargetGroupAttribute{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Cloud{} + result, err := c.buildTargetGroupAttributes(tt.targetGroup, tt.existingAttributes, tt.annotations) + + if len(tt.expectedError) > 0 { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + assert.Nil(t, result) + } else { + assert.NoError(t, err) + assert.Equal(t, len(tt.expectedAttributes), len(result)) + + // Convert to maps for easier comparison since order might vary + expectedMap := make(map[string]string) + for _, attr := range tt.expectedAttributes { + expectedMap[aws.ToString(attr.Key)] = aws.ToString(attr.Value) + } + + resultMap := make(map[string]string) + for _, attr := range result { + resultMap[aws.ToString(attr.Key)] = aws.ToString(attr.Value) + } + + assert.Equal(t, expectedMap, resultMap) + } + }) + } +} + +// Unit test generated by Cursor AI +func TestGetKeyValuePropertiesFromAnnotation_TargetGroupAttributes(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + annotation string + expected map[string]string + }{ + { + name: "valid target group attributes", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true,proxy_protocol_v2.enabled=false", + }, + annotation: ServiceAnnotationLoadBalancerTargetGroupAttributes, + expected: map[string]string{ + "preserve_client_ip.enabled": "true", + "proxy_protocol_v2.enabled": "false", + }, + }, + { + name: "single attribute", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true", + }, + annotation: ServiceAnnotationLoadBalancerTargetGroupAttributes, + expected: map[string]string{ + "preserve_client_ip.enabled": "true", + }, + }, + { + name: "empty annotation", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: "", + }, + annotation: ServiceAnnotationLoadBalancerTargetGroupAttributes, + expected: map[string]string{}, + }, + { + name: "annotation with spaces", + annotations: map[string]string{ + ServiceAnnotationLoadBalancerTargetGroupAttributes: " preserve_client_ip.enabled=true , proxy_protocol_v2.enabled=false ", + }, + annotation: ServiceAnnotationLoadBalancerTargetGroupAttributes, + expected: map[string]string{ + "preserve_client_ip.enabled": "true", + "proxy_protocol_v2.enabled": "false", + }, + }, + { + name: "annotation not present", + annotations: map[string]string{ + "other.annotation": "value", + }, + annotation: ServiceAnnotationLoadBalancerTargetGroupAttributes, + expected: map[string]string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getKeyValuePropertiesFromAnnotation(tt.annotations, tt.annotation) + assert.Equal(t, tt.expected, result) + }) + } +} + +// Test-specific mock for ELB v2 client that embeds MockedFakeELBV2 +type mockELBV2ClientForTargetGroupAttributes struct { + *MockedFakeELBV2 + describeTargetGroupsFunc func(ctx context.Context, input *elbv2.DescribeTargetGroupsInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupsOutput, error) + describeTargetGroupAttributesFunc func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) + modifyTargetGroupAttributesFunc func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) +} + +func (m *mockELBV2ClientForTargetGroupAttributes) DescribeTargetGroups(ctx context.Context, input *elbv2.DescribeTargetGroupsInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupsOutput, error) { + if m.describeTargetGroupsFunc != nil { + return m.describeTargetGroupsFunc(ctx, input, optFns...) + } + // Fall back to the embedded MockedFakeELBV2 implementation + return m.MockedFakeELBV2.DescribeTargetGroups(ctx, input, optFns...) +} + +func (m *mockELBV2ClientForTargetGroupAttributes) DescribeTargetGroupAttributes(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + if m.describeTargetGroupAttributesFunc != nil { + return m.describeTargetGroupAttributesFunc(ctx, input, optFns...) + } + return nil, fmt.Errorf("DescribeTargetGroupAttributes not mocked") +} + +func (m *mockELBV2ClientForTargetGroupAttributes) ModifyTargetGroupAttributes(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + if m.modifyTargetGroupAttributesFunc != nil { + return m.modifyTargetGroupAttributesFunc(ctx, input, optFns...) + } + return nil, fmt.Errorf("ModifyTargetGroupAttributes not mocked") +} + +// Unit test generated by Cursor AI +func TestCloud_ensureTargetGroupAttributes(t *testing.T) { + testTargetGroup := &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + } + + tests := []struct { + name string + targetGroup *elbv2types.TargetGroup + annotations map[string]string + mockDescribeTargetGroupAttribs func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) + mockModifyTargetGroupAttribs func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) + expectedError string + description string + }{ + { + name: "nil target group should return error", + targetGroup: nil, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true"}, + expectedError: "unable to reconcile target group attributes: target group is required", + description: "Function should validate target group is not nil before proceeding", + }, + // DescribeTargetGroupAttributes failure + { + name: "DescribeTargetGroupAttributes fails", + targetGroup: testTargetGroup, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false"}, + mockDescribeTargetGroupAttribs: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return nil, fmt.Errorf("AWS API error: target group not found") + }, + expectedError: "unable to retrieve target group attributes during attribute sync", + description: "Function should handle DescribeTargetGroupAttributes API failures", + }, + // No changes needed - attributes match (successful case with no updates) + { + name: "no changes needed - attributes already match desired state", + targetGroup: testTargetGroup, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true,proxy_protocol_v2.enabled=false"}, + mockDescribeTargetGroupAttribs: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, // matches annotation + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, // matches annotation + }, + }, nil + }, + description: "Function should succeed when attributes already match desired state", + }, + // No changes needed - no annotations (restore defaults, but they already match) + { + name: "no changes needed - no annotations and attributes match defaults", + targetGroup: testTargetGroup, + annotations: map[string]string{}, // No target group attributes annotation + mockDescribeTargetGroupAttribs: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, // matches default for instance target + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, // matches default + }, + }, nil + }, + description: "Function should succeed when no annotation provided and attributes match defaults", + }, + { + name: "ModifyTargetGroupAttributes fails", + targetGroup: testTargetGroup, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false"}, + mockDescribeTargetGroupAttribs: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, // different from annotation (false) + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, // matches default + }, + }, nil + }, + mockModifyTargetGroupAttribs: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + return nil, fmt.Errorf("AWS API error: access denied") + }, + expectedError: "unable to modify target group attributes during attribute sync", + description: "Function should handle ModifyTargetGroupAttributes API failures", + }, + // Successful case - changes needed and applied + { + name: "successful case - attributes updated", + targetGroup: testTargetGroup, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=false,proxy_protocol_v2.enabled=true"}, + mockDescribeTargetGroupAttribs: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, // different from annotation (false) + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, // different from annotation (true) + }, + }, nil + }, + mockModifyTargetGroupAttribs: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + expectedAttributes := map[string]string{ + "preserve_client_ip.enabled": "false", + "proxy_protocol_v2.enabled": "true", + } + + for _, attr := range input.Attributes { + key := aws.ToString(attr.Key) + value := aws.ToString(attr.Value) + if expectedValue, exists := expectedAttributes[key]; exists { + if value != expectedValue { + return nil, fmt.Errorf("unexpected attribute value for %s: got %s, expected %s", key, value, expectedValue) + } + } + } + + return &elbv2.ModifyTargetGroupAttributesOutput{}, nil + }, + description: "Function should successfully update target group attributes", + }, + // Successful case - restore defaults + { + name: "successful case - restore defaults for IP+TCP target group", + targetGroup: &elbv2types.TargetGroup{ + TargetGroupArn: aws.String("arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-ip-tg/1234567890123456"), + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumIp, + }, + annotations: map[string]string{}, // No annotation - should restore defaults + mockDescribeTargetGroupAttribs: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, // wrong, should be false for IP+TCP + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, // correct default + }, + }, nil + }, + mockModifyTargetGroupAttribs: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + // Should restore preserve_client_ip.enabled to false for IP+TCP combination + for _, attr := range input.Attributes { + if aws.ToString(attr.Key) == "preserve_client_ip.enabled" && aws.ToString(attr.Value) == "false" { + return &elbv2.ModifyTargetGroupAttributesOutput{}, nil + } + } + return nil, fmt.Errorf("expected preserve_client_ip.enabled=false to be set") + }, + description: "Function should successfully restore default values for IP+TCP target group combination", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockClient := &mockELBV2ClientForTargetGroupAttributes{ + MockedFakeELBV2: &MockedFakeELBV2{ + LoadBalancers: []elbv2types.LoadBalancer{}, + TargetGroups: []elbv2types.TargetGroup{}, + Listeners: []elbv2types.Listener{}, + LoadBalancerAttributes: make(map[string]map[string]string), + Tags: make(map[string][]elbv2types.Tag), + RegisteredInstances: make(map[string][]string), + }, + describeTargetGroupAttributesFunc: tt.mockDescribeTargetGroupAttribs, + modifyTargetGroupAttributesFunc: tt.mockModifyTargetGroupAttribs, + } + c := &Cloud{ + elbv2: mockClient, + } + + err := c.ensureTargetGroupAttributes(context.TODO(), tt.targetGroup, tt.annotations) + + if len(tt.expectedError) > 0 { + assert.Error(t, err, "Expected error for test case: %s", tt.description) + assert.Contains(t, err.Error(), tt.expectedError, "Error message should contain expected text for test case: %s", tt.description) + } else { + assert.NoError(t, err, "Expected no error for test case: %s", tt.description) + } + }) + } +} + +func TestCloud_reconcileTargetGroupsAttributes(t *testing.T) { + testLBARN := "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/net/test-lb/1234567890123456" + testTG1ARN := "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-1/1234567890123456" + testTG2ARN := "arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-2/1234567890123456" + + tests := []struct { + name string + lbARN string + annotations map[string]string + targetGroups []elbv2types.TargetGroup + describeTargetGroupsError error + describeTargetGroupAttributesFunc func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) + modifyTargetGroupAttributesFunc func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) + expectedError string + }{ + { + name: "empty load balancer ARN should return error", + lbARN: "", + annotations: map[string]string{}, + expectedError: "error updating target groups attributes: load balancer ARN is empty", + }, + { + name: "DescribeTargetGroups API failure", + lbARN: testLBARN, + annotations: map[string]string{}, + describeTargetGroupsError: fmt.Errorf("AWS API error: access denied"), + expectedError: "error updating target groups attributes from load balancer \"arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/net/test-lb/1234567890123456\": AWS API error: access denied", + }, + { + name: "no target groups found - success", + lbARN: testLBARN, + annotations: map[string]string{}, + targetGroups: []elbv2types.TargetGroup{}, + }, + { + name: "single target group - success", + lbARN: testLBARN, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true"}, + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("false")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + }, nil + }, + modifyTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + return &elbv2.ModifyTargetGroupAttributesOutput{}, nil + }, + }, + { + name: "multiple target groups - success", + lbARN: testLBARN, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "proxy_protocol_v2.enabled=true"}, + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + { + TargetGroupArn: aws.String(testTG2ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + }, nil + }, + modifyTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + return &elbv2.ModifyTargetGroupAttributesOutput{}, nil + }, + }, + { + name: "partial failure - some target groups fail", + lbARN: testLBARN, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true"}, + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + { + TargetGroupArn: aws.String(testTG2ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + if aws.ToString(input.TargetGroupArn) == testTG1ARN { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("false")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + }, nil + } + return nil, fmt.Errorf("target group not found") + }, + modifyTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + return &elbv2.ModifyTargetGroupAttributesOutput{}, nil + }, + expectedError: "one or more errors occurred while updating target group attributes: [error updating target group attributes for target group \"arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-2/1234567890123456\": unable to retrieve target group attributes during attribute sync: target group not found]", + }, + { + name: "all target groups fail", + lbARN: testLBARN, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true"}, + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + { + TargetGroupArn: aws.String(testTG2ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return nil, fmt.Errorf("target group not found") + }, + expectedError: "one or more errors occurred while updating target group attributes: [error updating target group attributes for target group \"arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-1/1234567890123456\": unable to retrieve target group attributes during attribute sync: target group not found error updating target group attributes for target group \"arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-2/1234567890123456\": unable to retrieve target group attributes during attribute sync: target group not found]", + }, + { + name: "ModifyTargetGroupAttributes fails for some target groups", + lbARN: testLBARN, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true"}, + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + { + TargetGroupArn: aws.String(testTG2ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumTcp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("false")}, + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, + }, + }, nil + }, + modifyTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.ModifyTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.ModifyTargetGroupAttributesOutput, error) { + if aws.ToString(input.TargetGroupArn) == testTG1ARN { + return &elbv2.ModifyTargetGroupAttributesOutput{}, nil + } + return nil, fmt.Errorf("permission denied") + }, + expectedError: "one or more errors occurred while updating target group attributes: [error updating target group attributes for target group \"arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-2/1234567890123456\": unable to modify target group attributes during attribute sync: permission denied]", + }, + { + name: "buildTargetGroupAttributes fails due to nil existing attributes", + lbARN: testLBARN, + annotations: map[string]string{ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true"}, + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: nil, // This will cause buildTargetGroupAttributes to fail + }, nil + }, + expectedError: "one or more errors occurred while updating target group attributes: [error updating target group attributes for target group \"arn:aws:elasticloadbalancing:us-east-1:123456789012:targetgroup/test-tg-1/1234567890123456\": unable to build target group attributes: error building target group attributes: target group attributes are nil]", + }, + { + name: "no annotations - success", + lbARN: testLBARN, + annotations: map[string]string{}, // No target group attributes annotation + targetGroups: []elbv2types.TargetGroup{ + { + TargetGroupArn: aws.String(testTG1ARN), + LoadBalancerArns: []string{testLBARN}, + Protocol: elbv2types.ProtocolEnumHttp, + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + describeTargetGroupAttributesFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupAttributesInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupAttributesOutput, error) { + return &elbv2.DescribeTargetGroupAttributesOutput{ + Attributes: []elbv2types.TargetGroupAttribute{ + {Key: aws.String("preserve_client_ip.enabled"), Value: aws.String("true")}, // Already at default + {Key: aws.String("proxy_protocol_v2.enabled"), Value: aws.String("false")}, // Already at default + }, + }, nil + }, + // No ModifyTargetGroupAttributes function since no changes needed + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var mockClient *mockELBV2ClientForTargetGroupAttributes + + // For empty ARN test, we don't need to set up mocks + if tt.lbARN != "" { + mockELBV2 := &MockedFakeELBV2{ + TargetGroups: tt.targetGroups, + } + + // Override DescribeTargetGroups if we need to simulate error + if tt.describeTargetGroupsError != nil { + // Create a custom mock that returns error for DescribeTargetGroups + mockClient = &mockELBV2ClientForTargetGroupAttributes{ + MockedFakeELBV2: &MockedFakeELBV2{}, + describeTargetGroupsFunc: func(ctx context.Context, input *elbv2.DescribeTargetGroupsInput, optFns ...func(*elbv2.Options)) (*elbv2.DescribeTargetGroupsOutput, error) { + return nil, tt.describeTargetGroupsError + }, + } + } else { + mockClient = &mockELBV2ClientForTargetGroupAttributes{ + MockedFakeELBV2: mockELBV2, + } + } + + // Set up target group attribute functions + if tt.describeTargetGroupAttributesFunc != nil { + mockClient.describeTargetGroupAttributesFunc = tt.describeTargetGroupAttributesFunc + } + if tt.modifyTargetGroupAttributesFunc != nil { + mockClient.modifyTargetGroupAttributesFunc = tt.modifyTargetGroupAttributesFunc + } + } + + c := &Cloud{ + elbv2: mockClient, + } + + err := c.reconcileTargetGroupsAttributes(context.TODO(), tt.lbARN, tt.annotations) + if err != nil { + if len(tt.expectedError) == 0 { + t.Fatalf("Expected no error for test case: %s, but got: %v", tt.name, err) + } + assert.Error(t, err, "Expected error for test case: %s", tt.name) + assert.Equal(t, tt.expectedError, err.Error(), "Error message should contain expected text for test case: %s", tt.name) + } else { + assert.NoError(t, err, "Expected no error for test case: %s", tt.name) + } + }) + } +}