Merge pull request #4948 from r4f4/target-group-name-fix

🐛 Fix Target Group's name exceeding 32 characters
kubernetes-sigs · Apr 24, 2024 · 4c5b811 · 4c5b811
2 parents 63c6635 + 077734c
commit 4c5b811
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 73 deletions.
diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go
@@ -223,6 +223,7 @@ var (
 // This is created first, and the ARN is then passed to the listener.
 type TargetGroupSpec struct {
 	// Name of the TargetGroup. Must be unique over the same group of listeners.
+	// +kubebuilder:validation:MaxLength=32
 	Name string `json:"name"`
 	// Port is the exposed port
 	Port int64 `json:"port"`

diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
@@ -1467,6 +1467,7 @@ spec:
                                 name:
                                   description: Name of the TargetGroup. Must be unique
                                     over the same group of listeners.
+                                  maxLength: 32
                                   type: string
                                 port:
                                   description: Port is the exposed port
@@ -1686,6 +1687,7 @@ spec:
                                 name:
                                   description: Name of the TargetGroup. Must be unique
                                     over the same group of listeners.
+                                  maxLength: 32
                                   type: string
                                 port:
                                   description: Port is the exposed port
@@ -3415,6 +3417,7 @@ spec:
                                 name:
                                   description: Name of the TargetGroup. Must be unique
                                     over the same group of listeners.
+                                  maxLength: 32
                                   type: string
                                 port:
                                   description: Port is the exposed port
@@ -3634,6 +3637,7 @@ spec:
                                 name:
                                   description: Name of the TargetGroup. Must be unique
                                     over the same group of listeners.
+                                  maxLength: 32
                                   type: string
                                 port:
                                   description: Port is the exposed port

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -2413,6 +2413,7 @@ spec:
                                 name:
                                   description: Name of the TargetGroup. Must be unique
                                     over the same group of listeners.
+                                  maxLength: 32
                                   type: string
                                 port:
                                   description: Port is the exposed port
@@ -2632,6 +2633,7 @@ spec:
                                 name:
                                   description: Name of the TargetGroup. Must be unique
                                     over the same group of listeners.
+                                  maxLength: 32
                                   type: string
                                 port:
                                   description: Port is the exposed port

diff --git a/pkg/cloud/services/elb/loadbalancer.go b/pkg/cloud/services/elb/loadbalancer.go
@@ -239,18 +239,6 @@ func (s *Service) getAdditionalTargetGroupHealthCheck(ln infrav1.AdditionalListe
 	return healthCheck
 }
 
-// getTargetGroupName creates the target group name based on LB Name, when defined, otherwise return
-// the standard name created from the timestamp.
-func (s *Service) getTargetGroupName(lbSpec *infrav1.AWSLoadBalancerSpec, defaultPrefix string, port int64) string {
-	targetName := fmt.Sprintf("%s-%d", defaultPrefix, time.Now().Unix())
-
-	if lbSpec != nil && lbSpec.Name != nil {
-		targetName = fmt.Sprintf("%s-%d", *lbSpec.Name, port)
-	}
-
-	return targetName
-}
-
 func (s *Service) getAPIServerLBSpec(elbName string, lbSpec *infrav1.AWSLoadBalancerSpec) (*infrav1.LoadBalancer, error) {
 	var securityGroupIDs []string
 	if lbSpec != nil {
@@ -266,7 +254,6 @@ func (s *Service) getAPIServerLBSpec(elbName string, lbSpec *infrav1.AWSLoadBala
 
 	// The default API health check is TCP, allowing customization to HTTP or HTTPS when HealthCheckProtocol is set.
 	apiHealthCheck := s.getAPITargetGroupHealthCheck(lbSpec)
-	apiTargetGroupName := s.getTargetGroupName(lbSpec, "apiserver-target", infrav1.DefaultAPIServerPort)
 	res := &infrav1.LoadBalancer{
 		Name:          elbName,
 		Scheme:        scheme,
@@ -276,7 +263,7 @@ func (s *Service) getAPIServerLBSpec(elbName string, lbSpec *infrav1.AWSLoadBala
 				Protocol: infrav1.ELBProtocolTCP,
 				Port:     infrav1.DefaultAPIServerPort,
 				TargetGroup: infrav1.TargetGroupSpec{
-					Name:        apiTargetGroupName,
+					Name:        fmt.Sprintf("apiserver-target-%d", time.Now().Unix()),
 					Port:        infrav1.DefaultAPIServerPort,
 					Protocol:    infrav1.ELBProtocolTCP,
 					VpcID:       s.scope.VPC().ID,
@@ -289,7 +276,6 @@ func (s *Service) getAPIServerLBSpec(elbName string, lbSpec *infrav1.AWSLoadBala
 
 	if lbSpec != nil {
 		for _, listener := range lbSpec.AdditionalListeners {
-			targetGroupName := s.getTargetGroupName(lbSpec, "additional-listener", listener.Port)
 			lnHealthCheck := &infrav1.TargetGroupHealthCheck{
 				Protocol: aws.String(string(listener.Protocol)),
 				Port:     aws.String(strconv.FormatInt(listener.Port, 10)),
@@ -302,7 +288,7 @@ func (s *Service) getAPIServerLBSpec(elbName string, lbSpec *infrav1.AWSLoadBala
 				Protocol: listener.Protocol,
 				Port:     listener.Port,
 				TargetGroup: infrav1.TargetGroupSpec{
-					Name:        targetGroupName,
+					Name:        fmt.Sprintf("additional-listener-%d", time.Now().Unix()),
 					Port:        listener.Port,
 					Protocol:    listener.Protocol,
 					VpcID:       s.scope.VPC().ID,

diff --git a/pkg/cloud/services/elb/loadbalancer_test.go b/pkg/cloud/services/elb/loadbalancer_test.go
@@ -3341,63 +3341,6 @@ func stubGetBaseService(t *testing.T, clusterName string) *Service {
 	}
 }
 
-func TestService_getTargetGroupName(t *testing.T) {
-	type argWant struct {
-		value      string
-		prefixOnly bool
-	}
-	type args struct {
-		lbSpec        *infrav1.AWSLoadBalancerSpec
-		defaultPrefix string
-		port          int64
-	}
-	tests := []struct {
-		name string
-		args args
-		want argWant
-	}{
-		{
-			name: "default name",
-			args: args{
-				lbSpec:        &infrav1.AWSLoadBalancerSpec{},
-				defaultPrefix: "apiserver-target",
-				port:          6443,
-			},
-			want: argWant{
-				value:      "apiserver-target-",
-				prefixOnly: true,
-			},
-		},
-		{
-			name: "custom name",
-			args: args{
-				lbSpec: &infrav1.AWSLoadBalancerSpec{
-					Name: ptr.To("foo"),
-				},
-				defaultPrefix: "api",
-				port:          6443,
-			},
-			want: argWant{
-				value: "foo-6443",
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			s := stubGetBaseService(t, "foo")
-			if tt.want.prefixOnly {
-				if got := s.getTargetGroupName(tt.args.lbSpec, tt.args.defaultPrefix, tt.args.port); !strings.HasPrefix(got, tt.want.value) {
-					t.Errorf("Service.getTargetGroupName() = %v, wantPrefix %v", got, tt.want.value)
-				}
-				return
-			}
-			if got := s.getTargetGroupName(tt.args.lbSpec, tt.args.defaultPrefix, tt.args.port); got != tt.want.value {
-				t.Errorf("Service.getTargetGroupName() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestService_getAPITargetGroupHealthCheck(t *testing.T) {
 	tests := []struct {
 		name   string