From 772417e5e9c1c43033b69f36a981549b74bcc942 Mon Sep 17 00:00:00 2001
From: Dario Tranchitella <dario@tranchitella.eu>
Date: Tue, 27 Feb 2024 17:34:38 +0100
Subject: [PATCH] fix: making CP LB immutable for disabled type

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>
---
 api/v1beta2/awscluster_webhook.go      | 10 +++++++
 api/v1beta2/awscluster_webhook_test.go | 38 +++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/api/v1beta2/awscluster_webhook.go b/api/v1beta2/awscluster_webhook.go
index 4e1a2dbb12..26f7be1711 100644
--- 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
index 85342552c6..3492608a89 100644
--- 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{