Add support for specifying the load balancer's name via annotation (#1880)

This adds a new "aws-load-balancer-name" annotation that can be used to specify the associated load balancer's name, overriding the default name format.

Some implementation notes:
* This doesn't perform any input validation on the annotation value to ensure it matches the allowed naming format. I'm happy to add this but didnt see validation on any other annotation value strings.
* Adding or updating the annotation on existing ingresses has no effect once a load balancer already exists.

Author: rifelpet

pkg/annotations/constants.go

@@ -50,6 +50,7 @@ const (
 	SvcLBSuffixLoadBalancerType              = "aws-load-balancer-type"
 	SvcLBSuffixInternal                      = "aws-load-balancer-internal"
 	SvcLBSuffixIPAddressType                 = "aws-load-balancer-ip-address-type"
+	SvcLBSuffixLoadBalancerName              = "aws-load-balancer-name"
 	SvcLBSuffixProxyProtocol                 = "aws-load-balancer-proxy-protocol"
 	SvcLBSuffixAccessLogEnabled              = "aws-load-balancer-access-log-enabled"
 	SvcLBSuffixAccessLogS3BucketName         = "aws-load-balancer-access-log-s3-bucket-name"

pkg/service/model_build_load_balancer.go

@@ -231,6 +231,10 @@ func (t *defaultModelBuildTask) buildLoadBalancerAttributes(_ context.Context) (
 var invalidLoadBalancerNamePattern = regexp.MustCompile("[[:^alnum:]]")
 
 func (t *defaultModelBuildTask) buildLoadBalancerName(_ context.Context, scheme elbv2model.LoadBalancerScheme) string {
+	var name string
+	if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixLoadBalancerName, &name, t.service.Annotations); exists {
+		return name
+	}
 	uuidHash := sha256.New()
 	_, _ = uuidHash.Write([]byte(t.clusterName))
 	_, _ = uuidHash.Write([]byte(t.service.UID))

pkg/service/model_build_load_balancer_test.go

@@ -708,3 +708,51 @@ func Test_defaultModelBuildTask_buildAdditionalResourceTags(t *testing.T) {
 		})
 	}
 }
+
+func Test_defaultModelBuildTask_buildLoadBalancerName(t *testing.T) {
+	tests := []struct {
+		name        string
+		service     *corev1.Service
+		clusterName string
+		scheme      elbv2.LoadBalancerScheme
+		want        string
+	}{
+		{
+			name: "no name annotation",
+
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   "foo",
+					Name:        "bar",
+					Annotations: map[string]string{},
+				},
+			},
+			scheme: elbv2.LoadBalancerSchemeInternetFacing,
+			want:   "k8s-foo-bar-e053368fb2",
+		},
+		{
+			name: "non-empty name annotation",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "foo",
+					Name:      "bar",
+					Annotations: map[string]string{
+						"service.beta.kubernetes.io/aws-load-balancer-name": "baz",
+					},
+				},
+			},
+			want: "baz",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			task := &defaultModelBuildTask{
+				service:          tt.service,
+				clusterName:      tt.clusterName,
+				annotationParser: annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io"),
+			}
+			got := task.buildLoadBalancerName(context.Background(), tt.scheme)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

test/e2e/service/service.go

@@ -48,6 +48,7 @@ type TargetGroupHC struct {
 }
 
 type LoadBalancerExpectation struct {
+	Name          *string
 	Type          string
 	Scheme        string
 	TargetType    string
@@ -166,6 +167,19 @@ func (m *ServiceTest) CheckLoadBalancerListeners(ctx context.Context, f *framewo
 	return nil
 }
 
+func (m *ServiceTest) CheckLoadBalancerName(ctx context.Context, f *framework.Framework, lbArns []string, lbName string) error {
+	By("Describing AWS Load Balancer", func() {
+		lbs, err := f.Cloud.ELBV2().DescribeLoadBalancersWithContext(ctx, &elbv2.DescribeLoadBalancersInput{
+			LoadBalancerArns: aws.StringSlice(lbArns),
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(lbs.LoadBalancers)).To(Equal(1))
+		lb := lbs.LoadBalancers[0]
+		Expect(aws.StringValue(lb.LoadBalancerName)).To(Equal(lbName))
+	})
+	return nil
+}
+
 func (m *ServiceTest) VerifyLoadBalancerAttributes(ctx context.Context, f *framework.Framework, expectedAttrs map[string]string) error {
 	lbArns, err := m.GetAwsLoadBalancerArns(ctx, f)
 	Expect(err).ToNot(HaveOccurred())
@@ -252,6 +266,11 @@ func (m *ServiceTest) VerifyAWSLoadBalancerResources(ctx context.Context, f *fra
 
 		err = m.CheckTargetGroups(ctx, f, lbArns[0], expected)
 		Expect(err).ToNot(HaveOccurred())
+
+		if expected.Name != nil {
+			err = m.CheckLoadBalancerName(ctx, f, lbArns, *expected.Name)
+			Expect(err).ToNot(HaveOccurred())
+		}
 	})
 	return nil
 }

test/e2e/service/service_test.go

@@ -310,4 +310,76 @@ var _ = Describe("Service", func() {
 			})
 		})
 	})
+
+	Context("NLB IP Load Balancer with name", func() {
+		var (
+			svcTest service.ServiceTest
+			svc     *corev1.Service
+		)
+		BeforeEach(func() {
+			svcTest = service.ServiceTest{}
+			svc = &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: ns.Name,
+					Annotations: map[string]string{
+						"service.beta.kubernetes.io/aws-load-balancer-name": name,
+						"service.beta.kubernetes.io/aws-load-balancer-type": "nlb-ip",
+					},
+				},
+				Spec: corev1.ServiceSpec{
+					Type:     corev1.ServiceTypeLoadBalancer,
+					Selector: labels,
+					Ports: []corev1.ServicePort{
+						{
+							Port:       80,
+							TargetPort: intstr.FromInt(80),
+							Protocol:   corev1.ProtocolTCP,
+						},
+					},
+				},
+			}
+		})
+		It("Should create and verify service", func() {
+			By("Creating service", func() {
+				err := svcTest.Create(ctx, tf, svc)
+				Expect(err).ToNot(HaveOccurred())
+			})
+			By("Verify Service with AWS", func() {
+				err := svcTest.VerifyAWSLoadBalancerResources(ctx, tf, service.LoadBalancerExpectation{
+					Name:       &name,
+					Type:       "network",
+					Scheme:     "internet-facing",
+					TargetType: "ip",
+					Listeners: map[string]string{
+						"80": "TCP",
+					},
+					TargetGroups: map[string]string{
+						"80": "TCP",
+					},
+					NumTargets: int(numReplicas),
+					TargetGroupHC: &service.TargetGroupHC{
+						Protocol:           "TCP",
+						Port:               "traffic-port",
+						Interval:           10,
+						Timeout:            10,
+						HealthyThreshold:   3,
+						UnhealthyThreshold: 3,
+					},
+				})
+				Expect(err).ToNot(HaveOccurred())
+			})
+			By("Send traffic to LB", func() {
+				err := svcTest.SendTrafficToLB(ctx, tf)
+				Expect(err).ToNot(HaveOccurred())
+			})
+			By("Deleting service", func() {
+				err := svcTest.Cleanup(ctx, tf, svc)
+				Expect(err).ToNot(HaveOccurred())
+				newSvc := &corev1.Service{}
+				err = tf.K8sClient.Get(ctx, k8s.NamespacedName(svc), newSvc)
+				Expect(apierrs.IsNotFound(err)).To(BeTrue())
+			})
+		})
+	})
 })