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

Error while assigning a Built-in Policy Initiative using SystemAssigned Identity #51

Closed
john-nicholls-development opened this issue Mar 16, 2021 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@john-nicholls-development

Our client would like the PCI v3.2.1:2018 Policy Initiative (id: /providers/Microsoft.Authorization/policySetDefinitions/496eeda9-8f2f-4d5e-8dfd-204f0a92ed41) assigned to their subscriptions, however when attempting this we get the following error:

Error: Duplicate object key

  on .terraform/modules/enterprise_scale/locals.policy_assignments.tf line 74, in locals:
  72:   azurerm_policy_set_definition_external_lookup = {
  73:     for policy_set_definition_id in local.policy_assignments_with_managed_identity_using_external_policy_set_definition :
  74:     policy_set_definition_id => {
  75:       name                  = basename(policy_set_definition_id)
  76:       management_group_name = try(regex(local.regex_extract_provider_scope, policy_set_definition_id), null)
  77:     }
  78:   }

Two different items produced the key
"/providers/Microsoft.Authorization/policySetDefinitions/496eeda9-8f2f-4d5e-8dfd-204f0a92ed41"
in this 'for' expression. If duplicates are expected, use the ellipsis (...)
after the value expression to enable grouping by key.

Policy Assignment document looks like this:

{
  "name": "PCI-Benchmark",
  "type": "Microsoft.Authorization/policyAssignments",
  "apiVersion": "2019-09-01",
  "properties": {
    "description": "Enables PCI benchmark initative.",
    "displayName": "PCI-Benchmark",
    "notScopes": [],
    "parameters": {},
    "policyDefinitionId": "/providers/Microsoft.Authorization/policySetDefinitions/496eeda9-8f2f-4d5e-8dfd-204f0a92ed41",
    "scope": "${current_scope_resource_id}",
    "enforcementMode": false
  },
  "location": "${default_location}",
  "identity": {
    "type": "SystemAssigned"
  }
}

Not quite sure what I'm doing wrong here, any help is appreciated.

@krowlandson krowlandson self-assigned this Mar 17, 2021
@krowlandson krowlandson added the bug Something isn't working label Mar 17, 2021
@krowlandson
Copy link
Contributor

Thank you for reporting this @john-nicholls-development... I'll try to reproduce this behaviour and come back to you on this ASAP.

Please can you confirm which version of the module you are using when you get this error?

@john-nicholls-development
Copy link
Author

We are currently building with 0.0.8

@krowlandson
Copy link
Contributor

@john-nicholls-development, we implemented a few fixes to the logic for this in v0.1.0 along with a fairly comprehensive policy refresh. Are you able to re-produce this issue in the latest release? If so, please provide details of the error and we will investigate further.

Please also take note of the upgrade guidance.

@magnus78boy
Copy link

magnus78boy commented Mar 29, 2021

Edit:
Our issue where configuration of ES module.
Which seems to give errors in the custom landing zone so we got the same policy assignment at the same level.
(But there were two different mgmt groups).

Hi,

I get the same problem in both 0.8 and in. 1.1.
We are trying to create a parallel pseudo root in our management group structure in our tenant which is the idea that we want to use as Dev Environment.

It is after we have the init backend and then run the plane step that we get the error message.
These are policies that bothered us:

  • ES-Deploy-VM-Antimalw
  • ES-Deploy-VM-Monitoring
  • ES-Deploy-ASC-Monitoring
  • Deploy-AKS-policy
  • Deploy-ASC-ContExport
  • Deploy-LX-Arv-monitoring
  • Deploy-SQL-DB-Auditing
  • Deploy-VM-Antimalw
  • Deploy-WS-ARC-monitoring

There were more policies that do not go through after our upgrade to 1.1.
Has a feeling that it has to do with how ID is created. It may have to do with several assignments in the same environment.

BR
Magnus

@krowlandson
Copy link
Contributor

Just a quick update. We have managed to duplicate this error and are working on a fix.

@krowlandson
Copy link
Contributor

We're making good progress on this, however we still have an outstanding issue which is preventing us from releasing a fix.

The original error reported above by @john-nicholls-development occurs when the module detects a Policy Assignment with a Policy Set Definition not known by the module. We refer to this within the module as an external definition, which covers both built-in and custom Policy Definitions and Policy Set Definitions which are not being managed within the current module context.

Under "normal" operation this part of the module processes correctly and I am able to correctly assign the PCI v3.2.1:2018 Policy Initiative mentioned above, without any changes. With the additional context provided by @magnus78boy I was able to identify that the error occurs when the module needs to lookup multiple assignments using the same external Policy Set Definition. This can also be repeated when a Policy Assignment uses a Policy Definition not known by the module, producing the same error but on line 128.

Error: Duplicate object key

  on .terraform/modules/enterprise_scale/locals.policy_assignments.tf line 128, in locals:
  126:  external_policy_definitions_from_internal_policy_assignments = {
  127:    for policy_set_definition_id in local.policy_assignments_with_managed_identity_using_external_policy_definition :
  128:    policy_set_definition_id => {
  129:      name                  = basename(policy_set_definition_id)
  130:      management_group_name = try(regex(local.regex_extract_provider_scope, policy_set_definition_id), null)
  131:    }
  132:  }

Two different items produced the key
"/providers/Microsoft.Authorization/policySetDefinitions/<redacted>"
in this 'for' expression. If duplicates are expected, use the ellipsis (...)
after the value expression to enable grouping by key.

We have identified a fix to handle these duplications, ensuring the module can successfully perform the lookup on external definitions when duplicates exist, however this has just pushed the problem further downstream.

Despite this not being an issue when no duplicates exist (with or without the "fix"), the module now generates the following error, which is actually harder to identify as this being the cause:

Error: Invalid for_each argument

  on .terraform/modules/enterprise_scale_lz1/resources.role_assignments.tf line 2, in resource "azurerm_role_assignment" "enterprise_scale":
   2:   for_each = local.azurerm_role_assignment_enterprise_scale

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

I will continue to investigate this particular issue (as the data model is correctly generating th required values as part of plan). Assuming this is within our control in the module and not something which needs addressing in the provider or Terraform itself, we hope to provide a full fix soon.

During our investigations, we also identified a minor bug causing the module to miss Role Assignments for internal Policy Set Definitions so will cut a new release with these initial fixes in place, but please note this will not resolve this issue fully and requires further work.

In the meantime, we will work on documentation for an example deployment to show how you can decompose your deployment into multiple instances of the module to better handle nested deployments and allow you to create these assignments at multiple scopes within a single hierarchy.

krowlandson pushed a commit that referenced this issue Mar 31, 2021
- Update the library templates to use `root_scope_resource_id` for policies deployed by this module
- Fix to remove incorrect Managed Identity from `Deploy-ASC-Monitoring` policy
- Fix to address missing Role Assignments for Policy Assignments using internal Policy Set Definitions
- Improve logic when processing Role Assignments for Policy Assignments with Managed Identities
- Initial work towards #51 where duplicate Policy Assignments at different scopes cause a duplicate key error
- Fix a bug where changing the `archetype_id` for an existing Management Group fails during the `plan` stage
@petr-stupka
Copy link

For the issue

Error: Invalid for_each argument

  on .terraform/modules/enterprise_scale_lz1/resources.role_assignments.tf line 2, in resource "azurerm_role_assignment" "enterprise_scale":
   2:   for_each = local.azurerm_role_assignment_enterprise_scale

It is still in place for version 0.3.1.
It works assigning empty map in local.azurerm_role_assignment_enterprise_scale for the first apply.

# The following locals are used to extract the Role Assignment
# configuration from the archetype module outputs.
locals {
  es_role_assignments_by_management_group = flatten([
    for archetype in values(module.management_group_archetypes) :
    archetype.configuration.azurerm_role_assignment
  ])
  es_role_assignments_by_subscription = []
  es_role_assignments = concat(
    local.es_role_assignments_by_management_group,
    local.es_role_assignments_by_subscription,
    local.es_role_assignments_by_policy_assignment,
  )
}

# The following locals are used to build the map of Role
# Assignments to deploy.
locals {
  /*
  azurerm_role_assignment_enterprise_scale = {
    for assignment in local.es_role_assignments :
    assignment.resource_id => assignment
  }
  */

  azurerm_role_assignment_enterprise_scale = {}
}

After the first apply, the change can be reverted and applied again. For the second apply the module output is already in state and TF can determine the values correctly.

So guess it cannot determine the output from the module if that may help.

@krowlandson
Copy link
Contributor

@petr-stupka... thank you for your additional feedback on this issue. Would you be able to share your configuration with us please so we can try to reproduce?

@krowlandson
Copy link
Contributor

Regarding my previous response where I mention:

In the meantime, we will work on documentation for an example deployment to show how you can decompose your deployment into multiple instances of the module to better handle nested deployments and allow you to create these assignments at multiple scopes within a single hierarchy.

Please refer to our recent Wiki page, Deploy Using Module Nesting. Hopefully this covers how to do this in a bit more detail.

@petr-stupka
Copy link

petr-stupka commented May 12, 2021

Hi @krowlandson

regarding the 'Error: Invalid for_each argument'

So I played with it a bit and realized I been wrong with the module like I mentioned before. So I hope I'm correct now :)

The issue is with built-in policy which have managed identity roles defined:

"roleDefinitionIds": [
  "/providers/microsoft.authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c"
],

This section in ES module is trying to create list of those roles for the specific policy from local.policy_roles

locals {
  es_role_assignments_by_policy_assignment = flatten([
    for policy_assignment_id, policy_id in local.policy_assignments_with_managed_identity : [
      for role_definition_id in try(local.policy_roles[policy_id], local.empty_list) : [
        {
          resource_id          = "${local.azurerm_policy_assignment_enterprise_scale[policy_assignment_id].scope_id}${local.provider_path.role_assignment}${uuidv5(uuidv5("url", role_definition_id), policy_assignment_id)}"
          scope_id             = local.azurerm_policy_assignment_enterprise_scale[policy_assignment_id].scope_id
          principal_id         = try(local.principal_id_by_policy_assignment[policy_assignment_id], null)
          role_definition_name = null
          role_definition_id   = role_definition_id
        }
      ]
    ]
  ])
}

However as you can see the Mod-Inherit-Tags-RG as example will be known after apply so it can not read the roleDefinitionIds from built-in policy during plan/apply

  policy_roles = {
    "/providers/Microsoft.Authorization/policyDefinitions/cd3aa116-8754-49c9-a813-ad46512ece54"                                                           = (known after apply)
    "/providers/Microsoft.Management/managementGroups/brz365-dev-management/providers/Microsoft.Authorization/policyDefinitions/Deny-Required-Tag-RG"     = []
    "/providers/Microsoft.Management/managementGroups/brz365-dev-management/providers/Microsoft.Authorization/policySetDefinitions/Deny-Required-Tags-RG" = []
    "/providers/Microsoft.Management/managementGroups/brz365-dev-management/providers/Microsoft.Authorization/policySetDefinitions/Mod-Inherit-Tags-RG"   = (known after apply)
  }

Any 'workaround' tip? I'm happy to give it try.

@krowlandson
Copy link
Contributor

@petr-stupka... I'm not sure whether this is just because you've setup an output for policy_roles, but something doesn't look quite right here. Would you mind just confirming whether you are consuming the module from the Terraform Registry, or have forked/cloned the repository and are using it by making configuration changes within the module?

The reason I ask is that a default deployment of the module would have many more values in the policy_roles object, and this isn't something we would expect a customer to typically need to investigate.

One observation I can make from the above (regardless of approach), is that Mod-Inherit-Tags-RG is showing as (known after apply) because the module doesn't know about it. This means it will try to lookup the Policy Set Definition from Azure, and fail as it presumably doesn't exist. This is usually caused by the policy (set) definition template not being defined within the module. All customer defined policies should be stored under the library_path location with the expected naming convention. They must also then be declared in the corresponding archetype_definition template to ensure the module will deploy them at the correct scope. In your case, this would be against the brz365-dev-management Management Group.

Our recommendation would be to follow the module user guide and examples page for a successful deployment. Specifically, I would recommend looking at our guide for Deploy Custom Landing Zone Archetypes and Expand built in archetype definitions. We have limited documentation currently on creating custom policies and roles, but this is on our backlog and will be published soon.

@krowlandson
Copy link
Contributor

Closing issue as we believe this was fixed in release v0.4.0

@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants