Validate providerID for user-assigned IDs in webhook

kubernetes-sigs · Jun 9, 2023 · 0451032 · 0451032
1 parent f514dce
commit 0451032
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 8 deletions.
diff --git a/api/v1beta1/azuremachine_validation.go b/api/v1beta1/azuremachine_validation.go
@@ -19,14 +19,16 @@ package v1beta1
 import (
 	"encoding/base64"
 	"fmt"
+	"strings"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
 	"github.com/google/uuid"
 	"golang.org/x/crypto/ssh"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 )
 
-// ValidateAzureMachineSpec check for validation errors of azuremachine.spec.
+// ValidateAzureMachineSpec checks an AzureMachineSpec and returns any validation errors.
 func ValidateAzureMachineSpec(spec AzureMachineSpec) field.ErrorList {
 	var allErrs field.ErrorList
 
@@ -124,9 +126,19 @@ func ValidateSystemAssignedIdentity(identityType VMIdentity, oldIdentity, newIde
 func ValidateUserAssignedIdentity(identityType VMIdentity, userAssignedIdentities []UserAssignedIdentity, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if identityType == VMIdentityUserAssigned && len(userAssignedIdentities) == 0 {
-		allErrs = append(allErrs, field.Required(fldPath, "must be specified for the 'UserAssigned' identity type"))
+	if identityType == VMIdentityUserAssigned {
+		if len(userAssignedIdentities) == 0 {
+			allErrs = append(allErrs, field.Required(fldPath, "must be specified for the 'UserAssigned' identity type"))
+		}
+		for _, identity := range userAssignedIdentities {
+			if identity.ProviderID != "" {
+				if _, err := arm.ParseResourceID(strings.TrimPrefix(identity.ProviderID, "azure://")); err != nil {
+					allErrs = append(allErrs, field.Invalid(fldPath, identity.ProviderID, "must be a valid Azure resource ID"))
+				}
+			}
+		}
 	}
+
 	return allErrs
 }
 

diff --git a/api/v1beta1/azuremachine_validation_test.go b/api/v1beta1/azuremachine_validation_test.go
@@ -580,6 +580,75 @@ func TestAzureMachine_ValidateSystemAssignedIdentityRole(t *testing.T) {
 	}
 }
 
+func TestAzureMachine_ValidateUserAssignedIdentity(t *testing.T) {
+	g := NewWithT(t)
+
+	tests := []struct {
+		name       string
+		idType     VMIdentity
+		identities []UserAssignedIdentity
+		wantErr    bool
+	}{
+		{
+			name:       "empty identity list",
+			idType:     VMIdentityUserAssigned,
+			identities: []UserAssignedIdentity{},
+			wantErr:    true,
+		},
+		{
+			name:   "invalid: providerID must start with slash",
+			idType: VMIdentityUserAssigned,
+			identities: []UserAssignedIdentity{
+				{
+					ProviderID: "subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-20202-control-plane-7w265",
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name:   "invalid: providerID must start with subscriptions or providers",
+			idType: VMIdentityUserAssigned,
+			identities: []UserAssignedIdentity{
+				{
+					ProviderID: "azure:///prescriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-20202-control-plane-7w265",
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name:   "valid",
+			idType: VMIdentityUserAssigned,
+			identities: []UserAssignedIdentity{
+				{
+					ProviderID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-20202-control-plane-7w265",
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name:   "valid with provider prefix",
+			idType: VMIdentityUserAssigned,
+			identities: []UserAssignedIdentity{
+				{
+					ProviderID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-20202-control-plane-7w265",
+				},
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			errs := ValidateUserAssignedIdentity(tc.idType, tc.identities, field.NewPath("userAssignedIdentities"))
+			if tc.wantErr {
+				g.Expect(errs).NotTo(BeEmpty())
+			} else {
+				g.Expect(errs).To(BeEmpty())
+			}
+		})
+	}
+}
+
 func TestAzureMachine_ValidateDataDisksUpdate(t *testing.T) {
 	g := NewWithT(t)
 

diff --git a/api/v1beta1/azuremachine_webhook_test.go b/api/v1beta1/azuremachine_webhook_test.go
@@ -90,8 +90,11 @@ func TestAzureMachine_ValidateCreate(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name:    "azuremachine with list of user-assigned identities",
-			machine: createMachineWithUserAssignedIdentities([]UserAssignedIdentity{{ProviderID: "azure:///123"}, {ProviderID: "azure:///456"}}),
+			name: "azuremachine with list of user-assigned identities",
+			machine: createMachineWithUserAssignedIdentities([]UserAssignedIdentity{
+				{ProviderID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-12345-control-plane-9d5x5"},
+				{ProviderID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-12345-control-plane-a1b2c"},
+			}),
 			wantErr: false,
 		},
 		{

diff --git a/api/v1beta1/azuremachinetemplate_webhook_test.go b/api/v1beta1/azuremachinetemplate_webhook_test.go
@@ -102,7 +102,10 @@ func TestAzureMachineTemplate_ValidateCreate(t *testing.T) {
 		{
 			name: "azuremachinetemplate with list of user-assigned identities",
 			machineTemplate: createAzureMachineTemplateFromMachine(
-				createMachineWithUserAssignedIdentities([]UserAssignedIdentity{{ProviderID: "azure:///123"}, {ProviderID: "azure:///456"}}),
+				createMachineWithUserAssignedIdentities([]UserAssignedIdentity{
+					{ProviderID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-09091-control-plane-f1b2c"},
+					{ProviderID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-09091-control-plane-9a8b7"},
+				}),
 			),
 			wantErr: false,
 		},

diff --git a/exp/api/v1beta1/azuremachinepool_webhook_test.go b/exp/api/v1beta1/azuremachinepool_webhook_test.go
@@ -146,8 +146,11 @@ func TestAzureMachinePool_ValidateCreate(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name:    "azuremachinepool with user assigned identity",
-			amp:     createMachinePoolWithUserAssignedIdentity([]string{"azure:://id1", "azure:://id2"}),
+			name: "azuremachinepool with user assigned identity",
+			amp: createMachinePoolWithUserAssignedIdentity([]string{
+				"azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-20202-control-plane-7w265",
+				"azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Compute/virtualMachines/default-20202-control-plane-a6b7d",
+			}),
 			wantErr: false,
 		},
 		{