Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_management_group - deprecated and rename group_id to name #6276

Merged
merged 4 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,18 @@ func dataSourceArmManagementGroup() *schema.Resource {
Schema: map[string]*schema.Schema{
"group_id": {
Type: schema.TypeString,
Required: true,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"name", "group_id"},
Deprecated: "Deprecated in favor of `name`",
ValidateFunc: validate.ManagementGroupName,
},

"name": {
Type: schema.TypeString,
Optional: true, // TODO -- change back to required after the deprecation
Computed: true, // TODO -- remove computed after the deprecation
ExactlyOneOf: []string{"name", "group_id"},
ValidateFunc: validate.ManagementGroupName,
},

Expand Down Expand Up @@ -52,24 +63,31 @@ func dataSourceArmManagementGroupRead(d *schema.ResourceData, meta interface{})
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

groupId := d.Get("group_id").(string)
groupName := ""
if v, ok := d.GetOk("name"); ok {
groupName = v.(string)
}
if v, ok := d.GetOk("group_id"); ok {
groupName = v.(string)
}

recurse := true
resp, err := client.Get(ctx, groupId, "children", &recurse, "", managementGroupCacheControl)
resp, err := client.Get(ctx, groupName, "children", &recurse, "", managementGroupCacheControl)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
return fmt.Errorf("Management Group %q was not found", groupId)
return fmt.Errorf("Management Group %q was not found", groupName)
}

return fmt.Errorf("Error reading Management Group %q: %+v", d.Id(), err)
}

if resp.ID == nil {
return fmt.Errorf("Client returned an nil ID for Management Group %q", groupId)
return fmt.Errorf("Client returned an nil ID for Management Group %q", groupName)
}

d.SetId(*resp.ID)
d.Set("group_id", groupId)
d.Set("name", groupName)
d.Set("group_id", groupName)

if props := resp.Properties; props != nil {
d.Set("display_name", props.DisplayName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/managementgroup/parse"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/managementgroup/validate"
azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)
Expand All @@ -27,9 +27,11 @@ func resourceArmManagementGroup() *schema.Resource {
Update: resourceArmManagementGroupCreateUpdate,
Read: resourceArmManagementGroupRead,
Delete: resourceArmManagementGroupDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Importer: azSchema.ValidateResourceIDPriorToImport(func(id string) error {
_, err := parse.ManagementGroupID(id)
return err
}),

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(30 * time.Minute),
Expand All @@ -40,11 +42,22 @@ func resourceArmManagementGroup() *schema.Resource {

Schema: map[string]*schema.Schema{
"group_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validate.ManagementGroupName,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"name"},
Deprecated: "Deprecated in favor of `name`",
ValidateFunc: validate.ManagementGroupName,
},

"name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"group_id"},
ValidateFunc: validate.ManagementGroupName,
},

"display_name": {
Expand Down Expand Up @@ -77,9 +90,12 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa
defer cancel()
armTenantID := meta.(*clients.Client).Account.TenantId

groupId := d.Get("group_id").(string)
if groupId == "" {
groupId = uuid.New().String()
groupName := uuid.New().String()
if v, ok := d.GetOk("group_name"); ok {
groupName = v.(string)
}
if v, ok := d.GetOk("group_id"); ok {
groupName = v.(string)
}

parentManagementGroupId := d.Get("parent_management_group_id").(string)
Expand All @@ -88,11 +104,11 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa
}

recurse := false
if features.ShouldResourcesBeImported() && d.IsNewResource() {
existing, err := client.Get(ctx, groupId, "children", &recurse, "", managementGroupCacheControl)
if d.IsNewResource() {
existing, err := client.Get(ctx, groupName, "children", &recurse, "", managementGroupCacheControl)
if err != nil {
if !utils.ResponseWasNotFound(existing.Response) {
return fmt.Errorf("unable to check for presence of existing Management Group %q: %s", groupId, err)
return fmt.Errorf("unable to check for presence of existing Management Group %q: %s", groupName, err)
}
}

Expand All @@ -101,10 +117,10 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa
}
}

log.Printf("[INFO] Creating Management Group %q", groupId)
log.Printf("[INFO] Creating Management Group %q", groupName)

properties := managementgroups.CreateManagementGroupRequest{
Name: utils.String(groupId),
Name: utils.String(groupName),
CreateManagementGroupProperties: &managementgroups.CreateManagementGroupProperties{
TenantID: utils.String(armTenantID),
Details: &managementgroups.CreateManagementGroupDetails{
Expand All @@ -119,18 +135,18 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa
properties.CreateManagementGroupProperties.DisplayName = utils.String(v.(string))
}

future, err := client.CreateOrUpdate(ctx, groupId, properties, managementGroupCacheControl)
future, err := client.CreateOrUpdate(ctx, groupName, properties, managementGroupCacheControl)
if err != nil {
return fmt.Errorf("unable to create Management Group %q: %+v", groupId, err)
return fmt.Errorf("unable to create Management Group %q: %+v", groupName, err)
}

if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("failed when waiting for creation of Management Group %q: %+v", groupId, err)
return fmt.Errorf("failed when waiting for creation of Management Group %q: %+v", groupName, err)
}

resp, err := client.Get(ctx, groupId, "children", &recurse, "", managementGroupCacheControl)
resp, err := client.Get(ctx, groupName, "children", &recurse, "", managementGroupCacheControl)
if err != nil {
return fmt.Errorf("unable to retrieve Management Group %q: %+v", groupId, err)
return fmt.Errorf("unable to retrieve Management Group %q: %+v", groupName, err)
}

d.SetId(*resp.ID)
Expand All @@ -139,32 +155,31 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa

// first remove any which need to be removed
if !d.IsNewResource() {
log.Printf("[DEBUG] Determine which Subscriptions should be removed from Management Group %q", groupId)
log.Printf("[DEBUG] Determine which Subscriptions should be removed from Management Group %q", groupName)
if props := resp.Properties; props != nil {
subscriptionIdsToRemove, err2 := determineManagementGroupSubscriptionsIdsToRemove(props.Children, subscriptionIds)
if err2 != nil {
return fmt.Errorf("unable to determine which subscriptions should be removed from Management Group %q: %+v", groupId, err2)
return fmt.Errorf("unable to determine which subscriptions should be removed from Management Group %q: %+v", groupName, err2)
}

for _, subscriptionId := range *subscriptionIdsToRemove {
log.Printf("[DEBUG] De-associating Subscription ID %q from Management Group %q", subscriptionId, groupId)
deleteResp, err2 := subscriptionsClient.Delete(ctx, groupId, subscriptionId, managementGroupCacheControl)
log.Printf("[DEBUG] De-associating Subscription ID %q from Management Group %q", subscriptionId, groupName)
deleteResp, err2 := subscriptionsClient.Delete(ctx, groupName, subscriptionId, managementGroupCacheControl)
if err2 != nil {
if !response.WasNotFound(deleteResp.Response) {
return fmt.Errorf("unable to de-associate Subscription %q from Management Group %q: %+v", subscriptionId, groupId, err2)
return fmt.Errorf("unable to de-associate Subscription %q from Management Group %q: %+v", subscriptionId, groupName, err2)
}
}
}
}
}

// then add the new ones
log.Printf("[DEBUG] Preparing to assign Subscriptions to Management Group %q", groupId)
log.Printf("[DEBUG] Preparing to assign Subscriptions to Management Group %q", groupName)
for _, subscriptionId := range subscriptionIds {
log.Printf("[DEBUG] Assigning Subscription ID %q to management group %q", subscriptionId, groupId)
_, err = subscriptionsClient.Create(ctx, groupId, subscriptionId, managementGroupCacheControl)
if err != nil {
return fmt.Errorf("[DEBUG] Error assigning Subscription ID %q to Management Group %q: %+v", subscriptionId, groupId, err)
log.Printf("[DEBUG] Assigning Subscription ID %q to management group %q", subscriptionId, groupName)
if _, err := subscriptionsClient.Create(ctx, groupName, subscriptionId, managementGroupCacheControl); err != nil {
return fmt.Errorf("[DEBUG] Error assigning Subscription ID %q to Management Group %q: %+v", subscriptionId, groupName, err)
}
}

Expand Down Expand Up @@ -193,6 +208,7 @@ func resourceArmManagementGroupRead(d *schema.ResourceData, meta interface{}) er
return fmt.Errorf("unable to read Management Group %q: %+v", d.Id(), err)
}

d.Set("name", id.GroupId)
d.Set("group_id", id.GroupId)

if props := resp.Properties; props != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ resource "azurerm_management_group" "test" {
}

data "azurerm_management_group" "test" {
group_id = azurerm_management_group.test.group_id
name = azurerm_management_group.test.name
}
`, data.RandomInteger)
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/terraform"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
)

func TestAccAzureRMManagementGroup_basic(t *testing.T) {
Expand All @@ -33,11 +32,6 @@ func TestAccAzureRMManagementGroup_basic(t *testing.T) {
}

func TestAccAzureRMManagementGroup_requiresImport(t *testing.T) {
if !features.ShouldResourcesBeImported() {
t.Skip("Skipping since resources aren't required to be imported")
return
}

data := acceptance.BuildTestData(t, "azurerm_management_group", "test")

resource.ParallelTest(t, resource.TestCase{
Expand Down Expand Up @@ -278,7 +272,7 @@ resource "azurerm_management_group" "test" {
}

resource "azurerm_management_group" "import" {
group_id = azurerm_management_group.test.group_id
name = azurerm_management_group.test.name
}
`
}
Expand Down
8 changes: 6 additions & 2 deletions website/docs/d/management_group.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Use this data source to access information about an existing Management Group.

```hcl
data "azurerm_management_group" "example" {
group_id = "00000000-0000-0000-0000-000000000000"
name = "00000000-0000-0000-0000-000000000000"
}

output "display_name" {
Expand All @@ -26,7 +26,11 @@ output "display_name" {

The following arguments are supported:

* `group_id` - Specifies the UUID of this Management Group.
* `name` - Specifies the name or UUID of this Management Group.

* `group_id` - Specifies the name or UUID of this Management Group.

~> **NOTE:** The field `group_id` has been deprecated in favor of `name`.

## Attributes Reference

Expand Down
8 changes: 5 additions & 3 deletions website/docs/r/management_group.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ resource "azurerm_management_group" "example_child" {

The following arguments are supported:

* `group_id` - (Optional) The UUID for this Management Group, which needs to be unique across your tenant - which will be generated if not provided. Changing this forces a new resource to be created.
* `name` - (Optional) The name or UUID for this Management Group, which needs to be unique across your tenant. A new UUID will be generated if not provided. Changing this forces a new resource to be created.

* `display_name` - (Optional) A friendly name for this Management Group. If not specified, this'll be the same as the `group_id`.
* `group_id` - (Optional) The name or UUID for this Management Group, which needs to be unique across your tenant. A new UUID will be generated if not provided. Changing this forces a new resource to be created.

* `display_name` - (Optional) A friendly name for this Management Group. If not specified, this'll be the same as the `name`.

* `parent_management_group_id` - (Optional) The ID of the Parent Management Group. Changing this forces a new resource to be created.

Expand All @@ -67,5 +69,5 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d
Management Groups can be imported using the `management group resource id`, e.g.

```shell
terraform import azurerm_management_group.example /providers/Microsoft.Management/ManagementGroups/group1
terraform import azurerm_management_group.example /providers/Microsoft.Management/managementGroups/group1
```