From c39eb3fc9fa101881b3a03713f079c682772f06f Mon Sep 17 00:00:00 2001 From: cmendible Date: Fri, 13 Mar 2020 17:07:03 +0000 Subject: [PATCH 1/3] Fixing destroy when role scope is a Management Group --- .../authorization/role_definition_resource.go | 19 +++++-- .../tests/role_definition_resource_test.go | 50 +++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/azurerm/internal/services/authorization/role_definition_resource.go b/azurerm/internal/services/authorization/role_definition_resource.go index 316d04020778..f6f96f037774 100644 --- a/azurerm/internal/services/authorization/role_definition_resource.go +++ b/azurerm/internal/services/authorization/role_definition_resource.go @@ -184,7 +184,7 @@ func resourceArmRoleDefinitionRead(d *schema.ResourceData, meta interface{}) err } if id := resp.ID; id != nil { - roleDefinitionId, err := parseRoleDefinitionId(*id) + roleDefinitionId, err := parseRoleDefinitionId(*id, *resp.RoleDefinitionProperties.AssignableScopes) if err != nil { return fmt.Errorf("Error parsing Role Definition ID: %+v", err) } @@ -217,7 +217,9 @@ func resourceArmRoleDefinitionDelete(d *schema.ResourceData, meta interface{}) e ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - id, err := parseRoleDefinitionId(d.Id()) + scope := d.Get("scope").(string) + + id, err := parseRoleDefinitionId(d.Id(), []string{scope}) if err != nil { return err } @@ -346,8 +348,19 @@ type roleDefinitionId struct { roleDefinitionId string } -func parseRoleDefinitionId(input string) (*roleDefinitionId, error) { +func parseRoleDefinitionId(input string, scopes []string) (*roleDefinitionId, error) { segments := strings.Split(input, "/providers/Microsoft.Authorization/roleDefinitions/") + + // First check if role is scoped to a Management Group + if len(scopes) == 1 && strings.HasPrefix(scopes[0], "/providers/Microsoft.Management/managementGroups/") { + id := roleDefinitionId{ + scope: strings.TrimPrefix(scopes[0], "/"), + roleDefinitionId: segments[1], + } + return &id, nil + } + + // Role is scoped to a Subscription if len(segments) != 2 { return nil, fmt.Errorf("Expected Role Definition ID to be in the format `{scope}/providers/Microsoft.Authorization/roleDefinitions/{name}` but got %q", input) } diff --git a/azurerm/internal/services/authorization/tests/role_definition_resource_test.go b/azurerm/internal/services/authorization/tests/role_definition_resource_test.go index 21867aad8a16..402ba6c2ac91 100644 --- a/azurerm/internal/services/authorization/tests/role_definition_resource_test.go +++ b/azurerm/internal/services/authorization/tests/role_definition_resource_test.go @@ -165,6 +165,26 @@ func TestAccAzureRMRoleDefinition_emptyName(t *testing.T) { }) } +func TestAccAzureRMRoleDefinition_managementGroup(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_role_definition", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMRoleDefinitionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMRoleDefinition_managementGroup(uuid.New().String(), data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMRoleDefinitionExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "scope", "/providers/Microsoft.Management/managementGroups/testMG"), + resource.TestCheckResourceAttr(data.ResourceName, "assignable_scopes.0", "/providers/Microsoft.Management/managementGroups/testMG"), + ), + }, + }, + }) +} + func testCheckAzureRMRoleDefinitionExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { client := acceptance.AzureProvider.Meta().(*clients.Client).Authorization.RoleDefinitionsClient @@ -369,3 +389,33 @@ resource "azurerm_role_definition" "test" { } `, data.RandomInteger) } + +func testAccAzureRMRoleDefinition_managementGroup(id string, data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_management_group" "test" { +} + +locals { + scope = azurerm_management_group.test.id +} + +resource "azurerm_role_definition" "test" { + role_definition_id = "%s" + name = "acctestrd-%d" + scope = local.scope + + permissions { + actions = ["*"] + not_actions = [] + } + + assignable_scopes = [ + local.scope + ] +} +`, id, data.RandomInteger) +} From 02330661600d4b890143835161a62b11b546440a Mon Sep 17 00:00:00 2001 From: jackofallops Date: Tue, 11 Aug 2020 08:13:34 +0100 Subject: [PATCH 2/3] Rebased, updated for multiple assignable scopes and tests. fixed potential crash --- .../authorization/role_definition_resource.go | 43 ++++++++-------- .../tests/role_definition_resource_test.go | 49 +++++-------------- website/docs/r/role_definition.html.markdown | 4 +- 3 files changed, 37 insertions(+), 59 deletions(-) diff --git a/azurerm/internal/services/authorization/role_definition_resource.go b/azurerm/internal/services/authorization/role_definition_resource.go index f6f96f037774..85166e7fc009 100644 --- a/azurerm/internal/services/authorization/role_definition_resource.go +++ b/azurerm/internal/services/authorization/role_definition_resource.go @@ -97,7 +97,8 @@ func resourceArmRoleDefinition() *schema.Resource { "assignable_scopes": { Type: schema.TypeList, - Required: true, + Optional: true, + Computed: true, Elem: &schema.Schema{ Type: schema.TypeString, }, @@ -184,13 +185,17 @@ func resourceArmRoleDefinitionRead(d *schema.ResourceData, meta interface{}) err } if id := resp.ID; id != nil { - roleDefinitionId, err := parseRoleDefinitionId(*id, *resp.RoleDefinitionProperties.AssignableScopes) - if err != nil { - return fmt.Errorf("Error parsing Role Definition ID: %+v", err) - } - if roleDefinitionId != nil { - d.Set("scope", roleDefinitionId.scope) - d.Set("role_definition_id", roleDefinitionId.roleDefinitionId) + if resp.RoleDefinitionProperties != nil && resp.RoleDefinitionProperties.AssignableScopes != nil { + roleDefinitionId, err := parseRoleDefinitionId(*id, *resp.RoleDefinitionProperties.AssignableScopes) + if err != nil { + return fmt.Errorf("Error parsing Role Definition ID: %+v", err) + } + if roleDefinitionId != nil { + d.Set("scope", roleDefinitionId.scope) + d.Set("role_definition_id", roleDefinitionId.roleDefinitionId) + } + } else { + return fmt.Errorf("assignable scopes not found for Role Definition with id %q", d.Id()) } } @@ -279,9 +284,15 @@ func expandRoleDefinitionPermissions(d *schema.ResourceData) []authorization.Per func expandRoleDefinitionAssignableScopes(d *schema.ResourceData) []string { scopes := make([]string, 0) + // The first scope in the list must be the target scope as it it not returned in any API call + assignedScope := d.Get("scope").(string) + scopes = append(scopes, assignedScope) assignableScopes := d.Get("assignable_scopes").([]interface{}) for _, scope := range assignableScopes { - scopes = append(scopes, scope.(string)) + // Ensure the assigned scope is not duplicated in the list if also specified in `assignable_scopes` + if scope != assignedScope { + scopes = append(scopes, scope.(string)) + } } return scopes @@ -351,24 +362,14 @@ type roleDefinitionId struct { func parseRoleDefinitionId(input string, scopes []string) (*roleDefinitionId, error) { segments := strings.Split(input, "/providers/Microsoft.Authorization/roleDefinitions/") - // First check if role is scoped to a Management Group - if len(scopes) == 1 && strings.HasPrefix(scopes[0], "/providers/Microsoft.Management/managementGroups/") { - id := roleDefinitionId{ - scope: strings.TrimPrefix(scopes[0], "/"), - roleDefinitionId: segments[1], - } - return &id, nil - } - - // Role is scoped to a Subscription if len(segments) != 2 { return nil, fmt.Errorf("Expected Role Definition ID to be in the format `{scope}/providers/Microsoft.Authorization/roleDefinitions/{name}` but got %q", input) } - // {scope}/providers/Microsoft.Authorization/roleDefinitions/{roleDefinitionId} id := roleDefinitionId{ - scope: segments[0], + scope: scopes[0], roleDefinitionId: segments[1], } + return &id, nil } diff --git a/azurerm/internal/services/authorization/tests/role_definition_resource_test.go b/azurerm/internal/services/authorization/tests/role_definition_resource_test.go index 402ba6c2ac91..fa7198181dd9 100644 --- a/azurerm/internal/services/authorization/tests/role_definition_resource_test.go +++ b/azurerm/internal/services/authorization/tests/role_definition_resource_test.go @@ -26,12 +26,7 @@ func TestAccAzureRMRoleDefinition_basic(t *testing.T) { testCheckAzureRMRoleDefinitionExists(data.ResourceName), ), }, - { - ResourceName: data.ResourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"role_definition_id", "scope"}, - }, + data.ImportStep(), }, }) } @@ -91,23 +86,16 @@ func TestAccAzureRMRoleDefinition_update(t *testing.T) { Config: testAccAzureRMRoleDefinition_basic(id, data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleDefinitionExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.0", "*"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.not_actions.#", "0"), ), }, + data.ImportStep(), { Config: testAccAzureRMRoleDefinition_updated(id, data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleDefinitionExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.0", "*"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.not_actions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.not_actions.0", "Microsoft.Authorization/*/read"), ), }, + data.ImportStep(), }, }) } @@ -124,23 +112,16 @@ func TestAccAzureRMRoleDefinition_updateEmptyId(t *testing.T) { Config: testAccAzureRMRoleDefinition_emptyId(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleDefinitionExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.0", "*"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.not_actions.#", "0"), ), }, + data.ImportStep(), { Config: testAccAzureRMRoleDefinition_updateEmptyId(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleDefinitionExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.actions.0", "*"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.not_actions.#", "1"), - resource.TestCheckResourceAttr(data.ResourceName, "permissions.0.not_actions.0", "Microsoft.Authorization/*/read"), ), }, + data.ImportStep(), }, }) } @@ -157,10 +138,9 @@ func TestAccAzureRMRoleDefinition_emptyName(t *testing.T) { Config: testAccAzureRMRoleDefinition_emptyId(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleDefinitionExists(data.ResourceName), - resource.TestCheckResourceAttrSet(data.ResourceName, "id"), - resource.TestCheckResourceAttrSet(data.ResourceName, "name"), ), }, + data.ImportStep(), }, }) } @@ -177,10 +157,9 @@ func TestAccAzureRMRoleDefinition_managementGroup(t *testing.T) { Config: testAccAzureRMRoleDefinition_managementGroup(uuid.New().String(), data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMRoleDefinitionExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "scope", "/providers/Microsoft.Management/managementGroups/testMG"), - resource.TestCheckResourceAttr(data.ResourceName, "assignable_scopes.0", "/providers/Microsoft.Management/managementGroups/testMG"), ), }, + data.ImportStep("scope"), }, }) } @@ -255,10 +234,6 @@ resource "azurerm_role_definition" "test" { actions = ["*"] not_actions = [] } - - assignable_scopes = [ - data.azurerm_subscription.primary.id, - ] } `, id, data.RandomInteger) } @@ -396,17 +371,16 @@ provider "azurerm" { features {} } -resource "azurerm_management_group" "test" { +data "azurerm_subscription" "primary" { } -locals { - scope = azurerm_management_group.test.id +resource "azurerm_management_group" "test" { } resource "azurerm_role_definition" "test" { role_definition_id = "%s" name = "acctestrd-%d" - scope = local.scope + scope = azurerm_management_group.test.id permissions { actions = ["*"] @@ -414,7 +388,8 @@ resource "azurerm_role_definition" "test" { } assignable_scopes = [ - local.scope + azurerm_management_group.test.id, + data.azurerm_subscription.primary.id, ] } `, id, data.RandomInteger) diff --git a/website/docs/r/role_definition.html.markdown b/website/docs/r/role_definition.html.markdown index f2363565382a..e6a2bf61acae 100644 --- a/website/docs/r/role_definition.html.markdown +++ b/website/docs/r/role_definition.html.markdown @@ -47,7 +47,9 @@ The following arguments are supported: * `permissions` - (Required) A `permissions` block as defined below. -* `assignable_scopes` - (Required) One or more assignable scopes for this Role Definition, such as `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333`, `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup`, or `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup/providers/Microsoft.Compute/virtualMachines/myVM`. +* `assignable_scopes` - (Optional) One or more assignable scopes for this Role Definition, such as `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333`, `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup`, or `/subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup/providers/Microsoft.Compute/virtualMachines/myVM`. + +~> **NOTE:** The value for `scope` is automatically included in this list. A `permissions` block as the following properties: From c509db9e5c496a83c27b59ba5b4e4994a298efd7 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Wed, 12 Aug 2020 09:55:01 +0100 Subject: [PATCH 3/3] added scope field to ID and created state migration --- .../authorization/parse/role_definition.go | 40 +++++++ .../role_definition_migration.go | 104 ++++++++++++++++++ .../role_definition_migration_test.go | 42 +++++++ .../authorization/role_definition_resource.go | 86 ++++++--------- 4 files changed, 221 insertions(+), 51 deletions(-) create mode 100644 azurerm/internal/services/authorization/parse/role_definition.go create mode 100644 azurerm/internal/services/authorization/role_definition_migration.go create mode 100644 azurerm/internal/services/authorization/role_definition_migration_test.go diff --git a/azurerm/internal/services/authorization/parse/role_definition.go b/azurerm/internal/services/authorization/parse/role_definition.go new file mode 100644 index 000000000000..2b2f803c5c51 --- /dev/null +++ b/azurerm/internal/services/authorization/parse/role_definition.go @@ -0,0 +1,40 @@ +package parse + +import ( + "fmt" + "strings" +) + +type RoleDefinitionID struct { + ResourceID string + Scope string + RoleID 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 +func RoleDefinitionId(input string) (*RoleDefinitionID, error) { + parts := strings.Split(input, "|") + if len(parts) != 2 { + return nil, fmt.Errorf("could not parse Role Definition ID, invalid format %q", input) + } + + idParts := strings.Split(parts[0], "roleDefinitions/") + + if !strings.HasPrefix(parts[1], "/subscriptions/") && !strings.HasPrefix(parts[1], "/providers/Microsoft.Management/managementGroups/") { + return nil, fmt.Errorf("failed to parse scope from Role Definition ID %q", input) + } + + roleDefinitionID := RoleDefinitionID{ + ResourceID: parts[0], + Scope: parts[1], + } + + if len(idParts) < 1 { + return nil, fmt.Errorf("failed to parse Role Definition ID from resource ID %q", input) + } else { + roleDefinitionID.RoleID = idParts[1] + } + + return &roleDefinitionID, nil +} diff --git a/azurerm/internal/services/authorization/role_definition_migration.go b/azurerm/internal/services/authorization/role_definition_migration.go new file mode 100644 index 000000000000..8b9457ae81ce --- /dev/null +++ b/azurerm/internal/services/authorization/role_definition_migration.go @@ -0,0 +1,104 @@ +package authorization + +import ( + "fmt" + "log" + + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func resourceArmRoleDefinitionV0() *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + "role_definition_id": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + }, + + "name": { + Type: schema.TypeString, + Required: true, + }, + + "scope": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "description": { + Type: schema.TypeString, + Optional: true, + }, + + "permissions": { + Type: schema.TypeList, + Required: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "actions": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "not_actions": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "data_actions": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + Set: schema.HashString, + }, + "not_data_actions": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + Set: schema.HashString, + }, + }, + }, + }, + + "assignable_scopes": { + Type: schema.TypeList, + Optional: true, + Computed: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + }, + } +} + +func resourceArmRoleDefinitionStateUpgradeV0(rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { + log.Println("[DEBUG] Migrating ID from v0 to v1 format") + + oldID := rawState["id"].(string) + if oldID == "" { + return nil, fmt.Errorf("failed to migrate state: old ID empty") + } + scope := rawState["scope"].(string) + if scope == "" { + return nil, fmt.Errorf("failed to migrate state: scope missing") + } + + newID := fmt.Sprintf("%s|%s", oldID, scope) + + rawState["id"] = newID + + return rawState, nil +} diff --git a/azurerm/internal/services/authorization/role_definition_migration_test.go b/azurerm/internal/services/authorization/role_definition_migration_test.go new file mode 100644 index 000000000000..7e4b7c96dafb --- /dev/null +++ b/azurerm/internal/services/authorization/role_definition_migration_test.go @@ -0,0 +1,42 @@ +package authorization + +import ( + "testing" +) + +func TestAzureRMRoleDefinitionMigrateState(t *testing.T) { + cases := map[string]struct { + StateVersion int + InputAttributes map[string]interface{} + ExpectedNewID string + }{ + "subscription_scope": { + StateVersion: 0, + InputAttributes: map[string]interface{}{ + "id": "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/11111111-1111-1111-1111-111111111111", + "name": "roleName", + "role_definition_id": "11111111-1111-1111-1111-111111111111", + "scope": "/subscriptions/00000000-0000-0000-0000-000000000000", + }, + ExpectedNewID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/11111111-1111-1111-1111-111111111111|/subscriptions/00000000-0000-0000-0000-000000000000", + }, + "managementGroup_scope": { + StateVersion: 0, + InputAttributes: map[string]interface{}{ + "id": "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/11111111-1111-1111-1111-111111111111", + "name": "roleName", + "role_definition_id": "11111111-1111-1111-1111-111111111111", + "scope": "/providers/Microsoft.Management/managementGroups/22222222-2222-2222-2222-222222222222", + }, + ExpectedNewID: "/subscriptions/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/roleDefinitions/11111111-1111-1111-1111-111111111111|/providers/Microsoft.Management/managementGroups/22222222-2222-2222-2222-222222222222", + }, + } + + for _, tc := range cases { + newID, _ := resourceArmRoleDefinitionStateUpgradeV0(tc.InputAttributes, nil) + + if newID["id"].(string) != tc.ExpectedNewID { + t.Fatalf("ID migration failed, expected %q, got: %q", tc.ExpectedNewID, newID["id"].(string)) + } + } +} diff --git a/azurerm/internal/services/authorization/role_definition_resource.go b/azurerm/internal/services/authorization/role_definition_resource.go index 85166e7fc009..8fa28f333d5b 100644 --- a/azurerm/internal/services/authorization/role_definition_resource.go +++ b/azurerm/internal/services/authorization/role_definition_resource.go @@ -3,9 +3,12 @@ package authorization import ( "fmt" "log" - "strings" "time" + azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema" + + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/authorization/parse" + "github.com/Azure/azure-sdk-for-go/services/preview/authorization/mgmt/2018-09-01-preview/authorization" "github.com/hashicorp/go-uuid" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" @@ -21,9 +24,11 @@ func resourceArmRoleDefinition() *schema.Resource { Read: resourceArmRoleDefinitionRead, Update: resourceArmRoleDefinitionCreateUpdate, Delete: resourceArmRoleDefinitionDelete, - Importer: &schema.ResourceImporter{ - State: schema.ImportStatePassthrough, - }, + + Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error { + _, err := parse.RoleDefinitionId(id) + return err + }), Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), @@ -32,6 +37,16 @@ func resourceArmRoleDefinition() *schema.Resource { Delete: schema.DefaultTimeout(30 * time.Minute), }, + SchemaVersion: 1, + + StateUpgraders: []schema.StateUpgrader{ + { + Type: resourceArmRoleDefinitionV0().CoreConfigSchema().ImpliedType(), + Upgrade: resourceArmRoleDefinitionStateUpgradeV0, + Version: 0, + }, + }, + Schema: map[string]*schema.Schema{ "role_definition_id": { Type: schema.TypeString, @@ -138,7 +153,8 @@ func resourceArmRoleDefinitionCreateUpdate(d *schema.ResourceData, meta interfac } if existing.ID != nil && *existing.ID != "" { - return tf.ImportAsExistsError("azurerm_role_definition", *existing.ID) + importID := fmt.Sprintf("%s|%s", *existing.ID, scope) + return tf.ImportAsExistsError("azurerm_role_definition", importID) } } @@ -160,11 +176,11 @@ func resourceArmRoleDefinitionCreateUpdate(d *schema.ResourceData, meta interfac if err != nil { return err } - if read.ID == nil { + if read.ID == nil || *read.ID == "" { return fmt.Errorf("Cannot read Role Definition ID for %q (Scope %q)", name, scope) } - d.SetId(*read.ID) + d.SetId(fmt.Sprintf("%s|%s", *read.ID, scope)) return resourceArmRoleDefinitionRead(d, meta) } @@ -173,7 +189,15 @@ func resourceArmRoleDefinitionRead(d *schema.ResourceData, meta interface{}) err ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - resp, err := client.GetByID(ctx, d.Id()) + roleDefinitionId, err := parse.RoleDefinitionId(d.Id()) + if err != nil { + return err + } + + d.Set("scope", roleDefinitionId.Scope) + d.Set("role_definition_id", roleDefinitionId.RoleID) + + resp, err := client.Get(ctx, roleDefinitionId.Scope, roleDefinitionId.RoleID) if err != nil { if utils.ResponseWasNotFound(resp.Response) { log.Printf("[DEBUG] Role Definition %q was not found - removing from state", d.Id()) @@ -184,21 +208,6 @@ func resourceArmRoleDefinitionRead(d *schema.ResourceData, meta interface{}) err return fmt.Errorf("Error loading Role Definition %q: %+v", d.Id(), err) } - if id := resp.ID; id != nil { - if resp.RoleDefinitionProperties != nil && resp.RoleDefinitionProperties.AssignableScopes != nil { - roleDefinitionId, err := parseRoleDefinitionId(*id, *resp.RoleDefinitionProperties.AssignableScopes) - if err != nil { - return fmt.Errorf("Error parsing Role Definition ID: %+v", err) - } - if roleDefinitionId != nil { - d.Set("scope", roleDefinitionId.scope) - d.Set("role_definition_id", roleDefinitionId.roleDefinitionId) - } - } else { - return fmt.Errorf("assignable scopes not found for Role Definition with id %q", d.Id()) - } - } - if props := resp.RoleDefinitionProperties; props != nil { d.Set("name", props.RoleName) d.Set("description", props.Description) @@ -222,17 +231,12 @@ func resourceArmRoleDefinitionDelete(d *schema.ResourceData, meta interface{}) e ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d) defer cancel() - scope := d.Get("scope").(string) - - id, err := parseRoleDefinitionId(d.Id(), []string{scope}) - if err != nil { - return err - } + id, _ := parse.RoleDefinitionId(d.Id()) - resp, err := client.Delete(ctx, id.scope, id.roleDefinitionId) + resp, err := client.Delete(ctx, id.Scope, id.RoleID) if err != nil { if !utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Error deleting Role Definition %q at Scope %q: %+v", id.roleDefinitionId, id.scope, err) + return fmt.Errorf("deleting Role Definition %q at Scope %q: %+v", id.RoleID, id.Scope, err) } } @@ -353,23 +357,3 @@ func flattenRoleDefinitionAssignableScopes(input *[]string) []interface{} { return scopes } - -type roleDefinitionId struct { - scope string - roleDefinitionId string -} - -func parseRoleDefinitionId(input string, scopes []string) (*roleDefinitionId, error) { - segments := strings.Split(input, "/providers/Microsoft.Authorization/roleDefinitions/") - - if len(segments) != 2 { - return nil, fmt.Errorf("Expected Role Definition ID to be in the format `{scope}/providers/Microsoft.Authorization/roleDefinitions/{name}` but got %q", input) - } - - id := roleDefinitionId{ - scope: scopes[0], - roleDefinitionId: segments[1], - } - - return &id, nil -}