From 54ac0d6133b980f249d081c8080fd6dd95fdb6f3 Mon Sep 17 00:00:00 2001 From: Marco Braga Date: Mon, 5 Jan 2026 18:09:33 -0300 Subject: [PATCH] Prevent Load Balancer type annotation changes after creation Adds validation to prevent users from changing the Load Balancer type annotation (service.beta.kubernetes.io/aws-load-balancer-type) after the load balancer has been created. This prevents undefined behavior and potential service disruptions. The validation detects the current load balancer type by analyzing the hostname pattern in the service's LoadBalancer status: - Classic Load Balancer: hostname ends with ".elb.amazonaws.com" - Network Load Balancer: hostname ends with ".elb..amazonaws.com" If a mismatch is detected between the annotation and the existing load balancer type, the controller returns a validation error preventing the update. Relatest to Issue 1254 Signed-off-by: Claude (AI Assistant) Co-Authored-By: Marco Braga --- pkg/providers/v1/aws_validations.go | 20 +++++ pkg/providers/v1/aws_validations_test.go | 108 +++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/pkg/providers/v1/aws_validations.go b/pkg/providers/v1/aws_validations.go index ae7c033e1d..cc8c440238 100644 --- a/pkg/providers/v1/aws_validations.go +++ b/pkg/providers/v1/aws_validations.go @@ -18,6 +18,7 @@ package aws import ( "fmt" + "strings" v1 "k8s.io/api/core/v1" ) @@ -53,6 +54,25 @@ func ensureLoadBalancerValidation(v *awsValidationInput) error { func validateServiceAnnotations(v *awsValidationInput) error { isNLB := isNLB(v.annotations) + // ServiceAnnotationLoadBalancerType + // Load Balancer Type annotation must not be updated after creation. + // Classic Load Balancer: hostname ends with ".elb.amazonaws.com" + // NLB: hostname ends with ".elb..amazonaws.com" + { + hostIsCreated := len(v.apiService.Status.LoadBalancer.Ingress) > 0 + if hostIsCreated { + hostIsClassic := strings.HasSuffix(v.apiService.Status.LoadBalancer.Ingress[0].Hostname, ".elb.amazonaws.com") + // If annotation is set to NLB and hostname has classic pattern, return an error. + if isNLB && hostIsClassic { + return fmt.Errorf("cannot update Load Balancer Type annotation %q after creation for NLB", ServiceAnnotationLoadBalancerType) + } + // If annotation is set to CLB and hostname has NLB pattern, return an error. + if !isNLB && !hostIsClassic { + return fmt.Errorf("cannot update Load Balancer Type annotation %q after creation for Classic Load Balancer", ServiceAnnotationLoadBalancerType) + } + } + } + // ServiceAnnotationLoadBalancerSecurityGroups // NLB only: ensure the BYO annotations are not supported and return an error. // FIXME: the BYO SG for NLB implementation is blocked by https://github.com/kubernetes/cloud-provider-aws/pull/1209 diff --git a/pkg/providers/v1/aws_validations_test.go b/pkg/providers/v1/aws_validations_test.go index 9c4aa322cc..01556de518 100644 --- a/pkg/providers/v1/aws_validations_test.go +++ b/pkg/providers/v1/aws_validations_test.go @@ -269,11 +269,14 @@ func TestValidateServiceAnnotationTargetGroupAttributes(t *testing.T) { func TestValidateServiceAnnotations(t *testing.T) { const byoSecurityGroupID = "sg-123456789" + const classicLBHostname = "my-classic-lb-1234567890.us-east-1.elb.amazonaws.com" + const nlbHostname = "my-nlb-1234567890.elb.us-east-1.amazonaws.com" tests := []struct { name string annotations map[string]string servicePorts []v1.ServicePort + ingressStatus []v1.LoadBalancerIngress expectedError string }{ // Valid cases - CLB (Classic Load Balancer) should allow BYO security groups @@ -420,6 +423,108 @@ func TestValidateServiceAnnotations(t *testing.T) { annotations: map[string]string{}, expectedError: "", }, + + // No existing ingress - any type should be allowed + { + name: "NLB in new service with no ingress should be allowed", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "nlb"}, + ingressStatus: nil, + expectedError: "", + }, + { + name: "CLB in new service with no ingress should be allowed", + annotations: map[string]string{}, + ingressStatus: nil, + expectedError: "", + }, + { + name: "NLB in new service with empty ingress list should be allowed", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "nlb"}, + ingressStatus: []v1.LoadBalancerIngress{}, + expectedError: "", + }, + + // Existing Classic LB - same type should succeed + { + name: "CLB in existing service with no type annotation should be allowed", + annotations: map[string]string{}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: classicLBHostname}, + }, + expectedError: "", + }, + { + name: "CLB in existing service with type annotation should be allowed", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "clb"}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: classicLBHostname}, + }, + expectedError: "", + }, + + // Existing NLB - same type should succeed + { + name: "NLB in existing service with type annotation should be allowed", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "nlb"}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: nlbHostname}, + }, + expectedError: "", + }, + + // Type change from CLB to NLB - should fail + { + name: "CLB in existing service with type annotation should not be allowed to change to NLB", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "nlb"}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: classicLBHostname}, + }, + expectedError: "cannot update Load Balancer Type annotation", + }, + + // Type change from NLB to CLB - should fail + { + name: "NLB in existing service with type annotation should not be allowed to change to CLB", + annotations: map[string]string{}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: nlbHostname}, + }, + expectedError: "cannot update Load Balancer Type annotation", + }, + { + name: "NLB in existing service with type annotation should not be allowed to change to CLB", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "clb"}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: nlbHostname}, + }, + expectedError: "cannot update Load Balancer Type annotation", + }, + + // Edge cases with hostname patterns + { + name: "CLB in existing service with regional hostname should be allowed", + annotations: map[string]string{}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: "internal-my-lb-123.eu-west-1.elb.amazonaws.com"}, + }, + expectedError: "", + }, + { + name: "NLB in existing service with different region hostname should be allowed", + annotations: map[string]string{ServiceAnnotationLoadBalancerType: "nlb"}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: "my-nlb-abc123.elb.ap-southeast-1.amazonaws.com"}, + }, + expectedError: "", + }, + { + name: "NLB in existing service with regional hostname should not be allowed to change to CLB", + annotations: map[string]string{}, + ingressStatus: []v1.LoadBalancerIngress{ + {Hostname: "my-nlb-abc123.elb.eu-central-1.amazonaws.com"}, + }, + expectedError: "cannot update Load Balancer Type annotation", + }, } for _, tt := range tests { @@ -436,6 +541,9 @@ func TestValidateServiceAnnotations(t *testing.T) { Ports: tt.servicePorts, }, } + if tt.ingressStatus != nil { + service.Status.LoadBalancer.Ingress = tt.ingressStatus + } // Create validation input input := &awsValidationInput{