Skip to content

Commit f2ee194

Browse files
aristosvoaristosvo
authored
azurerm_role_assignment: Fix assignments to resources (#12076)
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)
1 parent 64ea516 commit f2ee194

File tree

5 files changed

+178
-31
lines changed

5 files changed

+178
-31
lines changed

‎azurerm/helpers/azure/resourceid.go

+20-10
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import (
1111
// level fields, and other key-value pairs available via a map in the
1212
// Path field.
1313
type ResourceID struct {
14-
SubscriptionID string
15-
ResourceGroup string
16-
Provider string
17-
Path map[string]string
14+
SubscriptionID string
15+
ResourceGroup string
16+
Provider string
17+
SecondaryProvider string
18+
Path map[string]string
1819
}
1920

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

4243
var subscriptionID string
44+
var provider string
4345

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

55-
// Catch the subscriptionID before it can be overwritten by another "subscriptions"
56-
// value in the ID which is the case for the Service Bus subscription resource
57-
if key == "subscriptions" && subscriptionID == "" {
57+
switch {
58+
case key == "subscriptions" && subscriptionID == "":
59+
// Catch the subscriptionID before it can be overwritten by another "subscriptions"
60+
// value in the ID which is the case for the Service Bus subscription resource
5861
subscriptionID = value
59-
} else {
62+
case key == "providers" && provider == "":
63+
// Catch the provider before it can be overwritten by another "providers"
64+
// value in the ID which can be the case for the Role Assignment resource
65+
provider = value
66+
default:
6067
componentMap[key] = value
6168
}
6269
}
@@ -82,9 +89,12 @@ func ParseAzureResourceID(id string) (*ResourceID, error) {
8289
delete(componentMap, "resourcegroups")
8390
}
8491

85-
// It is OK not to have a provider in the case of a resource group
86-
if provider, ok := componentMap["providers"]; ok {
92+
if provider != "" {
8793
idObj.Provider = provider
94+
}
95+
96+
if secondaryProvider := componentMap["providers"]; secondaryProvider != "" {
97+
idObj.SecondaryProvider = secondaryProvider
8898
delete(componentMap, "providers")
8999
}
90100

‎azurerm/helpers/azure/resourceid_test.go

+14
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,20 @@ func TestParseAzureResourceID(t *testing.T) {
147147
},
148148
false,
149149
},
150+
{
151+
"/subscriptions/11111111-1111-1111-1111-111111111111/resourceGroups/example-resources/providers/Microsoft.Storage/storageAccounts/nameStorageAccount/providers/Microsoft.Authorization/roleAssignments/22222222-2222-2222-2222-222222222222",
152+
&azure.ResourceID{
153+
SubscriptionID: "11111111-1111-1111-1111-111111111111",
154+
ResourceGroup: "example-resources",
155+
Provider: "Microsoft.Storage",
156+
SecondaryProvider: "Microsoft.Authorization",
157+
Path: map[string]string{
158+
"storageAccounts": "nameStorageAccount",
159+
"roleAssignments": "22222222-2222-2222-2222-222222222222",
160+
},
161+
},
162+
false,
163+
},
150164
{
151165
// missing resource group
152166
"/subscriptions/11111111-1111-1111-1111-111111111111/providers/Microsoft.ApiManagement/service/service1/subscriptions/22222222-2222-2222-2222-222222222222",

‎azurerm/internal/services/authorization/parse/role_assignment.go

+29-11
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,16 @@ import (
88
)
99

1010
type RoleAssignmentId struct {
11-
SubscriptionID string
12-
ResourceGroup string
13-
ManagementGroup string
14-
Name string
15-
TenantId string
11+
SubscriptionID string
12+
ResourceGroup string
13+
ManagementGroup string
14+
ResourceScope string
15+
ResourceProvider string
16+
Name string
17+
TenantId string
1618
}
1719

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

3537
return &RoleAssignmentId{
36-
SubscriptionID: subscriptionId,
37-
ResourceGroup: resourceGroup,
38-
ManagementGroup: managementGroup,
39-
Name: name,
40-
TenantId: tenantId,
38+
SubscriptionID: subscriptionId,
39+
ResourceGroup: resourceGroup,
40+
ResourceProvider: resourceProvider,
41+
ResourceScope: resourceScope,
42+
ManagementGroup: managementGroup,
43+
Name: name,
44+
TenantId: tenantId,
4145
}, nil
4246
}
4347

4448
// in general case, the id format does not change
4549
// for cross tenant scenario, add the tenantId info
4650
func (id RoleAssignmentId) AzureResourceID() string {
51+
if id.ResourceScope != "" {
52+
fmtString := "/subscriptions/%s/resourceGroups/%s/providers/%s/%s/providers/Microsoft.Authorization/roleAssignments/%s"
53+
return fmt.Sprintf(fmtString, id.SubscriptionID, id.ResourceGroup, id.ResourceProvider, id.ResourceScope, id.Name)
54+
}
55+
4756
if id.ManagementGroup != "" {
4857
fmtString := "/providers/Microsoft.Management/managementGroups/%s/providers/Microsoft.Authorization/roleAssignments/%s"
4958
return fmt.Sprintf(fmtString, id.ManagementGroup, id.Name)
@@ -90,6 +99,15 @@ func RoleAssignmentID(input string) (*RoleAssignmentId, error) {
9099
}
91100
roleAssignmentId.SubscriptionID = id.SubscriptionID
92101
roleAssignmentId.ResourceGroup = id.ResourceGroup
102+
if id.Provider != "Microsoft.Authorization" && id.Provider != "" {
103+
roleAssignmentId.ResourceProvider = id.Provider
104+
// logic to save resource scope
105+
result := strings.Split(input, "/providers/")
106+
if len(result) == 3 {
107+
roleAssignmentId.ResourceScope = strings.TrimPrefix(result[1], fmt.Sprintf("%s/", id.Provider))
108+
}
109+
}
110+
93111
if roleAssignmentId.Name, err = id.PopSegment("roleAssignments"); err != nil {
94112
return nil, err
95113
}

‎azurerm/internal/services/authorization/parse/role_assignment_test.go

+62-10
Original file line numberDiff line numberDiff line change
@@ -10,37 +10,43 @@ var _ resourceid.Formatter = RoleAssignmentId{}
1010

1111
func TestRoleAssignmentIDFormatter(t *testing.T) {
1212
testData := []struct {
13-
SubscriptionId string
14-
ResourceGroup string
15-
ManagementGroup string
16-
Name string
17-
TenantId string
18-
Expected string
13+
SubscriptionId string
14+
ResourceGroup string
15+
ResourceProvider string
16+
ResourceScope string
17+
ManagementGroup string
18+
Name string
19+
TenantId string
20+
Expected string
1921
}{
2022
{
2123
SubscriptionId: "",
2224
ResourceGroup: "",
25+
ResourceScope: "",
2326
ManagementGroup: "",
2427
Name: "23456781-2349-8764-5631-234567890121",
2528
TenantId: "",
2629
},
2730
{
2831
SubscriptionId: "12345678-1234-9876-4563-123456789012",
2932
ResourceGroup: "group1",
33+
ResourceScope: "",
3034
ManagementGroup: "managementGroup1",
3135
Name: "23456781-2349-8764-5631-234567890121",
3236
TenantId: "",
3337
},
3438
{
3539
SubscriptionId: "12345678-1234-9876-4563-123456789012",
3640
ResourceGroup: "",
41+
ResourceScope: "",
3742
ManagementGroup: "managementGroup1",
3843
Name: "23456781-2349-8764-5631-234567890121",
3944
TenantId: "",
4045
},
4146
{
4247
SubscriptionId: "12345678-1234-9876-4563-123456789012",
4348
ResourceGroup: "",
49+
ResourceScope: "",
4450
ManagementGroup: "",
4551
Name: "23456781-2349-8764-5631-234567890121",
4652
TenantId: "",
@@ -49,6 +55,7 @@ func TestRoleAssignmentIDFormatter(t *testing.T) {
4955
{
5056
SubscriptionId: "12345678-1234-9876-4563-123456789012",
5157
ResourceGroup: "group1",
58+
ResourceScope: "",
5259
ManagementGroup: "",
5360
Name: "23456781-2349-8764-5631-234567890121",
5461
TenantId: "",
@@ -57,6 +64,7 @@ func TestRoleAssignmentIDFormatter(t *testing.T) {
5764
{
5865
SubscriptionId: "",
5966
ResourceGroup: "",
67+
ResourceScope: "",
6068
ManagementGroup: "12345678-1234-9876-4563-123456789012",
6169
Name: "23456781-2349-8764-5631-234567890121",
6270
TenantId: "",
@@ -65,15 +73,26 @@ func TestRoleAssignmentIDFormatter(t *testing.T) {
6573
{
6674
SubscriptionId: "",
6775
ResourceGroup: "",
76+
ResourceScope: "",
6877
ManagementGroup: "12345678-1234-9876-4563-123456789012",
6978
Name: "23456781-2349-8764-5631-234567890121",
7079
TenantId: "34567812-3456-7653-6742-345678901234",
7180
Expected: "/providers/Microsoft.Management/managementGroups/12345678-1234-9876-4563-123456789012/providers/Microsoft.Authorization/roleAssignments/23456781-2349-8764-5631-234567890121|34567812-3456-7653-6742-345678901234",
7281
},
82+
{
83+
SubscriptionId: "12345678-1234-9876-4563-123456789012",
84+
ResourceGroup: "group1",
85+
ResourceProvider: "Microsoft.Storage",
86+
ResourceScope: "storageAccounts/nameStorageAccount",
87+
ManagementGroup: "",
88+
Name: "23456781-2349-8764-5631-234567890121",
89+
TenantId: "34567812-3456-7653-6742-345678901234",
90+
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",
91+
},
7392
}
7493
for _, v := range testData {
7594
t.Logf("testing %+v", v)
76-
actual, err := NewRoleAssignmentID(v.SubscriptionId, v.ResourceGroup, v.ManagementGroup, v.Name, v.TenantId)
95+
actual, err := NewRoleAssignmentID(v.SubscriptionId, v.ResourceGroup, v.ResourceProvider, v.ResourceScope, v.ManagementGroup, v.Name, v.TenantId)
7796
if err != nil {
7897
if v.Expected == "" {
7998
continue
@@ -140,6 +159,7 @@ func TestRoleAssignmentID(t *testing.T) {
140159
Expected: &RoleAssignmentId{
141160
SubscriptionID: "12345678-1234-9876-4563-123456789012",
142161
ResourceGroup: "",
162+
ResourceScope: "",
143163
ManagementGroup: "",
144164
Name: "23456781-2349-8764-5631-234567890121",
145165
},
@@ -176,6 +196,30 @@ func TestRoleAssignmentID(t *testing.T) {
176196
TenantId: "34567812-3456-7653-6742-345678901234",
177197
},
178198
},
199+
{
200+
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",
201+
Expected: &RoleAssignmentId{
202+
SubscriptionID: "12345678-1234-9876-4563-123456789012",
203+
ResourceGroup: "group1",
204+
ResourceProvider: "Microsoft.Storage",
205+
ResourceScope: "storageAccounts/nameStorageAccount",
206+
ManagementGroup: "",
207+
Name: "23456781-2349-8764-5631-234567890121",
208+
TenantId: "34567812-3456-7653-6742-345678901234",
209+
},
210+
},
211+
{
212+
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",
213+
Expected: &RoleAssignmentId{
214+
SubscriptionID: "12345678-1234-9876-4563-123456789012",
215+
ResourceGroup: "group1",
216+
ResourceProvider: "Microsoft.AppPlatform",
217+
ResourceScope: "Spring/spring1/apps/app1",
218+
ManagementGroup: "",
219+
Name: "23456781-2349-8764-5631-234567890121",
220+
TenantId: "34567812-3456-7653-6742-345678901234",
221+
},
222+
},
179223
}
180224

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

201245
if actual.SubscriptionID != v.Expected.SubscriptionID {
202-
t.Fatalf("Expected %q but got %q for Role Assignment Name", v.Expected.SubscriptionID, actual.SubscriptionID)
246+
t.Fatalf("Expected %q but got %q for Role Assignment Subscription ID", v.Expected.SubscriptionID, actual.SubscriptionID)
203247
}
204248

205249
if actual.ResourceGroup != v.Expected.ResourceGroup {
206-
t.Fatalf("Expected %q but got %q for Role Assignment Name", v.Expected.ResourceGroup, actual.ResourceGroup)
250+
t.Fatalf("Expected %q but got %q for Role Assignment Resource Group", v.Expected.ResourceGroup, actual.ResourceGroup)
251+
}
252+
253+
if actual.ResourceProvider != v.Expected.ResourceProvider {
254+
t.Fatalf("Expected %q but got %q for Role Assignment Resource Provider", v.Expected.ResourceProvider, actual.ResourceProvider)
255+
}
256+
257+
if actual.ResourceScope != v.Expected.ResourceScope {
258+
t.Fatalf("Expected %q but got %q for Role Assignment Resource Scope", v.Expected.ResourceScope, actual.ResourceScope)
207259
}
208260

209261
if actual.ManagementGroup != v.Expected.ManagementGroup {
210-
t.Fatalf("Expected %q but got %q for Role Assignment Name", v.Expected.ManagementGroup, actual.ManagementGroup)
262+
t.Fatalf("Expected %q but got %q for Role Assignment Management Group", v.Expected.ManagementGroup, actual.ManagementGroup)
211263
}
212264
}
213265
}

‎azurerm/internal/services/authorization/role_assignment_resource_test.go

+53
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,23 @@ func TestAccRoleAssignment_condition(t *testing.T) {
236236
})
237237
}
238238

239+
func TestAccRoleAssignment_resourceScoped(t *testing.T) {
240+
data := acceptance.BuildTestData(t, "azurerm_role_assignment", "test")
241+
id := uuid.New().String()
242+
243+
r := RoleAssignmentResource{}
244+
245+
data.ResourceTest(t, r, []acceptance.TestStep{
246+
{
247+
Config: r.roleResourceScoped(data, id),
248+
Check: acceptance.ComposeTestCheckFunc(
249+
check.That(data.ResourceName).ExistsInAzure(r),
250+
),
251+
},
252+
data.ImportStep("skip_service_principal_aad_check"),
253+
})
254+
}
255+
239256
func (r RoleAssignmentResource) Exists(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
240257
id, err := parse.RoleAssignmentID(state.ID)
241258
if err != nil {
@@ -291,6 +308,42 @@ resource "azurerm_role_assignment" "test" {
291308
`, id)
292309
}
293310

311+
func (RoleAssignmentResource) roleResourceScoped(data acceptance.TestData, id string) string {
312+
return fmt.Sprintf(`
313+
provider "azurerm" {
314+
features {}
315+
}
316+
317+
data "azurerm_client_config" "test" {
318+
}
319+
320+
resource "azurerm_resource_group" "test" {
321+
name = "acctestRG-role-assigment-%d"
322+
location = "%s"
323+
}
324+
325+
resource "azurerm_storage_account" "test" {
326+
name = "unlikely23xst2acct%s"
327+
resource_group_name = azurerm_resource_group.test.name
328+
329+
location = azurerm_resource_group.test.location
330+
account_tier = "Standard"
331+
account_replication_type = "LRS"
332+
333+
tags = {
334+
environment = "production"
335+
}
336+
}
337+
338+
resource "azurerm_role_assignment" "test" {
339+
name = "%s"
340+
scope = azurerm_storage_account.test.id
341+
role_definition_name = "Storage Account Contributor"
342+
principal_id = data.azurerm_client_config.test.object_id
343+
}
344+
`, data.RandomInteger, data.Locations.Primary, data.RandomString, id)
345+
}
346+
294347
func (RoleAssignmentResource) requiresImportConfig(id string) string {
295348
return fmt.Sprintf(`
296349
%s

0 commit comments

Comments
 (0)