Skip to content

Commit

Permalink
azurerm_role_assignment: Fix assignments to resources (#12076)
Browse files Browse the repository at this point in the history 
Fixes  #12074 
Fixes  #12060 
Fixes  #12057
Fixes #12079
Fixes #12078
Fixes #12087

Related to/similar for [go-azure-helpers](https://github.com/hashicorp/go-azure-helpers): [this PR](hashicorp/go-azure-helpers#79)
  • Loading branch information
@aristosvo
aristosvo authored Jun 8, 2021
1 parent 64ea516 commit f2ee194
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 31 deletions.
30 changes: 20 additions & 10 deletions azurerm/helpers/azure/resourceid.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import (
// level fields, and other key-value pairs available via a map in the
// Path field.
type ResourceID struct {
SubscriptionID string
ResourceGroup string
Provider string
Path map[string]string
SubscriptionID string
ResourceGroup string
Provider string
SecondaryProvider string
Path map[string]string
}

// ParseAzureResourceID converts a long-form Azure Resource Manager ID
Expand All @@ -40,6 +41,7 @@ func ParseAzureResourceID(id string) (*ResourceID, error) {
}

var subscriptionID string
var provider string

// Put the constituent key-value pairs into a map
componentMap := make(map[string]string, len(components)/2)
Expand All @@ -52,11 +54,16 @@ func ParseAzureResourceID(id string) (*ResourceID, error) {
return nil, fmt.Errorf("Key/Value cannot be empty strings. Key: '%s', Value: '%s'", key, value)
}

// Catch the subscriptionID before it can be overwritten by another "subscriptions"
// value in the ID which is the case for the Service Bus subscription resource
if key == "subscriptions" && subscriptionID == "" {
switch {
case key == "subscriptions" && subscriptionID == "":
// Catch the subscriptionID before it can be overwritten by another "subscriptions"
// value in the ID which is the case for the Service Bus subscription resource
subscriptionID = value
} else {
case key == "providers" && provider == "":
// Catch the provider before it can be overwritten by another "providers"
// value in the ID which can be the case for the Role Assignment resource
provider = value
default:
componentMap[key] = value
}
}
Expand All @@ -82,9 +89,12 @@ func ParseAzureResourceID(id string) (*ResourceID, error) {
delete(componentMap, "resourcegroups")
}

// It is OK not to have a provider in the case of a resource group
if provider, ok := componentMap["providers"]; ok {
if provider != "" {
idObj.Provider = provider
}

if secondaryProvider := componentMap["providers"]; secondaryProvider != "" {
idObj.SecondaryProvider = secondaryProvider
delete(componentMap, "providers")
}

Expand Down
14 changes: 14 additions & 0 deletions azurerm/helpers/azure/resourceid_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,20 @@ func TestParseAzureResourceID(t *testing.T) {
},
false,
},
{
"/subscriptions/11111111-1111-1111-1111-111111111111/resourceGroups/example-resources/providers/Microsoft.Storage/storageAccounts/nameStorageAccount/providers/Microsoft.Authorization/roleAssignments/22222222-2222-2222-2222-222222222222",
&azure.ResourceID{
SubscriptionID: "11111111-1111-1111-1111-111111111111",
ResourceGroup: "example-resources",
Provider: "Microsoft.Storage",
SecondaryProvider: "Microsoft.Authorization",
Path: map[string]string{
"storageAccounts": "nameStorageAccount",
"roleAssignments": "22222222-2222-2222-2222-222222222222",
},
},
false,
},
{
// missing resource group
"/subscriptions/11111111-1111-1111-1111-111111111111/providers/Microsoft.ApiManagement/service/service1/subscriptions/22222222-2222-2222-2222-222222222222",
Expand Down
40 changes: 29 additions & 11 deletions azurerm/internal/services/authorization/parse/role_assignment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ import (
)

type RoleAssignmentId struct {
SubscriptionID string
ResourceGroup string
ManagementGroup string
Name string
TenantId string
SubscriptionID string
ResourceGroup string
ManagementGroup string
ResourceScope string
ResourceProvider string
Name string
TenantId string
}

func NewRoleAssignmentID(subscriptionId, resourceGroup, managementGroup, name, tenantId string) (*RoleAssignmentId, error) {
func NewRoleAssignmentID(subscriptionId, resourceGroup, resourceProvider, resourceScope, managementGroup, name, tenantId string) (*RoleAssignmentId, error) {
if subscriptionId == "" && resourceGroup == "" && managementGroup == "" {
return nil, fmt.Errorf("one of subscriptionId, resourceGroup, or managementGroup must be provided")
}
Expand All @@ -33,17 +35,24 @@ func NewRoleAssignmentID(subscriptionId, resourceGroup, managementGroup, name, t
}

return &RoleAssignmentId{
SubscriptionID: subscriptionId,
ResourceGroup: resourceGroup,
ManagementGroup: managementGroup,
Name: name,
TenantId: tenantId,
SubscriptionID: subscriptionId,
ResourceGroup: resourceGroup,
ResourceProvider: resourceProvider,
ResourceScope: resourceScope,
ManagementGroup: managementGroup,
Name: name,
TenantId: tenantId,
}, nil
}

// in general case, the id format does not change
// for cross tenant scenario, add the tenantId info
func (id RoleAssignmentId) AzureResourceID() string {
if id.ResourceScope != "" {
fmtString := "/subscriptions/%s/resourceGroups/%s/providers/%s/%s/providers/Microsoft.Authorization/roleAssignments/%s"
return fmt.Sprintf(fmtString, id.SubscriptionID, id.ResourceGroup, id.ResourceProvider, id.ResourceScope, id.Name)
}

if id.ManagementGroup != "" {
fmtString := "/providers/Microsoft.Management/managementGroups/%s/providers/Microsoft.Authorization/roleAssignments/%s"
return fmt.Sprintf(fmtString, id.ManagementGroup, id.Name)
Expand Down Expand Up @@ -90,6 +99,15 @@ func RoleAssignmentID(input string) (*RoleAssignmentId, error) {
}
roleAssignmentId.SubscriptionID = id.SubscriptionID
roleAssignmentId.ResourceGroup = id.ResourceGroup
if id.Provider != "Microsoft.Authorization" && id.Provider != "" {
roleAssignmentId.ResourceProvider = id.Provider
// logic to save resource scope
result := strings.Split(input, "/providers/")
if len(result) == 3 {
roleAssignmentId.ResourceScope = strings.TrimPrefix(result[1], fmt.Sprintf("%s/", id.Provider))
}
}

if roleAssignmentId.Name, err = id.PopSegment("roleAssignments"); err != nil {
return nil, err
}
Expand Down
72 changes: 62 additions & 10 deletions azurerm/internal/services/authorization/parse/role_assignment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,43 @@ var _ resourceid.Formatter = RoleAssignmentId{}

func TestRoleAssignmentIDFormatter(t *testing.T) {
testData := []struct {
SubscriptionId string
ResourceGroup string
ManagementGroup string
Name string
TenantId string
Expected string
SubscriptionId string
ResourceGroup string
ResourceProvider string
ResourceScope string
ManagementGroup string
Name string
TenantId string
Expected string
}{
{
SubscriptionId: "",
ResourceGroup: "",
ResourceScope: "",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "",
},
{
SubscriptionId: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "group1",
ResourceScope: "",
ManagementGroup: "managementGroup1",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "",
},
{
SubscriptionId: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "",
ResourceScope: "",
ManagementGroup: "managementGroup1",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "",
},
{
SubscriptionId: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "",
ResourceScope: "",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "",
Expand All @@ -49,6 +55,7 @@ func TestRoleAssignmentIDFormatter(t *testing.T) {
{
SubscriptionId: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "group1",
ResourceScope: "",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "",
Expand All @@ -57,6 +64,7 @@ func TestRoleAssignmentIDFormatter(t *testing.T) {
{
SubscriptionId: "",
ResourceGroup: "",
ResourceScope: "",
ManagementGroup: "12345678-1234-9876-4563-123456789012",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "",
Expand All @@ -65,15 +73,26 @@ func TestRoleAssignmentIDFormatter(t *testing.T) {
{
SubscriptionId: "",
ResourceGroup: "",
ResourceScope: "",
ManagementGroup: "12345678-1234-9876-4563-123456789012",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "34567812-3456-7653-6742-345678901234",
Expected: "/providers/Microsoft.Management/managementGroups/12345678-1234-9876-4563-123456789012/providers/Microsoft.Authorization/roleAssignments/23456781-2349-8764-5631-234567890121|34567812-3456-7653-6742-345678901234",
},
{
SubscriptionId: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "group1",
ResourceProvider: "Microsoft.Storage",
ResourceScope: "storageAccounts/nameStorageAccount",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "34567812-3456-7653-6742-345678901234",
Expected: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/group1/providers/Microsoft.Storage/storageAccounts/nameStorageAccount/providers/Microsoft.Authorization/roleAssignments/23456781-2349-8764-5631-234567890121|34567812-3456-7653-6742-345678901234",
},
}
for _, v := range testData {
t.Logf("testing %+v", v)
actual, err := NewRoleAssignmentID(v.SubscriptionId, v.ResourceGroup, v.ManagementGroup, v.Name, v.TenantId)
actual, err := NewRoleAssignmentID(v.SubscriptionId, v.ResourceGroup, v.ResourceProvider, v.ResourceScope, v.ManagementGroup, v.Name, v.TenantId)
if err != nil {
if v.Expected == "" {
continue
Expand Down Expand Up @@ -140,6 +159,7 @@ func TestRoleAssignmentID(t *testing.T) {
Expected: &RoleAssignmentId{
SubscriptionID: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "",
ResourceScope: "",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
},
Expand Down Expand Up @@ -176,6 +196,30 @@ func TestRoleAssignmentID(t *testing.T) {
TenantId: "34567812-3456-7653-6742-345678901234",
},
},
{
Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/group1/providers/Microsoft.Storage/storageAccounts/nameStorageAccount/providers/Microsoft.Authorization/roleAssignments/23456781-2349-8764-5631-234567890121|34567812-3456-7653-6742-345678901234",
Expected: &RoleAssignmentId{
SubscriptionID: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "group1",
ResourceProvider: "Microsoft.Storage",
ResourceScope: "storageAccounts/nameStorageAccount",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "34567812-3456-7653-6742-345678901234",
},
},
{
Input: "/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/group1/providers/Microsoft.AppPlatform/Spring/spring1/apps/app1/providers/Microsoft.Authorization/roleAssignments/23456781-2349-8764-5631-234567890121|34567812-3456-7653-6742-345678901234",
Expected: &RoleAssignmentId{
SubscriptionID: "12345678-1234-9876-4563-123456789012",
ResourceGroup: "group1",
ResourceProvider: "Microsoft.AppPlatform",
ResourceScope: "Spring/spring1/apps/app1",
ManagementGroup: "",
Name: "23456781-2349-8764-5631-234567890121",
TenantId: "34567812-3456-7653-6742-345678901234",
},
},
}

for _, v := range testData {
Expand All @@ -199,15 +243,23 @@ func TestRoleAssignmentID(t *testing.T) {
}

if actual.SubscriptionID != v.Expected.SubscriptionID {
t.Fatalf("Expected %q but got %q for Role Assignment Name", v.Expected.SubscriptionID, actual.SubscriptionID)
t.Fatalf("Expected %q but got %q for Role Assignment Subscription ID", v.Expected.SubscriptionID, actual.SubscriptionID)
}

if actual.ResourceGroup != v.Expected.ResourceGroup {
t.Fatalf("Expected %q but got %q for Role Assignment Name", v.Expected.ResourceGroup, actual.ResourceGroup)
t.Fatalf("Expected %q but got %q for Role Assignment Resource Group", v.Expected.ResourceGroup, actual.ResourceGroup)
}

if actual.ResourceProvider != v.Expected.ResourceProvider {
t.Fatalf("Expected %q but got %q for Role Assignment Resource Provider", v.Expected.ResourceProvider, actual.ResourceProvider)
}

if actual.ResourceScope != v.Expected.ResourceScope {
t.Fatalf("Expected %q but got %q for Role Assignment Resource Scope", v.Expected.ResourceScope, actual.ResourceScope)
}

if actual.ManagementGroup != v.Expected.ManagementGroup {
t.Fatalf("Expected %q but got %q for Role Assignment Name", v.Expected.ManagementGroup, actual.ManagementGroup)
t.Fatalf("Expected %q but got %q for Role Assignment Management Group", v.Expected.ManagementGroup, actual.ManagementGroup)
}
}
}
53 changes: 53 additions & 0 deletions azurerm/internal/services/authorization/role_assignment_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,23 @@ func TestAccRoleAssignment_condition(t *testing.T) {
})
}

func TestAccRoleAssignment_resourceScoped(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_role_assignment", "test")
id := uuid.New().String()

r := RoleAssignmentResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.roleResourceScoped(data, id),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep("skip_service_principal_aad_check"),
})
}

func (r RoleAssignmentResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
id, err := parse.RoleAssignmentID(state.ID)
if err != nil {
Expand Down Expand Up @@ -291,6 +308,42 @@ resource "azurerm_role_assignment" "test" {
`, id)
}

func (RoleAssignmentResource) roleResourceScoped(data acceptance.TestData, id string) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
data "azurerm_client_config" "test" {
}
resource "azurerm_resource_group" "test" {
name = "acctestRG-role-assigment-%d"
location = "%s"
}
resource "azurerm_storage_account" "test" {
name = "unlikely23xst2acct%s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
account_tier = "Standard"
account_replication_type = "LRS"
tags = {
environment = "production"
}
}
resource "azurerm_role_assignment" "test" {
name = "%s"
scope = azurerm_storage_account.test.id
role_definition_name = "Storage Account Contributor"
principal_id = data.azurerm_client_config.test.object_id
}
`, data.RandomInteger, data.Locations.Primary, data.RandomString, id)
}

func (RoleAssignmentResource) requiresImportConfig(id string) string {
return fmt.Sprintf(`
%s
Expand Down

0 comments on commit f2ee194

Please sign in to comment.