Skip to content

Commit

Permalink
Merge pull request kubernetes-sigs#4021 from zac-nixon/znixon/mc-same…
Browse files Browse the repository at this point in the history 
…-cluster

[feat] allow multiple targetgroupbindings to reference same tg arn if using multicluster mode
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 28, 2025
2 parents 7118c22 + 92ae3dc commit 227315a
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 7 deletions.
2 changes: 1 addition & 1 deletion docs/guide/targetgroupbinding/targetgroupbinding.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ spec:
name: awesome-service # route traffic to the awesome-service
port: 80
targetGroupARN: <arn-to-targetGroup>
multiClusterTargetGroup: "true"
multiClusterTargetGroup: true
```


Expand Down
22 changes: 19 additions & 3 deletions webhooks/elbv2/targetgroupbinding_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package elbv2
import (
"context"
"fmt"
"k8s.io/apimachinery/pkg/types"
"regexp"
"strings"

Expand Down Expand Up @@ -90,6 +91,9 @@ func (v *targetGroupBindingValidator) ValidateUpdate(ctx context.Context, obj ru
if err := v.checkAssumeRoleConfig(tgb); err != nil {
return err
}
if err := v.checkExistingTargetGroups(tgb); err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -162,17 +166,29 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG
}

// checkExistingTargetGroups will check for unique TargetGroup per TargetGroupBinding
func (v *targetGroupBindingValidator) checkExistingTargetGroups(tgb *elbv2api.TargetGroupBinding) error {
func (v *targetGroupBindingValidator) checkExistingTargetGroups(updatedTgb *elbv2api.TargetGroupBinding) error {
ctx := context.Background()
tgbList := elbv2api.TargetGroupBindingList{}
if err := v.k8sClient.List(ctx, &tgbList); err != nil {
return errors.Wrap(err, "failed to list TargetGroupBindings in the cluster")
}

duplicateTGBs := make([]types.NamespacedName, 0)
multiClusterSupported := updatedTgb.Spec.MultiClusterTargetGroup

for _, tgbObj := range tgbList.Items {
if tgbObj.Spec.TargetGroupARN == tgb.Spec.TargetGroupARN {
return errors.Errorf("TargetGroup %v is already bound to TargetGroupBinding %v", tgb.Spec.TargetGroupARN, k8s.NamespacedName(&tgbObj).String())
if tgbObj.UID != updatedTgb.UID && tgbObj.Spec.TargetGroupARN == updatedTgb.Spec.TargetGroupARN {
if !tgbObj.Spec.MultiClusterTargetGroup {
multiClusterSupported = false
}
duplicateTGBs = append(duplicateTGBs, k8s.NamespacedName(&tgbObj))
}
}

if len(duplicateTGBs) != 0 && !multiClusterSupported {
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)
}

return nil
}

Expand Down
127 changes: 124 additions & 3 deletions webhooks/elbv2/targetgroupbinding_validator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,14 @@ func Test_targetGroupBindingValidator_ValidateUpdate(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k8sSchema := runtime.NewScheme()
clientgoscheme.AddToScheme(k8sSchema)
elbv2api.AddToScheme(k8sSchema)
k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build()

v := &targetGroupBindingValidator{
logger: logr.New(&log.NullLogSink{}),
logger: logr.New(&log.NullLogSink{}),
k8sClient: k8sClient,
}
err := v.ValidateUpdate(context.Background(), tt.args.obj, tt.args.oldObj)
if tt.wantErr != nil {
Expand Down Expand Up @@ -954,6 +960,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
Expand All @@ -971,6 +978,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
Expand All @@ -984,6 +992,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns2",
UID: "tgb2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-2",
Expand All @@ -1001,6 +1010,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
Expand All @@ -1011,6 +1021,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns2",
UID: "tgb2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-2",
Expand All @@ -1021,19 +1032,32 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb3",
Namespace: "ns3",
UID: "tgb3",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-3",
TargetType: nil,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb22",
Namespace: "ns1",
UID: "tgb22",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-22",
TargetType: nil,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb22",
Namespace: "ns1",
UID: "tgb22",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-22",
Expand All @@ -1051,6 +1075,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
Expand All @@ -1064,14 +1089,106 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns1",
UID: "tgb2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
},
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."),
},
{
name: "[ok] duplicate target groups with multi cluster support",
env: env{
existingTGBs: []elbv2api.TargetGroupBinding{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
MultiClusterTargetGroup: true,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns1",
UID: "tgb2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
MultiClusterTargetGroup: true,
},
},
},
wantErr: nil,
},
{
name: "[err] try to add binding without multicluster support while multiple bindings are using the same tg arn",
env: env{
existingTGBs: []elbv2api.TargetGroupBinding{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
MultiClusterTargetGroup: true,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb3",
Namespace: "ns1",
UID: "tgb3",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
MultiClusterTargetGroup: true,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb4",
Namespace: "ns1",
UID: "tgb4",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
MultiClusterTargetGroup: true,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns1",
UID: "tgb2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
},
wantErr: errors.New("TargetGroup tg-1 is already bound to TargetGroupBinding ns1/tgb1"),
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."),
},
{
name: "[err] duplicate target groups - one target group binding",
Expand All @@ -1081,6 +1198,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
UID: "tgb1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
Expand All @@ -1091,6 +1209,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns2",
UID: "tgb2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-111",
Expand All @@ -1101,6 +1220,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb3",
Namespace: "ns3",
UID: "tgb3",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-3",
Expand All @@ -1114,14 +1234,15 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "tgb111",
Namespace: "ns1",
UID: "tgb111",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-111",
TargetType: nil,
},
},
},
wantErr: errors.New("TargetGroup tg-111 is already bound to TargetGroupBinding ns2/tgb2"),
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."),
},
}
for _, tt := range tests {
Expand Down

0 comments on commit 227315a

Please sign in to comment.