Skip to content

Commit 2f6df28

Browse files
mtuliomtulio
committed
svc/nlb/sg/validations: ensure annotations matches NLB SG
Ensure annotation matches feature NLB with Security Groups by preventing standard controller BYO SG annotations due existing controller limitations.
1 parent 76b579a commit 2f6df28

File tree

2 files changed

+207
-1
lines changed

2 files changed

+207
-1
lines changed

‎pkg/providers/v1/aws_validations.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,26 @@ func ensureLoadBalancerValidation(v *awsValidationInput) error {
5353
func validateServiceAnnotations(v *awsValidationInput) error {
5454
isNLB := isNLB(v.annotations)
5555

56+
// ServiceAnnotationLoadBalancerSecurityGroups
57+
// NLB only: ensure the BYO annotations are not supported and return an error.
58+
// FIXME: the BYO SG for NLB implementation is blocked by https://github.com/kubernetes/cloud-provider-aws/pull/1209
59+
if _, hasBYOAnnotation := v.annotations[ServiceAnnotationLoadBalancerSecurityGroups]; hasBYOAnnotation {
60+
if isNLB {
61+
return fmt.Errorf("BYO security group annotation %q is not supported by NLB", ServiceAnnotationLoadBalancerSecurityGroups)
62+
}
63+
}
64+
65+
// ServiceAnnotationLoadBalancerExtraSecurityGroups
66+
if _, hasExtraBYOAnnotation := v.annotations[ServiceAnnotationLoadBalancerExtraSecurityGroups]; hasExtraBYOAnnotation {
67+
if isNLB {
68+
return fmt.Errorf("BYO extra security group annotation %q is not supported by NLB", ServiceAnnotationLoadBalancerExtraSecurityGroups)
69+
}
70+
}
71+
5672
// ServiceAnnotationLoadBalancerTargetGroupAttributes
5773
if _, present := v.annotations[ServiceAnnotationLoadBalancerTargetGroupAttributes]; present {
5874
if !isNLB {
59-
return fmt.Errorf("target group annotations attribute is only supported for NLB")
75+
return fmt.Errorf("target group attributes annotation is only supported for NLB")
6076
}
6177
if err := validateServiceAnnotationTargetGroupAttributes(v); err != nil {
6278
return err

‎pkg/providers/v1/aws_validations_test.go‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,3 +266,193 @@ func TestValidateServiceAnnotationTargetGroupAttributes(t *testing.T) {
266266
})
267267
}
268268
}
269+
270+
func TestValidateServiceAnnotations(t *testing.T) {
271+
const byoSecurityGroupID = "sg-123456789"
272+
273+
tests := []struct {
274+
name string
275+
annotations map[string]string
276+
servicePorts []v1.ServicePort
277+
expectedError string
278+
}{
279+
// Valid cases - CLB (Classic Load Balancer) should allow BYO security groups
280+
{
281+
name: "CLB with BYO SG annotation - success",
282+
annotations: map[string]string{
283+
ServiceAnnotationLoadBalancerSecurityGroups: byoSecurityGroupID,
284+
},
285+
expectedError: "",
286+
},
287+
{
288+
name: "CLB with BYO extra SG annotation - success",
289+
annotations: map[string]string{
290+
ServiceAnnotationLoadBalancerExtraSecurityGroups: byoSecurityGroupID,
291+
},
292+
expectedError: "",
293+
},
294+
{
295+
name: "CLB with both BYO SG annotations - success",
296+
annotations: map[string]string{
297+
ServiceAnnotationLoadBalancerSecurityGroups: byoSecurityGroupID,
298+
ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-extra123",
299+
},
300+
expectedError: "",
301+
},
302+
303+
// Error cases - NLB should reject BYO security group annotations
304+
{
305+
name: "NLB with BYO SG annotation - error (not supported)",
306+
annotations: map[string]string{
307+
ServiceAnnotationLoadBalancerType: "nlb",
308+
ServiceAnnotationLoadBalancerSecurityGroups: byoSecurityGroupID,
309+
},
310+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
311+
},
312+
{
313+
name: "NLB with BYO extra SG annotation - error (not supported)",
314+
annotations: map[string]string{
315+
ServiceAnnotationLoadBalancerType: "nlb",
316+
ServiceAnnotationLoadBalancerExtraSecurityGroups: byoSecurityGroupID,
317+
},
318+
expectedError: "BYO extra security group annotation \"service.beta.kubernetes.io/aws-load-balancer-extra-security-groups\" is not supported by NLB",
319+
},
320+
{
321+
name: "NLB with both BYO SG annotations - error (not supported)",
322+
annotations: map[string]string{
323+
ServiceAnnotationLoadBalancerType: "nlb",
324+
ServiceAnnotationLoadBalancerSecurityGroups: byoSecurityGroupID,
325+
ServiceAnnotationLoadBalancerExtraSecurityGroups: "sg-extra123",
326+
},
327+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
328+
},
329+
{
330+
name: "NLB with BYO SG with empty value - error (not supported)",
331+
annotations: map[string]string{
332+
ServiceAnnotationLoadBalancerType: "nlb",
333+
ServiceAnnotationLoadBalancerSecurityGroups: "",
334+
},
335+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
336+
},
337+
{
338+
name: "NLB with BYO SG with multiple values - error (not supported)",
339+
annotations: map[string]string{
340+
ServiceAnnotationLoadBalancerType: "nlb",
341+
ServiceAnnotationLoadBalancerSecurityGroups: "sg-123,sg-456",
342+
},
343+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
344+
},
345+
{
346+
name: "NLB with single BYO SG - error (not supported)",
347+
annotations: map[string]string{
348+
ServiceAnnotationLoadBalancerType: "nlb",
349+
ServiceAnnotationLoadBalancerSecurityGroups: "sg-123456789",
350+
},
351+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
352+
},
353+
{
354+
name: "NLB with multiple BYO SGs - error (not supported)",
355+
annotations: map[string]string{
356+
ServiceAnnotationLoadBalancerType: "nlb",
357+
ServiceAnnotationLoadBalancerSecurityGroups: "sg-123456789,sg-987654321",
358+
},
359+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
360+
},
361+
{
362+
name: "NLB with invalid BYO SG format - error (not supported)",
363+
annotations: map[string]string{
364+
ServiceAnnotationLoadBalancerType: "nlb",
365+
ServiceAnnotationLoadBalancerSecurityGroups: "invalid-sg-format",
366+
},
367+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
368+
},
369+
{
370+
name: "NLB case sensitivity - BYO SG annotation with different casing should still be rejected",
371+
annotations: map[string]string{
372+
ServiceAnnotationLoadBalancerType: "nlb",
373+
ServiceAnnotationLoadBalancerSecurityGroups: "SG-123ABC",
374+
},
375+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
376+
},
377+
{
378+
name: "NLB mixed annotations - BYO and other annotations",
379+
annotations: map[string]string{
380+
ServiceAnnotationLoadBalancerSecurityGroups: "sg-123456",
381+
ServiceAnnotationLoadBalancerType: "nlb",
382+
ServiceAnnotationLoadBalancerInternal: "true",
383+
},
384+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
385+
},
386+
{
387+
name: "NLB whitespace in BYO SG values - should still be rejected",
388+
annotations: map[string]string{
389+
ServiceAnnotationLoadBalancerType: "nlb",
390+
ServiceAnnotationLoadBalancerSecurityGroups: " sg-123456 ",
391+
},
392+
expectedError: "BYO security group annotation \"service.beta.kubernetes.io/aws-load-balancer-security-groups\" is not supported by NLB",
393+
},
394+
395+
// Target group attributes validation for NLB (should succeed)
396+
{
397+
name: "NLB with target group attributes - success",
398+
annotations: map[string]string{
399+
ServiceAnnotationLoadBalancerType: "nlb",
400+
ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true",
401+
},
402+
servicePorts: []v1.ServicePort{
403+
{Port: 80, Protocol: v1.ProtocolTCP},
404+
},
405+
expectedError: "",
406+
},
407+
408+
// Target group attributes validation for CLB (should fail)
409+
{
410+
name: "CLB with target group attributes - error (only supported for NLB)",
411+
annotations: map[string]string{
412+
ServiceAnnotationLoadBalancerTargetGroupAttributes: "preserve_client_ip.enabled=true",
413+
},
414+
expectedError: "target group attributes annotation is only supported for NLB",
415+
},
416+
417+
// No annotations (should succeed)
418+
{
419+
name: "no annotations - success",
420+
annotations: map[string]string{},
421+
expectedError: "",
422+
},
423+
}
424+
425+
for _, tt := range tests {
426+
t.Run(tt.name, func(t *testing.T) {
427+
// Create a test service with the specified ports and annotations
428+
service := &v1.Service{
429+
ObjectMeta: metav1.ObjectMeta{
430+
Name: "test-service",
431+
Namespace: "test-namespace",
432+
Annotations: tt.annotations,
433+
},
434+
Spec: v1.ServiceSpec{
435+
Type: v1.ServiceTypeLoadBalancer,
436+
Ports: tt.servicePorts,
437+
},
438+
}
439+
440+
// Create validation input
441+
input := &awsValidationInput{
442+
apiService: service,
443+
annotations: tt.annotations,
444+
}
445+
446+
// Execute the validation
447+
err := validateServiceAnnotations(input)
448+
449+
// Verify the result
450+
if tt.expectedError == "" {
451+
assert.NoError(t, err, "Expected no error for test case: %s", tt.name)
452+
} else {
453+
assert.Error(t, err, "Expected error for test case: %s", tt.name)
454+
assert.Contains(t, err.Error(), tt.expectedError, "Error message should contain expected text for test case: %s", tt.name)
455+
}
456+
})
457+
}
458+
}

0 commit comments

Comments
 (0)