From bec483d1c71954ecf9fc1a8debc405e4ac4a193c Mon Sep 17 00:00:00 2001 From: Ian Kosen Date: Fri, 29 Mar 2024 22:15:22 -0700 Subject: [PATCH] fix: Adding VpcID to TGB broke upgradability. --- main.go | 2 +- .../elbv2/targetgroupbinding_validator.go | 7 ++-- .../targetgroupbinding_validator_test.go | 32 ++++++++++++++++--- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/main.go b/main.go index 6115ea0b6..88cc1755a 100644 --- a/main.go +++ b/main.go @@ -165,7 +165,7 @@ func main() { corewebhook.NewServiceMutator(controllerCFG.ServiceConfig.LoadBalancerClass, ctrl.Log).SetupWithManager(mgr) elbv2webhook.NewIngressClassParamsValidator().SetupWithManager(mgr) elbv2webhook.NewTargetGroupBindingMutator(cloud.ELBV2(), ctrl.Log).SetupWithManager(mgr) - elbv2webhook.NewTargetGroupBindingValidator(mgr.GetClient(), cloud.ELBV2(), ctrl.Log).SetupWithManager(mgr) + elbv2webhook.NewTargetGroupBindingValidator(mgr.GetClient(), cloud.ELBV2(), cloud.VpcID(), ctrl.Log).SetupWithManager(mgr) networkingwebhook.NewIngressValidator(mgr.GetClient(), controllerCFG.IngressConfig, ctrl.Log).SetupWithManager(mgr) //+kubebuilder:scaffold:builder diff --git a/webhooks/elbv2/targetgroupbinding_validator.go b/webhooks/elbv2/targetgroupbinding_validator.go index 58629f3b2..1b0ce5c82 100644 --- a/webhooks/elbv2/targetgroupbinding_validator.go +++ b/webhooks/elbv2/targetgroupbinding_validator.go @@ -21,11 +21,12 @@ import ( const apiPathValidateELBv2TargetGroupBinding = "/validate-elbv2-k8s-aws-v1beta1-targetgroupbinding" // NewTargetGroupBindingValidator returns a validator for TargetGroupBinding CRD. -func NewTargetGroupBindingValidator(k8sClient client.Client, elbv2Client services.ELBV2, logger logr.Logger) *targetGroupBindingValidator { +func NewTargetGroupBindingValidator(k8sClient client.Client, elbv2Client services.ELBV2, vpcID string, logger logr.Logger) *targetGroupBindingValidator { return &targetGroupBindingValidator{ k8sClient: k8sClient, elbv2Client: elbv2Client, logger: logger, + vpcID: vpcID, } } @@ -35,6 +36,7 @@ type targetGroupBindingValidator struct { k8sClient client.Client elbv2Client services.ELBV2 logger logr.Logger + vpcID string } func (v *targetGroupBindingValidator) Prototype(_ admission.Request) (runtime.Object, error) { @@ -112,7 +114,8 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG changedImmutableFields = append(changedImmutableFields, "spec.ipAddressType") } if (tgb.Spec.VpcID != "" && oldTGB.Spec.VpcID != "" && (tgb.Spec.VpcID) != (oldTGB.Spec.VpcID)) || - (tgb.Spec.VpcID == "") != (oldTGB.Spec.VpcID == "") { + (oldTGB.Spec.VpcID != "" && tgb.Spec.VpcID == "") || + (oldTGB.Spec.VpcID == "" && tgb.Spec.VpcID != "" && tgb.Spec.VpcID != v.vpcID) { changedImmutableFields = append(changedImmutableFields, "spec.vpcID") } if len(changedImmutableFields) != 0 { diff --git a/webhooks/elbv2/targetgroupbinding_validator_test.go b/webhooks/elbv2/targetgroupbinding_validator_test.go index 1cf07a45a..b083bc4ca 100644 --- a/webhooks/elbv2/targetgroupbinding_validator_test.go +++ b/webhooks/elbv2/targetgroupbinding_validator_test.go @@ -39,6 +39,7 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { } instanceTargetType := elbv2api.TargetTypeInstance ipTargetType := elbv2api.TargetTypeIP + clusterVpcID := "vpcid-02" tests := []struct { name string fields fields @@ -194,7 +195,7 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { TargetGroupArn: awssdk.String("tg-2"), TargetType: awssdk.String("instance"), IpAddressType: awssdk.String("ipv6"), - VpcId: awssdk.String("vpcid-02"), + VpcId: &clusterVpcID, }, }, }, @@ -207,7 +208,7 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { TargetGroupArn: awssdk.String("tg-2"), TargetType: awssdk.String("instance"), IpAddressType: awssdk.String("ipv6"), - VpcId: awssdk.String("vpcid-02"), + VpcId: &clusterVpcID, }, }, }, @@ -219,7 +220,7 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { TargetGroupARN: "tg-2", TargetType: &instanceTargetType, IPAddressType: &targetGroupIPAddressTypeIPv6, - VpcID: "vpcid-02", + VpcID: clusterVpcID, }, }, }, @@ -238,7 +239,7 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { TargetGroupArn: awssdk.String("tg-2"), TargetType: awssdk.String("instance"), IpAddressType: awssdk.String("ipv6"), - VpcId: awssdk.String("vpcid-02"), + VpcId: &clusterVpcID, }, }, }, @@ -251,7 +252,7 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { TargetGroupArn: awssdk.String("tg-2"), TargetType: awssdk.String("instance"), IpAddressType: awssdk.String("ipv6"), - VpcId: awssdk.String("vpcid-02"), + VpcId: &clusterVpcID, }, }, }, @@ -479,6 +480,7 @@ func Test_targetGroupBindingValidator_checkImmutableFields(t *testing.T) { } instanceTargetType := elbv2api.TargetTypeInstance ipTargetType := elbv2api.TargetTypeIP + clusterVpcID := "cluster-vpc-id" tests := []struct { name string args args @@ -746,11 +748,31 @@ func Test_targetGroupBindingValidator_checkImmutableFields(t *testing.T) { }, wantErr: errors.New("TargetGroupBinding update may not change these fields: spec.vpcID"), }, + { + name: "VpcID modified from nil to cluster vpc-id is allowed", + args: args{ + tgb: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + VpcID: clusterVpcID, + }, + }, + oldTGB: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + }, + }, + }, + wantErr: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := &targetGroupBindingValidator{ logger: logr.New(&log.NullLogSink{}), + vpcID: clusterVpcID, } err := v.checkImmutableFields(tt.args.tgb, tt.args.oldTGB) if tt.wantErr != nil {