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 issues introduced by #8270 #8663

Closed
krowlandson opened this issue Sep 28, 2020 · 10 comments · Fixed by #8668
Closed

New issues introduced by #8270 #8663

krowlandson opened this issue Sep 28, 2020 · 10 comments · Fixed by #8668

Comments

@krowlandson
Copy link
Contributor

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureRM Provider) Version

Terraform v0.13.3
+ provider registry.terraform.io/hashicorp/azurerm v2.29.0

Affected Resource(s)

  • azurerm_policy_set_definition

Terraform Configuration Files

The code below is used to create multiple Policy Set Definitions from a list object local.es_policy_set_definitions_by_management_group. This list is generated dynamically 'per Management Group' based on a map of Policy Set Definitions to assign per Management Group. The Policy Set Definitions are written in native ARM and stored as .json files within our module, and loaded into Terraform using the file() and jsondecode() functions.

A typical local.es_policy_set_definitions_by_management_group object looks like this:

[
  {
    resource_id = <calculated_resource_id_of_resource_to_create>
    scope_id    = <resource_id_of_the_target_management_group>
    template    = <hcl_object_representation_of_policy_set_definition_from_template_file>
  }
]

The code used to deploy the Policy Set Definitions is as follows:

resource "azurerm_policy_set_definition" "enterprise_scale" {
  for_each = {
    for policy_set in local.es_policy_set_definitions_by_management_group :
    policy_set.resource_id => policy_set
  }

  # Mandatory resource attributes
  name         = each.value.template.displayName
  policy_type  = "Custom"
  display_name = each.value.template.displayName

  # Dynamic configuration blocks
  dynamic "policy_definition_reference" {
    for_each = [
      for item in each.value.template.policyDefinitions :
      {
        policyDefinitionId          = item.policyDefinitionId
        parameters                  = item.parameters
        policyDefinitionReferenceId = item.policyDefinitionReferenceId
      }
      if try(length(each.value.template.policyDefinitions) > 0, false)
    ]
    content {
      policy_definition_id = policy_definition_reference.value["policyDefinitionId"]
      parameter_values     = try(length(policy_definition_reference.value["parameters"]) > 0, false) ? jsonencode(policy_definition_reference.value["parameters"]) : jsonencode(local.empty_map)
      reference_id         = try(length(policy_definition_reference.value["policyDefinitionReferenceId"]) > 0, false) ? policy_definition_reference.value["policyDefinitionReferenceId"] : policy_definition_reference.value["policyDefinitionId"]
    }
  }

  # Optional resource attributes
  description           = try(length(each.value.template.description) > 0, false) ? each.value.template.description : "${each.value.template.displayName} Policy Set Definition at scope ${each.value.scope_id}"
  management_group_name = try(length(each.value.scope_id) > 0, false) ? basename(each.value.scope_id) : null
  metadata              = try(length(each.value.template.metadata) > 0, false) ? jsonencode(each.value.template.metadata) : local.empty_string
  parameters            = try(length(each.value.template.parameters) > 0, false) ? jsonencode(each.value.template.parameters) : local.empty_string

  # Set explicit dependency on Policy Definition deployments
  depends_on = [
    azurerm_policy_definition.enterprise_scale,
  ]

}

Much of the logic is to handle situations where a source Policy Set Definition template may not contain certain optional values, and we use a dynamic block to generate the policy_definition_reference block based on the template content.

Following the updates released in v2.29.0 we have discovered a couple of issues which appear to be related as described below.

Issue Description

Below is an extract of our code before #8270:

  # Dynamic configuration blocks
  dynamic "policy_definition_reference" {
    for_each = [
      for item in each.value.template.policyDefinitions :
      {
        policyDefinitionId          = item.policyDefinitionId
        parameters                  = item.parameters
        policyDefinitionReferenceId = item.policyDefinitionReferenceId
      }
      if try(length(each.value.template.policyDefinitions) > 0, false)
    ]
    content {
      policy_definition_id = policy_definition_reference.value["policyDefinitionId"]
      parameters = {
        for parameter_id, parameter_value in policy_definition_reference.value["parameters"] :
        parameter_id => parameter_value.value
      }
      reference_id = try(length(policy_definition_reference.value["policyDefinitionReferenceId"]) > 0, false) ? policy_definition_reference.value["policyDefinitionReferenceId"] : policy_definition_reference.value["policyDefinitionId"]
    }
  }

Upon release of v2.29.0 we started to receive the following error:

Error: expanding `policy_definition_reference`: unmarshalling `parameter_values`: unexpected end of JSON input

  on modules/enterprise_scale/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {


Error: expanding `policy_definition_reference`: unmarshalling `parameter_values`: unexpected end of JSON input

  on modules/enterprise_scale/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {


Error: expanding `policy_definition_reference`: unmarshalling `parameter_values`: unexpected end of JSON input

  on modules/enterprise_scale/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {

We were able to narrow this down to the fact that since the changes introduced in #8270, the provider appears to no longer accept null for optional parameter values. This usually isn't an issue and is a pattern we use in multiple places within our code.

To fix this, we moved to the newly recommended parameter_values format which is actually beneficial to us as it simplifies the code as follows:

  # Dynamic configuration blocks
  dynamic "policy_definition_reference" {
    for_each = [
      for item in each.value.template.policyDefinitions :
      {
        policyDefinitionId          = item.policyDefinitionId
        parameters                  = item.parameters
        policyDefinitionReferenceId = item.policyDefinitionReferenceId
      }
      if try(length(each.value.template.policyDefinitions) > 0, false)
    ]
    content {
      policy_definition_id = policy_definition_reference.value["policyDefinitionId"]
      parameter_values     = try(length(policy_definition_reference.value["parameters"]) > 0, false) ? jsonencode(policy_definition_reference.value["parameters"]) : jsonencode(local.empty_map)
      reference_id         = try(length(policy_definition_reference.value["policyDefinitionReferenceId"]) > 0, false) ? policy_definition_reference.value["policyDefinitionReferenceId"] : policy_definition_reference.value["policyDefinitionId"]
    }
  }

When no parameter value is provided in the original ARM template, we simply replace is with an empty JSON string "{}" which is generated using the syntax jsonencode(local.empty_map), where locals { empty_map = {} }.

This initially appears to resolve the issue we were facing, however we've observed the following issues.

Issue 1

When trying to run terraform apply for the first time after the code update, Terraform wanted to update our Policy Set Definitions, but upon doing so we were faced with the following error:

Error: expanding `policy_definition_reference`: cannot set both `parameters` and `parameter_values`

  on .terraform/modules/enterprise_scale/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {

To work around this, we had to use terraform taint to force redeployment, at which point the deployment completed successfully.

Issue 2

Having got past the above issue, every time we try to run terraform apply we get prompted to update any Policy Set Definitions where no parameters are defined, or "parameters": {} and when apply is confirmed, we repeatedly get the same error:

Error: expanding `policy_definition_reference`: cannot set both `parameters` and `parameter_values`

  on .terraform/modules/enterprise_scale/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {

Debug Output

Extract of debug at error point:

2020-09-28T19:24:24.804Z [DEBUG] plugin.terraform-provider-azurerm_v2.29.0_x5:
Id":"8251035221074005943","policyDefinitionId":"/providers/Microsoft.Management/managementGroups/tf/providers/Microsoft.Authorization/policyDefinitions/ES-Deny-PublicEndpoint-Aks","parameters":{}}]},"id":"/providers/Microsoft.Management/managementgroups/tf/providers/Microsoft.Authorization/policySetDefinitions/ES-Deny-Public-Endpoints-for-PaaS-Services","type":"Microsoft.Authorization/policySetDefinitions","name":"ES-Deny-Public-Endpoints-for-PaaS-Services"}
2020/09/28 19:24:24 [WARN] Provider "registry.terraform.io/hashicorp/azurerm" produced an unexpected new value for module.enterprise_scale.azurerm_policy_set_definition.enterprise_scale["/providers/Microsoft.Management/managementGroups/tf/providers/Microsoft.Authorization/policySetDefinitions/ES-Deny-Public-Endpoints-for-PaaS-Services"], but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .metadata: was cty.StringVal("{\"createdBy\":\"78665f88-b16d-473b-a073-9aaa5b6f6af3\",\"createdOn\":\"2020-09-28T12:06:47.1045376Z\",\"updatedBy\":\"78665f88-b16d-473b-a073-9aaa5b6f6af3\",\"updatedOn\":\"2020-09-28T19:17:57.8070067Z\"}"), but now cty.StringVal("{\"createdBy\":\"78665f88-b16d-473b-a073-9aaa5b6f6af3\",\"createdOn\":\"2020-09-28T12:06:47.1045376Z\",\"updatedBy\":\"78665f88-b16d-473b-a073-9aaa5b6f6af3\",\"updatedOn\":\"2020-09-28T19:22:53.6514379Z\"}")
      - .policy_definition_reference[0].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[1].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[2].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[3].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[4].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[5].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[6].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")
      - .policy_definition_reference[7].parameter_values: was cty.StringVal("{}"), but now cty.StringVal("")

If you can access our Terraform Cloud logs, the latest failed run with debug enabled is under ID run-oLJ9fvae2xdzu4dt.

I previously tested using just an empty string "" as the default value, but this didn't work either.

Panic Output

None.

Expected Behavior

Should be able to run terraform apply multiple times and not have the apply update resources which haven't fundamentally changed, and complete without errors.

Actual Behavior

As described above.

Steps to Reproduce

  1. terraform apply

Important Factoids

We are running this in standard public Azure, and are using Terraform Cloud for the remote backend (including deployment).

References

  • #0000
@ArcturusZhang
Copy link
Contributor

Hi @krowlandson thanks for this issue!

This is very similar to this issue therefore I have made some similar fix for this issue: #8668

@tombuildsstuff tombuildsstuff added this to the v2.30.0 milestone Sep 30, 2020
@jackofallops jackofallops modified the milestones: v2.30.0, v2.31.0 Oct 1, 2020
@krowlandson
Copy link
Contributor Author

Just to add some more information for this as I see it didn't make the latest release.

If I update my code to use an empty string "" when no parameters are specified (see code snippet below), I am then able to correctly run terraform apply multiple times on an existing deployment without any changes being detected:

  # Dynamic configuration blocks
  dynamic "policy_definition_reference" {
    for_each = [
      for item in each.value.template.policyDefinitions :
      {
        policyDefinitionId          = item.policyDefinitionId
        parameters                  = item.parameters
        policyDefinitionReferenceId = item.policyDefinitionReferenceId
      }
      if try(length(each.value.template.policyDefinitions) > 0, false)
    ]
    content {
      policy_definition_id = policy_definition_reference.value["policyDefinitionId"]
      parameter_values     = try(length(policy_definition_reference.value["parameters"]) > 0, false) ? jsonencode(policy_definition_reference.value["parameters"]) : ""
      reference_id         = try(length(policy_definition_reference.value["policyDefinitionReferenceId"]) > 0, false) ? policy_definition_reference.value["policyDefinitionReferenceId"] : policy_definition_reference.value["policyDefinitionId"]
    }
  }

Unfortunately, if I then destroy the azurerm_policy_set_definition resources and try to recreate them, terraform apply then throws the error below which is what I introduced the default value of "{}" to fix:

Error: expanding `policy_definition_reference`: unmarshalling `parameter_values`: unexpected end of JSON input

  on .terraform/modules/enterprise_scale/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {

Not sure whether this makes any difference to the fix, but thought might be worth sharing.

jackofallops added a commit that referenced this issue Oct 8, 2020
Co-authored-by: jackofallops <ste@hashicorp.com>
@ghost
Copy link

ghost commented Oct 8, 2020

This has been released in version 2.31.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.31.0"
}
# ... other configuration ...

@krowlandson
Copy link
Contributor Author

Thank you for this fix. I've been running a few more checks following this update, and we still got the following with our original code when running terraform apply more than once (i.e. after the Policy Set Definitions are initially created):

During terraform plan:

      ~ policy_definition_reference {
          + parameter_values     = jsonencode({})
            parameters           = {}
            policy_definition_id = "/providers/Microsoft.Management/managementGroups/tf/providers/Microsoft.Authorization/policyDefinitions/ES-Deploy-Sql-Tde"
            reference_id         = "ES-Deploy-Sql-Tde"
        }
      ~ policy_definition_reference {
          + parameter_values     = jsonencode({})
            parameters           = {}
            policy_definition_id = "/providers/Microsoft.Management/managementGroups/tf/providers/Microsoft.Authorization/policyDefinitions/ES-Deploy-Sql-SecurityAlertPolicies"
            reference_id         = "ES-Deploy-Sql-SecurityAlertPolicies"
        }
      ~ policy_definition_reference {
          + parameter_values     = jsonencode({})
            parameters           = {}
            policy_definition_id = "/providers/Microsoft.Management/managementGroups/tf/providers/Microsoft.Authorization/policyDefinitions/ES-Deploy-Sql-AuditingSettings"
            reference_id         = "ES-Deploy-Sql-AuditingSettings"
        }

During terraform apply:

Error: expanding `policy_definition_reference`: cannot set both `parameters` and `parameter_values`

  on .terraform/modules/enterprise_scale_custom/resources.policy_set_definitions.tf line 1, in resource "azurerm_policy_set_definition" "enterprise_scale":
   1: resource "azurerm_policy_set_definition" "enterprise_scale" {

Changing our default response from "{}" to null when no parameters are actually specified in the source template appears to resolve this (and works on both first apply, and subsequent applies now) but just wanted to check whether this is the expected behaviour?

@cgerber3
Copy link

I have upgraded the TF provider to 2.31 and the error described above still occurs

@ArcturusZhang
Copy link
Contributor

I have upgraded the TF provider to 2.31 and the error described above still occurs

Could you please provide some details on how to reproduce your issue?

@cgerber3
Copy link

Please refer to the entry from krowlandson:

_If I update my code to use an empty string "" when no parameters are specified (see code snippet below), I am then able to correctly run terraform apply multiple times on an existing deployment without any changes being detected:

Dynamic configuration blocks

dynamic "policy_definition_reference" {
for_each = [
for item in each.value.template.policyDefinitions :
{
policyDefinitionId = item.policyDefinitionId
parameters = item.parameters
policyDefinitionReferenceId = item.policyDefinitionReferenceId
}
if try(length(each.value.template.policyDefinitions) > 0, false)
]
content {
policy_definition_id = policy_definition_reference.value["policyDefinitionId"]
parameter_values = try(length(policy_definition_reference.value["parameters"]) > 0, false) ? jsonencode(policy_definition_reference.value["parameters"]) : ""
reference_id = try(length(policy_definition_reference.value["policyDefinitionReferenceId"]) > 0, false) ? policy_definition_reference.value["policyDefinitionReferenceId"] : policy_definition_reference.value["policyDefinitionId"]
}
}
Unfortunately, if I then destroy the azurerm_policy_set_definition resources and try to recreate them, terraform apply then throws the error below which is what I introduced the default value of "{}" to fix

My own workaround for the error is to delete the Initiative completely then redeploy

@ArcturusZhang
Copy link
Contributor

ArcturusZhang commented Oct 28, 2020

I cannot reproduce this issue by using the following config (the definition of the policy set definition is the same, but I have to make up my own variable definitions)

resource "azurerm_policy_set_definition" "enterprise_scale" {
  for_each = {
    for policy_set in local.es_policy_set_definitions_by_management_group :
    policy_set.resource_id => policy_set
  }

  # Mandatory resource attributes
  name         = each.value.template.displayName
  policy_type  = "Custom"
  display_name = each.value.template.displayName

  # Dynamic configuration blocks
  dynamic "policy_definition_reference" {
    for_each = [
      for item in each.value.template.policyDefinitions :
      {
        policyDefinitionId          = item.policyDefinitionId
        parameters                  = item.parameters
        policyDefinitionReferenceId = item.policyDefinitionReferenceId
      }
      if try(length(each.value.template.policyDefinitions) > 0, false)
    ]
    content {
      policy_definition_id = policy_definition_reference.value["policyDefinitionId"]
      parameter_values     = try(length(policy_definition_reference.value["parameters"]) > 0, false) ? jsonencode(policy_definition_reference.value["parameters"]) : ""
      reference_id         = try(length(policy_definition_reference.value["policyDefinitionReferenceId"]) > 0, false) ? policy_definition_reference.value["policyDefinitionReferenceId"] : policy_definition_reference.value["policyDefinitionId"]
    }
  }

  # Optional resource attributes
  description           = try(length(each.value.template.description) > 0, false) ? each.value.template.description : "${each.value.template.displayName} Policy Set Definition"
  management_group_name = try(length(each.value.scope_id) > 0, false) ? basename(each.value.scope_id) : null
  metadata              = try(length(each.value.template.metadata) > 0, false) ? jsonencode(each.value.template.metadata) : local.empty_string
  parameters            = try(length(each.value.template.parameters) > 0, false) ? jsonencode(each.value.template.parameters) : local.empty_string
}

locals {
  empty_string = ""
  es_policy_set_definitions_by_management_group = [
    {
      resource_id = "1"
      template = {
        displayName = "test-policyset"
        parameters = {
          effect = {
            type = "String"
            allowedValues = ["audit","deny","disabled"]
            defaultValue = "audit"
          }
          excludedNamespaces = {
            type = "Array"
            defaultValue = ["kube-system","gatekeeper-system","azure-arc"]
          }
        }
        policyDefinitions = [
          {
            policyDefinitionId = "/providers/Microsoft.Authorization/policyDefinitions/82985f06-dc18-4a48-bc1c-b9f4f0098cfe"
            parameters = {
              allowHostNetwork = {
                value = false
              }
              effect = {
                value = "[parameters('effect')]"
              }
              excludedNamespaces = {
                value = "[parameters('excludedNamespaces')]"
              }
              maxPort = {
                value = 0
              }
              minPort = {
                value = 0
              }
            }
            policyDefinitionReferenceId = "testRef"
          },
          
          {
            policyDefinitionId = "/providers/Microsoft.Authorization/policyDefinitions/0004bbf0-5099-4179-869e-e9ffe5fb0945"
            parameters = ""
            policyDefinitionReferenceId = "testRef2"
          }
        ]
      }
    }
  ]
}

I tried the following steps:

  1. apply (create policy set definition)
  2. plan (shows no diff)
  3. destroy (the policy set definition is destroyed)
  4. apply (policy set definition is created again)
  5. plan (shows no diff)

@cgerber3
Copy link

do not perform step 3 if you are executing it as a separate command vs part of the apply

@ghost
Copy link

ghost commented Nov 7, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Nov 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants