Merge pull request #3194 from willie-yao/fix-machinepool-default

MachinePools: Fix AzureMachinePool default for SystemAssignedIdentityRole

Author: k8s-ci-robot

api/v1beta1/azuremachine_default_test.go

@@ -79,7 +79,10 @@ func TestAzureMachineSpec_SetIdentityDefaults(t *testing.T) {
 			DefinitionID: fakeRoleDefinitionID,
 		},
 	}}}
-
+	deprecatedRoleAssignmentNameTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
+		Identity:           VMIdentitySystemAssigned,
+		RoleAssignmentName: existingRoleAssignmentName,
+	}}}
 	emptyTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
 		Identity:                   VMIdentitySystemAssigned,
 		SystemAssignedIdentityRole: &SystemAssignedIdentityRole{},
@@ -95,6 +98,10 @@ func TestAzureMachineSpec_SetIdentityDefaults(t *testing.T) {
 	g.Expect(systemAssignedIdentityRoleExistTest.machine.Spec.SystemAssignedIdentityRole.Scope).To(Equal(fakeScope))
 	g.Expect(systemAssignedIdentityRoleExistTest.machine.Spec.SystemAssignedIdentityRole.DefinitionID).To(Equal(fakeRoleDefinitionID))
 
+	deprecatedRoleAssignmentNameTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
+	g.Expect(deprecatedRoleAssignmentNameTest.machine.Spec.SystemAssignedIdentityRole.Name).To(Equal(existingRoleAssignmentName))
+	g.Expect(deprecatedRoleAssignmentNameTest.machine.Spec.RoleAssignmentName).To(BeEmpty())
+
 	emptyTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
 	g.Expect(emptyTest.machine.Spec.SystemAssignedIdentityRole.Name).To(Not(BeEmpty()))
 	_, err := uuid.Parse(emptyTest.machine.Spec.SystemAssignedIdentityRole.Name)

exp/api/v1beta1/azuremachinepool_default.go

@@ -72,18 +72,14 @@ func (amp *AzureMachinePool) SetIdentityDefaults(subscriptionID string) {
 		return
 	}
 	if amp.Spec.Identity == infrav1.VMIdentitySystemAssigned {
-		if amp.Spec.RoleAssignmentName == "" {
-			amp.Spec.RoleAssignmentName = string(uuid.NewUUID())
-		}
 		if amp.Spec.SystemAssignedIdentityRole == nil {
 			amp.Spec.SystemAssignedIdentityRole = &infrav1.SystemAssignedIdentityRole{}
 		}
-		if amp.Spec.SystemAssignedIdentityRole.Name == "" {
+		if amp.Spec.RoleAssignmentName != "" {
+			amp.Spec.SystemAssignedIdentityRole.Name = amp.Spec.RoleAssignmentName
+			amp.Spec.RoleAssignmentName = ""
+		} else if amp.Spec.SystemAssignedIdentityRole.Name == "" {
 			amp.Spec.SystemAssignedIdentityRole.Name = string(uuid.NewUUID())
-			if amp.Spec.RoleAssignmentName != "" {
-				amp.Spec.SystemAssignedIdentityRole.Name = amp.Spec.RoleAssignmentName
-				amp.Spec.RoleAssignmentName = ""
-			}
 		}
 		if amp.Spec.SystemAssignedIdentityRole.Scope == "" {
 			// Default scope to the subscription.

exp/api/v1beta1/azuremachinepool_default_test.go

@@ -75,6 +75,10 @@ func TestAzureMachinePool_SetIdentityDefaults(t *testing.T) {
 			Scope:        fakeScope,
 		},
 	}}}
+	deprecatedRoleAssignmentNameTest := test{machinePool: &AzureMachinePool{Spec: AzureMachinePoolSpec{
+		Identity:           infrav1.VMIdentitySystemAssigned,
+		RoleAssignmentName: existingRoleAssignmentName,
+	}}}
 	emptyTest := test{machinePool: &AzureMachinePool{Spec: AzureMachinePoolSpec{
 		Identity:                   infrav1.VMIdentitySystemAssigned,
 		SystemAssignedIdentityRole: &infrav1.SystemAssignedIdentityRole{},
@@ -90,6 +94,10 @@ func TestAzureMachinePool_SetIdentityDefaults(t *testing.T) {
 	g.Expect(systemAssignedIdentityRoleExistTest.machinePool.Spec.SystemAssignedIdentityRole.Scope).To(Equal(fakeScope))
 	g.Expect(systemAssignedIdentityRoleExistTest.machinePool.Spec.SystemAssignedIdentityRole.DefinitionID).To(Equal(fakeRoleDefinitionID))
 
+	deprecatedRoleAssignmentNameTest.machinePool.SetIdentityDefaults(fakeSubscriptionID)
+	g.Expect(deprecatedRoleAssignmentNameTest.machinePool.Spec.SystemAssignedIdentityRole.Name).To(Equal(existingRoleAssignmentName))
+	g.Expect(deprecatedRoleAssignmentNameTest.machinePool.Spec.RoleAssignmentName).To(BeEmpty())
+
 	emptyTest.machinePool.SetIdentityDefaults(fakeSubscriptionID)
 	g.Expect(emptyTest.machinePool.Spec.SystemAssignedIdentityRole.Name).To(Not(BeEmpty()))
 	_, err := uuid.Parse(emptyTest.machinePool.Spec.SystemAssignedIdentityRole.Name)

exp/api/v1beta1/azuremachinepool_webhook.go

@@ -208,7 +208,7 @@ func (amp *AzureMachinePool) ValidateSystemAssignedIdentity(old runtime.Object)
 // ValidateSystemAssignedIdentityRole validates the scope and roleDefinitionID for the system-assigned identity.
 func (amp *AzureMachinePool) ValidateSystemAssignedIdentityRole() error {
 	var allErrs field.ErrorList
-	if amp.Spec.RoleAssignmentName != "" && amp.Spec.SystemAssignedIdentityRole.Name != "" {
+	if amp.Spec.RoleAssignmentName != "" && amp.Spec.SystemAssignedIdentityRole != nil && amp.Spec.SystemAssignedIdentityRole.Name != "" {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("systemAssignedIdentityRole"), amp.Spec.SystemAssignedIdentityRole.Name, "cannot set both roleAssignmentName and systemAssignedIdentityRole.name"))
 	}
 	if amp.Spec.Identity == infrav1.VMIdentitySystemAssigned {