Skip to content

Enforce one target group per target group binding #2098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 25, 2021
Merged

Enforce one target group per target group binding #2098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func main() {
mgr.GetClient(), ctrl.Log.WithName("pod-readiness-gate-injector"))
corewebhook.NewPodMutator(podReadinessGateInjector).SetupWithManager(mgr)
elbv2webhook.NewTargetGroupBindingMutator(cloud.ELBV2(), ctrl.Log).SetupWithManager(mgr)
elbv2webhook.NewTargetGroupBindingValidator(ctrl.Log).SetupWithManager(mgr)
elbv2webhook.NewTargetGroupBindingValidator(mgr.GetClient(), ctrl.Log).SetupWithManager(mgr)
networkingwebhook.NewIngressValidator(mgr.GetClient(), controllerCFG.IngressConfig, ctrl.Log).SetupWithManager(mgr)
//+kubebuilder:scaffold:builder

Expand Down
28 changes: 25 additions & 3 deletions webhooks/elbv2/targetgroupbinding_validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,28 @@ import (
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/runtime"
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
"sigs.k8s.io/aws-load-balancer-controller/pkg/webhook"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

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

// NewTargetGroupBindingMutator returns a mutator for TargetGroupBinding CRD.
func NewTargetGroupBindingValidator(logger logr.Logger) *targetGroupBindingValidator {
func NewTargetGroupBindingValidator(k8sClient client.Client, logger logr.Logger) *targetGroupBindingValidator {
return &targetGroupBindingValidator{
logger: logger,
k8sClient: k8sClient,
logger: logger,
}
}

var _ webhook.Validator = &targetGroupBindingValidator{}

type targetGroupBindingValidator struct {
logger logr.Logger
k8sClient client.Client
logger logr.Logger
}

func (v *targetGroupBindingValidator) Prototype(_ admission.Request) (runtime.Object, error) {
Expand All @@ -40,6 +44,9 @@ func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj ru
if err := v.checkNodeSelector(tgb); err != nil {
return err
}
if err := v.checkExistingTargetGroups(tgb); err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -93,6 +100,21 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG
return nil
}

// checkExistingTargetGroups will check for unique TargetGroup per TargetGroupBinding
func (v *targetGroupBindingValidator) checkExistingTargetGroups(tgb *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")
}
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())
}
}
return nil
}

//checkNodeSelector ensures that NodeSelector is only set when TargetType is ip
func (v *targetGroupBindingValidator) checkNodeSelector(tgb *elbv2api.TargetGroupBinding) error {
if (*tgb.Spec.TargetType == elbv2api.TargetTypeIP) && (tgb.Spec.NodeSelector != nil) {
Expand Down
229 changes: 228 additions & 1 deletion webhooks/elbv2/targetgroupbinding_validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ import (

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/log"

"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
testclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) {
Expand Down Expand Up @@ -61,8 +66,13 @@ func Test_targetGroupBindingValidator_ValidateCreate(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.NewFakeClientWithScheme(k8sSchema)
v := &targetGroupBindingValidator{
logger: &log.NullLogger{},
k8sClient: k8sClient,
logger: &log.NullLogger{},
}
err := v.ValidateCreate(context.Background(), tt.args.obj)
if tt.wantErr != nil {
Expand Down Expand Up @@ -431,3 +441,220 @@ func Test_targetGroupBindingValidator_checkNodeSelector(t *testing.T) {
})
}
}

func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {

type env struct {
existingTGBs []elbv2api.TargetGroupBinding
}

type args struct {
tgb *elbv2api.TargetGroupBinding
}

tests := []struct {
name string
env env
args args
wantErr error
}{
{
name: "[ok] no existing target groups",
env: env{},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
},
wantErr: nil,
},
{
name: "[ok] no duplicate target groups - one target group binding",
env: env{
existingTGBs: []elbv2api.TargetGroupBinding{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-2",
TargetType: nil,
},
},
},
wantErr: nil,
},
{
name: "[ok] no duplicate target groups - multiple target group bindings",
env: env{
existingTGBs: []elbv2api.TargetGroupBinding{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-2",
TargetType: nil,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb3",
Namespace: "ns3",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-3",
TargetType: nil,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb22",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-22",
TargetType: nil,
},
},
},
wantErr: nil,
},
{
name: "[err] duplicate target groups - one target group binding",
env: env{
existingTGBs: []elbv2api.TargetGroupBinding{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
},
wantErr: errors.New("TargetGroup tg-1 is already bound to TargetGroupBinding ns1/tgb1"),
},
{
name: "[err] duplicate target groups - one target group binding",
env: env{
existingTGBs: []elbv2api.TargetGroupBinding{
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb1",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-1",
TargetType: nil,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb2",
Namespace: "ns2",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-111",
TargetType: nil,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb3",
Namespace: "ns3",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-3",
TargetType: nil,
},
},
},
},
args: args{
tgb: &elbv2api.TargetGroupBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "tgb111",
Namespace: "ns1",
},
Spec: elbv2api.TargetGroupBindingSpec{
TargetGroupARN: "tg-111",
TargetType: nil,
},
},
},
wantErr: errors.New("TargetGroup tg-111 is already bound to TargetGroupBinding ns2/tgb2"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k8sSchema := runtime.NewScheme()
clientgoscheme.AddToScheme(k8sSchema)
elbv2api.AddToScheme(k8sSchema)
k8sClient := testclient.NewFakeClientWithScheme(k8sSchema)
v := &targetGroupBindingValidator{
k8sClient: k8sClient,
logger: &log.NullLogger{},
}
for _, tgb := range tt.env.existingTGBs {
assert.NoError(t, k8sClient.Create(context.Background(), tgb.DeepCopy()))
}
err := v.checkExistingTargetGroups(tt.args.tgb)
if tt.wantErr == nil {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantErr.Error())
}
})
}
}