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

New resource azurerm_policy_remediation #5746

Merged
merged 32 commits into from
Mar 27, 2020
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0c6555f
Add ID parsing and validation functions, add name validate function
ArcturusZhang Feb 25, 2020
5582e41
Add validate function for data source
ArcturusZhang Feb 25, 2020
cb3a052
Goimports
ArcturusZhang Feb 25, 2020
83435db
Added new resource and data source azurerm_policy_remediation
ArcturusZhang Feb 14, 2020
3c6ba09
Add some more test cases
ArcturusZhang Feb 14, 2020
c53b8db
Some changes to conform the changes in 2.0
ArcturusZhang Feb 25, 2020
689bcdc
Resolve some comments
ArcturusZhang Mar 6, 2020
b281d69
Add ID parsing and validation functions, add name validate function
ArcturusZhang Feb 25, 2020
5eb2e9b
Add validate function for data source
ArcturusZhang Feb 25, 2020
22dc3b7
Goimports
ArcturusZhang Feb 25, 2020
cf1d469
Resolve comments
ArcturusZhang Mar 5, 2020
48dc397
Use parsing functions from mgmt group
ArcturusZhang Mar 6, 2020
aae0f5d
Resolve more comments
ArcturusZhang Mar 6, 2020
be89874
Resolve more comments
ArcturusZhang Mar 6, 2020
8142ed5
Fix CI failures
ArcturusZhang Mar 6, 2020
e4cf05a
Some refactor
ArcturusZhang Mar 6, 2020
d083570
Removed a useless comment
ArcturusZhang Mar 6, 2020
4ebd2f0
Some refinement on file structure
ArcturusZhang Mar 6, 2020
dcc9859
Fix CI failures
ArcturusZhang Mar 6, 2020
383ae37
Resolve comments
ArcturusZhang Mar 6, 2020
4171232
Merge branch 'Add-ID-parsing-function-for-mgmt-group' into PolicyInsi…
ArcturusZhang Mar 10, 2020
5c8dd44
Merge remote-tracking branch 'origin/master' into PolicyInsightsRemed…
ArcturusZhang Mar 10, 2020
080d94b
Merge branch 'master' into PolicyInsightsRemediation
ArcturusZhang Mar 19, 2020
abddb70
Merge branch 'master' into PolicyInsightsRemediation
ArcturusZhang Mar 19, 2020
2e42ed3
Refresh dependencies for policyinsights
ArcturusZhang Mar 19, 2020
f31c25a
Merge branch 'master' into PolicyInsightsRemediation
ArcturusZhang Mar 20, 2020
02cac69
Resolve comments
ArcturusZhang Mar 25, 2020
1e0db42
Resolve comments
ArcturusZhang Mar 26, 2020
97bd696
Include some changes on policy definition
ArcturusZhang Mar 27, 2020
e53b998
Fix name validation to forbid upper case letters
ArcturusZhang Mar 27, 2020
3407903
make fmt
katbyte Mar 27, 2020
88cb232
Goimports :(
ArcturusZhang Mar 27, 2020
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 @@ -7,6 +7,7 @@ import (
"github.com/Azure/azure-sdk-for-go/services/preview/resources/mgmt/2018-03-01-preview/managementgroups"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/managementgroup/validate"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)
Expand All @@ -21,8 +22,9 @@ func dataSourceArmManagementGroup() *schema.Resource {

Schema: map[string]*schema.Schema{
"group_id": {
Type: schema.TypeString,
Required: true,
Type: schema.TypeString,
Required: true,
ValidateFunc: validate.ManagementGroupName,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the property is group_id but this is validating the name?

Copy link
Contributor Author

@ArcturusZhang ArcturusZhang Mar 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here group_id means this one in its ID:

/providers/Microsoft.ManagementGroup/managementGroups/{group_id}

the thing in other resources' IDs at the same position would be called as a name, for instance

/subscriptions/{subsID}/resourceGroups/group1/providers/Microsoft.Compute/virtualMachines/{name}

And another reason is that if we call this group_id in the validation function, the validation function would looks like ManagementGroupGroupID or ManagementGroupID. This would be confusing with the validation function of the real management group ID. Therefore I took this name.

},

"display_name": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package parse

import (
"fmt"
"regexp"
"strings"
)

type ManagementGroupId struct {
GroupId string
}

func ManagementGroupID(input string) (*ManagementGroupId, error) {
regex := regexp.MustCompile(`^/providers/[Mm]icrosoft\.[Mm]anagement/management[Gg]roups/`)
if !regex.MatchString(input) {
return nil, fmt.Errorf("Unable to parse Management Group ID %q", input)
}

// Split the input ID by the regex
segments := regex.Split(input, -1)
if len(segments) != 2 {
return nil, fmt.Errorf("Unable to parse Management Group ID %q: expected id to have two segments after splitting", input)
}

groupID := segments[1]
if groupID == "" {
return nil, fmt.Errorf("unable to parse Management Group ID %q: management group name is empty", input)
}
if segments := strings.Split(groupID, "/"); len(segments) != 1 {
return nil, fmt.Errorf("unable to parse Management Group ID %q: ID has extra segments", input)
}

id := ManagementGroupId{
GroupId: groupID,
}

return &id, nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package parse

import "testing"

func TestManagementGroupID(t *testing.T) {
testData := []struct {
Name string
Input string
Error bool
Expected *ManagementGroupId
}{
{
Name: "Empty",
Input: "",
Error: true,
},
{
Name: "No Management Groups Segment",
Input: "/providers/Microsoft.Management",
Error: true,
},
{
Name: "No Management Group ID",
Input: "/providers/Microsoft.Management/managementGroups/",
Error: true,
},
{
Name: "Management Group ID in UUID",
Input: "/providers/Microsoft.Management/managementGroups/00000000-0000-0000-0000-000000000000",
Expected: &ManagementGroupId{
GroupId: "00000000-0000-0000-0000-000000000000",
},
},
{
Name: "Management Group ID in Readable ID",
Input: "/providers/Microsoft.Management/managementGroups/myGroup",
Expected: &ManagementGroupId{
GroupId: "myGroup",
},
},
{
Name: "Management Group ID in UUID with wrong casing",
Input: "/providers/microsoft.management/managementgroups/00000000-0000-0000-0000-000000000000",
Expected: &ManagementGroupId{
GroupId: "00000000-0000-0000-0000-000000000000",
},
},
{
Name: "Management Group ID in Readable ID with wrong casing",
Input: "/providers/microsoft.management/managementgroups/group1",
Expected: &ManagementGroupId{
GroupId: "group1",
},
},
{
Name: "Invalid Management group id",
Input: "/providers/Microsoft.Management/managementGroups/myGroup/another",
Error: true,
},
{
Name: "Resource ID in management group",
Input: "/providers/Microsoft.Management/managementGroups/myGroup/providers/Microsoft.Authorization/policyDefinitions/def1",
Error: true,
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

actual, err := ManagementGroupID(v.Input)
if err != nil {
if v.Error {
continue
}

t.Fatalf("Expected a value but got an error: %s", err)
}

if actual.GroupId != v.Expected.GroupId {
t.Fatalf("Expected %q but got %q for Name", v.Expected.GroupId, actual.GroupId)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"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"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)
Expand All @@ -38,10 +40,11 @@ func resourceArmManagementGroup() *schema.Resource {

Schema: map[string]*schema.Schema{
"group_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validate.ManagementGroupName,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The is a group_id property but we are validating a name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason as the above comment

},

"display_name": {
Expand All @@ -51,9 +54,10 @@ func resourceArmManagementGroup() *schema.Resource {
},

"parent_management_group_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validate.ManagementGroupID,
},

"subscription_ids": {
Expand Down Expand Up @@ -88,7 +92,7 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa
existing, err := client.Get(ctx, groupId, "children", &recurse, "", managementGroupCacheControl)
if err != nil {
if !utils.ResponseWasNotFound(existing.Response) {
return fmt.Errorf("Error checking for presence of existing Management Group %q: %s", groupId, err)
return fmt.Errorf("unable to check for presence of existing Management Group %q: %s", groupId, err)
}
}

Expand Down Expand Up @@ -117,16 +121,16 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa

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

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

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

d.SetId(*resp.ID)
Expand All @@ -139,15 +143,15 @@ func resourceArmManagementGroupCreateUpdate(d *schema.ResourceData, meta interfa
if props := resp.Properties; props != nil {
subscriptionIdsToRemove, err2 := determineManagementGroupSubscriptionsIdsToRemove(props.Children, subscriptionIds)
if err2 != nil {
return fmt.Errorf("Error determining 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", groupId, 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)
if err2 != nil {
if !response.WasNotFound(deleteResp.Response) {
return fmt.Errorf("Error de-associating 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, groupId, err2)
}
}
}
Expand All @@ -172,31 +176,31 @@ func resourceArmManagementGroupRead(d *schema.ResourceData, meta interface{}) er
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parseManagementGroupId(d.Id())
id, err := parse.ManagementGroupID(d.Id())
if err != nil {
return err
}

recurse := true
resp, err := client.Get(ctx, id.groupId, "children", &recurse, "", managementGroupCacheControl)
resp, err := client.Get(ctx, id.GroupId, "children", &recurse, "", managementGroupCacheControl)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
log.Printf("[INFO] Management Group %q doesn't exist - removing from state", d.Id())
d.SetId("")
return nil
}

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

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

if props := resp.Properties; props != nil {
d.Set("display_name", props.DisplayName)

subscriptionIds, err := flattenArmManagementGroupSubscriptionIds(props.Children)
if err != nil {
return fmt.Errorf("Error flattening `subscription_ids`: %+v", err)
return fmt.Errorf("unable to flatten `subscription_ids`: %+v", err)
}
d.Set("subscription_ids", subscriptionIds)

Expand All @@ -220,20 +224,20 @@ func resourceArmManagementGroupDelete(d *schema.ResourceData, meta interface{})
ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parseManagementGroupId(d.Id())
id, err := parse.ManagementGroupID(d.Id())
if err != nil {
return err
}

recurse := true
group, err := client.Get(ctx, id.groupId, "children", &recurse, "", managementGroupCacheControl)
group, err := client.Get(ctx, id.GroupId, "children", &recurse, "", managementGroupCacheControl)
if err != nil {
if utils.ResponseWasNotFound(group.Response) {
log.Printf("[DEBUG] Management Group %q doesn't exist in Azure - nothing to do!", id.groupId)
log.Printf("[DEBUG] Management Group %q doesn't exist in Azure - nothing to do!", id.GroupId)
return nil
}

return fmt.Errorf("Error retrieving Management Group %q: %+v", id.groupId, err)
return fmt.Errorf("unable to retrieve Management Group %q: %+v", id.GroupId, err)
}

// before deleting a management group, return any subscriptions to the root management group
Expand All @@ -245,26 +249,26 @@ func resourceArmManagementGroupDelete(d *schema.ResourceData, meta interface{})
}

subscriptionId := *v.ID
log.Printf("[DEBUG] De-associating Subscription %q from Management Group %q..", subscriptionId, id.groupId)
log.Printf("[DEBUG] De-associating Subscription %q from Management Group %q..", subscriptionId, id.GroupId)
// NOTE: whilst this says `Delete` it's actually `Deassociate` - which is /really/ helpful
deleteResp, err2 := subscriptionsClient.Delete(ctx, id.groupId, subscriptionId, managementGroupCacheControl)
deleteResp, err2 := subscriptionsClient.Delete(ctx, id.GroupId, subscriptionId, managementGroupCacheControl)
if err2 != nil {
if !response.WasNotFound(deleteResp.Response) {
return fmt.Errorf("Error de-associating Subscription %q from Management Group %q: %+v", subscriptionId, id.groupId, err2)
return fmt.Errorf("unable to de-associate Subscription %q from Management Group %q: %+v", subscriptionId, id.GroupId, err2)
}
}
}
}
}

resp, err := client.Delete(ctx, id.groupId, managementGroupCacheControl)
resp, err := client.Delete(ctx, id.GroupId, managementGroupCacheControl)
if err != nil {
return fmt.Errorf("Error deleting Management Group %q: %+v", id.groupId, err)
return fmt.Errorf("unable to delete Management Group %q: %+v", id.GroupId, err)
}

err = resp.WaitForCompletionRef(ctx, client.Client)
if err != nil {
return fmt.Errorf("Error waiting for the deletion of Management Group %q: %+v", id.groupId, err)
return fmt.Errorf("failed when waiting for the deletion of Management Group %q: %+v", id.GroupId, err)
}

return nil
Expand Down Expand Up @@ -295,7 +299,7 @@ func flattenArmManagementGroupSubscriptionIds(input *[]managementgroups.ChildInf

id, err := parseManagementGroupSubscriptionID(*child.ID)
if err != nil {
return nil, fmt.Errorf("Unable to parse child Subscription ID %+v", err)
return nil, fmt.Errorf("unable to parse child Subscription ID %+v", err)
}

if id != nil {
Expand All @@ -306,31 +310,13 @@ func flattenArmManagementGroupSubscriptionIds(input *[]managementgroups.ChildInf
return subscriptionIds, nil
}

type managementGroupId struct {
groupId string
}

type subscriptionId struct {
subscriptionId string
}

func parseManagementGroupId(input string) (*managementGroupId, error) {
// /providers/Microsoft.Management/managementGroups/00000000-0000-0000-0000-000000000000
segments := strings.Split(input, "/")
if len(segments) != 5 {
return nil, fmt.Errorf("Expected there to be 5 segments but got %d", len(segments))
}

id := managementGroupId{
groupId: segments[4],
}
return &id, nil
}

func parseManagementGroupSubscriptionID(input string) (*subscriptionId, error) {
// this is either:
// /subscriptions/00000000-0000-0000-0000-000000000000
// /providers/Microsoft.Management/managementGroups/e4115b99-6be7-4153-a73f-5ff5e778ce28

// we skip out the managementGroup ID's
if strings.HasPrefix(input, "/providers/Microsoft.Management/managementGroups/") {
Expand All @@ -340,11 +326,11 @@ func parseManagementGroupSubscriptionID(input string) (*subscriptionId, error) {
components := strings.Split(input, "/")

if len(components) == 0 {
return nil, fmt.Errorf("Subscription Id is empty or not formatted correctly: %s", input)
return nil, fmt.Errorf("subscription Id is empty or not formatted correctly: %s", input)
}

if len(components) != 3 {
return nil, fmt.Errorf("Subscription Id should have 2 segments, got %d: %q", len(components)-1, input)
return nil, fmt.Errorf("subscription Id should have 2 segments, got %d: %q", len(components)-1, input)
}

id := subscriptionId{
Expand All @@ -366,7 +352,7 @@ func determineManagementGroupSubscriptionsIdsToRemove(existing *[]managementgrou

id, err := parseManagementGroupSubscriptionID(*v.ID)
if err != nil {
return nil, fmt.Errorf("Error parsing Subscription ID %q: %+v", *v.ID, err)
return nil, fmt.Errorf("unable to parse Subscription ID %q: %+v", *v.ID, err)
}

// not a Subscription - so let's skip it
Expand Down
Loading