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

Assignment name scoped to Management group causes duplicate Assignment Ids when attempting to create Set Assignments using the same Policy Set Initiative to multiple locations #111

Closed
jinkang23 opened this issue Aug 16, 2024 · 4 comments
Labels

Comments

@jinkang23
Copy link

jinkang23 commented Aug 16, 2024

When deploying the same Policy initiative to multiple Azure regions scoped at Management group (i.e. Tenant Root Group), this creates a duplicate Assignment Id causing the deployment to fail. This can be worked around by generating a globally unique guid and passing that in as assignment_name value, but I would like to ask that both def_assignment and set_assignment sub-modules support generating a random value using random_id resource instead.

  • Module Version: 2.9.2
  • Terraform Version: 1.9.4
  • AzureRM Provider Version: 3.115.0
locals {
  policy_set_definitions_built_in = {
    enable_allLogs_category_group_resource_logging_for_supported_resources_to_storage       = "b6b86da9-e527-49de-ac59-6af0a9db10b8"
  }
}
data "azurerm_subscription" "current" {}

data "azurerm_management_group" "root" {
  name = data.azurerm_subscription.current.tenant_id
}

data "azurerm_policy_set_definition" "this" {
  for_each = local.policy_set_definitions_built_in

  name = each.value 
}

module "asgmt_root_mg_platform_resource_diagnostics_to_storage_this" {
  for_each = toset(["centralus","eastus2"]) #*<-- deploys to all azure locations specified 

  source  = "gettek/policy-as-code/azurerm//modules/set_assignment"
  version = "2.9.2"

  initiative          = data.azurerm_policy_set_definition.this["enable_allLogs_category_group_resource_logging_for_supported_resources_to_storage"]
  assignment_scope    = data.azurerm_management_group.root.id
  assignment_location = each.key
  assignment_display_name = "Resource Diagnostics Settings to Storage - ${each.key}"

  # resource remediation options
  re_evaluate_compliance = true
  skip_remediation       = true 
  skip_role_assignment   = false
  role_definition_ids    = [data.azurerm_role_definition.contributor.id] # using explicit roles

  # NOTE: You may omit parameters at assignment to use the definitions 'defaultValue'
  assignment_parameters = {
    effect                = "DeployIfNotExists"
    resourceLocation      = each.key
    storageAccount        = local.diagnostics_storage_accounts[each.key].storage_account_id
    diagnosticSettingName = "setByPolicy-Storage" 
  }

  non_compliance_messages = {
    null = "Flagged by Initiative: ${data.azurerm_policy_set_definition.this["enable_allLogs_category_group_resource_logging_for_supported_resources_to_storage"].display_name}"
  }
}

Expected Behavior

Policy Set Assignments for module.asgmt_root_mg_platform_resource_diagnostics_to_storage_this should successfully create two assignments, one per each region specified using the for_each: centralus, eastus2.

Current Behavior

First Policy Set Assignment is created succesfully (i.e. centralus), however, the second Policy Set Assignment fails with 400 Bad Request due to duplicate Policy set Assignment ID.

│ Error: creating Scoped Policy Assignment (Scope: "/providers/Microsoft.Management/managementGroups/00000000-0000-0000-0000-000000000000"
│ Policy Assignment Name: "b6b86da9-e527-49de-ac59-"): unexpected status 400 (400 Bad Request) with error: FailedIdentityOperation: Identity operation for resource '/providers/Microsoft.Management/managementGroups/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/policyAssignments/b6b86da9-e527-49de-ac59-' failed with error 'Failed to perform resource identity operation. Status: 'BadRequest'. Response: '{"error":{"code":"BadRequest","message":""}}'.'.
│
│   with module.asgmt_root_mg_platform_resource_diagnostics_to_storage_this["centralus"].azurerm_management_group_policy_assignment.set[0],
│   on .terraform/modules/asgmt_root_mg_platform_resource_diagnostics_to_storage_this/modules/set_assignment/main.tf line 5, in resource "azurerm_management_group_policy_assignment" "set":
│    5: resource "azurerm_management_group_policy_assignment" "set" {
│
│ creating Scoped Policy Assignment (Scope: "/providers/Microsoft.Management/managementGroups/00000000-0000-0000-0000-000000000000"
│ Policy Assignment Name: "b6b86da9-e527-49de-ac59-"): unexpected status 400 (400 Bad Request) with error: FailedIdentityOperation: Identity operation for resource
│ '/providers/Microsoft.Management/managementGroups/00000000-0000-0000-0000-000000000000/providers/Microsoft.Authorization/policyAssignments/b6b86da9-e527-49de-ac59-' failed with error 'Failed to perform
│ resource identity operation. Status: 'BadRequest'. Response: '{"error":{"code":"BadRequest","message":""}}'.'.

Possible Solution

Update set_assignment and def_assignment sub-modules with following changes:

# variables.tf 
variable "generate_assignment_id" {
  type        = bool
  description = "Generate a random assignment ID"
  default     = false

  validation {
    condition     = var.generate_assignment_id == true ? var.assignment_name == null : true
    error_message = "You must not specify `assignment_name` if `generate_assignment_id` is true."
  }
}

resource "random_id" "assignment_id" {
  keepers = {
    assignment_scope = var.assignment_scope
    assignment_name  = var.assignment_name
    initiative_name  = var.initiative.name
  }
  byte_length = 12
}

locals {

# if `var.generate_assignment_id` == `true`, then use random hex id (exactly 24 chars long) as assignment name, otherwise, use assignment_name or initiative name. This allows for backwards compatibility. 

  assignment_name_trim = local.assignment_scope.mg > 0 ? 24 : 64
  assignment_name      = var.generate_assignment_id ? random_id.assignment_id.hex : try((lower(substr(coalesce(var.assignment_name, var.initiative.name), 0, local.assignment_name_trim))), "")


}
@gettek
Copy link
Owner

gettek commented Aug 19, 2024

Hi @jinkang23,

Thank you for raising this, unfortunately I cannot see how this will be beneficial when one can set assignment_name to give unique values at runtime.

Perhaps you could consider creating the random_id in a for_each loop instead and parsing the key/value mapping to the assignment, thus populating assignment_name to meet your requirements.

Hope this helps

@jinkang23
Copy link
Author

Hello @gettek - Thank you for your response.

I have considered that workaround and will technically work in this case. However, I raised this issue in hoping that the modules can be updated to support randomly generating the assignment_name natively as I believe it would be a more elegant approach to solving this issue instead of trying to track the random_id key/value mapping external to the module. That degree of overhead can become a bit complex once we start to manage many Policy assignments at management group scopes and would hope that you would reconsider adding this as a QoL enhancement. Thank you!

Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Sep 21, 2024
Copy link

github-actions bot commented Oct 5, 2024

This issue was closed because it has been inactive for 14 days since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants