diff --git a/azurerm/internal/services/managementgroup/management_group_data_source.go b/azurerm/internal/services/managementgroup/management_group_data_source.go index fdc49353e7b7..dcd5b2a495cf 100644 --- a/azurerm/internal/services/managementgroup/management_group_data_source.go +++ b/azurerm/internal/services/managementgroup/management_group_data_source.go @@ -74,7 +74,7 @@ func dataSourceArmManagementGroupRead(d *schema.ResourceData, meta interface{}) recurse := true resp, err := client.Get(ctx, groupName, "children", &recurse, "", managementGroupCacheControl) if err != nil { - if utils.ResponseWasNotFound(resp.Response) { + if utils.ResponseWasForbidden(resp.Response) { return fmt.Errorf("Management Group %q was not found", groupName) } diff --git a/azurerm/internal/services/managementgroup/management_group_resource.go b/azurerm/internal/services/managementgroup/management_group_resource.go index 661036045290..8736410c8c42 100644 --- a/azurerm/internal/services/managementgroup/management_group_resource.go +++ b/azurerm/internal/services/managementgroup/management_group_resource.go @@ -90,14 +90,18 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa defer cancel() armTenantID := meta.(*clients.Client).Account.TenantId - groupName := uuid.New().String() - if v, ok := d.GetOk("group_name"); ok { + var groupName string + if v := d.Get("group_id"); v != "" { groupName = v.(string) } - if v, ok := d.GetOk("group_id"); ok { + if v := d.Get("name"); v != "" { groupName = v.(string) } + if groupName == "" { + groupName = uuid.New().String() + } + parentManagementGroupId := d.Get("parent_management_group_id").(string) if parentManagementGroupId == "" { parentManagementGroupId = fmt.Sprintf("/providers/Microsoft.Management/managementGroups/%s", armTenantID) @@ -107,11 +111,11 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa if d.IsNewResource() { existing, err := client.Get(ctx, groupName, "children", &recurse, "", managementGroupCacheControl) if err != nil { - if !utils.ResponseWasNotFound(existing.Response) { + // 403 is returned if group does not exist, bug tracked at: https://github.com/Azure/azure-rest-api-specs/issues/9549 + if !utils.ResponseWasNotFound(existing.Response) && !utils.ResponseWasForbidden(existing.Response) { return fmt.Errorf("unable to check for presence of existing Management Group %q: %s", groupName, err) } } - if existing.ID != nil && *existing.ID != "" { return tf.ImportAsExistsError("azurerm_management_group", *existing.ID) } @@ -131,7 +135,7 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa }, } - if v, ok := d.GetOk("display_name"); ok { + if v := d.Get("display_name"); v != "" { properties.CreateManagementGroupProperties.DisplayName = utils.String(v.(string)) } @@ -199,7 +203,7 @@ func resourceArmManagementGroupRead(d *schema.ResourceData, meta interface{}) er recurse := true resp, err := client.Get(ctx, id.GroupId, "children", &recurse, "", managementGroupCacheControl) if err != nil { - if utils.ResponseWasNotFound(resp.Response) { + if utils.ResponseWasForbidden(resp.Response) || utils.ResponseWasNotFound(resp.Response) { log.Printf("[INFO] Management Group %q doesn't exist - removing from state", d.Id()) d.SetId("") return nil @@ -248,7 +252,7 @@ func resourceArmManagementGroupDelete(d *schema.ResourceData, meta interface{}) recurse := true group, err := client.Get(ctx, id.GroupId, "children", &recurse, "", managementGroupCacheControl) if err != nil { - if utils.ResponseWasNotFound(group.Response) { + if utils.ResponseWasNotFound(group.Response) || utils.ResponseWasForbidden(group.Response) { log.Printf("[DEBUG] Management Group %q doesn't exist in Azure - nothing to do!", id.GroupId) return nil } diff --git a/azurerm/internal/services/managementgroup/tests/management_group_resource_test.go b/azurerm/internal/services/managementgroup/tests/management_group_resource_test.go index 254aeb15f4d9..25ab059fcb1c 100644 --- a/azurerm/internal/services/managementgroup/tests/management_group_resource_test.go +++ b/azurerm/internal/services/managementgroup/tests/management_group_resource_test.go @@ -15,7 +15,7 @@ import ( func TestAccAzureRMManagementGroup_basic(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_management_group", "test") - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -34,7 +34,7 @@ func TestAccAzureRMManagementGroup_basic(t *testing.T) { func TestAccAzureRMManagementGroup_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_management_group", "test") - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -54,7 +54,7 @@ func TestAccAzureRMManagementGroup_requiresImport(t *testing.T) { } func TestAccAzureRMManagementGroup_nested(t *testing.T) { - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -76,7 +76,7 @@ func TestAccAzureRMManagementGroup_nested(t *testing.T) { } func TestAccAzureRMManagementGroup_multiLevel(t *testing.T) { - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -99,7 +99,7 @@ func TestAccAzureRMManagementGroup_multiLevel(t *testing.T) { } func TestAccAzureRMManagementGroup_multiLevelUpdated(t *testing.T) { - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -126,7 +126,7 @@ func TestAccAzureRMManagementGroup_multiLevelUpdated(t *testing.T) { func TestAccAzureRMManagementGroup_withName(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_management_group", "test") - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -137,6 +137,7 @@ func TestAccAzureRMManagementGroup_withName(t *testing.T) { testCheckAzureRMManagementGroupExists(data.ResourceName), ), }, + data.ImportStep(), }, }) } @@ -144,7 +145,7 @@ func TestAccAzureRMManagementGroup_withName(t *testing.T) { func TestAccAzureRMManagementGroup_updateName(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_management_group", "test") - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -159,9 +160,10 @@ func TestAccAzureRMManagementGroup_updateName(t *testing.T) { Config: testAzureRMManagementGroup_withName(data), Check: resource.ComposeTestCheckFunc( testCheckAzureRMManagementGroupExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "display_name", fmt.Sprintf("acctestmg-%d", data.RandomInteger)), + resource.TestCheckResourceAttr(data.ResourceName, "name", fmt.Sprintf("acctestmg-%d", data.RandomInteger)), ), }, + data.ImportStep(), }, }) } @@ -170,7 +172,7 @@ func TestAccAzureRMManagementGroup_withSubscriptions(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_management_group", "test") subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID") - resource.ParallelTest(t, resource.TestCase{ + resource.Test(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, CheckDestroy: testCheckAzureRMManagementGroupDestroy, @@ -218,8 +220,8 @@ func testCheckAzureRMManagementGroupExists(resourceName string) resource.TestChe return fmt.Errorf("Bad: Get on managementGroupsClient: %s", err) } - if resp.StatusCode == http.StatusNotFound { - return fmt.Errorf("Management Group does not exist: %s", groupName) + if resp.StatusCode == http.StatusForbidden { + return fmt.Errorf("Management Group does not exist or you do not have proper permissions: %s", groupName) } return nil @@ -243,7 +245,7 @@ func testCheckAzureRMManagementGroupDestroy(s *terraform.State) error { return nil } - if resp.StatusCode != http.StatusNotFound { + if resp.StatusCode == http.StatusAccepted { return fmt.Errorf("Management Group still exists: %s", *resp.Name) } } @@ -318,9 +320,10 @@ provider "azurerm" { } resource "azurerm_management_group" "test" { - display_name = "acctestmg-%d" + name = "acctestmg-%d" + display_name = "accTestMG-%d" } -`, data.RandomInteger) +`, data.RandomInteger, data.RandomInteger) } // TODO: switch this out for dynamically creating a subscription once that's supported in the future