diff --git a/CHANGELOG.md b/CHANGELOG.md index fb071fa63213..50b81f65179e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,15 @@ IMPROVEMENTS: +* `azurerm_cosmosdb_sql_container` - support for composite indexes [GH-8792] * `azurerm_mssql_database` - do not set longterm and shortterm retention policies when using the `DW` SKUs [GH-8899] -* `azurerm_search_service` - add support for `identity` [GH-8907] +* `azurerm_search_service` - support for the `identity` block [GH-8907] +* `azurerm_sql_firewall_rule` - adding validation for the `start_ip_address` and `end_ip_address` fields [GH-8935] + +BUG FIXES: + +* `azurerm_application_gateway` - now supports `ignore_changes` for `ssl_certificate` when using pre-existing certificates [GH-8761] +* `azurerm_policy_set_definition` - Fix updates for `parameters` and `parameter_values` in `policy_definition_reference` blocks [GH-8882] ## 2.32.0 (October 15, 2020) diff --git a/azurerm/internal/services/cosmos/common/indexing_policy.go b/azurerm/internal/services/cosmos/common/indexing_policy.go index 9c55e6cd6cb7..10c3cbd34ffd 100644 --- a/azurerm/internal/services/cosmos/common/indexing_policy.go +++ b/azurerm/internal/services/cosmos/common/indexing_policy.go @@ -44,6 +44,27 @@ func expandAzureRmCosmosDBIndexingPolicyExcludedPaths(input []interface{}) *[]do return &paths } +func expandAzureRmCosmosDBIndexingPolicyCompositeIndexes(input []interface{}) *[][]documentdb.CompositePath { + indexes := make([][]documentdb.CompositePath, 0) + + for _, i := range input { + indexPairs := make([]documentdb.CompositePath, 0) + indexPair := i.(map[string]interface{}) + for _, idxPair := range indexPair["index"].([]interface{}) { + data := idxPair.(map[string]interface{}) + + index := documentdb.CompositePath{ + Path: utils.String(data["path"].(string)), + Order: documentdb.CompositePathSortOrder(data["order"].(string)), + } + indexPairs = append(indexPairs, index) + } + indexes = append(indexes, indexPairs) + } + + return &indexes +} + func ExpandAzureRmCosmosDbIndexingPolicy(d *schema.ResourceData) *documentdb.IndexingPolicy { i := d.Get("indexing_policy").([]interface{}) @@ -60,6 +81,9 @@ func ExpandAzureRmCosmosDbIndexingPolicy(d *schema.ResourceData) *documentdb.Ind policy.ExcludedPaths = expandAzureRmCosmosDBIndexingPolicyExcludedPaths(v) } + if v, ok := input["composite_index"].([]interface{}); ok { + policy.CompositeIndexes = expandAzureRmCosmosDBIndexingPolicyCompositeIndexes(v) + } return policy } @@ -85,6 +109,43 @@ func flattenCosmosDBIndexingPolicyExcludedPaths(input *[]documentdb.ExcludedPath return excludedPaths } +func flattenCosmosDBIndexingPolicyCompositeIndex(input []documentdb.CompositePath) []interface{} { + if input == nil { + return []interface{}{} + } + + indexPairs := make([]interface{}, 0) + for _, v := range input { + path := "" + if v.Path != nil { + path = *v.Path + } + + block := make(map[string]interface{}) + block["path"] = path + block["order"] = string(v.Order) + indexPairs = append(indexPairs, block) + } + + return indexPairs +} + +func flattenCosmosDBIndexingPolicyCompositeIndexes(input *[][]documentdb.CompositePath) []interface{} { + if input == nil { + return []interface{}{} + } + + indexes := make([]interface{}, 0) + + for _, v := range *input { + block := make(map[string][]interface{}) + block["index"] = flattenCosmosDBIndexingPolicyCompositeIndex(v) + indexes = append(indexes, block) + } + + return indexes +} + func flattenCosmosDBIndexingPolicyIncludedPaths(input *[]documentdb.IncludedPath) []interface{} { if input == nil { return nil @@ -111,6 +172,7 @@ func FlattenAzureRmCosmosDbIndexingPolicy(indexingPolicy *documentdb.IndexingPol result["indexing_mode"] = string(indexingPolicy.IndexingMode) result["included_path"] = flattenCosmosDBIndexingPolicyIncludedPaths(indexingPolicy.IncludedPaths) result["excluded_path"] = flattenCosmosDBIndexingPolicyExcludedPaths(indexingPolicy.ExcludedPaths) + result["composite_index"] = flattenCosmosDBIndexingPolicyCompositeIndexes(indexingPolicy.CompositeIndexes) results = append(results, result) return results diff --git a/azurerm/internal/services/cosmos/common/schema.go b/azurerm/internal/services/cosmos/common/schema.go index 9ae33dd1252d..d24208b993bc 100644 --- a/azurerm/internal/services/cosmos/common/schema.go +++ b/azurerm/internal/services/cosmos/common/schema.go @@ -89,6 +89,39 @@ func CosmosDbIndexingPolicySchema() *schema.Schema { }, }, }, + "composite_index": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "index": { + Type: schema.TypeList, + MinItems: 1, + Required: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "path": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringIsNotEmpty, + }, + "order": { + Type: schema.TypeString, + Required: true, + // Workaround for Azure/azure-rest-api-specs#11222 + DiffSuppressFunc: suppress.CaseDifference, + ValidateFunc: validation.StringInSlice( + []string{ + string(documentdb.Ascending), + string(documentdb.Descending), + }, false), + }, + }, + }, + }, + }, + }, + }, }, }, } diff --git a/azurerm/internal/services/cosmos/cosmosdb_sql_container_resource_test.go b/azurerm/internal/services/cosmos/cosmosdb_sql_container_resource_test.go index f00d2008963d..7bc96e4bf0f6 100644 --- a/azurerm/internal/services/cosmos/cosmosdb_sql_container_resource_test.go +++ b/azurerm/internal/services/cosmos/cosmosdb_sql_container_resource_test.go @@ -259,6 +259,27 @@ resource "azurerm_cosmosdb_sql_container" "test" { excluded_path { path = "/testing/id2/*" } + composite_index { + index { + path = "/path1" + order = "Descending" + } + index { + path = "/path2" + order = "Ascending" + } + } + + composite_index { + index { + path = "/path3" + order = "Ascending" + } + index { + path = "/path4" + order = "Descending" + } + } } } `, testAccAzureRMCosmosDbSqlDatabase_basic(data), data.RandomInteger) @@ -293,6 +314,28 @@ resource "azurerm_cosmosdb_sql_container" "test" { excluded_path { path = "/testing/id1/*" } + + composite_index { + index { + path = "/path1" + order = "Ascending" + } + index { + path = "/path2" + order = "Descending" + } + } + + composite_index { + index { + path = "/path3" + order = "Ascending" + } + index { + path = "/path4" + order = "Descending" + } + } } } `, testAccAzureRMCosmosDbSqlDatabase_basic(data), data.RandomInteger) @@ -338,6 +381,28 @@ resource "azurerm_cosmosdb_sql_container" "test" { excluded_path { path = "%s" } + + composite_index { + index { + path = "/path1" + order = "Ascending" + } + index { + path = "/path2" + order = "Descending" + } + } + + composite_index { + index { + path = "/path3" + order = "Ascending" + } + index { + path = "/path4" + order = "Descending" + } + } } } `, testAccAzureRMCosmosDbSqlDatabase_basic(data), data.RandomInteger, includedPath, excludedPath) diff --git a/azurerm/internal/services/network/application_gateway_resource.go b/azurerm/internal/services/network/application_gateway_resource.go index a2558f9cd5bb..c9471651f13a 100644 --- a/azurerm/internal/services/network/application_gateway_resource.go +++ b/azurerm/internal/services/network/application_gateway_resource.go @@ -3243,6 +3243,7 @@ func expandApplicationGatewaySslCertificates(d *schema.ResourceData) (*[]network data := v["data"].(string) password := v["password"].(string) kvsid := v["key_vault_secret_id"].(string) + cert := v["public_cert_data"].(string) output := network.ApplicationGatewaySslCertificate{ Name: utils.String(name), @@ -3263,6 +3264,8 @@ func expandApplicationGatewaySslCertificates(d *schema.ResourceData) (*[]network } output.ApplicationGatewaySslCertificatePropertiesFormat.KeyVaultSecretID = utils.String(kvsid) + } else if cert != "" { + output.ApplicationGatewaySslCertificatePropertiesFormat.PublicCertData = utils.String(cert) } else { return nil, fmt.Errorf("either `key_vault_secret_id` or `data` must be specified for the `ssl_certificate` block %q", name) } diff --git a/azurerm/internal/services/network/tests/application_gateway_resource_test.go b/azurerm/internal/services/network/tests/application_gateway_resource_test.go index 1599c3405447..8fb0c340d571 100644 --- a/azurerm/internal/services/network/tests/application_gateway_resource_test.go +++ b/azurerm/internal/services/network/tests/application_gateway_resource_test.go @@ -1,10 +1,15 @@ package tests import ( + "encoding/base64" "fmt" + "io/ioutil" + "log" "regexp" "testing" + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-05-01/network" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance" @@ -662,6 +667,33 @@ func TestAccAzureRMApplicationGateway_sslCertificate_EmptyPassword(t *testing.T) }) } +func TestAccAzureRMApplicationGateway_manualSslCertificateChangeIgnoreChanges(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_application_gateway", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMApplicationGatewayDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMApplicationGateway_manualSslCertificateChangeIgnoreChangesConfig(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApplicationGatewayExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "ssl_certificate.0.name", "acctestcertificate1"), + testCheckAzureRMApplicationGatewayChangeCert(data.ResourceName, "acctestcertificate2"), + ), + }, + { + Config: testAccAzureRMApplicationGateway_manualSslCertificateChangeIgnoreChangesUpdatedConfig(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMApplicationGatewayExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "ssl_certificate.0.name", "acctestcertificate2"), + ), + }, + }, + }) +} + func TestAccAzureRMApplicationGateway_sslCertificate(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_application_gateway", "test") @@ -4011,6 +4043,225 @@ resource "azurerm_application_gateway" "test" { `, template, data.RandomInteger) } +func testCheckAzureRMApplicationGatewayChangeCert(resourceName, certName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + client := acceptance.AzureProvider.Meta().(*clients.Client).Network.ApplicationGatewaysClient + ctx := acceptance.AzureProvider.Meta().(*clients.Client).StopContext + + // Ensure we have enough information in state to look up in API + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return fmt.Errorf("Not found: %s", resourceName) + } + + gatewayName := rs.Primary.Attributes["name"] + resourceGroup := rs.Primary.Attributes["resource_group_name"] + + agw, err := client.Get(ctx, resourceGroup, gatewayName) + if err != nil { + return fmt.Errorf("Bad: Get on ApplicationGatewaysClient: %+v", err) + } + + certPfx, err := ioutil.ReadFile("testdata/application_gateway_test.pfx") + if err != nil { + log.Fatal(err) + } + certB64 := base64.StdEncoding.EncodeToString(certPfx) + + newSslCertificates := make([]network.ApplicationGatewaySslCertificate, 1) + newSslCertificates[0] = network.ApplicationGatewaySslCertificate{ + Name: utils.String(certName), + Etag: utils.String("*"), + + ApplicationGatewaySslCertificatePropertiesFormat: &network.ApplicationGatewaySslCertificatePropertiesFormat{ + Data: utils.String(certB64), + Password: utils.String("terraform"), + }, + } + + agw.SslCertificates = &newSslCertificates + + future, err := client.CreateOrUpdate(ctx, resourceGroup, gatewayName, agw) + if err != nil { + return fmt.Errorf("Bad: updating AGW: %+v", err) + } + + if err := future.WaitForCompletionRef(ctx, client.Client); err != nil { + return fmt.Errorf("Bad: waiting for update of AGW: %+v", err) + } + + return nil + } +} + +func testAccAzureRMApplicationGateway_manualSslCertificateChangeIgnoreChangesConfig(data acceptance.TestData) string { + template := testAccAzureRMApplicationGateway_template(data) + return fmt.Sprintf(` +%s + +# since these variables are re-used - a locals block makes this more maintainable +locals { + backend_address_pool_name = "${azurerm_virtual_network.test.name}-beap" + frontend_port_name = "${azurerm_virtual_network.test.name}-feport" + frontend_ip_configuration_name = "${azurerm_virtual_network.test.name}-feip" + http_setting_name = "${azurerm_virtual_network.test.name}-be-htst" + listener_name = "${azurerm_virtual_network.test.name}-httplstn" + request_routing_rule_name = "${azurerm_virtual_network.test.name}-rqrt" + ssl_certificate_name = "acctestcertificate1" +} + +resource "azurerm_application_gateway" "test" { + name = "acctesttag" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + + sku { + name = "Standard_Small" + tier = "Standard" + capacity = 2 + } + + gateway_ip_configuration { + name = "my-gateway-ip-configuration" + subnet_id = azurerm_subnet.test.id + } + + frontend_port { + name = local.frontend_port_name + port = 80 + } + + frontend_ip_configuration { + name = local.frontend_ip_configuration_name + public_ip_address_id = azurerm_public_ip.test.id + } + + backend_address_pool { + name = local.backend_address_pool_name + } + + backend_http_settings { + name = local.http_setting_name + cookie_based_affinity = "Disabled" + port = 80 + protocol = "Http" + request_timeout = 1 + } + + http_listener { + name = local.listener_name + frontend_ip_configuration_name = local.frontend_ip_configuration_name + frontend_port_name = local.frontend_port_name + protocol = "Http" + } + + request_routing_rule { + name = local.request_routing_rule_name + rule_type = "Basic" + http_listener_name = local.listener_name + backend_address_pool_name = local.backend_address_pool_name + backend_http_settings_name = local.http_setting_name + } + + ssl_certificate { + name = local.ssl_certificate_name + data = filebase64("testdata/application_gateway_test.pfx") + password = "terraform" + } + + lifecycle { + ignore_changes = [ + ssl_certificate, + ] + } +} +`, template) +} + +func testAccAzureRMApplicationGateway_manualSslCertificateChangeIgnoreChangesUpdatedConfig(data acceptance.TestData) string { + template := testAccAzureRMApplicationGateway_template(data) + return fmt.Sprintf(` +%s + +# since these variables are re-used - a locals block makes this more maintainable +locals { + backend_address_pool_name = "${azurerm_virtual_network.test.name}-beap" + frontend_port_name = "${azurerm_virtual_network.test.name}-feport" + frontend_ip_configuration_name = "${azurerm_virtual_network.test.name}-feip" + http_setting_name = "${azurerm_virtual_network.test.name}-be-htst" + listener_name = "${azurerm_virtual_network.test.name}-httplstn" + request_routing_rule_name = "${azurerm_virtual_network.test.name}-rqrt" + ssl_certificate_name = "acctestcertificate3" +} + +resource "azurerm_application_gateway" "test" { + name = "acctesttag" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + + sku { + name = "Standard_Small" + tier = "Standard" + capacity = 2 + } + + gateway_ip_configuration { + name = "my-gateway-ip-configuration" + subnet_id = azurerm_subnet.test.id + } + + frontend_port { + name = local.frontend_port_name + port = 80 + } + + frontend_ip_configuration { + name = local.frontend_ip_configuration_name + public_ip_address_id = azurerm_public_ip.test.id + } + + backend_address_pool { + name = local.backend_address_pool_name + } + + backend_http_settings { + name = local.http_setting_name + cookie_based_affinity = "Disabled" + port = 80 + protocol = "Http" + request_timeout = 1 + } + + http_listener { + name = local.listener_name + frontend_ip_configuration_name = local.frontend_ip_configuration_name + frontend_port_name = local.frontend_port_name + protocol = "Http" + } + + request_routing_rule { + name = local.request_routing_rule_name + rule_type = "Basic" + http_listener_name = local.listener_name + backend_address_pool_name = local.backend_address_pool_name + backend_http_settings_name = local.http_setting_name + } + + ssl_certificate { + name = local.ssl_certificate_name + data = filebase64("testdata/application_gateway_test.pfx") + password = "terraform" + } + + lifecycle { + ignore_changes = [ + ssl_certificate, + ] + } +} +`, template) +} + func testAccAzureRMApplicationGateway_webApplicationFirewall(data acceptance.TestData) string { template := testAccAzureRMApplicationGateway_template(data) return fmt.Sprintf(` diff --git a/azurerm/internal/services/policy/policy_set_definition_resource.go b/azurerm/internal/services/policy/policy_set_definition_resource.go index c12abe91d626..602f846e9491 100644 --- a/azurerm/internal/services/policy/policy_set_definition_resource.go +++ b/azurerm/internal/services/policy/policy_set_definition_resource.go @@ -26,8 +26,8 @@ import ( func resourceArmPolicySetDefinition() *schema.Resource { return &schema.Resource{ - Create: resourceArmPolicySetDefinitionCreateUpdate, - Update: resourceArmPolicySetDefinitionCreateUpdate, + Create: resourceArmPolicySetDefinitionCreate, + Update: resourceArmPolicySetDefinitionUpdate, Read: resourceArmPolicySetDefinitionRead, Delete: resourceArmPolicySetDefinitionDelete, @@ -205,9 +205,9 @@ type DefinitionReferenceInOldApiVersion struct { Parameters map[string]*policy.ParameterValuesValue `json:"parameters"` } -func resourceArmPolicySetDefinitionCreateUpdate(d *schema.ResourceData, meta interface{}) error { +func resourceArmPolicySetDefinitionCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Policy.SetDefinitionsClient - ctx, cancel := timeouts.ForCreateUpdate(meta.(*clients.Client).StopContext, d) + ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() name := d.Get("name").(string) @@ -219,17 +219,15 @@ func resourceArmPolicySetDefinitionCreateUpdate(d *schema.ResourceData, meta int managementGroupName = v.(string) } - if d.IsNewResource() { - existing, err := getPolicySetDefinitionByName(ctx, client, name, managementGroupName) - if err != nil { - if !utils.ResponseWasNotFound(existing.Response) { - return fmt.Errorf("checking for presence of existing Policy Set Definition %q: %+v", name, err) - } + existing, err := getPolicySetDefinitionByName(ctx, client, name, managementGroupName) + if err != nil { + if !utils.ResponseWasNotFound(existing.Response) { + return fmt.Errorf("checking for presence of existing Policy Set Definition %q: %+v", name, err) } + } - if existing.ID != nil && *existing.ID != "" { - return tf.ImportAsExistsError("azurerm_policy_set_definition", *existing.ID) - } + if existing.ID != nil && *existing.ID != "" { + return tf.ImportAsExistsError("azurerm_policy_set_definition", *existing.ID) } properties := policy.SetDefinitionProperties{ @@ -274,7 +272,6 @@ func resourceArmPolicySetDefinitionCreateUpdate(d *schema.ResourceData, meta int SetDefinitionProperties: &properties, } - var err error if managementGroupName == "" { _, err = client.CreateOrUpdate(ctx, name, definition) } else { @@ -282,7 +279,7 @@ func resourceArmPolicySetDefinitionCreateUpdate(d *schema.ResourceData, meta int } if err != nil { - return fmt.Errorf("creating/updating Policy Set Definition %q: %+v", name, err) + return fmt.Errorf("creating Policy Set Definition %q: %+v", name, err) } // Policy Definitions are eventually consistent; wait for them to stabilize @@ -316,6 +313,106 @@ func resourceArmPolicySetDefinitionCreateUpdate(d *schema.ResourceData, meta int return resourceArmPolicySetDefinitionRead(d, meta) } +func resourceArmPolicySetDefinitionUpdate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*clients.Client).Policy.SetDefinitionsClient + ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) + defer cancel() + + id, err := parse.PolicySetDefinitionID(d.Id()) + if err != nil { + return err + } + + managementGroupName := "" + if scopeId, ok := id.PolicyScopeId.(parse.ScopeAtManagementGroup); ok { + managementGroupName = scopeId.ManagementGroupName + } + + // retrieve + existing, err := getPolicySetDefinitionByName(ctx, client, id.Name, managementGroupName) + if err != nil { + return fmt.Errorf("retrieving Policy Set Definition %q (Scope %q): %+v", id.Name, id.ScopeId(), err) + } + if existing.SetDefinitionProperties == nil { + return fmt.Errorf("retrieving Policy Set Definition %q (Scope %q): `properties` was nil", id.Name, id.ScopeId()) + } + + if d.HasChange("policy_type") { + existing.SetDefinitionProperties.PolicyType = policy.Type(d.Get("policy_type").(string)) + } + + if d.HasChange("display_name") { + existing.SetDefinitionProperties.DisplayName = utils.String(d.Get("display_name").(string)) + } + + if d.HasChange("description") { + existing.SetDefinitionProperties.Description = utils.String(d.Get("description").(string)) + } + + if d.HasChange("metadata") { + metaDataString := d.Get("metadata").(string) + if metaDataString != "" { + metaData, err := structure.ExpandJsonFromString(metaDataString) + if err != nil { + return fmt.Errorf("expanding JSON for `metadata`: %+v", err) + } + existing.SetDefinitionProperties.Metadata = metaData + } else { + existing.SetDefinitionProperties.Metadata = nil + } + } + + if d.HasChange("parameters") { + parametersString := d.Get("parameters").(string) + if parametersString != "" { + parameters, err := expandParameterDefinitionsValueFromString(parametersString) + if err != nil { + return fmt.Errorf("expanding JSON for `parameters`: %+v", err) + } + existing.SetDefinitionProperties.Parameters = parameters + } else { + existing.SetDefinitionProperties.Parameters = nil + } + } + + if d.HasChange("policy_definitions") { + var policyDefinitions []policy.DefinitionReference + err := json.Unmarshal([]byte(d.Get("policy_definitions").(string)), &policyDefinitions) + if err != nil { + return fmt.Errorf("expanding JSON for `policy_definitions`: %+v", err) + } + existing.SetDefinitionProperties.PolicyDefinitions = &policyDefinitions + } + + if d.HasChange("policy_definition_reference") { + definitions, err := expandAzureRMPolicySetDefinitionPolicyDefinitionsUpdate(d) + if err != nil { + return fmt.Errorf("expanding `policy_definition_reference`: %+v", err) + } + existing.SetDefinitionProperties.PolicyDefinitions = definitions + } + + if managementGroupName == "" { + _, err = client.CreateOrUpdate(ctx, id.Name, existing) + } else { + _, err = client.CreateOrUpdateAtManagementGroup(ctx, id.Name, existing, managementGroupName) + } + + if err != nil { + return fmt.Errorf("updating Policy Set Definition %q: %+v", id.Name, err) + } + + var resp policy.SetDefinition + resp, err = getPolicySetDefinitionByName(ctx, client, id.Name, managementGroupName) + if err != nil { + return fmt.Errorf("retrieving Policy Set Definition %q: %+v", id.Name, err) + } + + d.SetId(*resp.ID) + + return resourceArmPolicySetDefinitionRead(d, meta) +} + func resourceArmPolicySetDefinitionRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*clients.Client).Policy.SetDefinitionsClient ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) @@ -327,8 +424,7 @@ func resourceArmPolicySetDefinitionRead(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 } @@ -437,6 +533,44 @@ func policySetDefinitionRefreshFunc(ctx context.Context, client *policy.SetDefin } } +func expandAzureRMPolicySetDefinitionPolicyDefinitionsUpdate(d *schema.ResourceData) (*[]policy.DefinitionReference, error) { + result := make([]policy.DefinitionReference, 0) + input := d.Get("policy_definition_reference").([]interface{}) + + for i := range input { + if d.HasChange(fmt.Sprintf("policy_definition_reference.%d.parameter_values", i)) && d.HasChange(fmt.Sprintf("policy_definition_reference.%d.parameters", i)) { + return nil, fmt.Errorf("cannot set both `parameters` and `parameter_values`") + } + parameters := make(map[string]*policy.ParameterValuesValue) + if d.HasChange(fmt.Sprintf("policy_definition_reference.%d.parameters", i)) { + // there is change in `parameters` - the user is will to use this attribute as parameter values + log.Printf("[DEBUG] updating %s", fmt.Sprintf("policy_definition_reference.%d.parameters", i)) + p := d.Get(fmt.Sprintf("policy_definition_reference.%d.parameters", i)).(map[string]interface{}) + for k, v := range p { + parameters[k] = &policy.ParameterValuesValue{ + Value: v, + } + } + } else { + // in this case, it is either parameter_values updated or no update on both, we took the value in `parameter_values` as the final value + log.Printf("[DEBUG] updating %s", fmt.Sprintf("policy_definition_reference.%d.parameter_values", i)) + if p, ok := d.Get(fmt.Sprintf("policy_definition_reference.%d.parameter_values", i)).(string); ok && p != "" { + if err := json.Unmarshal([]byte(p), ¶meters); err != nil { + return nil, fmt.Errorf("unmarshalling `parameter_values`: %+v", err) + } + } + } + + result = append(result, policy.DefinitionReference{ + PolicyDefinitionID: utils.String(d.Get(fmt.Sprintf("policy_definition_reference.%d.policy_definition_id", i)).(string)), + Parameters: parameters, + PolicyDefinitionReferenceID: utils.String(d.Get(fmt.Sprintf("policy_definition_reference.%d.reference_id", i)).(string)), + }) + } + + return &result, nil +} + func expandAzureRMPolicySetDefinitionPolicyDefinitions(input []interface{}) (*[]policy.DefinitionReference, error) { result := make([]policy.DefinitionReference, 0) @@ -449,12 +583,11 @@ func expandAzureRMPolicySetDefinitionPolicyDefinitions(input []interface{}) (*[] return nil, fmt.Errorf("unmarshalling `parameter_values`: %+v", err) } } - if p, ok := v["parameters"]; ok { - m := p.(map[string]interface{}) - if len(parameters) > 0 && len(m) > 0 { + if p, ok := v["parameters"].(map[string]interface{}); ok { + if len(parameters) > 0 && len(p) > 0 { return nil, fmt.Errorf("cannot set both `parameters` and `parameter_values`") } - for k, value := range p.(map[string]interface{}) { + for k, value := range p { parameters[k] = &policy.ParameterValuesValue{ Value: value, } 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 ef611e109dfb..dcb7d8af4d62 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 @@ -121,6 +121,82 @@ func TestAccAzureRMPolicySetDefinition_customNoParameter(t *testing.T) { }) } +func TestAccAzureRMPolicySetDefinition_customUpdateDisplayName(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_custom(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMPolicySetDefinition_customUpdateDisplayName(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), + }, + }) +} + +func TestAccAzureRMPolicySetDefinition_customUpdateParameters(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_custom(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMPolicySetDefinition_customUpdateParameters(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + }, + }) +} + +func TestAccAzureRMPolicySetDefinition_customUpdateAddNewReference(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_custom(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + data.ImportStep(), + { + Config: testAccAzureRMPolicySetDefinition_customUpdateAddNewReference(data), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMPolicySetDefinitionExists(data.ResourceName), + ), + }, + }, + }) +} + func TestAccAzureRMPolicySetDefinition_customWithPolicyReferenceID(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_policy_set_definition", "test") @@ -385,6 +461,124 @@ VALUES `, template, data.RandomInteger, data.RandomInteger) } +func testAccAzureRMPolicySetDefinition_customUpdateDisplayName(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-updated" + + parameters = < **NOTE:** This value is required when the specified Policy Definition contains the `parameters` field. diff --git a/website/docs/r/policy_definition.html.markdown b/website/docs/r/policy_definition.html.markdown index 1b052a7b9a70..1844c14988e5 100644 --- a/website/docs/r/policy_definition.html.markdown +++ b/website/docs/r/policy_definition.html.markdown @@ -83,15 +83,15 @@ The following arguments are supported: ~> **Note:** if you are using `azurerm_management_group` to assign a value to `management_group_id`, be sure to use `name` or `group_id` attribute, but not `id`. * `policy_rule` - (Optional) The policy rule for the policy definition. This - is a json object representing the rule that contains an if and + is a JSON string representing the rule that contains an if and a then block. * `metadata` - (Optional) The metadata for the policy definition. This - is a json object representing additional metadata that should be stored + is a JSON string representing additional metadata that should be stored with the policy definition. * `parameters` - (Optional) Parameters for the policy definition. This field - is a json object that allows you to parameterize your policy definition. + is a JSON string that allows you to parameterize your policy definition. ## Attributes Reference diff --git a/website/docs/r/policy_set_definition.html.markdown b/website/docs/r/policy_set_definition.html.markdown index e88f022d3bc4..2c3ba32da771 100644 --- a/website/docs/r/policy_set_definition.html.markdown +++ b/website/docs/r/policy_set_definition.html.markdown @@ -76,7 +76,7 @@ A `policy_definition_reference` block supports the following: * `policy_definition_id` - (Required) The ID of the policy definition or policy set definition that will be included in this policy set definition. -* `parameter_values` - (Optional) Parameter values for the referenced policy rule. This field is a json object that allows you to assign parameters to this policy rule. +* `parameter_values` - (Optional) Parameter values for the referenced policy rule. This field is a JSON string that allows you to assign parameters to this policy rule. * `reference_id` - (Optional) A unique ID within this policy set definition for this policy definition reference.