Merge pull request #3 from schubergphilis/rbac_fix

bug: Rbac fix
schubergphilis · Feb 20, 2025 · 02557e8 · 02557e8
2 parents 5f117a6 + ca39fee
commit 02557e8
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 14 deletions.
diff --git a/examples/basic/main.tf b/examples/basic/main.tf
@@ -23,6 +23,13 @@ module "acr" {
     admin_enabled                 = false
     public_network_access_enabled = false
     network_rule_bypass_option    = "AzureServices"
+
+    role_assignments = {
+      acr = {
+        role_definition_name = "AcrPull"
+        principal_id         = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myrg/providers/Microsoft.ContainerRegistry/registries/myacr123"
+      }
+    }
   }
   tags = {
     "deploymentmodel" = "Terraform"

diff --git a/main.tf b/main.tf
@@ -108,18 +108,7 @@ resource "azurerm_role_assignment" "acr" {
 
   scope                = azurerm_container_registry.this.id
   role_definition_name = each.value.role_definition_name
-  principal_id         = each.value.id
-
-  lifecycle {
-    precondition {
-      condition     = each.value.role_definition_name == "AcrPush" || each.value.role_definition_name == "AcrPull"
-      error_message = "The role definition must be either 'AcrPull' or 'AcrPush'."
-    }
-    precondition {
-      condition     = can(regex("^([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})$", each.value.id))
-      error_message = "The principal ID must be a valid object ID or principal ID."
-    }
-  }
+  principal_id         = each.value.principal_id
 }
 
 resource "azurerm_monitor_diagnostic_setting" "this" {

diff --git a/variables.acr.tf b/variables.acr.tf
@@ -32,8 +32,8 @@ variable "acr" {
     })), [])
     zone_redundancy_enabled = optional(bool, true)
     role_assignments = optional(map(object({
-      principal_id = string
-      role         = string
+      principal_id         = string
+      role_definition_name = string
     })))
     tags = optional(map(string))
   })
@@ -95,6 +95,10 @@ ACR_DETAILS
     condition     = can(regex("^[[:alnum:]]{5,50}$", var.acr.name))
     error_message = "The name must be between 5 and 50 characters long and can only contain letters and numbers."
   }
+  validation {
+    condition     = alltrue([for ra in var.acr.role_assignments : ra.role_definition_name == "AcrPush" || ra.role_definition_name == "AcrPull"])
+    error_message = "All role definitions must be either 'AcrPull' or 'AcrPush'."
+  }
 }
 
 variable "customer_managed_key" {