Skip to content

Commit 6d1a33c

Browse files
zac-nixonzac-nixon
committed
allow multiple targetgroupbindings to reference same tg arn if using multi cluster mode
1 parent 369bdac commit 6d1a33c

File tree

3 files changed

+154
-22
lines changed

3 files changed

+154
-22
lines changed

‎docs/guide/targetgroupbinding/targetgroupbinding.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ spec:
160160
name: awesome-service # route traffic to the awesome-service
161161
port: 80
162162
targetGroupARN: <arn-to-targetGroup>
163-
multiClusterTargetGroup: "true"
163+
multiClusterTargetGroup: true
164164
```
165165

166166

‎webhooks/elbv2/targetgroupbinding_validator.go‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,19 @@ package elbv2
33
import (
44
"context"
55
"fmt"
6+
elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
7+
"k8s.io/apimachinery/pkg/types"
68
"regexp"
9+
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
710
"strings"
811

9-
elbv2types "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
10-
1112
awssdk "github.com/aws/aws-sdk-go-v2/aws"
1213
elbv2sdk "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
1314
"github.com/go-logr/logr"
1415
"github.com/pkg/errors"
1516
"k8s.io/apimachinery/pkg/runtime"
1617
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
1718
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
18-
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
19-
"sigs.k8s.io/aws-load-balancer-controller/pkg/targetgroupbinding"
2019
"sigs.k8s.io/aws-load-balancer-controller/pkg/webhook"
2120
ctrl "sigs.k8s.io/controller-runtime"
2221
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -56,7 +55,6 @@ func (v *targetGroupBindingValidator) Prototype(_ admission.Request) (runtime.Ob
5655

5756
func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
5857
tgb := obj.(*elbv2api.TargetGroupBinding)
59-
targetgroupbinding.AnnotationsToFields(tgb)
6058
if err := v.checkRequiredFields(ctx, tgb); err != nil {
6159
return err
6260
}
@@ -78,7 +76,6 @@ func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj ru
7876
func (v *targetGroupBindingValidator) ValidateUpdate(ctx context.Context, obj runtime.Object, oldObj runtime.Object) error {
7977
tgb := obj.(*elbv2api.TargetGroupBinding)
8078
oldTgb := oldObj.(*elbv2api.TargetGroupBinding)
81-
targetgroupbinding.AnnotationsToFields(tgb)
8279
if err := v.checkRequiredFields(ctx, tgb); err != nil {
8380
return err
8481
}
@@ -88,6 +85,9 @@ func (v *targetGroupBindingValidator) ValidateUpdate(ctx context.Context, obj ru
8885
if err := v.checkNodeSelector(tgb); err != nil {
8986
return err
9087
}
88+
if err := v.checkExistingTargetGroups(tgb); err != nil {
89+
return err
90+
}
9191
return nil
9292
}
9393

@@ -160,17 +160,29 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG
160160
}
161161

162162
// checkExistingTargetGroups will check for unique TargetGroup per TargetGroupBinding
163-
func (v *targetGroupBindingValidator) checkExistingTargetGroups(tgb *elbv2api.TargetGroupBinding) error {
163+
func (v *targetGroupBindingValidator) checkExistingTargetGroups(updatedTgb *elbv2api.TargetGroupBinding) error {
164164
ctx := context.Background()
165165
tgbList := elbv2api.TargetGroupBindingList{}
166+
167+
duplicateTGBs := make([]types.NamespacedName, 0)
168+
multiClusterSupported := updatedTgb.Spec.MultiClusterTargetGroup
169+
166170
if err := v.k8sClient.List(ctx, &tgbList); err != nil {
167171
return errors.Wrap(err, "failed to list TargetGroupBindings in the cluster")
168172
}
169173
for _, tgbObj := range tgbList.Items {
170-
if tgbObj.Spec.TargetGroupARN == tgb.Spec.TargetGroupARN {
171-
return errors.Errorf("TargetGroup %v is already bound to TargetGroupBinding %v", tgb.Spec.TargetGroupARN, k8s.NamespacedName(&tgbObj).String())
174+
if tgbObj.UID != updatedTgb.UID && tgbObj.Spec.TargetGroupARN == updatedTgb.Spec.TargetGroupARN {
175+
if !tgbObj.Spec.MultiClusterTargetGroup {
176+
multiClusterSupported = false
177+
}
178+
duplicateTGBs = append(duplicateTGBs, k8s.NamespacedName(&tgbObj))
172179
}
173180
}
181+
182+
if len(duplicateTGBs) != 0 && !multiClusterSupported {
183+
return errors.Errorf("TargetGroup %v is already bound to following TargetGroupBindings %v. Please enable MultiCluster mode on all TargetGroupBindings referencing %v or choose a different Target Group ARN.", updatedTgb.Spec.TargetGroupARN, duplicateTGBs, updatedTgb.Spec.TargetGroupARN)
184+
}
185+
174186
return nil
175187
}
176188

@@ -184,7 +196,7 @@ func (v *targetGroupBindingValidator) checkNodeSelector(tgb *elbv2api.TargetGrou
184196

185197
// checkTargetGroupIPAddressType ensures IP address type matches with that on the AWS target group
186198
func (v *targetGroupBindingValidator) checkTargetGroupIPAddressType(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error {
187-
targetGroupIPAddressType, err := v.getTargetGroupIPAddressTypeFromAWS(ctx, tgb)
199+
targetGroupIPAddressType, err := v.getTargetGroupIPAddressTypeFromAWS(ctx, tgb.Spec.TargetGroupARN)
188200
if err != nil {
189201
return errors.Wrap(err, "unable to get target group IP address type")
190202
}
@@ -203,7 +215,7 @@ func (v *targetGroupBindingValidator) checkTargetGroupVpcID(ctx context.Context,
203215
if !vpcIDPatternRegex.MatchString(tgb.Spec.VpcID) {
204216
return errors.Errorf(vpcIDValidationErr, tgb.Spec.VpcID)
205217
}
206-
vpcID, err := v.getVpcIDFromAWS(ctx, tgb)
218+
vpcID, err := v.getVpcIDFromAWS(ctx, tgb.Spec.TargetGroupARN)
207219
if err != nil {
208220
return errors.Wrap(err, "unable to get target group VpcID")
209221
}
@@ -214,8 +226,8 @@ func (v *targetGroupBindingValidator) checkTargetGroupVpcID(ctx context.Context,
214226
}
215227

216228
// getTargetGroupIPAddressTypeFromAWS returns the target group IP address type of AWS target group
217-
func (v *targetGroupBindingValidator) getTargetGroupIPAddressTypeFromAWS(ctx context.Context, tgb *elbv2api.TargetGroupBinding) (elbv2api.TargetGroupIPAddressType, error) {
218-
targetGroup, err := v.getTargetGroupFromAWS(ctx, tgb)
229+
func (v *targetGroupBindingValidator) getTargetGroupIPAddressTypeFromAWS(ctx context.Context, tgARN string) (elbv2api.TargetGroupIPAddressType, error) {
230+
targetGroup, err := v.getTargetGroupFromAWS(ctx, tgARN)
219231
if err != nil {
220232
return "", err
221233
}
@@ -232,12 +244,11 @@ func (v *targetGroupBindingValidator) getTargetGroupIPAddressTypeFromAWS(ctx con
232244
}
233245

234246
// getTargetGroupFromAWS returns the AWS target group corresponding to the ARN
235-
func (v *targetGroupBindingValidator) getTargetGroupFromAWS(ctx context.Context, tgb *elbv2api.TargetGroupBinding) (*elbv2types.TargetGroup, error) {
236-
tgARN := tgb.Spec.TargetGroupARN
247+
func (v *targetGroupBindingValidator) getTargetGroupFromAWS(ctx context.Context, tgARN string) (*elbv2types.TargetGroup, error) {
237248
req := &elbv2sdk.DescribeTargetGroupsInput{
238249
TargetGroupArns: []string{tgARN},
239250
}
240-
tgList, err := v.elbv2Client.AssumeRole(ctx, tgb.Spec.IamRoleArnToAssume, tgb.Spec.AssumeRoleExternalId).DescribeTargetGroupsAsList(ctx, req)
251+
tgList, err := v.elbv2Client.DescribeTargetGroupsAsList(ctx, req)
241252
if err != nil {
242253
return nil, err
243254
}
@@ -247,8 +258,8 @@ func (v *targetGroupBindingValidator) getTargetGroupFromAWS(ctx context.Context,
247258
return &tgList[0], nil
248259
}
249260

250-
func (v *targetGroupBindingValidator) getVpcIDFromAWS(ctx context.Context, tgb *elbv2api.TargetGroupBinding) (string, error) {
251-
targetGroup, err := v.getTargetGroupFromAWS(ctx, tgb)
261+
func (v *targetGroupBindingValidator) getVpcIDFromAWS(ctx context.Context, tgARN string) (string, error) {
262+
targetGroup, err := v.getTargetGroupFromAWS(ctx, tgARN)
252263
if err != nil {
253264
return "", err
254265
}

‎webhooks/elbv2/targetgroupbinding_validator_test.go‎

Lines changed: 124 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,14 @@ func Test_targetGroupBindingValidator_ValidateUpdate(t *testing.T) {
462462
}
463463
for _, tt := range tests {
464464
t.Run(tt.name, func(t *testing.T) {
465+
k8sSchema := runtime.NewScheme()
466+
clientgoscheme.AddToScheme(k8sSchema)
467+
elbv2api.AddToScheme(k8sSchema)
468+
k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build()
469+
465470
v := &targetGroupBindingValidator{
466-
logger: logr.New(&log.NullLogSink{}),
471+
logger: logr.New(&log.NullLogSink{}),
472+
k8sClient: k8sClient,
467473
}
468474
err := v.ValidateUpdate(context.Background(), tt.args.obj, tt.args.oldObj)
469475
if tt.wantErr != nil {
@@ -954,6 +960,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
954960
ObjectMeta: metav1.ObjectMeta{
955961
Name: "tgb1",
956962
Namespace: "ns1",
963+
UID: "tgb1",
957964
},
958965
Spec: elbv2api.TargetGroupBindingSpec{
959966
TargetGroupARN: "tg-1",
@@ -971,6 +978,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
971978
ObjectMeta: metav1.ObjectMeta{
972979
Name: "tgb1",
973980
Namespace: "ns1",
981+
UID: "tgb1",
974982
},
975983
Spec: elbv2api.TargetGroupBindingSpec{
976984
TargetGroupARN: "tg-1",
@@ -984,6 +992,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
984992
ObjectMeta: metav1.ObjectMeta{
985993
Name: "tgb2",
986994
Namespace: "ns2",
995+
UID: "tgb2",
987996
},
988997
Spec: elbv2api.TargetGroupBindingSpec{
989998
TargetGroupARN: "tg-2",
@@ -1001,6 +1010,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10011010
ObjectMeta: metav1.ObjectMeta{
10021011
Name: "tgb1",
10031012
Namespace: "ns1",
1013+
UID: "tgb1",
10041014
},
10051015
Spec: elbv2api.TargetGroupBindingSpec{
10061016
TargetGroupARN: "tg-1",
@@ -1011,6 +1021,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10111021
ObjectMeta: metav1.ObjectMeta{
10121022
Name: "tgb2",
10131023
Namespace: "ns2",
1024+
UID: "tgb2",
10141025
},
10151026
Spec: elbv2api.TargetGroupBindingSpec{
10161027
TargetGroupARN: "tg-2",
@@ -1021,19 +1032,32 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10211032
ObjectMeta: metav1.ObjectMeta{
10221033
Name: "tgb3",
10231034
Namespace: "ns3",
1035+
UID: "tgb3",
10241036
},
10251037
Spec: elbv2api.TargetGroupBindingSpec{
10261038
TargetGroupARN: "tg-3",
10271039
TargetType: nil,
10281040
},
10291041
},
1042+
{
1043+
ObjectMeta: metav1.ObjectMeta{
1044+
Name: "tgb22",
1045+
Namespace: "ns1",
1046+
UID: "tgb22",
1047+
},
1048+
Spec: elbv2api.TargetGroupBindingSpec{
1049+
TargetGroupARN: "tg-22",
1050+
TargetType: nil,
1051+
},
1052+
},
10301053
},
10311054
},
10321055
args: args{
10331056
tgb: &elbv2api.TargetGroupBinding{
10341057
ObjectMeta: metav1.ObjectMeta{
10351058
Name: "tgb22",
10361059
Namespace: "ns1",
1060+
UID: "tgb22",
10371061
},
10381062
Spec: elbv2api.TargetGroupBindingSpec{
10391063
TargetGroupARN: "tg-22",
@@ -1051,6 +1075,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10511075
ObjectMeta: metav1.ObjectMeta{
10521076
Name: "tgb1",
10531077
Namespace: "ns1",
1078+
UID: "tgb1",
10541079
},
10551080
Spec: elbv2api.TargetGroupBindingSpec{
10561081
TargetGroupARN: "tg-1",
@@ -1064,14 +1089,106 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10641089
ObjectMeta: metav1.ObjectMeta{
10651090
Name: "tgb2",
10661091
Namespace: "ns1",
1092+
UID: "tgb2",
1093+
},
1094+
Spec: elbv2api.TargetGroupBindingSpec{
1095+
TargetGroupARN: "tg-1",
1096+
TargetType: nil,
1097+
},
1098+
},
1099+
},
1100+
wantErr: errors.New("TargetGroup tg-1 is already bound to following TargetGroupBindings [ns1/tgb1]. Please enable MultiCluster mode on all TargetGroupBindings referencing tg-1 or choose a different Target Group ARN."),
1101+
},
1102+
{
1103+
name: "[ok] duplicate target groups with multi cluster support",
1104+
env: env{
1105+
existingTGBs: []elbv2api.TargetGroupBinding{
1106+
{
1107+
ObjectMeta: metav1.ObjectMeta{
1108+
Name: "tgb1",
1109+
Namespace: "ns1",
1110+
UID: "tgb1",
1111+
},
1112+
Spec: elbv2api.TargetGroupBindingSpec{
1113+
TargetGroupARN: "tg-1",
1114+
TargetType: nil,
1115+
MultiClusterTargetGroup: true,
1116+
},
1117+
},
1118+
},
1119+
},
1120+
args: args{
1121+
tgb: &elbv2api.TargetGroupBinding{
1122+
ObjectMeta: metav1.ObjectMeta{
1123+
Name: "tgb2",
1124+
Namespace: "ns1",
1125+
UID: "tgb2",
1126+
},
1127+
Spec: elbv2api.TargetGroupBindingSpec{
1128+
TargetGroupARN: "tg-1",
1129+
TargetType: nil,
1130+
MultiClusterTargetGroup: true,
1131+
},
1132+
},
1133+
},
1134+
wantErr: nil,
1135+
},
1136+
{
1137+
name: "[err] try to add binding without multicluster support while multiple bindings are using the same tg arn",
1138+
env: env{
1139+
existingTGBs: []elbv2api.TargetGroupBinding{
1140+
{
1141+
ObjectMeta: metav1.ObjectMeta{
1142+
Name: "tgb1",
1143+
Namespace: "ns1",
1144+
UID: "tgb1",
1145+
},
1146+
Spec: elbv2api.TargetGroupBindingSpec{
1147+
TargetGroupARN: "tg-1",
1148+
TargetType: nil,
1149+
MultiClusterTargetGroup: true,
1150+
},
1151+
},
1152+
{
1153+
ObjectMeta: metav1.ObjectMeta{
1154+
Name: "tgb3",
1155+
Namespace: "ns1",
1156+
UID: "tgb3",
1157+
},
1158+
Spec: elbv2api.TargetGroupBindingSpec{
1159+
TargetGroupARN: "tg-1",
1160+
TargetType: nil,
1161+
MultiClusterTargetGroup: true,
1162+
},
1163+
},
1164+
{
1165+
ObjectMeta: metav1.ObjectMeta{
1166+
Name: "tgb4",
1167+
Namespace: "ns1",
1168+
UID: "tgb4",
1169+
},
1170+
Spec: elbv2api.TargetGroupBindingSpec{
1171+
TargetGroupARN: "tg-1",
1172+
TargetType: nil,
1173+
MultiClusterTargetGroup: true,
1174+
},
1175+
},
1176+
},
1177+
},
1178+
args: args{
1179+
tgb: &elbv2api.TargetGroupBinding{
1180+
ObjectMeta: metav1.ObjectMeta{
1181+
Name: "tgb2",
1182+
Namespace: "ns1",
1183+
UID: "tgb2",
10671184
},
10681185
Spec: elbv2api.TargetGroupBindingSpec{
10691186
TargetGroupARN: "tg-1",
10701187
TargetType: nil,
10711188
},
10721189
},
10731190
},
1074-
wantErr: errors.New("TargetGroup tg-1 is already bound to TargetGroupBinding ns1/tgb1"),
1191+
wantErr: errors.New("TargetGroup tg-1 is already bound to following TargetGroupBindings [ns1/tgb1 ns1/tgb3 ns1/tgb4]. Please enable MultiCluster mode on all TargetGroupBindings referencing tg-1 or choose a different Target Group ARN."),
10751192
},
10761193
{
10771194
name: "[err] duplicate target groups - one target group binding",
@@ -1081,6 +1198,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10811198
ObjectMeta: metav1.ObjectMeta{
10821199
Name: "tgb1",
10831200
Namespace: "ns1",
1201+
UID: "tgb1",
10841202
},
10851203
Spec: elbv2api.TargetGroupBindingSpec{
10861204
TargetGroupARN: "tg-1",
@@ -1091,6 +1209,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10911209
ObjectMeta: metav1.ObjectMeta{
10921210
Name: "tgb2",
10931211
Namespace: "ns2",
1212+
UID: "tgb2",
10941213
},
10951214
Spec: elbv2api.TargetGroupBindingSpec{
10961215
TargetGroupARN: "tg-111",
@@ -1101,6 +1220,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
11011220
ObjectMeta: metav1.ObjectMeta{
11021221
Name: "tgb3",
11031222
Namespace: "ns3",
1223+
UID: "tgb3",
11041224
},
11051225
Spec: elbv2api.TargetGroupBindingSpec{
11061226
TargetGroupARN: "tg-3",
@@ -1114,14 +1234,15 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
11141234
ObjectMeta: metav1.ObjectMeta{
11151235
Name: "tgb111",
11161236
Namespace: "ns1",
1237+
UID: "tgb111",
11171238
},
11181239
Spec: elbv2api.TargetGroupBindingSpec{
11191240
TargetGroupARN: "tg-111",
11201241
TargetType: nil,
11211242
},
11221243
},
11231244
},
1124-
wantErr: errors.New("TargetGroup tg-111 is already bound to TargetGroupBinding ns2/tgb2"),
1245+
wantErr: errors.New("TargetGroup tg-111 is already bound to following TargetGroupBindings [ns2/tgb2]. Please enable MultiCluster mode on all TargetGroupBindings referencing tg-111 or choose a different Target Group ARN."),
11251246
},
11261247
}
11271248
for _, tt := range tests {

0 commit comments

Comments
 (0)