From c1026fff84f6580fc6dee63e128798cfe23bf26a Mon Sep 17 00:00:00 2001 From: Dapeng Zhang Date: Fri, 27 Mar 2020 12:55:28 +0800 Subject: [PATCH] Make name as a possible input variable of data source azurerm_policy_definition --- .../policy/data_source_policy_definition.go | 83 +++++++++++-------- .../data_source_policy_definition_test.go | 71 ++++++++++++---- .../docs/d/policy_definition.html.markdown | 14 +++- .../docs/r/policy_definition.html.markdown | 3 +- 4 files changed, 115 insertions(+), 56 deletions(-) diff --git a/azurerm/internal/services/policy/data_source_policy_definition.go b/azurerm/internal/services/policy/data_source_policy_definition.go index 4a23564024cae..1756564349b6c 100644 --- a/azurerm/internal/services/policy/data_source_policy_definition.go +++ b/azurerm/internal/services/policy/data_source_policy_definition.go @@ -1,13 +1,13 @@ package policy import ( + "context" "fmt" "time" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/policy" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts" ) @@ -23,17 +23,22 @@ func dataSourceArmPolicyDefinition() *schema.Resource { Schema: map[string]*schema.Schema{ "display_name": { Type: schema.TypeString, - Required: true, + Optional: true, + Computed: true, ValidateFunc: validation.StringIsNotEmpty, + ExactlyOneOf: []string{"name", "display_name"}, }, - "management_group_id": { + "name": { Type: schema.TypeString, Optional: true, - ValidateFunc: azure.ValidateResourceIDOrEmpty, + Computed: true, + ValidateFunc: validation.StringIsNotEmpty, + ExactlyOneOf: []string{"name", "display_name"}, }, - "name": { + "management_group_id": { Type: schema.TypeString, - Computed: true, + Optional: true, + // TODO -- temporary removed this validation, since a management group ID is always not a resource ID. add this back when there is a proper function for validation of mgmt group IDs }, "type": { Type: schema.TypeString, @@ -68,39 +73,24 @@ func dataSourceArmPolicyDefinitionRead(d *schema.ResourceData, meta interface{}) ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) defer cancel() - name := d.Get("display_name").(string) + displayName := d.Get("display_name").(string) + name := d.Get("name").(string) managementGroupID := d.Get("management_group_id").(string) - var policyDefinitions policy.DefinitionListResultIterator - var err error - - if managementGroupID != "" { - policyDefinitions, err = client.ListByManagementGroupComplete(ctx, managementGroupID) - } else { - policyDefinitions, err = client.ListComplete(ctx) - } - - if err != nil { - return fmt.Errorf("Error loading Policy Definition List: %+v", err) - } - var policyDefinition policy.Definition - - for policyDefinitions.NotDone() { - def := policyDefinitions.Value() - if def.DisplayName != nil && *def.DisplayName == name { - policyDefinition = def - break + var err error + if displayName != "" { + policyDefinition, err = getPolicyDefinitionByDisplayName(ctx, client, displayName, managementGroupID) + if err != nil { + return fmt.Errorf("failed to read Policy Definition (Display Name %q): %+v", displayName, err) } - - err = policyDefinitions.NextWithContext(ctx) + } else if name != "" { + policyDefinition, err = getPolicyDefinition(ctx, client, name, managementGroupID) if err != nil { - return fmt.Errorf("Error loading Policy Definition List: %s", err) + return fmt.Errorf("failed to read Policy Definition %q: %+v", name, err) } - } - - if policyDefinition.ID == nil { - return fmt.Errorf("Error loading Policy Definition List: could not find policy '%s'", name) + } else { + return fmt.Errorf("one of `display_name` or `name` must be set") } d.SetId(*policyDefinition.ID) @@ -124,3 +114,30 @@ func dataSourceArmPolicyDefinitionRead(d *schema.ResourceData, meta interface{}) return nil } + +func getPolicyDefinitionByDisplayName(ctx context.Context, client *policy.DefinitionsClient, displayName, managementGroupID string) (policy.Definition, error) { + var policyDefinitions policy.DefinitionListResultIterator + var err error + + if managementGroupID != "" { + policyDefinitions, err = client.ListByManagementGroupComplete(ctx, managementGroupID) + } else { + policyDefinitions, err = client.ListComplete(ctx) + } + if err != nil { + return policy.Definition{}, fmt.Errorf("failed to load Policy Definition List: %+v", err) + } + + for policyDefinitions.NotDone() { + def := policyDefinitions.Value() + if def.DisplayName != nil && *def.DisplayName == displayName && def.ID != nil { + return def, nil + } + + if err := policyDefinitions.NextWithContext(ctx); err != nil { + return policy.Definition{}, fmt.Errorf("failed to load Policy Definition List: %s", err) + } + } + + return policy.Definition{}, fmt.Errorf("failed to load Policy Definition List: could not find policy '%s'", displayName) +} diff --git a/azurerm/internal/services/policy/tests/data_source_policy_definition_test.go b/azurerm/internal/services/policy/tests/data_source_policy_definition_test.go index ca1b76bfcb168..4676b1f6c93f4 100644 --- a/azurerm/internal/services/policy/tests/data_source_policy_definition_test.go +++ b/azurerm/internal/services/policy/tests/data_source_policy_definition_test.go @@ -10,6 +10,7 @@ import ( func TestAccDataSourceAzureRMPolicyDefinition_builtIn(t *testing.T) { data := acceptance.BuildTestData(t, "data.azurerm_policy_definition", "test") + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, @@ -30,6 +31,7 @@ func TestAccDataSourceAzureRMPolicyDefinition_builtIn(t *testing.T) { func TestAccDataSourceAzureRMPolicyDefinition_builtIn_AtManagementGroup(t *testing.T) { data := acceptance.BuildTestData(t, "data.azurerm_policy_definition", "test") + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, @@ -44,17 +46,41 @@ func TestAccDataSourceAzureRMPolicyDefinition_builtIn_AtManagementGroup(t *testi }) } -func TestAccDataSourceAzureRMPolicyDefinition_custom(t *testing.T) { +func TestAccDataSourceAzureRMPolicyDefinition_customByDisplayName(t *testing.T) { data := acceptance.BuildTestData(t, "data.azurerm_policy_definition", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceAzureRMPolicyDefinition_customByDisplayName(data), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(data.ResourceName, "name", fmt.Sprintf("acctestpol-%d", data.RandomInteger)), + resource.TestCheckResourceAttr(data.ResourceName, "display_name", fmt.Sprintf("acctestpol-display-%d", data.RandomInteger)), + resource.TestCheckResourceAttr(data.ResourceName, "type", "Microsoft.Authorization/policyDefinitions"), + resource.TestCheckResourceAttr(data.ResourceName, "policy_type", "Custom"), + resource.TestCheckResourceAttr(data.ResourceName, "policy_rule", "{\"if\":{\"not\":{\"field\":\"location\",\"in\":\"[parameters('allowedLocations')]\"}},\"then\":{\"effect\":\"audit\"}}"), + resource.TestCheckResourceAttr(data.ResourceName, "parameters", "{\"allowedLocations\":{\"metadata\":{\"description\":\"The list of allowed locations for resources.\",\"displayName\":\"Allowed locations\",\"strongType\":\"location\"},\"type\":\"Array\"}}"), + resource.TestCheckResourceAttrSet(data.ResourceName, "metadata"), + ), + }, + }, + }) +} + +func TestAccDataSourceAzureRMPolicyDefinition_customByName(t *testing.T) { + data := acceptance.BuildTestData(t, "data.azurerm_policy_definition", "test") + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acceptance.PreCheck(t) }, Providers: acceptance.SupportedProviders, Steps: []resource.TestStep{ { - Config: testAccDataSourceCustomPolicyDefinition(data), + Config: testAccDataSourceAzureRMPolicyDefinition_customByName(data), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(data.ResourceName, "name", fmt.Sprintf("acctestpol-%d", data.RandomInteger)), - resource.TestCheckResourceAttr(data.ResourceName, "display_name", fmt.Sprintf("acctestpol-%d", data.RandomInteger)), + resource.TestCheckResourceAttr(data.ResourceName, "display_name", fmt.Sprintf("acctestpol-display-%d", data.RandomInteger)), resource.TestCheckResourceAttr(data.ResourceName, "type", "Microsoft.Authorization/policyDefinitions"), resource.TestCheckResourceAttr(data.ResourceName, "policy_type", "Custom"), resource.TestCheckResourceAttr(data.ResourceName, "policy_rule", "{\"if\":{\"not\":{\"field\":\"location\",\"in\":\"[parameters('allowedLocations')]\"}},\"then\":{\"effect\":\"audit\"}}"), @@ -94,7 +120,29 @@ data "azurerm_policy_definition" "test" { `, name) } -func testAccDataSourceCustomPolicyDefinition(data acceptance.TestData) string { +func testAccDataSourceAzureRMPolicyDefinition_customByDisplayName(data acceptance.TestData) string { + template := testAccDataSourceAzureRMPolicyDefinition_template(data) + return fmt.Sprintf(` +%s + +data "azurerm_policy_definition" "test" { + display_name = azurerm_policy_definition.test_policy.display_name +} +`, template) +} + +func testAccDataSourceAzureRMPolicyDefinition_customByName(data acceptance.TestData) string { + template := testAccDataSourceAzureRMPolicyDefinition_template(data) + return fmt.Sprintf(` +%s + +data "azurerm_policy_definition" "test" { + name = azurerm_policy_definition.test_policy.name +} +`, template) +} + +func testAccDataSourceAzureRMPolicyDefinition_template(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { features {} @@ -104,7 +152,7 @@ resource "azurerm_policy_definition" "test_policy" { name = "acctestpol-%d" policy_type = "Custom" mode = "All" - display_name = "acctestpol-%d" + display_name = "acctestpol-display-%d" policy_rule = <