Skip to content

Commit 27803e3

Browse files
author
Fawad Khaliq
authored
Enforce one target group per target group binding (#2098)
1 parent 88f3733 commit 27803e3

File tree

3 files changed

+254
-5
lines changed

3 files changed

+254
-5
lines changed

‎main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func main() {
144144
mgr.GetClient(), ctrl.Log.WithName("pod-readiness-gate-injector"))
145145
corewebhook.NewPodMutator(podReadinessGateInjector).SetupWithManager(mgr)
146146
elbv2webhook.NewTargetGroupBindingMutator(cloud.ELBV2(), ctrl.Log).SetupWithManager(mgr)
147-
elbv2webhook.NewTargetGroupBindingValidator(ctrl.Log).SetupWithManager(mgr)
147+
elbv2webhook.NewTargetGroupBindingValidator(mgr.GetClient(), ctrl.Log).SetupWithManager(mgr)
148148
networkingwebhook.NewIngressValidator(mgr.GetClient(), controllerCFG.IngressConfig, ctrl.Log).SetupWithManager(mgr)
149149
//+kubebuilder:scaffold:builder
150150

‎webhooks/elbv2/targetgroupbinding_validator.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,28 @@ import (
88
"github.com/pkg/errors"
99
"k8s.io/apimachinery/pkg/runtime"
1010
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
11+
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
1112
"sigs.k8s.io/aws-load-balancer-controller/pkg/webhook"
1213
ctrl "sigs.k8s.io/controller-runtime"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
1315
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1416
)
1517

1618
const apiPathValidateELBv2TargetGroupBinding = "/validate-elbv2-k8s-aws-v1beta1-targetgroupbinding"
1719

1820
// NewTargetGroupBindingMutator returns a mutator for TargetGroupBinding CRD.
19-
func NewTargetGroupBindingValidator(logger logr.Logger) *targetGroupBindingValidator {
21+
func NewTargetGroupBindingValidator(k8sClient client.Client, logger logr.Logger) *targetGroupBindingValidator {
2022
return &targetGroupBindingValidator{
21-
logger: logger,
23+
k8sClient: k8sClient,
24+
logger: logger,
2225
}
2326
}
2427

2528
var _ webhook.Validator = &targetGroupBindingValidator{}
2629

2730
type targetGroupBindingValidator struct {
28-
logger logr.Logger
31+
k8sClient client.Client
32+
logger logr.Logger
2933
}
3034

3135
func (v *targetGroupBindingValidator) Prototype(_ admission.Request) (runtime.Object, error) {
@@ -40,6 +44,9 @@ func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj ru
4044
if err := v.checkNodeSelector(tgb); err != nil {
4145
return err
4246
}
47+
if err := v.checkExistingTargetGroups(tgb); err != nil {
48+
return err
49+
}
4350
return nil
4451
}
4552

@@ -93,6 +100,21 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG
93100
return nil
94101
}
95102

103+
// checkExistingTargetGroups will check for unique TargetGroup per TargetGroupBinding
104+
func (v *targetGroupBindingValidator) checkExistingTargetGroups(tgb *elbv2api.TargetGroupBinding) error {
105+
ctx := context.Background()
106+
tgbList := elbv2api.TargetGroupBindingList{}
107+
if err := v.k8sClient.List(ctx, &tgbList); err != nil {
108+
return errors.Wrap(err, "failed to list TargetGroupBindings in the cluster")
109+
}
110+
for _, tgbObj := range tgbList.Items {
111+
if tgbObj.Spec.TargetGroupARN == tgb.Spec.TargetGroupARN {
112+
return errors.Errorf("TargetGroup %v is already bound to TargetGroupBinding %v", tgb.Spec.TargetGroupARN, k8s.NamespacedName(&tgbObj).String())
113+
}
114+
}
115+
return nil
116+
}
117+
96118
//checkNodeSelector ensures that NodeSelector is only set when TargetType is ip
97119
func (v *targetGroupBindingValidator) checkNodeSelector(tgb *elbv2api.TargetGroupBinding) error {
98120
if (*tgb.Spec.TargetType == elbv2api.TargetTypeIP) && (tgb.Spec.NodeSelector != nil) {

‎webhooks/elbv2/targetgroupbinding_validator_test.go

Lines changed: 228 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@ import (
66

77
"github.com/pkg/errors"
88
"github.com/stretchr/testify/assert"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1011
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
1112
"sigs.k8s.io/controller-runtime/pkg/log"
13+
14+
"k8s.io/apimachinery/pkg/runtime"
15+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
16+
testclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
1217
)
1318

1419
func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) {
@@ -61,8 +66,13 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) {
6166
}
6267
for _, tt := range tests {
6368
t.Run(tt.name, func(t *testing.T) {
69+
k8sSchema := runtime.NewScheme()
70+
clientgoscheme.AddToScheme(k8sSchema)
71+
elbv2api.AddToScheme(k8sSchema)
72+
k8sClient := testclient.NewFakeClientWithScheme(k8sSchema)
6473
v := &targetGroupBindingValidator{
65-
logger: &log.NullLogger{},
74+
k8sClient: k8sClient,
75+
logger: &log.NullLogger{},
6676
}
6777
err := v.ValidateCreate(context.Background(), tt.args.obj)
6878
if tt.wantErr != nil {
@@ -431,3 +441,220 @@ func Test_targetGroupBindingValidator_checkNodeSelector(t *testing.T) {
431441
})
432442
}
433443
}
444+
445+
func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
446+
447+
type env struct {
448+
existingTGBs []elbv2api.TargetGroupBinding
449+
}
450+
451+
type args struct {
452+
tgb *elbv2api.TargetGroupBinding
453+
}
454+
455+
tests := []struct {
456+
name string
457+
env env
458+
args args
459+
wantErr error
460+
}{
461+
{
462+
name: "[ok] no existing target groups",
463+
env: env{},
464+
args: args{
465+
tgb: &elbv2api.TargetGroupBinding{
466+
ObjectMeta: metav1.ObjectMeta{
467+
Name: "tgb1",
468+
Namespace: "ns1",
469+
},
470+
Spec: elbv2api.TargetGroupBindingSpec{
471+
TargetGroupARN: "tg-1",
472+
TargetType: nil,
473+
},
474+
},
475+
},
476+
wantErr: nil,
477+
},
478+
{
479+
name: "[ok] no duplicate target groups - one target group binding",
480+
env: env{
481+
existingTGBs: []elbv2api.TargetGroupBinding{
482+
{
483+
ObjectMeta: metav1.ObjectMeta{
484+
Name: "tgb1",
485+
Namespace: "ns1",
486+
},
487+
Spec: elbv2api.TargetGroupBindingSpec{
488+
TargetGroupARN: "tg-1",
489+
TargetType: nil,
490+
},
491+
},
492+
},
493+
},
494+
args: args{
495+
tgb: &elbv2api.TargetGroupBinding{
496+
ObjectMeta: metav1.ObjectMeta{
497+
Name: "tgb2",
498+
Namespace: "ns2",
499+
},
500+
Spec: elbv2api.TargetGroupBindingSpec{
501+
TargetGroupARN: "tg-2",
502+
TargetType: nil,
503+
},
504+
},
505+
},
506+
wantErr: nil,
507+
},
508+
{
509+
name: "[ok] no duplicate target groups - multiple target group bindings",
510+
env: env{
511+
existingTGBs: []elbv2api.TargetGroupBinding{
512+
{
513+
ObjectMeta: metav1.ObjectMeta{
514+
Name: "tgb1",
515+
Namespace: "ns1",
516+
},
517+
Spec: elbv2api.TargetGroupBindingSpec{
518+
TargetGroupARN: "tg-1",
519+
TargetType: nil,
520+
},
521+
},
522+
{
523+
ObjectMeta: metav1.ObjectMeta{
524+
Name: "tgb2",
525+
Namespace: "ns2",
526+
},
527+
Spec: elbv2api.TargetGroupBindingSpec{
528+
TargetGroupARN: "tg-2",
529+
TargetType: nil,
530+
},
531+
},
532+
{
533+
ObjectMeta: metav1.ObjectMeta{
534+
Name: "tgb3",
535+
Namespace: "ns3",
536+
},
537+
Spec: elbv2api.TargetGroupBindingSpec{
538+
TargetGroupARN: "tg-3",
539+
TargetType: nil,
540+
},
541+
},
542+
},
543+
},
544+
args: args{
545+
tgb: &elbv2api.TargetGroupBinding{
546+
ObjectMeta: metav1.ObjectMeta{
547+
Name: "tgb22",
548+
Namespace: "ns1",
549+
},
550+
Spec: elbv2api.TargetGroupBindingSpec{
551+
TargetGroupARN: "tg-22",
552+
TargetType: nil,
553+
},
554+
},
555+
},
556+
wantErr: nil,
557+
},
558+
{
559+
name: "[err] duplicate target groups - one target group binding",
560+
env: env{
561+
existingTGBs: []elbv2api.TargetGroupBinding{
562+
{
563+
ObjectMeta: metav1.ObjectMeta{
564+
Name: "tgb1",
565+
Namespace: "ns1",
566+
},
567+
Spec: elbv2api.TargetGroupBindingSpec{
568+
TargetGroupARN: "tg-1",
569+
TargetType: nil,
570+
},
571+
},
572+
},
573+
},
574+
args: args{
575+
tgb: &elbv2api.TargetGroupBinding{
576+
ObjectMeta: metav1.ObjectMeta{
577+
Name: "tgb2",
578+
Namespace: "ns1",
579+
},
580+
Spec: elbv2api.TargetGroupBindingSpec{
581+
TargetGroupARN: "tg-1",
582+
TargetType: nil,
583+
},
584+
},
585+
},
586+
wantErr: errors.New("TargetGroup tg-1 is already bound to TargetGroupBinding ns1/tgb1"),
587+
},
588+
{
589+
name: "[err] duplicate target groups - one target group binding",
590+
env: env{
591+
existingTGBs: []elbv2api.TargetGroupBinding{
592+
{
593+
ObjectMeta: metav1.ObjectMeta{
594+
Name: "tgb1",
595+
Namespace: "ns1",
596+
},
597+
Spec: elbv2api.TargetGroupBindingSpec{
598+
TargetGroupARN: "tg-1",
599+
TargetType: nil,
600+
},
601+
},
602+
{
603+
ObjectMeta: metav1.ObjectMeta{
604+
Name: "tgb2",
605+
Namespace: "ns2",
606+
},
607+
Spec: elbv2api.TargetGroupBindingSpec{
608+
TargetGroupARN: "tg-111",
609+
TargetType: nil,
610+
},
611+
},
612+
{
613+
ObjectMeta: metav1.ObjectMeta{
614+
Name: "tgb3",
615+
Namespace: "ns3",
616+
},
617+
Spec: elbv2api.TargetGroupBindingSpec{
618+
TargetGroupARN: "tg-3",
619+
TargetType: nil,
620+
},
621+
},
622+
},
623+
},
624+
args: args{
625+
tgb: &elbv2api.TargetGroupBinding{
626+
ObjectMeta: metav1.ObjectMeta{
627+
Name: "tgb111",
628+
Namespace: "ns1",
629+
},
630+
Spec: elbv2api.TargetGroupBindingSpec{
631+
TargetGroupARN: "tg-111",
632+
TargetType: nil,
633+
},
634+
},
635+
},
636+
wantErr: errors.New("TargetGroup tg-111 is already bound to TargetGroupBinding ns2/tgb2"),
637+
},
638+
}
639+
for _, tt := range tests {
640+
t.Run(tt.name, func(t *testing.T) {
641+
k8sSchema := runtime.NewScheme()
642+
clientgoscheme.AddToScheme(k8sSchema)
643+
elbv2api.AddToScheme(k8sSchema)
644+
k8sClient := testclient.NewFakeClientWithScheme(k8sSchema)
645+
v := &targetGroupBindingValidator{
646+
k8sClient: k8sClient,
647+
logger: &log.NullLogger{},
648+
}
649+
for _, tgb := range tt.env.existingTGBs {
650+
assert.NoError(t, k8sClient.Create(context.Background(), tgb.DeepCopy()))
651+
}
652+
err := v.checkExistingTargetGroups(tt.args.tgb)
653+
if tt.wantErr == nil {
654+
assert.NoError(t, err)
655+
} else {
656+
assert.EqualError(t, err, tt.wantErr.Error())
657+
}
658+
})
659+
}
660+
}

0 commit comments

Comments
 (0)