fix: making CP LB immutable for disabled type

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>
kubernetes-sigs · Feb 27, 2024 · 772417e · 772417e
1 parent 9b8a4a7
commit 772417e
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 1 deletion.
diff --git a/api/v1beta2/awscluster_webhook.go b/api/v1beta2/awscluster_webhook.go
@@ -153,6 +153,16 @@ func (r *AWSCluster) validateControlPlaneLoadBalancerUpdate(oldlb, newlb *AWSLoa
 			)
 		}
 	} else {
+		// A disabled Load Balancer has many implications that must be treated as immutable/
+		// this is mostly used by externally managed Control Plane, and there's no need to support type changes.
+		// More info: https://kubernetes.slack.com/archives/CD6U2V71N/p1708983246100859?thread_ts=1708973478.410979&cid=CD6U2V71N
+		if (oldlb.LoadBalancerType == LoadBalancerTypeDisabled && newlb.LoadBalancerType != LoadBalancerTypeDisabled) ||
+			(newlb.LoadBalancerType == LoadBalancerTypeDisabled && oldlb.LoadBalancerType != LoadBalancerTypeDisabled) {
+			allErrs = append(allErrs,
+				field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "type"),
+					newlb.Scheme, "field is immutable when created of disabled type"),
+			)
+		}
 		// If old scheme was not nil, the new scheme should be the same.
 		if !cmp.Equal(oldlb.Scheme, newlb.Scheme) {
 			allErrs = append(allErrs,

diff --git a/api/v1beta2/awscluster_webhook_test.go b/api/v1beta2/awscluster_webhook_test.go
@@ -607,12 +607,48 @@ func TestAWSClusterValidateCreate(t *testing.T) {
 }
 
 func TestAWSClusterValidateUpdate(t *testing.T) {
-	tests := []struct {
+	var tests = []struct {
 		name       string
 		oldCluster *AWSCluster
 		newCluster *AWSCluster
 		wantErr    bool
 	}{
+		{
+			name: "Control Plane LB type is immutable when switching from disabled to any",
+			oldCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						LoadBalancerType: LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			newCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						LoadBalancerType: LoadBalancerTypeClassic,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Control Plane LB type is immutable when switching from any to disabled",
+			oldCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						LoadBalancerType: LoadBalancerTypeClassic,
+					},
+				},
+			},
+			newCluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						LoadBalancerType: LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "region is immutable",
 			oldCluster: &AWSCluster{