Merge pull request #3630 from ikosenn/fix-tgb-backwards-compatibility

fix: Adding VpcID to TGB broke upgradability.
kubernetes-sigs · Apr 1, 2024 · c2515fe · c2515fe
2 parents 8393192 + bec483d
commit c2515fe
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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 {