From d92ea9aea0531fa25679809e252cd43d578563da Mon Sep 17 00:00:00 2001
From: Nawaz Hussain K <k.nawaz.h@gmail.com>
Date: Tue, 20 Dec 2022 10:59:13 -0800
Subject: [PATCH] Fix_Bug: validate non nil AzureMachine.Spec.Diagnostic

- when upgrading from v1alpha4 -> v1beta1, AzureMachine CRD gets updated upon cluster upgrade.
This upgrade updates AzureMachine.Spec.Diagnostics to "Managed", matching the existing behavior as implemented in https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/901

- This PR enables upgrading nil AzureMachine.Spec.Diagnostic to "Managed" and unblocks v1alpha4 -> v1beta1 upgrades.
- adds test cases to test validateUpdate() in case of nil AzureMachine.Spec.Diagnostics
---
 api/v1beta1/azuremachine_webhook.go      | 12 ++++---
 api/v1beta1/azuremachine_webhook_test.go | 42 ++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/api/v1beta1/azuremachine_webhook.go b/api/v1beta1/azuremachine_webhook.go
index aa7d29365bb..ba2e1c8c525 100644
--- a/api/v1beta1/azuremachine_webhook.go
+++ b/api/v1beta1/azuremachine_webhook.go
@@ -143,11 +143,13 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error {
 		allErrs = append(allErrs, err)
 	}
 
-	if err := webhookutils.ValidateImmutable(
-		field.NewPath("Spec", "Diagnostics"),
-		old.Spec.Diagnostics,
-		m.Spec.Diagnostics); err != nil {
-		allErrs = append(allErrs, err)
+	if old.Spec.Diagnostics != nil {
+		if err := webhookutils.ValidateImmutable(
+			field.NewPath("Spec", "Diagnostics"),
+			old.Spec.Diagnostics,
+			m.Spec.Diagnostics); err != nil {
+			allErrs = append(allErrs, err)
+		}
 	}
 
 	if len(allErrs) == 0 {
diff --git a/api/v1beta1/azuremachine_webhook_test.go b/api/v1beta1/azuremachine_webhook_test.go
index 0d04eb39bc5..03ce5906052 100644
--- a/api/v1beta1/azuremachine_webhook_test.go
+++ b/api/v1beta1/azuremachine_webhook_test.go
@@ -562,6 +562,48 @@ func TestAzureMachine_ValidateUpdate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "validTest: azuremachine.spec.Diagnostics should not error on updating nil diagnostics",
+			oldMachine: &AzureMachine{
+				Spec: AzureMachineSpec{},
+			},
+			newMachine: &AzureMachine{
+				Spec: AzureMachineSpec{
+					Diagnostics: &Diagnostics{Boot: &BootDiagnostics{StorageAccountType: ManagedDiagnosticsStorage}},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalidTest: azuremachine.spec.Diagnostics is immutable",
+			oldMachine: &AzureMachine{
+				Spec: AzureMachineSpec{
+					Diagnostics: &Diagnostics{},
+				},
+			},
+			newMachine: &AzureMachine{
+				Spec: AzureMachineSpec{
+					Diagnostics: &Diagnostics{Boot: &BootDiagnostics{StorageAccountType: ManagedDiagnosticsStorage}},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalidTest: azuremachine.spec.Diagnostics is immutable",
+			oldMachine: &AzureMachine{
+				Spec: AzureMachineSpec{
+					Diagnostics: &Diagnostics{
+						Boot: &BootDiagnostics{},
+					},
+				},
+			},
+			newMachine: &AzureMachine{
+				Spec: AzureMachineSpec{
+					Diagnostics: &Diagnostics{Boot: &BootDiagnostics{StorageAccountType: ManagedDiagnosticsStorage}},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {