reject custom load balancer name longer than 32 characters (#2295)

Author: kishorj

docs/guide/ingress/annotations.md

@@ -157,8 +157,7 @@ Traffic Listening can be controlled with following annotations:
 ## Traffic Routing
 Traffic Routing can be controlled with following annotations:
 
-- <a name="load-balancer-name">`alb.ingress.kubernetes.io/load-balancer-name`</a> specifies the custom name to use for the load balancer. Name longer than 32 characters will be trimmed.
-
+- <a name="load-balancer-name">`alb.ingress.kubernetes.io/load-balancer-name`</a> specifies the custom name to use for the load balancer. Name longer than 32 characters will be treated as an error.
  !!!note "Merge Behavior"
  `name` is exclusive across all Ingresses in an IngressGroup.
 

docs/guide/service/annotations.md

@@ -45,7 +45,7 @@
 ## Traffic Routing
 Traffic Routing can be controlled with following annotations:
 
-- <a name="load-balancer-name">`service.beta.kubernetes.io/aws-load-balancer-name`</a> specifies the custom name to use for the load balancer. Name longer than 32 characters will be trimmed.
+- <a name="load-balancer-name">`service.beta.kubernetes.io/aws-load-balancer-name`</a> specifies the custom name to use for the load balancer. Name longer than 32 characters will be treated as an error.
 
  !!!note "limitations"
  - If you modify this annotation after service creation, there is no effect.

pkg/ingress/model_build_load_balancer.go

@@ -100,7 +100,7 @@ func (t *defaultModelBuildTask) buildLoadBalancerName(_ context.Context, scheme
 name, _ := explicitNames.PopAny()
 // The name of the loadbalancer can only have up to 32 characters
 if len(name) > 32 {
-name = name[:32]
+return "", errors.New("load balancer name cannot be longer than 32 characters")
 }
 return name, nil
 }

pkg/ingress/model_build_load_balancer_test.go

@@ -579,7 +579,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) {
 },
 scheme: elbv2.LoadBalancerSchemeInternetFacing,
 },
-want: "bazbazfoofoobazbazfoofoobazbazfo",
+wantErr: errors.New("load balancer name cannot be longer than 32 characters"),
 },
 {
 name: "name annotation on single ingress only",

pkg/service/model_build_load_balancer.go

@@ -57,7 +57,10 @@ func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, schem
 if err != nil {
 return elbv2model.LoadBalancerSpec{}, err
 }
-name := t.buildLoadBalancerName(ctx, scheme)
+name, err := t.buildLoadBalancerName(ctx, scheme)
+if err != nil {
+return elbv2model.LoadBalancerSpec{}, err
+}
 spec := elbv2model.LoadBalancerSpec{
 Name: name,
 Type: elbv2model.LoadBalancerTypeNetwork,
@@ -354,14 +357,14 @@ func (t *defaultModelBuildTask) getAnnotationSpecificLbAttributes() (map[string]
 
 var invalidLoadBalancerNamePattern = regexp.MustCompile("[[:^alnum:]]")
 
-func (t *defaultModelBuildTask) buildLoadBalancerName(_ context.Context, scheme elbv2model.LoadBalancerScheme) string {
+func (t *defaultModelBuildTask) buildLoadBalancerName(_ context.Context, scheme elbv2model.LoadBalancerScheme) (string, error) {
 var name string
 if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixLoadBalancerName, &name, t.service.Annotations); exists {
 // The name of the loadbalancer can only have up to 32 characters
 if len(name) > 32 {
-name = name[:32]
+return "", errors.New("load balancer name cannot be longer than 32 characters")
 }
-return name
+return name, nil
 }
 uuidHash := sha256.New()
 _, _ = uuidHash.Write([]byte(t.clusterName))
@@ -371,5 +374,5 @@ func (t *defaultModelBuildTask) buildLoadBalancerName(_ context.Context, scheme
 
 sanitizedNamespace := invalidLoadBalancerNamePattern.ReplaceAllString(t.service.Namespace, "")
 sanitizedName := invalidLoadBalancerNamePattern.ReplaceAllString(t.service.Name, "")
-return fmt.Sprintf("k8s-%.8s-%.8s-%.10s", sanitizedNamespace, sanitizedName, uuid)
+return fmt.Sprintf("k8s-%.8s-%.8s-%.10s", sanitizedNamespace, sanitizedName, uuid), nil
 }

pkg/service/model_build_load_balancer_test.go

@@ -912,6 +912,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) {
 clusterName string
 scheme elbv2.LoadBalancerScheme
 want string
+wantErrerror
 }{
 {
 name: "no name annotation",
@@ -940,7 +941,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) {
 want: "baz",
 },
 {
-name: "trim name annotation",
+name: "reject name longer than 32 characters",
 service: &corev1.Service{
 ObjectMeta: metav1.ObjectMeta{
 Namespace: "foo",
@@ -950,7 +951,7 @@ func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) {
 },
 },
 },
-want: "bazbazfoofoobazbazfoofoobazbazfo",
+wantErr: errors.New("load balancer name cannot be longer than 32 characters"),
 },
 }
 for _, tt := range tests {
@@ -960,8 +961,12 @@ func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) {
 clusterName: tt.clusterName,
 annotationParser: annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io"),
 }
-got := task.buildLoadBalancerName(context.Background(), tt.scheme)
-assert.Equal(t, tt.want, got)
+got, err := task.buildLoadBalancerName(context.Background(), tt.scheme)
+if err != nil {
+assert.EqualError(t, err, tt.wantErr.Error())
+} else {
+assert.Equal(t, tt.want, got)
+}
 })
 }
 }