TargetGroupBinding now support targetGroupName - #2655 refresh (#3903)

* TargetGroupBinding now support targetGroupName * documentation for targetGroupName * unit tests now validate if either either TargetGroupARN or TargetGroupName has been provided * more unit tests * updated documentation * Unit Test for MutateCreate * implemented changes requested by reviewers * Fixes following migration to `aws-sdk-go-v2` * make crds Also tweaked the description for the `TargetGroupName` field --------- Co-authored-by: Marcos Diez <[email protected]>
kubernetes-sigs · Dec 6, 2024 · 283ebee · 283ebee
1 parent 8ba34e2
commit 283ebee
Show file tree

Hide file tree

Showing 9 changed files with 258 additions and 26 deletions.
diff --git a/apis/elbv2/v1alpha1/targetgroupbinding_types.go b/apis/elbv2/v1alpha1/targetgroupbinding_types.go
@@ -107,7 +107,12 @@ type TargetGroupBindingNetworking struct {
 // TargetGroupBindingSpec defines the desired state of TargetGroupBinding
 type TargetGroupBindingSpec struct {
 	// targetGroupARN is the Amazon Resource Name (ARN) for the TargetGroup.
-	TargetGroupARN string `json:"targetGroupARN"`
+	// +optional
+	TargetGroupARN string `json:"targetGroupARN,omitempty"`
+
+	// targetGroupName is the Name of the TargetGroup.
+	// +optional
+	TargetGroupName string `json:"targetGroupName,omitempty"`
 
 	// MultiClusterTargetGroup Denotes if the TargetGroup is shared among multiple clusters
 	// +optional
@@ -138,6 +143,7 @@ type TargetGroupBindingStatus struct {
 // +kubebuilder:printcolumn:name="SERVICE-PORT",type="string",JSONPath=".spec.serviceRef.port",description="The Kubernetes Service's port"
 // +kubebuilder:printcolumn:name="TARGET-TYPE",type="string",JSONPath=".spec.targetType",description="The AWS TargetGroup's TargetType"
 // +kubebuilder:printcolumn:name="ARN",type="string",JSONPath=".spec.targetGroupARN",description="The AWS TargetGroup's Amazon Resource Name",priority=1
+// +kubebuilder:printcolumn:name="NAME",type="string",JSONPath=".spec.targetGroupName",description="The AWS TargetGroup's Name",priority=2
 // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp"
 // TargetGroupBinding is the Schema for the TargetGroupBinding API
 type TargetGroupBinding struct {

diff --git a/apis/elbv2/v1beta1/targetgroupbinding_types.go b/apis/elbv2/v1beta1/targetgroupbinding_types.go
@@ -124,8 +124,12 @@ type TargetGroupBindingNetworking struct {
 // TargetGroupBindingSpec defines the desired state of TargetGroupBinding
 type TargetGroupBindingSpec struct {
 	// targetGroupARN is the Amazon Resource Name (ARN) for the TargetGroup.
-	// +kubebuilder:validation:MinLength=1
-	TargetGroupARN string `json:"targetGroupARN"`
+	// +optional
+	TargetGroupARN string `json:"targetGroupARN,omitempty"`
+
+	// targetGroupName is the Name of the TargetGroup.
+	// +optional
+	TargetGroupName string `json:"targetGroupName,omitempty"`
 
 	// MultiClusterTargetGroup Denotes if the TargetGroup is shared among multiple clusters
 	// +optional
@@ -169,6 +173,7 @@ type TargetGroupBindingStatus struct {
 // +kubebuilder:printcolumn:name="SERVICE-PORT",type="string",JSONPath=".spec.serviceRef.port",description="The Kubernetes Service's port"
 // +kubebuilder:printcolumn:name="TARGET-TYPE",type="string",JSONPath=".spec.targetType",description="The AWS TargetGroup's TargetType"
 // +kubebuilder:printcolumn:name="ARN",type="string",JSONPath=".spec.targetGroupARN",description="The AWS TargetGroup's Amazon Resource Name",priority=1
+// +kubebuilder:printcolumn:name="NAME",type="string",JSONPath=".spec.targetGroupName",description="The AWS TargetGroup's Name",priority=2
 // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp"
 // TargetGroupBinding is the Schema for the TargetGroupBinding API
 type TargetGroupBinding struct {

diff --git a/config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml b/config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml
@@ -32,6 +32,11 @@ spec:
       name: ARN
       priority: 1
       type: string
+    - description: The AWS TargetGroup's Name
+      jsonPath: .spec.targetGroupName
+      name: NAME
+      priority: 2
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE
       type: date
@@ -160,6 +165,9 @@ spec:
                 description: targetGroupARN is the Amazon Resource Name (ARN) for
                   the TargetGroup.
                 type: string
+              targetGroupName:
+                description: targetGroupName is the Name of the TargetGroup.
+                type: string
               targetType:
                 description: targetType is the TargetType of TargetGroup. If unspecified,
                   it will be automatically inferred.
@@ -169,7 +177,6 @@ spec:
                 type: string
             required:
             - serviceRef
-            - targetGroupARN
             type: object
           status:
             description: TargetGroupBindingStatus defines the observed state of TargetGroupBinding
@@ -202,6 +209,11 @@ spec:
       name: ARN
       priority: 1
       type: string
+    - description: The AWS TargetGroup's Name
+      jsonPath: .spec.targetGroupName
+      name: NAME
+      priority: 2
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE
       type: date
@@ -387,7 +399,9 @@ spec:
               targetGroupARN:
                 description: targetGroupARN is the Amazon Resource Name (ARN) for
                   the TargetGroup.
-                minLength: 1
+                type: string
+              targetGroupName:
+                description: targetGroupName is the Name of the TargetGroup.
                 type: string
               targetType:
                 description: targetType is the TargetType of TargetGroup. If unspecified,
@@ -402,7 +416,6 @@ spec:
                 type: string
             required:
             - serviceRef
-            - targetGroupARN
             type: object
           status:
             description: TargetGroupBindingStatus defines the observed state of TargetGroupBinding

diff --git a/docs/guide/targetgroupbinding/targetgroupbinding.md b/docs/guide/targetgroupbinding/targetgroupbinding.md
@@ -16,8 +16,25 @@ TargetGroupBinding CR supports TargetGroups of either `instance` or `ip` TargetT
 !!!tip ""
     If TargetType is not explicitly specified, a mutating webhook will automatically call AWS API to find the TargetType for your TargetGroup and set it to correct value.
 
+## Choosing the Target Group
+One can either use ``targetGroupARN`` of ``targetGroupName`` to identify a Target Group. Although both are unique and immutable in an AWS region, one only has control of the ``targetGroupName``, for ``targetGroupARN`` is generated by AWS and contain random characters.
+
+If you provide both ``targetGroupARN`` and ``targetGroupName``, beware that ``targetGroupARN`` prevails.
+
+
+## Sample YAMLs
+```yaml
+apiVersion: elbv2.k8s.aws/v1beta1
+kind: TargetGroupBinding
+metadata:
+  name: my-tgb
+spec:
+  serviceRef:
+    name: awesome-service # route traffic to the awesome-service
+    port: 80
+  targetGroupName: <name-of-the-targetGroup>
+```
 
-## Sample YAML
 ```yaml
 apiVersion: elbv2.k8s.aws/v1beta1
 kind: TargetGroupBinding

diff --git a/helm/aws-load-balancer-controller/crds/crds.yaml b/helm/aws-load-balancer-controller/crds/crds.yaml
@@ -272,6 +272,11 @@ spec:
       name: ARN
       priority: 1
       type: string
+    - description: The AWS TargetGroup's Name
+      jsonPath: .spec.targetGroupName
+      name: NAME
+      priority: 2
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE
       type: date
@@ -400,6 +405,9 @@ spec:
                 description: targetGroupARN is the Amazon Resource Name (ARN) for
                   the TargetGroup.
                 type: string
+              targetGroupName:
+                description: targetGroupName is the Name of the TargetGroup.
+                type: string
               targetType:
                 description: targetType is the TargetType of TargetGroup. If unspecified,
                   it will be automatically inferred.
@@ -409,7 +417,6 @@ spec:
                 type: string
             required:
             - serviceRef
-            - targetGroupARN
             type: object
           status:
             description: TargetGroupBindingStatus defines the observed state of TargetGroupBinding
@@ -442,6 +449,11 @@ spec:
       name: ARN
       priority: 1
       type: string
+    - description: The AWS TargetGroup's Name
+      jsonPath: .spec.targetGroupName
+      name: NAME
+      priority: 2
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE
       type: date
@@ -627,7 +639,9 @@ spec:
               targetGroupARN:
                 description: targetGroupARN is the Amazon Resource Name (ARN) for
                   the TargetGroup.
-                minLength: 1
+                type: string
+              targetGroupName:
+                description: targetGroupName is the Name of the TargetGroup.
                 type: string
               targetType:
                 description: targetType is the TargetType of TargetGroup. If unspecified,
@@ -642,7 +656,6 @@ spec:
                 type: string
             required:
             - serviceRef
-            - targetGroupARN
             type: object
           status:
             description: TargetGroupBindingStatus defines the observed state of TargetGroupBinding

diff --git a/webhooks/elbv2/targetgroupbinding_mutator.go b/webhooks/elbv2/targetgroupbinding_mutator.go
@@ -39,6 +39,12 @@ func (m *targetGroupBindingMutator) Prototype(_ admission.Request) (runtime.Obje
 
 func (m *targetGroupBindingMutator) MutateCreate(ctx context.Context, obj runtime.Object) (runtime.Object, error) {
 	tgb := obj.(*elbv2api.TargetGroupBinding)
+	if tgb.Spec.TargetGroupARN == "" && tgb.Spec.TargetGroupName == "" {
+		return nil, errors.Errorf("must provide either TargetGroupARN or TargetGroupName")
+	}
+	if err := m.getArnFromNameIfNeeded(ctx, tgb); err != nil {
+		return nil, err
+	}
 	if err := m.defaultingTargetType(ctx, tgb); err != nil {
 		return nil, err
 	}
@@ -51,6 +57,17 @@ func (m *targetGroupBindingMutator) MutateCreate(ctx context.Context, obj runtim
 	return tgb, nil
 }
 
+func (m *targetGroupBindingMutator) getArnFromNameIfNeeded(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error {
+	if tgb.Spec.TargetGroupARN == "" && tgb.Spec.TargetGroupName != "" {
+		tgObj, err := m.getTargetGroupsByNameFromAWS(ctx, tgb.Spec.TargetGroupName)
+		if err != nil {
+			return err
+		}
+		tgb.Spec.TargetGroupARN = *tgObj.TargetGroupArn
+	}
+	return nil
+}
+
 func (m *targetGroupBindingMutator) MutateUpdate(ctx context.Context, obj runtime.Object, oldObj runtime.Object) (runtime.Object, error) {
 	return obj, nil
 }
@@ -142,6 +159,20 @@ func (m *targetGroupBindingMutator) getTargetGroupFromAWS(ctx context.Context, t
 	return &tgList[0], nil
 }
 
+func (m *targetGroupBindingMutator) getTargetGroupsByNameFromAWS(ctx context.Context, tgName string) (*elbv2types.TargetGroup, error) {
+	req := &elbv2sdk.DescribeTargetGroupsInput{
+		Names: []string{tgName},
+	}
+	tgList, err := m.elbv2Client.DescribeTargetGroupsAsList(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+	if len(tgList) != 1 {
+		return nil, errors.Errorf("expecting a single targetGroup with name [%s] but got %v", tgName, len(tgList))
+	}
+	return &tgList[0], nil
+}
+
 func (m *targetGroupBindingMutator) getVpcIDFromAWS(ctx context.Context, tgARN string) (string, error) {
 	targetGroup, err := m.getTargetGroupFromAWS(ctx, tgARN)
 	if err != nil {

diff --git a/webhooks/elbv2/targetgroupbinding_mutator_test.go b/webhooks/elbv2/targetgroupbinding_mutator_test.go
@@ -239,6 +239,53 @@ func Test_targetGroupBindingMutator_MutateCreate(t *testing.T) {
 			},
 			wantErr: errors.New("unable to get target group VpcID: vpcid not found"),
 		},
+		{
+			name: "targetGroupBinding with TargetGroupName instead of TargetGroupARN",
+			fields: fields{
+				describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{
+					{
+						req: &elbv2sdk.DescribeTargetGroupsInput{
+							Names: []string{"tg-name"},
+						},
+						resp: []elbv2types.TargetGroup{
+							{
+								TargetGroupArn:  awssdk.String("tg-arn"),
+								TargetGroupName: awssdk.String("tg-name"),
+								TargetType:      elbv2types.TargetTypeEnumInstance,
+							},
+						},
+					},
+					{
+						req: &elbv2sdk.DescribeTargetGroupsInput{
+							TargetGroupArns: []string{"tg-arn"},
+						},
+						resp: []elbv2types.TargetGroup{
+							{
+								TargetGroupArn: awssdk.String("tg-arn"),
+								TargetType:     elbv2types.TargetTypeEnumInstance,
+							},
+						},
+					},
+				},
+			},
+			args: args{
+				obj: &elbv2api.TargetGroupBinding{
+					Spec: elbv2api.TargetGroupBindingSpec{
+						TargetGroupName: "tg-name",
+						TargetType:      &instanceTargetType,
+						IPAddressType:   &targetGroupIPAddressTypeIPv4,
+					},
+				},
+			},
+			want: &elbv2api.TargetGroupBinding{
+				Spec: elbv2api.TargetGroupBindingSpec{
+					TargetGroupARN:  "tg-arn",
+					TargetGroupName: "tg-name",
+					TargetType:      &instanceTargetType,
+					IPAddressType:   &targetGroupIPAddressTypeIPv4,
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/webhooks/elbv2/targetgroupbinding_validator.go b/webhooks/elbv2/targetgroupbinding_validator.go
@@ -2,6 +2,7 @@ package elbv2
 
 import (
 	"context"
+	"fmt"
 	elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
 	"regexp"
 	"strings"
@@ -53,7 +54,7 @@ func (v *targetGroupBindingValidator) Prototype(_ admission.Request) (runtime.Ob
 
 func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
 	tgb := obj.(*elbv2api.TargetGroupBinding)
-	if err := v.checkRequiredFields(tgb); err != nil {
+	if err := v.checkRequiredFields(ctx, tgb); err != nil {
 		return err
 	}
 	if err := v.checkNodeSelector(tgb); err != nil {
@@ -74,7 +75,7 @@ func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj ru
 func (v *targetGroupBindingValidator) ValidateUpdate(ctx context.Context, obj runtime.Object, oldObj runtime.Object) error {
 	tgb := obj.(*elbv2api.TargetGroupBinding)
 	oldTgb := oldObj.(*elbv2api.TargetGroupBinding)
-	if err := v.checkRequiredFields(tgb); err != nil {
+	if err := v.checkRequiredFields(ctx, tgb); err != nil {
 		return err
 	}
 	if err := v.checkImmutableFields(tgb, oldTgb); err != nil {
@@ -91,8 +92,30 @@ func (v *targetGroupBindingValidator) ValidateDelete(ctx context.Context, obj ru
 }
 
 // checkRequiredFields will check required fields are not absent.
-func (v *targetGroupBindingValidator) checkRequiredFields(tgb *elbv2api.TargetGroupBinding) error {
+func (v *targetGroupBindingValidator) checkRequiredFields(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error {
 	var absentRequiredFields []string
+	if tgb.Spec.TargetGroupARN == "" {
+		if tgb.Spec.TargetGroupName == "" {
+			absentRequiredFields = append(absentRequiredFields, "either TargetGroupARN or TargetGroupName")
+		} else if tgb.Spec.TargetGroupName != "" {
+			/*
+				The purpose of this code is to guarantee that the either the ARN of the TargetGroup exists
+				or it's possible to infer the ARN by the name of the TargetGroup (since it's unique).
+
+				And even though the validator can't mutate, I added tgb.Spec.TargetGroupARN = *tgObj.TargetGroupArn
+				to guarantee the object is in a consistent state though the rest of the process.
+
+				The whole code of aws-load-balancer-controller was written assuming there is an ARN.
+				By changing the object here I guarantee as early as possible that that assumption is true.
+			*/
+
+			tgObj, err := v.getTargetGroupsByNameFromAWS(ctx, tgb.Spec.TargetGroupName)
+			if err != nil {
+				return fmt.Errorf("searching TargetGroup with name %s: %w", tgb.Spec.TargetGroupName, err)
+			}
+			tgb.Spec.TargetGroupARN = *tgObj.TargetGroupArn
+		}
+	}
 	if tgb.Spec.TargetType == nil {
 		absentRequiredFields = append(absentRequiredFields, "spec.targetType")
 	}
@@ -227,6 +250,21 @@ func (v *targetGroupBindingValidator) getVpcIDFromAWS(ctx context.Context, tgARN
 	return awssdk.ToString(targetGroup.VpcId), nil
 }
 
+// getTargetGroupFromAWS returns the AWS target group corresponding to the tgName
+func (v *targetGroupBindingValidator) getTargetGroupsByNameFromAWS(ctx context.Context, tgName string) (*elbv2types.TargetGroup, error) {
+	req := &elbv2sdk.DescribeTargetGroupsInput{
+		Names: []string{tgName},
+	}
+	tgList, err := v.elbv2Client.DescribeTargetGroupsAsList(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+	if len(tgList) != 1 {
+		return nil, errors.Errorf("expecting a single targetGroup with name [%s] but got %v", tgName, len(tgList))
+	}
+	return &tgList[0], nil
+}
+
 // +kubebuilder:webhook:path=/validate-elbv2-k8s-aws-v1beta1-targetgroupbinding,mutating=false,failurePolicy=fail,groups=elbv2.k8s.aws,resources=targetgroupbindings,verbs=create;update,versions=v1beta1,name=vtargetgroupbinding.elbv2.k8s.aws,sideEffects=None,webhookVersions=v1,admissionReviewVersions=v1beta1
 
 func (v *targetGroupBindingValidator) SetupWithManager(mgr ctrl.Manager) {