Skip to content

Fix: Resolve duplicate Key Vault Administrator role assignment in workload zone deployment#998

Merged
KimForss merged 1 commit intoAzure:release/january-2026from
nnoaman:fix/wz-duplicate-keyvault-admin-role
Jan 15, 2026
Merged

Fix: Resolve duplicate Key Vault Administrator role assignment in workload zone deployment#998
KimForss merged 1 commit intoAzure:release/january-2026from
nnoaman:fix/wz-duplicate-keyvault-admin-role

Conversation

@nnoaman
Copy link
Contributor

@nnoaman nnoaman commented Jan 14, 2026

Summary

Fixes duplicate Key Vault Administrator role assignment error when deploying workload zones with existing Key Vaults.

Problem

Workload zone deployments fail with error:

Error: RoleAssignmentExists: The role assignment already exists.
at: module.sap_landscape.azurerm_role_assignment.role_assignment_msi[0]

This occurs because when var.key_vault.user.exists = true, both:

  • role_assignment_msi (line 103)
  • kv_user_msi_rbac (line 183)

attempt to assign the Key Vault Administrator role to the MSI, creating a conflict.

Solution

Modified role assignment count conditions in key_vault_sap_landscape.tf

This ensures MSI role assignments only apply to existing Key Vaults, while kv_user_msi_rbac handles new Key Vaults

Testing

Validated against workload zone deployment scenario

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a duplicate Key Vault Administrator role assignment error that occurs during workload zone deployments with new Key Vaults. The issue was caused by two resources attempting to assign the same role to the same managed identity on the same Key Vault scope.

Changes:

  • Modified role_assignment_msi count condition to only execute for existing Key Vaults
  • Modified role_assignment_msi_officer count condition to only execute for existing Key Vaults
  • Ensures mutually exclusive role assignment behavior: role_assignment_msi* for existing KVs, kv_user_msi_rbac* for new KVs
Comments suppressed due to low confidence (2)

deploy/terraform/terraform-units/modules/sap_landscape/key_vault_sap_landscape.tf:107

  • The scope conditional expression is now redundant. Since the count condition already ensures that this resource only runs when var.key_vault.user.exists is true, the ternary operator in the scope will always evaluate to the first branch. Consider simplifying the scope to just use data.azurerm_key_vault.kv_user[0].id directly for clarity.
  scope                                = var.key_vault.user.exists ? (
                                           data.azurerm_key_vault.kv_user[0].id) : (
                                           azurerm_key_vault.kv_user[0].id
                                         )

deploy/terraform/terraform-units/modules/sap_landscape/key_vault_sap_landscape.tf:139

  • The scope conditional expression is now redundant. Since the count condition already ensures that this resource only runs when var.key_vault.user.exists is true, the ternary operator in the scope will always evaluate to the first branch. Consider simplifying the scope to just use data.azurerm_key_vault.kv_user[0].id directly for clarity.
  scope                                = var.key_vault.user.exists ? (
                                           data.azurerm_key_vault.kv_user[0].id) : (
                                           azurerm_key_vault.kv_user[0].id
                                         )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nnoaman nnoaman requested a review from KimForss January 14, 2026 18:13
@nnoaman nnoaman marked this pull request as ready for review January 14, 2026 18:13
@nnoaman nnoaman requested a review from a team as a code owner January 14, 2026 18:13
@nnoaman nnoaman removed the request for review from a team January 14, 2026 18:14
@KimForss KimForss merged commit 5378bd4 into Azure:release/january-2026 Jan 15, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants