implemented changes requested by reviewers

Author: marcosdiez

webhooks/elbv2/targetgroupbinding_mutator.go

@@ -2,6 +2,7 @@ package elbv2
 
 import (
 	"context"
+
 	awssdk "github.com/aws/aws-sdk-go/aws"
 	elbv2sdk "github.com/aws/aws-sdk-go/service/elbv2"
 	"github.com/go-logr/logr"
@@ -38,7 +39,7 @@ 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("You must provide either TargetGroupARN or TargetGroupName")
+		return nil, errors.Errorf("must provide either TargetGroupARN or TargetGroupName")
 	}
 	if err := m.getArnFromNameIfNeeded(ctx, tgb); err != nil {
 		return nil, err

webhooks/elbv2/targetgroupbinding_validator.go

@@ -2,6 +2,7 @@ package elbv2
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	awssdk "github.com/aws/aws-sdk-go/aws"
@@ -84,9 +85,20 @@ func (v *targetGroupBindingValidator) checkRequiredFields(ctx context.Context, t
 		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 errors.Errorf("Can't locate TargetGroup with name %s", tgb.Spec.TargetGroupName)
+				return fmt.Errorf("searching TargetGroup with name %s: %w", tgb.Spec.TargetGroupName, err)
 			}
 			tgb.Spec.TargetGroupARN = *tgObj.TargetGroupArn
 		}
@@ -194,6 +206,7 @@ func (v *targetGroupBindingValidator) getTargetGroupFromAWS(ctx context.Context,
 	return tgList[0], nil
 }
 
+// getTargetGroupFromAWS returns the AWS target group corresponding to the tgName
 func (v *targetGroupBindingValidator) getTargetGroupsByNameFromAWS(ctx context.Context, tgName string) (*elbv2sdk.TargetGroup, error) {
 	req := &elbv2sdk.DescribeTargetGroupsInput{
 		Names: awssdk.StringSlice([]string{tgName}),