From b40bddf50e01b903a091335427d7fadfcb5125de Mon Sep 17 00:00:00 2001 From: Chris Dituri Date: Sun, 18 Aug 2024 11:50:23 -0500 Subject: [PATCH] fixup! fixup! Better error handling when importing azure role definitions --- .../authorization/parse/role_definition.go | 29 ++--- .../parse/role_definition_test.go | 118 ++++++++++++++---- 2 files changed, 105 insertions(+), 42 deletions(-) diff --git a/internal/services/authorization/parse/role_definition.go b/internal/services/authorization/parse/role_definition.go index 5843c5774ad4a..ccb22d2bb0eea 100644 --- a/internal/services/authorization/parse/role_definition.go +++ b/internal/services/authorization/parse/role_definition.go @@ -43,18 +43,18 @@ func (r RoleDefinitionID) Segments() []resourceids.Segment { } var roleDefinitionIdRegexp *regexp.Regexp = regexp.MustCompile( - "^(/subscriptions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})?" + - "/providers/Microsoft.Authorization/roleDefinitions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$") + "^(?i)(?:/subscriptions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})?" + + "/providers/Microsoft.Authorization/roleDefinitions/([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})$") var subscriptionScopeRegexp *regexp.Regexp = regexp.MustCompile( - "^(/subscriptions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})" + - "(/resourceGroups/[_\\-.()0-9a-zA-Z]{1,89}[_\\-()0-9a-zA-Z]{1})?$") + "^(?i)(?:/subscriptions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})" + + "(?:/resourceGroups/[_\\-.()0-9a-zA-Z]{1,89}[_\\-()0-9a-zA-Z]{1})?$") var mgmtGroupScopeRegexp *regexp.Regexp = regexp.MustCompile( - "^(/providers/Microsoft.Management/managementGroups/[_\\-.()0-9a-zA-Z]{1,89}[_\\-()0-9a-zA-Z]{1})" + - "(/subscriptions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})?") + "^(?i)(?:/providers/Microsoft.Management/managementGroups/[_\\-.()0-9a-zA-Z]{1,89}[_\\-()0-9a-zA-Z]{1})" + + "(?:/subscriptions/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})?$") -const allScopesToken string = "/" +const allScopes string = "/" // RoleDefinitionId is a pseudo ID for storing Scope parameter as this it not retrievable from API // It is formed of the Azure Resource ID for the Role and the Scope it is created against @@ -64,8 +64,8 @@ func RoleDefinitionId(input string) (*RoleDefinitionID, error) { return nil, fmt.Errorf("could not parse Role Definition ID, invalid format %q", input) } - resourceId := roleDefinitionIdRegexp.FindString(parts[0]) - if resourceId == "" { + roleID := roleDefinitionIdRegexp.FindStringSubmatch(parts[0]) + if len(roleID) != 2 { return nil, fmt.Errorf("could not parse Role Definition ID, invalid format %q", parts[0]) } @@ -74,21 +74,16 @@ func RoleDefinitionId(input string) (*RoleDefinitionID, error) { scope = mgmtGroupScopeRegexp.FindString(parts[1]) if scope == "" { scope = parts[1] - if scope != allScopesToken { + if scope != allScopes { return nil, fmt.Errorf("could not parse scope from Role Definition ID, invalid format %q", parts[1]) } } } roleDefinitionID := RoleDefinitionID{ - ResourceID: resourceId, + ResourceID: roleID[0], Scope: scope, - } - - if idParts := strings.Split(resourceId, "roleDefinitions/"); len(idParts) < 1 { - return nil, fmt.Errorf("failed to parse Role Definition ID from resource ID %q", input) - } else { - roleDefinitionID.RoleID = idParts[1] + RoleID: roleID[1], } return &roleDefinitionID, nil diff --git a/internal/services/authorization/parse/role_definition_test.go b/internal/services/authorization/parse/role_definition_test.go index 7329a82d19bc7..4253bf0ac1013 100644 --- a/internal/services/authorization/parse/role_definition_test.go +++ b/internal/services/authorization/parse/role_definition_test.go @@ -5,57 +5,125 @@ package parse import ( "testing" + + "github.com/google/go-cmp/cmp" ) func TestParseRoleDefinitionID(t *testing.T) { testData := []struct { - RoleDefinitionID string - Expect bool + Input string + Expected *RoleDefinitionID + ShouldParse bool }{ { - RoleDefinitionID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/00000000-0000-0000-0000-000000000000|/subscriptions/00000000-0000-0000-0000-000000000000", - Expect: true, + Input: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/12345678-1234-1234-1234-1234567890ab|/subscriptions/00000000-0000-0000-0000-000000000000", + Expected: &RoleDefinitionID{ + ResourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/12345678-1234-1234-1234-1234567890ab", + Scope: "/subscriptions/00000000-0000-0000-0000-000000000000", + RoleID: "12345678-1234-1234-1234-1234567890ab", + }, + ShouldParse: true, + }, + { + Input: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/12345678-1234-1234-1234-1234567890ab|/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/valid.resource()-_group", + Expected: &RoleDefinitionID{ + ResourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/12345678-1234-1234-1234-1234567890ab", + Scope: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/valid.resource()-_group", + RoleID: "12345678-1234-1234-1234-1234567890ab", + }, + ShouldParse: true, + }, + { + Input: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/00000000-0000-0000-0000-000000000000|/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/invalid.resource()-_group.", + Expected: nil, + ShouldParse: false, + }, + { + Input: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd|/providers/Microsoft.Management/managementGroups/Some-Management-Group", + Expected: &RoleDefinitionID{ + ResourceID: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd", + Scope: "/providers/Microsoft.Management/managementGroups/Some-Management-Group", + RoleID: "ab65ee35-dc39-43ac-86a0-accaadf4abcd", + }, + ShouldParse: true, + }, + { + Input: "/pRoVidErs/MiCrOsOft.AuThORiZaTiOn/RoLeDeFinItiOns/ab65ee35-DC39-43AC-86a0-accaadf4abcd|/PrOviDeRs/MiCrOsOfT.MaNaGeMeNt/maNAGementGroups/Some-Management-Group", + Expected: &RoleDefinitionID{ + ResourceID: "/pRoVidErs/MiCrOsOft.AuThORiZaTiOn/RoLeDeFinItiOns/ab65ee35-DC39-43AC-86a0-accaadf4abcd", + Scope: "/PrOviDeRs/MiCrOsOfT.MaNaGeMeNt/maNAGementGroups/Some-Management-Group", + RoleID: "ab65ee35-DC39-43AC-86a0-accaadf4abcd", + }, + ShouldParse: true, }, { - RoleDefinitionID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/00000000-0000-0000-0000-000000000000|/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/valid.resource()-_group", - Expect: true, + Input: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd|/providers/Microsoft.Management/managementGroups/Some-Management-Group/subscriptions/12345678-1234-1234-1234-1234567890ab", + Expected: &RoleDefinitionID{ + ResourceID: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd", + Scope: "/providers/Microsoft.Management/managementGroups/Some-Management-Group/subscriptions/12345678-1234-1234-1234-1234567890ab", + RoleID: "ab65ee35-dc39-43ac-86a0-accaadf4abcd", + }, + ShouldParse: true, }, { - RoleDefinitionID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/00000000-0000-0000-0000-000000000000|/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/invalid.resource()-_group.", - Expect: false, + Input: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd|/", + Expected: &RoleDefinitionID{ + ResourceID: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd", + Scope: "/", + RoleID: "ab65ee35-dc39-43ac-86a0-accaadf4abcd", + }, + ShouldParse: true, }, { - RoleDefinitionID: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd|/providers/Microsoft.Management/managementGroups/Some-Management-Group", - Expect: true, + Input: "/providers/Microsoft.Authorization/rOlEdEfiNiTiOns/AB65ee35-Dc39-43aC-86a0-aCCaaDf4abcd|/", + Expected: &RoleDefinitionID{ + ResourceID: "/providers/Microsoft.Authorization/rOlEdEfiNiTiOns/AB65ee35-Dc39-43aC-86a0-aCCaaDf4abcd", + Scope: "/", + RoleID: "AB65ee35-Dc39-43aC-86a0-aCCaaDf4abcd", + }, + ShouldParse: true, }, { - RoleDefinitionID: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd|/providers/Microsoft.Management/managementGroups/Some-Management-Group/subscriptions/12345678-1234-1234-1234567890ab", - Expect: true, + Input: "12345678-1234-1234-1234-1234567890ab|/providers/Microsoft.Management/managementGroups/Some-Management-Group", + Expected: nil, + ShouldParse: false, }, { - RoleDefinitionID: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-dc39-43ac-86a0-accaadf4abcd|/", - Expect: true, + Input: "12345678-1234-inva-lid1-|/providers/Microsoft.Management/managementGroups/Some-Management-Group", + Expected: nil, + ShouldParse: false, }, { - RoleDefinitionID: "12345678-1234-1234-1234-1234567890ab|/providers/Microsoft.Management/managementGroups/Some-Management-Group", - Expect: false, + Input: "12345678-nonsense", + Expected: nil, + ShouldParse: false, + }, + { + Input: "/providers/Microsoft.Authorization/roleDefinitions/ab65ee35-INVALID-86a0-accaadf4abcd|/", + Expected: nil, + ShouldParse: false, }, } for _, v := range testData { - t.Logf("parsing %q, expecting %+v", v.RoleDefinitionID, v.Expect) + t.Logf("[DEBUG] parsing %q, ShouldParse %+v", v.Input, v.ShouldParse) + + roleDefinitionId, err := RoleDefinitionId(v.Input) - roleDefinitionID, err := RoleDefinitionId(v.RoleDefinitionID) - if err != nil { - if v.Expect == true { - t.Fatalf("expected %q parse success, got %q", v.RoleDefinitionID, err) + if v.ShouldParse { + switch { + case err != nil: + t.Fatalf("expected %q parse success, got failure: %q", v.Input, err) + case !cmp.Equal(v.Expected, roleDefinitionId): + t.Fatalf("parse succeeded but expected %+v, got %+v", v.Expected, roleDefinitionId) + default: + t.Logf("[DEBUG] parse succeeded as expected, got:\n%+v", roleDefinitionId) } - t.Logf("parse failed as expected, got %q", err) } else { - if v.Expect == false { - t.Fatalf("expected %q parse failure, got:\n%+v", v.RoleDefinitionID, roleDefinitionID) + if err == nil { + t.Fatalf("expected %q parse failure, got success: %+v", v.Input, roleDefinitionId) } - t.Logf("parse succeeded as expected, got:\n%+v", roleDefinitionID) + t.Logf("[DEBUG] parse failed as expected, got %q", err) } } }