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{