From 82c42d8f7451dde8d26c6b25202f7d1dae7db8fb Mon Sep 17 00:00:00 2001 From: arcturusZhang Date: Wed, 11 Nov 2020 15:03:04 +0800 Subject: [PATCH 1/4] Policy set definition enhancement --- azurerm/internal/services/policy/policy.go | 9 +- .../policy/policy_definition_data_source.go | 2 +- .../policy/policy_definition_resource.go | 4 +- .../policy_set_definition_data_source.go | 47 +++++- .../policy/policy_set_definition_resource.go | 144 +++++++++++++++++- .../policy_set_definition_resource_test.go | 67 ++++++++ .../d/policy_set_definition.html.markdown | 18 +++ .../r/policy_set_definition.html.markdown | 18 +++ 8 files changed, 296 insertions(+), 13 deletions(-) diff --git a/azurerm/internal/services/policy/policy.go b/azurerm/internal/services/policy/policy.go index 57a6aa440018..e65bb0f59043 100644 --- a/azurerm/internal/services/policy/policy.go +++ b/azurerm/internal/services/policy/policy.go @@ -6,9 +6,8 @@ import ( "encoding/json" "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" - "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-09-01/policy" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) func getPolicyDefinitionByDisplayName(ctx context.Context, client *policy.DefinitionsClient, displayName, managementGroupName string) (policy.Definition, error) { @@ -49,7 +48,7 @@ func getPolicyDefinitionByDisplayName(ctx context.Context, client *policy.Defini return results[0], nil } -func getPolicyDefinitionByName(ctx context.Context, client *policy.DefinitionsClient, name string, managementGroupName string) (res policy.Definition, err error) { +func getPolicyDefinitionByName(ctx context.Context, client *policy.DefinitionsClient, name, managementGroupName string) (res policy.Definition, err error) { if managementGroupName == "" { res, err = client.Get(ctx, name) if utils.ResponseWasNotFound(res.Response) { @@ -62,7 +61,7 @@ func getPolicyDefinitionByName(ctx context.Context, client *policy.DefinitionsCl return res, err } -func getPolicySetDefinitionByName(ctx context.Context, client *policy.SetDefinitionsClient, name string, managementGroupID string) (res policy.SetDefinition, err error) { +func getPolicySetDefinitionByName(ctx context.Context, client *policy.SetDefinitionsClient, name, managementGroupID string) (res policy.SetDefinition, err error) { if managementGroupID == "" { res, err = client.Get(ctx, name) if utils.ResponseWasNotFound(res.Response) { @@ -121,7 +120,7 @@ func expandParameterDefinitionsValueFromString(jsonString string) (map[string]*p return result, err } -func flattenParameterDefintionsValueToString(input map[string]*policy.ParameterDefinitionsValue) (string, error) { +func flattenParameterDefinitionsValueToString(input map[string]*policy.ParameterDefinitionsValue) (string, error) { if len(input) == 0 { return "", nil } diff --git a/azurerm/internal/services/policy/policy_definition_data_source.go b/azurerm/internal/services/policy/policy_definition_data_source.go index 92df4351cee7..f26bd63449ee 100644 --- a/azurerm/internal/services/policy/policy_definition_data_source.go +++ b/azurerm/internal/services/policy/policy_definition_data_source.go @@ -131,7 +131,7 @@ func dataSourceArmPolicyDefinitionRead(d *schema.ResourceData, meta interface{}) d.Set("metadata", metadataStr) } - if parametersStr, err := flattenParameterDefintionsValueToString(policyDefinition.Parameters); err == nil { + if parametersStr, err := flattenParameterDefinitionsValueToString(policyDefinition.Parameters); err == nil { d.Set("parameters", parametersStr) } else { return fmt.Errorf("failed to flatten Policy Parameters %q: %+v", name, err) diff --git a/azurerm/internal/services/policy/policy_definition_resource.go b/azurerm/internal/services/policy/policy_definition_resource.go index 928445b4132c..168029b180ce 100644 --- a/azurerm/internal/services/policy/policy_definition_resource.go +++ b/azurerm/internal/services/policy/policy_definition_resource.go @@ -312,7 +312,7 @@ func resourceArmPolicyDefinitionRead(d *schema.ResourceData, meta interface{}) e d.Set("metadata", metadataStr) } - if parametersStr, err := flattenParameterDefintionsValueToString(props.Parameters); err == nil { + if parametersStr, err := flattenParameterDefinitionsValueToString(props.Parameters); err == nil { d.Set("parameters", parametersStr) } else { return fmt.Errorf("flattening policy definition parameters %+v", err) @@ -356,7 +356,7 @@ func resourceArmPolicyDefinitionDelete(d *schema.ResourceData, meta interface{}) return nil } -func policyDefinitionRefreshFunc(ctx context.Context, client *policy.DefinitionsClient, name string, managementGroupID string) resource.StateRefreshFunc { +func policyDefinitionRefreshFunc(ctx context.Context, client *policy.DefinitionsClient, name, managementGroupID string) resource.StateRefreshFunc { return func() (interface{}, string, error) { res, err := getPolicyDefinitionByName(ctx, client, name, managementGroupID) diff --git a/azurerm/internal/services/policy/policy_set_definition_data_source.go b/azurerm/internal/services/policy/policy_set_definition_data_source.go index e24438d6a3bd..3643d12d937d 100644 --- a/azurerm/internal/services/policy/policy_set_definition_data_source.go +++ b/azurerm/internal/services/policy/policy_set_definition_data_source.go @@ -89,6 +89,14 @@ func dataSourceArmPolicySetDefinition() *schema.Resource { Type: schema.TypeString, Computed: true, }, + + "group_names": { + Type: schema.TypeList, + Computed: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, }, }, }, @@ -97,6 +105,39 @@ func dataSourceArmPolicySetDefinition() *schema.Resource { Type: schema.TypeString, Computed: true, }, + + "policy_definition_group": { + Type: schema.TypeList, + Computed: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Computed: true, + }, + + "display_name": { + Type: schema.TypeString, + Computed: true, + }, + + "category": { + Type: schema.TypeString, + Computed: true, + }, + + "description": { + Type: schema.TypeString, + Computed: true, + }, + + "additional_metadata_id": { + Type: schema.TypeString, + Computed: true, + }, + }, + }, + }, }, } } @@ -137,7 +178,7 @@ func dataSourceArmPolicySetDefinitionRead(d *schema.ResourceData, meta interface d.Set("policy_type", setDefinition.PolicyType) d.Set("metadata", flattenJSON(setDefinition.Metadata)) - if paramsStr, err := flattenParameterDefintionsValueToString(setDefinition.Parameters); err != nil { + if paramsStr, err := flattenParameterDefinitionsValueToString(setDefinition.Parameters); err != nil { return fmt.Errorf("flattening JSON for `parameters`: %+v", err) } else { d.Set("parameters", paramsStr) @@ -157,5 +198,9 @@ func dataSourceArmPolicySetDefinitionRead(d *schema.ResourceData, meta interface return fmt.Errorf("setting `policy_definition_reference`: %+v", err) } + if err := d.Set("policy_definition_group", flattenAzureRMPolicySetDefinitionPolicyGroups(setDefinition.PolicyDefinitionGroups)); err != nil { + return fmt.Errorf("setting `policy_definition_group`: %+v", err) + } + return nil } diff --git a/azurerm/internal/services/policy/policy_set_definition_resource.go b/azurerm/internal/services/policy/policy_set_definition_resource.go index 84c000c793b0..43a28e899f91 100644 --- a/azurerm/internal/services/policy/policy_set_definition_resource.go +++ b/azurerm/internal/services/policy/policy_set_definition_resource.go @@ -1,6 +1,7 @@ package policy import ( + "bytes" "context" "encoding/json" "fmt" @@ -11,6 +12,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-09-01/policy" "github.com/Azure/go-autorest/autorest" + "github.com/hashicorp/terraform-plugin-sdk/helper/hashcode" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/structure" @@ -152,9 +154,57 @@ func resourceArmPolicySetDefinition() *schema.Resource { Optional: true, Computed: true, }, + + "group_names": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.StringIsNotEmpty, + }, + }, }, }, }, + + "policy_definition_group": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringIsNotEmpty, + }, + + "display_name": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringIsNotEmpty, + }, + + "category": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringIsNotEmpty, + }, + + "description": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringIsNotEmpty, + }, + + "additional_metadata_id": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringIsNotEmpty, + }, + }, + }, + Set: resourceARMPolicySetDefinitionPolicyDefinitionGroupHash, + }, }, } } @@ -271,6 +321,10 @@ func resourceArmPolicySetDefinitionCreate(d *schema.ResourceData, meta interface properties.PolicyDefinitions = definitions } + if v, ok := d.GetOk("policy_definition_group"); ok { + properties.PolicyDefinitionGroups = expandAzureRMPolicySetDefinitionPolicyGroups(v.(*schema.Set).List()) + } + definition := policy.SetDefinition{ SetDefinitionProperties: &properties, } @@ -463,7 +517,7 @@ func resourceArmPolicySetDefinitionRead(d *schema.ResourceData, meta interface{} } if parameters := props.Parameters; parameters != nil { - parametersStr, err := flattenParameterDefintionsValueToString(parameters) + parametersStr, err := flattenParameterDefinitionsValueToString(parameters) if err != nil { return fmt.Errorf("flattening JSON for `parameters`: %+v", err) } @@ -486,6 +540,10 @@ func resourceArmPolicySetDefinitionRead(d *schema.ResourceData, meta interface{} if err := d.Set("policy_definition_reference", references); err != nil { return fmt.Errorf("setting `policy_definition_reference`: %+v", err) } + + if err := d.Set("policy_definition_group", flattenAzureRMPolicySetDefinitionPolicyGroups(props.PolicyDefinitionGroups)); err != nil { + return fmt.Errorf("setting `policy_definition_group`: %+v", err) + } } return nil @@ -502,8 +560,7 @@ func resourceArmPolicySetDefinitionDelete(d *schema.ResourceData, meta interface } managementGroupName := "" - switch scopeId := id.PolicyScopeId.(type) { // nolint gocritic - case parse.ScopeAtManagementGroup: + if scopeId, ok := id.PolicyScopeId.(parse.ScopeAtManagementGroup); ok { managementGroupName = scopeId.ManagementGroupName } @@ -525,7 +582,7 @@ func resourceArmPolicySetDefinitionDelete(d *schema.ResourceData, meta interface return nil } -func policySetDefinitionRefreshFunc(ctx context.Context, client *policy.SetDefinitionsClient, name string, managementGroupId string) resource.StateRefreshFunc { +func policySetDefinitionRefreshFunc(ctx context.Context, client *policy.SetDefinitionsClient, name, managementGroupId string) resource.StateRefreshFunc { return func() (interface{}, string, error) { res, err := getPolicySetDefinitionByName(ctx, client, name, managementGroupId) if err != nil { @@ -601,6 +658,7 @@ func expandAzureRMPolicySetDefinitionPolicyDefinitions(input []interface{}) (*[] PolicyDefinitionID: utils.String(v["policy_definition_id"].(string)), Parameters: parameters, PolicyDefinitionReferenceID: utils.String(v["reference_id"].(string)), + GroupNames: utils.ExpandStringSlice(v["group_names"].(*schema.Set).List()), }) } @@ -642,7 +700,85 @@ func flattenAzureRMPolicySetDefinitionPolicyDefinitions(input *[]policy.Definiti "parameters": parametersMap, "parameter_values": parameterValues, "reference_id": policyDefinitionReference, + "group_names": utils.FlattenStringSlice(definition.GroupNames), }) } return result, nil } + +func expandAzureRMPolicySetDefinitionPolicyGroups(input []interface{}) *[]policy.DefinitionGroup { + result := make([]policy.DefinitionGroup, 0) + + for _, item := range input { + v := item.(map[string]interface{}) + group := policy.DefinitionGroup{} + if name := v["name"].(string); name != "" { + group.Name = utils.String(name) + } + if displayName := v["display_name"].(string); displayName != "" { + group.DisplayName = utils.String(displayName) + } + if category := v["category"].(string); category != "" { + group.Category = utils.String(category) + } + if description := v["description"].(string); description != "" { + group.Description = utils.String(description) + } + if metadataID := v["additional_metadata_id"].(string); metadataID != "" { + group.AdditionalMetadataID = utils.String(metadataID) + } + result = append(result, group) + } + + return &result +} + +func flattenAzureRMPolicySetDefinitionPolicyGroups(input *[]policy.DefinitionGroup) []interface{} { + result := make([]interface{}, 0) + if input == nil { + return result + } + + for _, group := range *input { + name := "" + if group.Name != nil { + name = *group.Name + } + displayName := "" + if group.DisplayName != nil { + displayName = *group.DisplayName + } + category := "" + if group.Category != nil { + category = *group.Category + } + description := "" + if group.Description != nil { + description = *group.Description + } + metadataID := "" + if group.AdditionalMetadataID != nil { + metadataID = *group.AdditionalMetadataID + } + + result = append(result, map[string]interface{}{ + "name": name, + "display_name": displayName, + "category": category, + "description": description, + "additional_metadata_id": metadataID, + }) + } + + return result +} + +func resourceARMPolicySetDefinitionPolicyDefinitionGroupHash(v interface{}) int { + var buf bytes.Buffer + + if m, ok := v.(map[string]interface{}); ok { + buf.WriteString(m["name"].(string)) + } + + return hashcode.String(buf.String()) +} diff --git a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go index dcb7d8af4d62..3859fc350cff 100644 --- a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go +++ b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go @@ -216,6 +216,25 @@ func TestAccAzureRMPolicySetDefinition_customWithPolicyReferenceID(t *testing.T) }) } +func TestAccAzureRMPolicySetDefinition_customWithDefinitionGroups(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_policy_set_definition", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMPolicySetDefinitionDestroy, + Steps: []resource.TestStep{ + { + Config: testAzureRMPolicySetDefinition_customWithDefinitionGroups(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + func TestAccAzureRMPolicySetDefinition_managementGroupDeprecated(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_policy_set_definition", "test") resource.ParallelTest(t, resource.TestCase{ @@ -806,6 +825,54 @@ VALUES `, template, data.RandomInteger, data.RandomInteger) } +func testAzureRMPolicySetDefinition_customWithDefinitionGroups(data acceptance.TestData) string { + template := testAzureRMPolicySetDefinition_template(data) + return fmt.Sprintf(` +%s + +resource "azurerm_policy_set_definition" "test" { + name = "acctestPolSet-%d" + policy_type = "Custom" + display_name = "acctestPolSet-display-%d" + + parameters = < Date: Mon, 16 Nov 2020 11:41:13 +0800 Subject: [PATCH 2/4] Resolve comments --- .../policy_set_definition_data_source.go | 4 +- .../policy/policy_set_definition_resource.go | 20 +++--- .../policy_set_definition_resource_test.go | 63 ++++++++++++++++++- 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/azurerm/internal/services/policy/policy_set_definition_data_source.go b/azurerm/internal/services/policy/policy_set_definition_data_source.go index 3643d12d937d..d854103c2d4e 100644 --- a/azurerm/internal/services/policy/policy_set_definition_data_source.go +++ b/azurerm/internal/services/policy/policy_set_definition_data_source.go @@ -90,7 +90,7 @@ func dataSourceArmPolicySetDefinition() *schema.Resource { Computed: true, }, - "group_names": { + "policy_group_names": { Type: schema.TypeList, Computed: true, Elem: &schema.Schema{ @@ -131,7 +131,7 @@ func dataSourceArmPolicySetDefinition() *schema.Resource { Computed: true, }, - "additional_metadata_id": { + "additional_metadata_resource_id": { Type: schema.TypeString, Computed: true, }, diff --git a/azurerm/internal/services/policy/policy_set_definition_resource.go b/azurerm/internal/services/policy/policy_set_definition_resource.go index 43a28e899f91..37c97580fe97 100644 --- a/azurerm/internal/services/policy/policy_set_definition_resource.go +++ b/azurerm/internal/services/policy/policy_set_definition_resource.go @@ -155,7 +155,7 @@ func resourceArmPolicySetDefinition() *schema.Resource { Computed: true, }, - "group_names": { + "policy_group_names": { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{ @@ -196,7 +196,7 @@ func resourceArmPolicySetDefinition() *schema.Resource { ValidateFunc: validation.StringIsNotEmpty, }, - "additional_metadata_id": { + "additional_metadata_resource_id": { Type: schema.TypeString, Optional: true, ValidateFunc: validation.StringIsNotEmpty, @@ -658,7 +658,7 @@ func expandAzureRMPolicySetDefinitionPolicyDefinitions(input []interface{}) (*[] PolicyDefinitionID: utils.String(v["policy_definition_id"].(string)), Parameters: parameters, PolicyDefinitionReferenceID: utils.String(v["reference_id"].(string)), - GroupNames: utils.ExpandStringSlice(v["group_names"].(*schema.Set).List()), + GroupNames: utils.ExpandStringSlice(v["policy_group_names"].(*schema.Set).List()), }) } @@ -700,7 +700,7 @@ func flattenAzureRMPolicySetDefinitionPolicyDefinitions(input *[]policy.Definiti "parameters": parametersMap, "parameter_values": parameterValues, "reference_id": policyDefinitionReference, - "group_names": utils.FlattenStringSlice(definition.GroupNames), + "policy_group_names": utils.FlattenStringSlice(definition.GroupNames), }) } return result, nil @@ -724,7 +724,7 @@ func expandAzureRMPolicySetDefinitionPolicyGroups(input []interface{}) *[]policy if description := v["description"].(string); description != "" { group.Description = utils.String(description) } - if metadataID := v["additional_metadata_id"].(string); metadataID != "" { + if metadataID := v["additional_metadata_resource_id"].(string); metadataID != "" { group.AdditionalMetadataID = utils.String(metadataID) } result = append(result, group) @@ -762,11 +762,11 @@ func flattenAzureRMPolicySetDefinitionPolicyGroups(input *[]policy.DefinitionGro } result = append(result, map[string]interface{}{ - "name": name, - "display_name": displayName, - "category": category, - "description": description, - "additional_metadata_id": metadataID, + "name": name, + "display_name": displayName, + "category": category, + "description": description, + "additional_metadata_resource_id": metadataID, }) } diff --git a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go index 3859fc350cff..f64bf000800b 100644 --- a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go +++ b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go @@ -231,6 +231,13 @@ func TestAccAzureRMPolicySetDefinition_customWithDefinitionGroups(t *testing.T) ), }, data.ImportStep(), + { + Config: testAzureRMPolicySetDefinition_customWithDefinitionGroupsUpdate(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), }, }) } @@ -855,7 +862,7 @@ PARAMETERS "allowedLocations": {"value": "[parameters('allowedLocations')]"} } VALUES - group_names = ["group-1", "group-2"] + policy_group_names = ["group-1", "group-2"] } policy_definition_group { @@ -873,6 +880,60 @@ VALUES `, template, data.RandomInteger, data.RandomInteger) } +func testAzureRMPolicySetDefinition_customWithDefinitionGroupsUpdate(data acceptance.TestData) string { + template := testAzureRMPolicySetDefinition_template(data) + return fmt.Sprintf(` +%s + +resource "azurerm_policy_set_definition" "test" { + name = "acctestPolSet-%d" + policy_type = "Custom" + display_name = "acctestPolSet-display-%d" + + parameters = < Date: Mon, 16 Nov 2020 15:33:01 +0800 Subject: [PATCH 3/4] Add another step to change things back --- .../policy/tests/policy_set_definition_resource_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go index f64bf000800b..cdae2ec49925 100644 --- a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go +++ b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go @@ -238,6 +238,13 @@ func TestAccAzureRMPolicySetDefinition_customWithDefinitionGroups(t *testing.T) ), }, data.ImportStep(), + { + Config: testAzureRMPolicySetDefinition_customWithDefinitionGroups(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), }, }) } From ca86a5948c35755794d9bdefa488609d9c1c2798 Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Tue, 17 Nov 2020 08:42:43 +0800 Subject: [PATCH 4/4] resolve comment --- .../policy/tests/policy_set_definition_resource_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go index cdae2ec49925..2d10dd0a2c4a 100644 --- a/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go +++ b/azurerm/internal/services/policy/tests/policy_set_definition_resource_test.go @@ -877,7 +877,7 @@ VALUES } policy_definition_group { - name = "group-1" + name = "Group-1" } policy_definition_group { @@ -925,8 +925,8 @@ VALUES } policy_definition_group { - name = "group-1" - display_name = "group-display-1" + name = "Group-1" + display_name = "Group-Display-1" category = "My Access Control" description = "Controls accesses" }