diff --git a/apis/elbv2/v1alpha1/targetgroupbinding_types.go b/apis/elbv2/v1alpha1/targetgroupbinding_types.go index 78c4f5b8e..4af605afa 100644 --- 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 index 437f2d3a6..337e89202 100644 --- 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 index 8f2dd41d0..f9af43e6f 100644 --- 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 index 8da8ff89e..cec6b6f02 100644 --- 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: +``` -## 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 index 048d51e84..750666392 100644 --- 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 index 8a005927c..dd9fb557e 100644 --- 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 index 1b6df3261..f23692f80 100644 --- 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 index 367c52a38..c4aa4df90 100644 --- 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) { diff --git a/webhooks/elbv2/targetgroupbinding_validator_test.go b/webhooks/elbv2/targetgroupbinding_validator_test.go index 7ce250047..a023e526a 100644 --- a/webhooks/elbv2/targetgroupbinding_validator_test.go +++ b/webhooks/elbv2/targetgroupbinding_validator_test.go @@ -96,8 +96,9 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { args: args{ obj: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &ipTargetType, - NodeSelector: &v1.LabelSelector{}, + TargetGroupARN: "tg-1", + TargetType: &ipTargetType, + NodeSelector: &v1.LabelSelector{}, }, }, }, @@ -131,6 +132,47 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { }, wantErr: nil, }, + { + name: "TargetGroupName can be resolved", + 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"), + TargetGroupName: awssdk.String("tg-name"), + TargetType: elbv2types.TargetTypeEnumInstance, + }, + }, + }, + }, + }, + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupName: "tg-name", + TargetType: &instanceTargetType, + IPAddressType: &targetGroupIPAddressTypeIPv4, + }, + }, + }, + wantErr: nil, + }, { name: "ipAddressType mismatch with TargetGroup", fields: fields{ @@ -360,14 +402,16 @@ func Test_targetGroupBindingValidator_ValidateUpdate(t *testing.T) { args: args{ obj: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &ipTargetType, - NodeSelector: &v1.LabelSelector{}, + TargetGroupARN: "tg-1", + TargetType: &ipTargetType, + NodeSelector: &v1.LabelSelector{}, }, }, oldObj: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &ipTargetType, - NodeSelector: &v1.LabelSelector{}, + TargetGroupARN: "tg-1", + TargetType: &ipTargetType, + NodeSelector: &v1.LabelSelector{}, }, }, }, @@ -449,6 +493,20 @@ func Test_targetGroupBindingValidator_checkRequiredFields(t *testing.T) { }, wantErr: errors.New("TargetGroupBinding must specify these fields: spec.targetType"), }, + { + name: "either TargetGroupARN or TargetGroupName must be specified", + args: args{ + tgb: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "", + TargetGroupName: "", + // TargetType: &ipTargetType, + TargetType: &instanceTargetType, + }, + }, + }, + wantErr: errors.New("TargetGroupBinding must specify these fields: either TargetGroupARN or TargetGroupName"), + }, { name: "targetType is set", args: args{ @@ -467,7 +525,7 @@ func Test_targetGroupBindingValidator_checkRequiredFields(t *testing.T) { v := &targetGroupBindingValidator{ logger: logr.New(&log.NullLogSink{}), } - err := v.checkRequiredFields(tt.args.tgb) + err := v.checkRequiredFields(context.Background(), tt.args.tgb) if tt.wantErr != nil { assert.EqualError(t, err, tt.wantErr.Error()) } else { @@ -807,7 +865,8 @@ func Test_targetGroupBindingValidator_checkNodeSelector(t *testing.T) { args: args{ tgb: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &ipTargetType, + TargetGroupARN: "tg-4", + TargetType: &ipTargetType, }, }, }, @@ -818,7 +877,8 @@ func Test_targetGroupBindingValidator_checkNodeSelector(t *testing.T) { args: args{ tgb: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &instanceTargetType, + TargetGroupARN: "tg-5", + TargetType: &instanceTargetType, }, }, }, @@ -829,8 +889,9 @@ func Test_targetGroupBindingValidator_checkNodeSelector(t *testing.T) { args: args{ tgb: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &instanceTargetType, - NodeSelector: &nodeSelector, + TargetGroupARN: "tg-6", + TargetType: &instanceTargetType, + NodeSelector: &nodeSelector, }, }, }, @@ -841,8 +902,9 @@ func Test_targetGroupBindingValidator_checkNodeSelector(t *testing.T) { args: args{ tgb: &elbv2api.TargetGroupBinding{ Spec: elbv2api.TargetGroupBindingSpec{ - TargetType: &ipTargetType, - NodeSelector: &nodeSelector, + TargetGroupARN: "tg-7", + TargetType: &ipTargetType, + NodeSelector: &nodeSelector, }, }, },