Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pkg/providers/v1/aws_validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package aws

import (
"fmt"
"strings"

v1 "k8s.io/api/core/v1"
)
Expand Down Expand Up @@ -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.<region>.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
Expand Down
108 changes: 108 additions & 0 deletions pkg/providers/v1/aws_validations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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{
Expand Down
Loading