Merge pull request #1098 from Nordix/azCluster-validation-furkat

Add validations for immutable AzureCluster Updates

Author: k8s-ci-robot

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/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

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 {