From 9c0af5b747a4da506c0b55db44c501ff03754a4b Mon Sep 17 00:00:00 2001
From: Furkat Gofurov <furkat.gofurov@est.tech>
Date: Wed, 23 Dec 2020 22:12:47 +0200
Subject: [PATCH] Add validations for AzureCluster Updates

---
 api/v1alpha3/azurecluster_webhook.go      | 33 +++++++++++++-
 api/v1alpha3/azurecluster_webhook_test.go | 52 +++++++++++++++++++++--
 2 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/api/v1alpha3/azurecluster_webhook.go b/api/v1alpha3/azurecluster_webhook.go
index c6f351a4f02..a6d73138d39 100644
--- a/api/v1alpha3/azurecluster_webhook.go
+++ b/api/v1alpha3/azurecluster_webhook.go
@@ -17,7 +17,11 @@ limitations under the License.
 package v1alpha3
 
 import (
+	"reflect"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -56,8 +60,35 @@ func (c *AzureCluster) ValidateCreate() error {
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
 	clusterlog.Info("validate update", "name", c.Name)
+	var allErrs field.ErrorList
 	old := oldRaw.(*AzureCluster)
-	return c.validateCluster(old)
+
+	if !reflect.DeepEqual(c.Spec.ResourceGroup, old.Spec.ResourceGroup) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "ResourceGroup"),
+				c.Spec.ResourceGroup, "field is immutable"),
+		)
+	}
+
+	if !reflect.DeepEqual(c.Spec.SubscriptionID, old.Spec.SubscriptionID) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "SubscriptionID"),
+				c.Spec.SubscriptionID, "field is immutable"),
+		)
+	}
+
+	if !reflect.DeepEqual(c.Spec.Location, old.Spec.Location) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "Location"),
+				c.Spec.Location, "field is immutable"),
+		)
+	}
+
+	if len(allErrs) == 0 {
+		return c.validateCluster(old)
+	}
+
+	return apierrors.NewInvalid(GroupVersion.WithKind("AzureCluster").GroupKind(), c.Name, allErrs)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
diff --git a/api/v1alpha3/azurecluster_webhook_test.go b/api/v1alpha3/azurecluster_webhook_test.go
index ee25a804521..03bd2e9a611 100644
--- a/api/v1alpha3/azurecluster_webhook_test.go
+++ b/api/v1alpha3/azurecluster_webhook_test.go
@@ -100,9 +100,10 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 	g := NewWithT(t)
 
 	tests := []struct {
-		name    string
-		cluster *AzureCluster
-		wantErr bool
+		name       string
+		oldCluster *AzureCluster
+		cluster    *AzureCluster
+		wantErr    bool
 	}{
 		{
 			name: "azurecluster with pre-existing vnet - valid spec",
@@ -157,10 +158,53 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 			}(),
 			wantErr: true,
 		},
+		{
+			name: "azurecluster resource group is immutable",
+			oldCluster: &AzureCluster{
+				Spec: AzureClusterSpec{
+					ResourceGroup: "demoResourceGroup",
+				},
+			},
+			cluster: &AzureCluster{
+				Spec: AzureClusterSpec{
+					ResourceGroup: "demoResourceGroup-2",
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "azurecluster subscription ID is immutable",
+			oldCluster: &AzureCluster{
+				Spec: AzureClusterSpec{
+					SubscriptionID: "212ec1q8",
+				},
+			},
+			cluster: &AzureCluster{
+				Spec: AzureClusterSpec{
+					SubscriptionID: "212ec1q9",
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "azurecluster location is immutable",
+			oldCluster: &AzureCluster{
+				Spec: AzureClusterSpec{
+					Location: "North Europe",
+				},
+			},
+			cluster: &AzureCluster{
+				Spec: AzureClusterSpec{
+					Location: "West Europe",
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			err := tc.cluster.ValidateUpdate(createValidCluster())
+			t.Parallel()
+			err := tc.cluster.ValidateUpdate(tc.oldCluster)
 			if tc.wantErr {
 				g.Expect(err).To(HaveOccurred())
 			} else {