azurerm_policy_set_definition update behaviour broken in 2.19 using policy_definition_reference

biggles007 commented 4 years ago

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.12.28

provider.azurerm v2.19.0

Affected Resource(s)

azure_policy_set_definition

Terraform Configuration Files

I'm unable to provide all the relevant code due to restrictions on releasing code from the end client.

resource "azurerm_policy_set_definition" "diags" {
  name         = "diagnostics-initiative"
  policy_type  = "Custom"
  display_name = "Apply Default Diagnostic Settings"
  management_group_name           = var.definition_management_group_name

  parameters                      = file("${path.module}/json/diagnostics/_globals/initiative_parameters.json")

  dynamic "policy_definition_reference" {
      for_each = [for item in var.diagnostic_settings: {
              name = item.resource_name
          }
          if item.include_in_initiative==true && item.policy_type =="logs"
      ]

      content {
          policy_definition_id = azurerm_policy_definition.definition["${policy_definition_reference.value.name}_logs"].id
          parameters = {
            eventHubAuthorizationRuleId = "[parameters('eventHubAuthorizationRuleId')]"
            eventHubName = "[parameters('eventHubName')]"
            logsEnabled = "[parameters('logsEnabled')]"
            profileName = "[parameters('profileName')]"
            storageAccountId = "[parameters('storageAccountId')]"
            workspace = "[parameters('workspace')]"
            retentionInDays = "[parameters('retentionInDays')]"

          }
       }

    }

  dynamic "policy_definition_reference" {
      for_each = [for item in var.diagnostic_settings: {
              name = item.resource_name
          }
          if item.include_in_initiative==true && item.policy_type =="metrics"
      ]

      content {
          policy_definition_id = azurerm_policy_definition.definition["${policy_definition_reference.value.name}_metrics"].id
          parameters = {
              eventHubAuthorizationRuleId = "[parameters('eventHubAuthorizationRuleId')]"
                eventHubName = "[parameters('eventHubName')]"
                metricsEnabled = "[parameters('metricsEnabled')]"
                profileName = "[parameters('profileName')]"
                workspace = "[parameters('workspace')]"

          }
       }
    }
}

The input consists of input files containing the following which are created using a for_each against the azurerm_policy_definition resource.

variable "diagnostic_settings" {
    default = {
        # Azure Firewall
        firewall_logs = {
            resource_name = "firewall"
            resource_type = "Microsoft.Network/azureFirewalls"
            display_name = "Azure Firewall Logs"
            policy_type = "logs"
            include_in_initiative = true
        }
        firewall_metrics = {
            resource_name = "firewall"
            resource_type = "Microsoft.Network/azureFirewalls"
            display_name = "Azure Firewall Metrics"
            policy_type = "metrics"
            include_in_initiative = true
        }
    }
}

Expected Behavior

Policy Initiative should have been amended with additional or removal of a policy.

Actual Behavior

When adding/removing items it throws errors on the resource_id as it tries to re-map the resource_ids between different policies.

azurerm_policy_set_definition.diags: Modifying... [id=/providers/Microsoft.Management/managementgroups/sandbox/providers/Microsoft.Authorization/policySetDefinitions/diagnostics-initiative]

Error: creating/updating Policy Set Definition "diagnostics-initiative": policy.SetDefinitionsClient#CreateOrUpdateAtManagementGroup: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="DuplicateReferencesInPolicySet" Message="The policy set definition 'diagnostics-initiative' request is invalid. Policy set definition contains duplicate policy references. Duplicate reference Ids are '14127430907021476224'. Please remove any exact duplicates from the policy set definition. Violating policy definition Ids are '/providers/Microsoft.Management/managementgroups/sandbox/providers/Microsoft.Authorization/policyDefinitions/diag-logic_apps-metrics, /providers/Microsoft.Management/managementgroups/sandbox/providers/Microsoft.Authorization/policyDefinitions/diag-virtual_network-metrics'."

on diagnostics_policies_initiative.tf line 34, in resource "azurerm_policy_set_definition" "diags": 34: resource "azurerm_policy_set_definition" "diags" {

Steps to Reproduce

Apply an initiative with multiple policies
Inject an additional policy into the middle of the list OR remove an item from the list
Plan and apply the update

ArcturusZhang commented 4 years ago

Hi @bigglesuk69 thanks for opening this issue!

This should be caused during the expanding of the array of policy_definition_reference, some elements are added multiple times somehow.

Similar with issue #7802

I am working on this to find the root cause.

ArcturusZhang commented 4 years ago

Similar symptom in issue #7751

caldwell2000 commented 4 years ago

Having the same issue. Is there a workaround or a fix coming soon? Using Versions: Terraform v0.12.21 provider.azurerm v2.25.0

ezubriski commented 4 years ago

@caldwell2000 I have been working around this by recreating the policy initiative. I have been working around this by reordering my policy reference blocks. When that doesn't work, I have recreated the resource so that the update can happen.

This works for dev but for production scenarios with policy attachments its not too useful.

biggles007 commented 4 years ago

I'm wondering whether setting a unique value in the optional 'reference_id' will resolve the issue. I believe having that passed in alongside the definition ID may get this to work. Looking at the Security Center initiative, they have the ref ID set to a text string not numeric ID.

I haven't had time to test that yet, but will let you know once I do.

caldwell2000 commented 4 years ago

@bigglesuk69 - Looks like your solution works. I set the reference_id to the same value as the policy_definition_id and the "apply" works. Thanks for the idea!

ArcturusZhang commented 3 years ago

Hi @bigglesuk69 and @caldwell2000 thanks for your discussions and I just had some experiments about this.

This is truly caused by the reference_id. When you create the set definition without assigning a reference_id, Azure will automatically create one for you and send it back - therefore this attribute is Computed. But in the terraform implementation, we do not ensure this kind bind, which is saying that in a case of a reordering of the policy_definition_reference blocks, terraform will treat your change as you have changed the definition_id, but the reference_id remains the same. In this case, Azure will return error like posted in the description complaining about DuplicateReferencesInPolicySet. And the reason for the reordering should be that you are using a set to iterate the policy_definition_reference. In a hash set, the order is deterministic but not guaranteed, every time you add or remove or change the element of it, the iteration may reorder, and therefore comes this issue.

And also, to work this around, explicitly assigning reference_id by yourself works - this manages the bind between the reference_id and the definition_id, and therefore no duplicate error will pop out.

I am considering that we could change the type of policy_definition_reference from List to Set (but this would be considered as a breaking change), but I am not absolutely sure, is this list of policy_definition_reference guaranteed to be distinct and orderless?

ArcturusZhang commented 3 years ago

And just had a test that Azure actually allow two policy_definition_references with same definition_id, therefore set does not make sense on this block.

I am not quite sure ordering matters or not, but since it is not required to be distinct, we cannot make this a set.

dgcaron commented 3 years ago

the solution provided by @bigglesuk69 worked for me, i was looping over the created policies to create a initiative:

resource "azurerm_policy_set_definition" "initiative" {
  name         = "my_initiative"
  policy_type  = "Custom"
  display_name = "MyPolicy Set"

  parameters = <<PARAMETERS
 .. deleted
PARAMETERS

  dynamic "policy_definition_reference" {
    for_each = local.targets
    content {
      policy_definition_id = module.policies[policy_definition_reference.key].definition_id
      --> reference_id         = policy_definition_reference.key <-- adding this fixed it for me
      parameter_values     = <<VALUE
     .. deleted
    VALUE
    }
  }
}

mlcooper commented 3 years ago

I have tried putting a unique value for reference_id in all of my policy_definition_reference blocks, however it did not appear to fix the issue.

For example, if I remove one policy_definition_reference block from the resource, and then do a plan or apply, it tries to re-map reference_id's for no apparent reason:

Terraform will perform the following actions:

  # azurerm_policy_set_definition.azure_logging will be updated in-place
  ~ resource "azurerm_policy_set_definition" "azure_logging" {
        id                    = "/providers/Microsoft.Management/managementGroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policySetDefinitions/6c88d2a48ab8479e8697fa4f"
        name                  = "6c88d2a48ab8479e8697fa4f"
        # (7 unchanged attributes hidden)

      ~ policy_definition_reference {
          ~ policy_definition_id = "/providers/Microsoft.Management/managementgroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policyDefinitions/Managed-DAML-ResourceLogs-AutomationAccount" -> "/providers/Microsoft.Management/managementgroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policyDefinitions/Managed-DAML-ResourceLogs-KeyVaults"
          ~ reference_id         = "ResourceLogs-AutomationAccount" -> "ResourceLogs-KeyVaults"
            # (3 unchanged attributes hidden)
        }
      ~ policy_definition_reference {
          ~ policy_definition_id = "/providers/Microsoft.Management/managementgroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policyDefinitions/Managed-DAML-ResourceLogs-CognitaveServices" -> "/providers/Microsoft.Management/managementgroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policyDefinitions/Managed-DAML-ResourceLogs-AutomationAccount"
          ~ reference_id         = "ResourceLogs-CognitaveServices" -> "ResourceLogs-AutomationAccount"
            # (3 unchanged attributes hidden)
        }
      ~ policy_definition_reference {
          ~ policy_definition_id = "/providers/Microsoft.Management/managementgroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policyDefinitions/Managed-DAML-ResourceLogs-DataLakeAnalytics" -> "/providers/Microsoft.Management/managementgroups/xxxxxxxx-a2ac-4c32-bf6b-616810e913c6/providers/Microsoft.Authorization/policyDefinitions/Managed-DAML-ResourceLogs-CognitaveServices"
          ~ reference_id         = "ResourceLogs-DataLakeAnalytics" -> "ResourceLogs-CognitaveServices"
            # (3 unchanged attributes hidden)
        }

Please note how the reference_ids are being re-mapped however I have NOT changed them in my code.

I am running:

Terraform v0.14.11
+ provider registry.terraform.io/hashicorp/archive v2.2.0
+ provider registry.terraform.io/hashicorp/azuread v1.4.0
+ provider registry.terraform.io/hashicorp/azurerm v2.59.0
+ provider registry.terraform.io/hashicorp/random v3.1.0

garretth9 commented 3 years ago

@mlcooper i see the same behavior in my plan output after adding the reference_id, but when i actually apply it it does update the initiative as expected. Unless you're seeing different behavior on the apply i think it might just be a matter of the plan generating confusing output and not really recognizing that an element has been added or removed. The actual functionality (for me at least) appears to be correct as long as i have the reference ID set.

AshutoshNirkhe commented 2 years ago

The solution of adding 'reference_id' to all the policies referenced in the initiative indeed fixed the Duplicate policy references issue for me too, thanks!

schwarzzz commented 10 months ago

I cannot confirm, that the workaround is working. Actually I was using explicit reference_ids for quite a while (for another reasons) and saw the above mentioned error every now and then when adding or removing policies to/from an initiative.

I removed the reference_ids because I was hoping that this would fix it. Which it doesn't.

Now I found this issue...

Im using + provider registry.terraform.io/hashicorp/azurerm v3.77.0

I also have a hard time reproducing this error. It seems like you need quite a large initiative to see this.

schwarzzz commented 10 months ago

Another thing to mention: Using explicit reference_ids helps in one regard: Exemptions. Exemptions will not be deleted, if the policy or initiative is deleted. Exemptions reference policies via initiative assignment name and policy reference id.

In case the initiaitve needs to be re-created from scratch, reliable, re-producible names and reference IDs will ensure your exemptions will still be in place.

schwarzzz commented 10 months ago

Nevermind. The workaround is indeed working. I found a single policy reference in my set, which did not have a reference_id set...

Netkracker commented 9 months ago

We at @swisspost miss this feature, it is extremely tedious to generate a random id for every policy reference when you have initiatives that hold 100+ policy references, i am aware that this only seems to affect the "big guys" but this is a very undesired situation and is an issue for 3 years by now.

IMO we have 3 possibilities to pass a random (unique) ID to terraform:

uuid Function https://developer.hashicorp.com/terraform/language/functions/uuid Will redeploy my policy definition references each time

random_uuid (Resource) have to create a resource block per definition (way to much not "really" needed code)

add random GUID in Plaintext (generated using something like New-Guid in PowerShell) manual steps required which is never good

Any news on the situation ?

wvannuffele4 commented 9 months ago

Agreed this ought to be fixed at the provider level. Fortunately the workaround is simple and outside of generating a load of pointless random_string resources, doesn't really get in the way.

# workaround for https://github.com/hashicorp/terraform-provider-azurerm/issues/7810
resource "random_string" "uuid" {
  for_each = var.policy_definition_references
}

resource "azurerm_policy_set_definition" "policy_set_definition" {
  ...

  dynamic "policy_definition_reference" {
    for_each = var.policy_definition_references

    content {
      reference_id         = random_string.uuid[policy_definition_reference.key].result
    }
  }
}

duladuda commented 1 week ago

I am using the latest azurerm provider 4.1.0 also facing the same issue. I have over 17 policies in the policy set definition, will try adding the workaround to check this works.

hashicorp / terraform-provider-azurerm