feat: accelerator permission flattening#132
Conversation
There was a problem hiding this comment.
Pull request overview
This PR flattens and simplifies RBAC role assignment handling across the accelerator, centralizes the concept of an intermediate root management group, and enhances e2e test orchestration to validate these changes end-to-end.
Changes:
- Introduces generic role assignment definitions that can target either built-in or custom roles and centralizes management-group scope under an optional intermediate root management group.
- Adds Terraform architecture–aware file manipulation to derive and update the intermediate root management group definition, wiring those values into Azure module configuration for Terraform, GitHub, Azure DevOps, and local flows.
- Expands the e2e test workflow to dynamically create/find per-test subscriptions, adjust architecture definitions, and clean up management groups and subscriptions before and after test runs.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
modules/file_manipulation/variables.tf |
Adds terraform_architecture_file_path input so the module can read and manipulate the Terraform architecture definition. |
modules/file_manipulation/outputs.tf |
Extends repository_files to include rewritten architecture files and exposes intermediate_root_management_group_id/display_name outputs for downstream modules. |
modules/file_manipulation/locals.intermediate_root_management_group.tf |
Derives the intermediate root management group from the Terraform architecture (or Bicep parameters) and rewrites the architecture to mark the root MG as exists = true. |
modules/azure/variables.tf |
Adds configuration toggles for creating an intermediate root management group, moving target subscriptions under it, and passing its id/display name into the Azure module; updates role_assignments schema to support built-in and custom roles. |
modules/azure/subscription_placements.tf |
New resource that optionally associates target subscriptions under the intermediate root or parent management group via azapi_resource if configured. |
modules/azure/storage.tf |
Removes additional “helper” role assignments to storage for extra principal IDs and the temporary Reader assignments, aligning with the new flattened permissions model. |
modules/azure/role_definitions.tf |
Scopes custom role definitions to the intermediate root management group when it is created, otherwise to the parent management group as before. |
modules/azure/role_assignments.tf |
Refactors role assignment generation to work with either built-in role names or custom role IDs, and to scope management-group assignments to the intermediate root management group when enabled. |
modules/azure/management_group.tf |
New azapi_resource that creates the intermediate root management group under the configured parent management group with retry and timeout behavior. |
alz/local/variables.tf |
Simplifies Terraform/Bicep role definition defaults (empty or reduced set), introduces new role assignment schemas for Terraform/Bicep/Bicep Classic, removes grant_permissions_to_current_user, and adds terraform_architecture_file_path. |
alz/local/scripts-bicep/deploy-local.ps1 |
Stops passing an explicit management group ID parameter and relies on environment-derived intermediate root management group ID when invoking Bicep deployments. |
alz/local/scripts-bicep/bicep-deploy.ps1 |
Removes the managementGroupId parameter and consistently derives the target management group from the MANAGEMENT_GROUP_ID_PREFIX/INTERMEDIATE_ROOT_MANAGEMENT_GROUP_ID/MANAGEMENT_GROUP_ID_POSTFIX environment variables for what-if, cleanup, and deployment stack operations. |
alz/local/main.tf |
Wires file-manipulation’s intermediate root management group outputs into the Azure module, selects the appropriate role assignment map per IaC type (including Bicep Classic), and passes terraform_architecture_file_path to file manipulation. |
alz/github/variables.tf |
Mirrors local variable changes for GitHub scenarios: simplifies custom role defaults, adds flexible role assignment maps for Terraform/Bicep/Bicep Classic, and introduces terraform_architecture_file_path. |
alz/github/main.tf |
Aligns GitHub bootstrap with the new Azure module interface, including selecting the correct role assignment map, toggling intermediate root management group creation/migration, and passing terraform_architecture_file_path into file manipulation. |
alz/github/actions/bicep/templates/workflows/ci-template.yaml |
Updates CI Bicep workflow to stop passing a management group ID into the Bicep deploy action (now derived from env) while keeping the rest of the deployment inputs intact. |
alz/github/actions/bicep/templates/workflows/cd-template.yaml |
Makes the same management group ID handling adjustment for CD Bicep workflows (plan/apply jobs) to align with the new env-based deployment behavior. |
alz/github/actions/bicep/templates/actions/bicep-first-deployment-check/action.yaml |
Changes the “first deployment” detection to compute the intermediate root management group ID from env and to consider the presence of child management groups when deciding if this is the first deployment. |
alz/github/actions/bicep/templates/actions/bicep-deploy/action.yaml |
Removes the managementGroupId input in favor of deriving the intermediate root mg from env, and updates what-if, cleanup, and deployment stack calls to use that computed ID. |
alz/azuredevops/variables.tf |
Mirrors the local/GitHub variable changes for Azure DevOps, including streamlined custom roles, new role assignment maps per IaC type, and terraform_architecture_file_path. |
alz/azuredevops/pipelines/bicep/templates/helpers/bicep-first-deployment-check.yaml |
Updates the Azure DevOps helper template to compute the intermediate root management group ID and use its children count to infer first deployment instead of scanning all management groups. |
alz/azuredevops/pipelines/bicep/templates/helpers/bicep-deploy.yaml |
Removes the explicit management group ID parameter, derives it from environment variables, and updates what-if, cleanup, and deployment stack logic accordingly. |
alz/azuredevops/pipelines/bicep/templates/ci-template.yaml |
Adjusts the Bicep CI pipeline template to call the updated deploy helper without a managementGroupId parameter while leaving other behavior unchanged. |
alz/azuredevops/pipelines/bicep/templates/cd-template.yaml |
Mirrors the CI template changes for CD (plan/apply) Azure DevOps Bicep deployments. |
alz/azuredevops/main.tf |
Aligns Azure DevOps bootstrap with the new Azure module API and file manipulation outputs, including the intermediate root management group wiring and architecture file path. |
.github/workflows/end-to-end-test.yml |
Expands the e2e test workflow to log in to Azure, resolve per-test subscriptions, mutate Terraform architecture definitions with unique IDs, run pre/post clean-up of landing zones and management groups, and reuse/install the ALZ PowerShell module more efficiently. |
.github/tests/scripts/generate-matrix.ps1 |
Simplifies and normalizes the test matrix (e.g., using the test starter module, updating Terraform versions) and introduces ShortNamePrefix to support predictable subscription naming. |
.github/tests/scripts/create-subscriptions.ps1 |
New helper script that plans/creates per-test subscriptions (using alias API with retry/rate-limit handling) and optionally registers required resource providers across all relevant subscriptions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Default includes 1 predefined roles: | ||
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag)s |
There was a problem hiding this comment.
Minor wording/typo in the new Bicep custom role docs: "Default includes 1 predefined roles" should be singular ("role"), and there is an extra trailing s after flag) in the alz_reader description. Please adjust the text so the grammar is correct and the phrase ends with flag) rather than flag)s.
| Default includes 1 predefined roles: | |
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag)s | |
| Default includes 1 predefined role: | |
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag) |
| Default includes 1 predefined roles: | ||
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag)s |
There was a problem hiding this comment.
Minor wording/typo in the new Bicep custom role docs: "Default includes 1 predefined roles" should be singular ("role"), and there is an extra trailing s after flag) in the alz_reader description. Please adjust the text so the grammar is correct and the phrase ends with flag) rather than flag)s.
| Default includes 1 predefined roles: | |
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag)s | |
| Default includes 1 predefined role: | |
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag) |
| **(Required)** Relative path to the Terraform architecture definition JSON file within the module folder. | ||
|
|
||
| This file defines the structure and components of the Terraform deployment architecture. | ||
| Used for dynamic file manipulation based on architecture specifics. |
There was a problem hiding this comment.
The description calls this a "Terraform architecture definition JSON file", but the default value is a .yaml file and the implementation supports YAML as well as JSON. It would be clearer to describe this as a Terraform architecture definition YAML/JSON file so that consumers understand YAML is expected by default.
| **(Required)** Relative path to the Terraform architecture definition JSON file within the module folder. | |
| This file defines the structure and components of the Terraform deployment architecture. | |
| Used for dynamic file manipulation based on architecture specifics. | |
| **(Required)** Relative path to the Terraform architecture definition YAML/JSON file within the module folder. | |
| This file defines the structure and components of the Terraform deployment architecture. | |
| The default value points to a YAML-based architecture definition, but JSON files are also supported. |
| variable "terraform_architecture_file_path" { | ||
| description = <<-EOT | ||
| **(Required)** Relative path to the Terraform architecture definition JSON file within the module folder. | ||
|
|
||
| This file defines the structure and components of the Terraform deployment architecture. | ||
| Used for dynamic file manipulation based on architecture specifics. | ||
| EOT | ||
| type = string | ||
| default = "lib/architecture_definitions/alz_custom.alz_architecture_definition.yaml" |
There was a problem hiding this comment.
The description calls this a "Terraform architecture definition JSON file", but the default value is a .yaml file and the implementation supports YAML as well as JSON. It would be clearer to describe this as a Terraform architecture definition YAML/JSON file so that consumers understand YAML is expected by default.
| description = <<-EOT | ||
| **(Required)** Relative path to the Terraform architecture definition JSON file within the module folder. | ||
|
|
||
| This file defines the structure and components of the Terraform deployment architecture. | ||
| Used for dynamic file manipulation based on architecture specifics. | ||
| EOT | ||
| type = string | ||
| default = "lib/architecture_definitions/alz_custom.alz_architecture_definition.yaml" | ||
| } |
There was a problem hiding this comment.
The description calls this a "Terraform architecture definition JSON file", but the default value is a .yaml file and the implementation supports YAML as well as JSON. It would be clearer to describe this as a Terraform architecture definition YAML/JSON file so that consumers understand YAML is expected by default.
| variable "terraform_architecture_file_path" { | ||
| description = <<-EOT | ||
| **(Required)** Relative path to the Terraform architecture definition JSON file within the module folder. | ||
|
|
||
| This file defines the structure and components of the Terraform deployment architecture. | ||
| Used for dynamic file manipulation based on architecture specifics. | ||
| EOT | ||
| type = string |
There was a problem hiding this comment.
The description here refers to a "Terraform architecture definition JSON file", but the implementation supports both YAML and JSON (and callers default this variable to a .yaml file). To avoid confusion for consumers, the description should be updated to state that the path can point to a YAML or JSON architecture definition file.
| Default includes 1 predefined roles: | ||
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag)s |
There was a problem hiding this comment.
Minor wording/typo in the new Bicep custom role docs: "Default includes 1 predefined roles" should be singular ("role"), and there is an extra trailing s after flag) in the alz_reader description. Please adjust the text so the grammar is correct and the phrase ends with flag) rather than flag)s.
| Default includes 1 predefined roles: | |
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag)s | |
| Default includes 1 predefined role: | |
| - `alz_reader` - Run Bicep What-If validations (requires --validation-level providerNoRbac flag) |
Overview/Summary
Flatten and simplify role assignments.
Testing Evidence
e2e test run: https://github.com/Azure/accelerator-bootstrap-modules/actions/runs/21515210327
As part of this Pull Request I have
mainbranch