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

Add support for user managed identity for policy assignments (re-submission) #867

Merged
merged 11 commits into from
Mar 13, 2024

Conversation

LaurentLesle
Copy link
Contributor

Overview/Summary

Addresses a PR merge regression -> #806 (comment)

Azure policies who implement a deploy if not exist (DINE policies) require an identity to have the right permission to deploy the missing resources. By leveraging the user managed identities, customers can reduce the number of system identities created by the assignments by using a user managed identity. The other benefit of using a user managed identity is the decouple the role assignment from the policy.

This PR fixes/adds/changes/removes

  1. Add support for UserAssigned msi to a policy assignment
  2. Support migration from SystemAssign to UserAssigned by overriding the policy assignment in the lib folder
  3. Closes issue Allow UserAssigned Identities for Policy Assignments #712 User Assigned identity for Policy #500

Breaking Changes

no breaking changes

Testing Evidence

Updated examples/400-multi-with-orchestration to show how to modify an existing policy assignment from SystemAssigned to UserAssigned

# module.core.module.alz.data.azapi_resource.user_msi["/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/policyAssignments/Deploy-VMSS-Monitoring"] will be read during apply
  # (depends on a resource or a module with changes pending)
 <= data "azapi_resource" "user_msi" {
      + id                     = (known after apply)
      + location               = (known after apply)
      + output                 = (known after apply)
      + parent_id              = (known after apply)
      + resource_id            = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/msi/providers/Microsoft.ManagedIdentity/userAssignedIdentities/ms1"
      + response_export_values = [
          + "properties.principalId",
        ]
      + tags                   = (known after apply)
      + type                   = "Microsoft.ManagedIdentity/userAssignedIdentities@2018-11-30"
    }

  # module.core.module.alz.azurerm_management_group_policy_assignment.enterprise_scale["/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/policyAssignments/Deploy-VMSS-Monitoring"] will be updated in-place
  ~ resource "azurerm_management_group_policy_assignment" "enterprise_scale" {
        id                   = "/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/policyAssignments/Deploy-VMSS-Monitoring"
        name                 = "Deploy-VMSS-Monitoring"
        # (9 unchanged attributes hidden)

      ~ identity {
          ~ identity_ids = [
              + "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/msi/providers/Microsoft.ManagedIdentity/userAssignedIdentities/ms1",
            ]
          ~ type         = "SystemAssigned" -> "UserAssigned"
            # (2 unchanged attributes hidden)
        }

        # (1 unchanged block hidden)
    }

  # module.core.module.alz.module.role_assignments_for_policy["/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/policyAssignments/Deploy-VMSS-Monitoring"].azurerm_role_assignment.for_policy["/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/roleAssignments/1e6eb635-dc9a-54d5-9bb5-7506132bff67"] must be replaced
-/+ resource "azurerm_role_assignment" "for_policy" {
      ~ id                               = "/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/roleAssignments/1e6eb635-dc9a-54d5-9bb5-7506132bff67" -> (known after apply)
        name                             = "1e6eb635-dc9a-54d5-9bb5-7506132bff67"
      ~ principal_id                     = "9ce8936e-cdd6-4a7c-a2a5-0e695c9ef8a5" # forces replacement -> (known after apply) # forces replacement
      ~ principal_type                   = "ServicePrincipal" -> (known after apply)
      ~ role_definition_name             = "Virtual Machine Contributor" -> (known after apply)
      + skip_service_principal_aad_check = (known after apply)
        # (2 unchanged attributes hidden)
    }

  # module.core.module.alz.module.role_assignments_for_policy["/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/policyAssignments/Deploy-VMSS-Monitoring"].azurerm_role_assignment.for_policy["/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/roleAssignments/55ffe1be-e389-5d46-9488-8d6915a8b60e"] must be replaced
-/+ resource "azurerm_role_assignment" "for_policy" {
      ~ id                               = "/providers/Microsoft.Management/managementGroups/myorg/providers/Microsoft.Authorization/roleAssignments/55ffe1be-e389-5d46-9488-8d6915a8b60e" -> (known after apply)
        name                             = "55ffe1be-e389-5d46-9488-8d6915a8b60e"
      ~ principal_id                     = "9ce8936e-cdd6-4a7c-a2a5-0e695c9ef8a5" # forces replacement -> (known after apply) # forces replacement
      ~ principal_type                   = "ServicePrincipal" -> (known after apply)
      ~ role_definition_name             = "Log Analytics Contributor" -> (known after apply)
      + skip_service_principal_aad_check = (known after apply)
        # (2 unchanged attributes hidden)
    }

Plan: 2 to add, 1 to change, 2 to destroy.

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant issues, for tracking and closure.
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.

@matt-FFFFFF
Copy link
Member

/azp run unit

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matt-FFFFFF
Copy link
Member

Hi @LaurentLesle

The tests fail - this is why the original PR was reverted. Please can you investigate?

│ Error: Invalid function argument
│ 
│   on ../../../resources.role_assignments.tf line 62, in data "azapi_resource" "user_msi":
│   62:     if one(azurerm_management_group_policy_assignment.enterprise_scale[ik].identity[0].identity_ids) != null
│     ├────────────────
│     │ azurerm_management_group_policy_assignment.enterprise_scale is object with 38 attributes
│ 
│ Invalid value for "list" parameter: argument must not be null.

@LaurentLesle
Copy link
Contributor Author

@matt-FFFFFF can you confirm this is the tests the failed pipeline was running? if it is the case I will repro locally and fix

image

@matt-FFFFFF
Copy link
Member

Correct

@LaurentLesle
Copy link
Contributor Author

@matt-FFFFFF managed to reproduce and fix.

@matt-FFFFFF
Copy link
Member

/azp run unit

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matt-FFFFFF
Copy link
Member

Error: Invalid for_each argument
│ 
│   on ../../../resources.role_assignments.tf line 60, in data "azapi_resource" "user_msi":
│   60:   for_each = {
│   61:     for ik, iv in local.es_role_assignments_by_policy_assignment : ik => iv
│   62:     if try(azurerm_management_group_policy_assignment.enterprise_scale[ik].identity[0].type, null) == "UserAssigned"
│   63:   }
│     ├────────────────
│     │ azurerm_management_group_policy_assignment.enterprise_scale is object with 38 attributes
│     │ local.es_role_assignments_by_policy_assignment is object with 21 attributes
│ 
│ The "for_each" map includes keys derived from resource attributes that
│ cannot be determined until apply, and so Terraform cannot determine the
│ full set of keys that will identify the instances of this resource.
│ 
│ When working with unknown values in for_each, it's better to define the map
│ keys statically in your configuration and place apply-time results only in
│ the map values.
│ 
│ Alternatively, you could use the -target planning option to first apply
│ only the resources that the for_each value depends on, and then apply a
│ second time to fully converge.

@LaurentLesle

@luke-taylor
Copy link
Contributor

/azp run unit

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@luke-taylor
Copy link
Contributor

/azp run unit

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@luke-taylor
Copy link
Contributor

/azp run update

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@luke-taylor
Copy link
Contributor

/azp run update

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@luke-taylor
Copy link
Contributor

/azp run unit

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@luke-taylor
Copy link
Contributor

/azp run unit

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@luke-taylor luke-taylor merged commit c84e4e8 into Azure:main Mar 13, 2024
9 checks passed
@madhulikasarangam
Copy link

Hi Laurent and Team,

After merge the user assigned identity code to entraprice scale master repo, still we are facing the issue if we put the user assigned identity in the policy assignment. Could someone help on this

on /home/vscode/.terraform.cache/singtel-dev/rover_jobs/20240604071653261147708/modules/enterprise_scale/resources.policy_assignments.tf line 28, in resource "azurerm_management_group_policy_assignment" "enterprise_scale": -- 04-Jun-2024 15:17:59 | 28:       if lower(iv) == "systemassigned" 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | Invalid value for "str" parameter: string required. 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | Error: Invalid function argument 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | on /home/vscode/.terraform.cache/singtel-dev/rover_jobs/20240604071653261147708/modules/enterprise_scale/resources.policy_assignments.tf line 28, in resource "azurerm_management_group_policy_assignment" "enterprise_scale": 04-Jun-2024 15:17:59 | 28:       if lower(iv) == "systemassigned" 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | Invalid value for "str" parameter: string required. 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | Error: Invalid function argument 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | on /home/vscode/.terraform.cache/singtel-dev/rover_jobs/20240604071653261147708/modules/enterprise_scale/resources.policy_assignments.tf line 28, in resource "azurerm_management_group_policy_assignment" "enterprise_scale": 04-Jun-2024 15:17:59 | 28:       if lower(iv) == "systemassigned" 04-Jun-2024 15:17:59 |   04-Jun-2024 15:17:59 | Invalid value for "str" parameter: string required.

@madhulikasarangam
Copy link

If someone can join the call and let us know how we can pass the template file in the CAF for user assigned managed identity issue will be resolve. Please help on the issue

@madhulikasarangam
Copy link

Error:

4 | Error: Error in function call -- | -- 04-Jun-2024 23:28:24 |   04-Jun-2024 23:28:24 | on /home/vscode/.terraform.cache/singtel-dev/modules/enterprise_scale/modules/archetypes/locals.policy_assignments.tf line 30, in locals: 04-Jun-2024 23:28:24 | 30:     filepath => jsondecode(templatefile("${local.custom_library_path}/${filepath}", local.template_file_vars)) 04-Jun-2024 23:28:24 | ├──────────────── 04-Jun-2024 23:28:24 | │ local.custom_library_path is "../../../../platform/components/lib" 04-Jun-2024 23:28:24 | │ local.template_file_vars is object with 25 attributes 04-Jun-2024 23:28:24 |   04-Jun-2024 23:28:24 | Call to function "jsondecode" failed: invalid character 'f'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants