fix(iac/azure): custom role grants Microsoft.Capacity/reservationOrders/purchase/action#744
Conversation
…rs/purchase/action The built-in Reservation Purchaser role lacks Microsoft.Capacity/reservationOrders/purchase/action, which the two-step calculatePrice -> purchase flow (PR #680) requires. Replace it with a custom role that explicitly enumerates the runtime actions used by the reservation + savings-plan purchase paths, assignable at subscription scope AND /providers/Microsoft.Capacity. Fixes 403 AuthorizationFailed on live reservation purchases for newly onboarded Azure subscriptions. Closes #731 (if open).
cristim
added
triaged
|
Warning Review limit reached
More reviews will be available in 44 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughBoth ARM template and Terraform configurations replace the built-in ChangesCustom RBAC role for purchase authorization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@arm/CUDly-CrossSubscription/template.json`:
- Around line 88-95: The role assignment `name` GUID at provider scope is not
unique per subscription and can collide across deployments; update the `name`
expression for the Microsoft.Authorization/roleAssignments resource so it
includes the subscription identifier (e.g., subscription().subscriptionId) along
with parameters('servicePrincipalObjectId') and the fixed salt
'cudlyCustomRoleCapacity' to produce a subscription-scoped GUID; locate the
resource that sets "name": "[guid(parameters('servicePrincipalObjectId'),
'cudlyCustomRoleCapacity')]" and replace the GUID inputs to include the
subscription id so each subscription gets a distinct roleAssignment name while
keeping the same salt and principalId/roleDefinitionId usage.
- Around line 45-56: The custom role's "actions" array is missing required RBAC
actions which cause 403s for exchange and savings-plan flows; update the actions
list (the JSON property "actions" in the role definition) to include
Microsoft.Capacity/calculateExchange/action, Microsoft.Capacity/exchange/action,
Microsoft.BillingBenefits/savingsPlanOrders/write, and
Microsoft.BillingBenefits/validate/action so the role covers exchange and
savings-plan operations.
In `@iac/federation/azure-target/terraform/main.tf`:
- Around line 97-101: Add a second role assignment at the Microsoft.Capacity
provider scope so the service principal can call provider-root endpoints: create
a new azurerm_role_assignment (e.g.,
azurerm_role_assignment.cudly_reservation_purchaser_provider_scope) with scope
set to "/subscriptions/${local.subscription_id}/providers/Microsoft.Capacity",
role_definition_id set to
azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id,
principal_id set to azuread_service_principal.cudly.object_id, and ensure it
depends_on azurerm_role_definition.cudly_reservation_purchaser (and any related
resources) just like the existing subscription-scoped assignment.
- Around line 71-82: The actions list in the Terraform RBAC assignment is
missing required permissions for exchange and savings-plan flows; update the
actions array (the "actions" attribute in the relevant IAM role/resource block)
to include Microsoft.Capacity/calculateExchange/action,
Microsoft.Capacity/exchange/action,
Microsoft.BillingBenefits/savingsPlanOrders/write, and
Microsoft.BillingBenefits/validate/action so tenants can perform exchange and
savings-plan validation/purchase operations; locate the actions array shown in
the diff and append these four action strings to it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2d9b7b8-7170-46a5-a98e-bcb315726467
📒 Files selected for processing (2)
arm/CUDly-CrossSubscription/template.jsoniac/federation/azure-target/terraform/main.tf
| "actions": [ | ||
| "Microsoft.Capacity/calculateprice/action", | ||
| "Microsoft.Capacity/reservationorders/write", | ||
| "Microsoft.Capacity/reservationorders/read", | ||
| "Microsoft.Capacity/reservationorders/reservations/read", | ||
| "Microsoft.Capacity/register/action", | ||
| "Microsoft.Capacity/calculatePrice/action", | ||
| "Microsoft.Capacity/catalogs/read", | ||
| "Microsoft.Capacity/reservationOrders/read", | ||
| "Microsoft.Capacity/reservationOrders/write", | ||
| "Microsoft.Capacity/reservationOrders/purchase/action", | ||
| "Microsoft.Capacity/reservationOrders/reservations/read", | ||
| "Microsoft.BillingBenefits/savingsPlanOrderAliases/write", | ||
| "Microsoft.BillingBenefits/savingsPlanOrders/read", | ||
| "Microsoft.BillingBenefits/savingsPlanOrders/savingsPlans/read", | ||
| "Microsoft.BillingBenefits/savingsPlanOrders/action" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
template="arm/CUDly-CrossSubscription/template.json"
echo "Current custom-role actions:"
jq -r '
.resources[]
| select(.type=="Microsoft.Authorization/roleDefinitions")
| .properties.permissions[0].actions[]
' "$template" | sort
echo
for action in \
"Microsoft.Capacity/calculateExchange/action" \
"Microsoft.Capacity/exchange/action" \
"Microsoft.BillingBenefits/savingsPlanOrders/write" \
"Microsoft.BillingBenefits/validate/action"
do
if jq -e --arg a "$action" '
.resources[]
| select(.type=="Microsoft.Authorization/roleDefinitions")
| .properties.permissions[0].actions
| index($a)
' "$template" >/dev/null; then
echo "FOUND $action"
else
echo "MISSING $action"
fi
doneRepository: LeanerCloud/CUDly
Length of output: 814
Add the missing RBAC actions to the custom role permissions
The custom role action list in arm/CUDly-CrossSubscription/template.json omits these required actions, so exchange and some savings-plan flows can still fail with 403:
Microsoft.Capacity/calculateExchange/actionMicrosoft.Capacity/exchange/actionMicrosoft.BillingBenefits/savingsPlanOrders/writeMicrosoft.BillingBenefits/validate/action
Suggested patch
"actions": [
"Microsoft.Capacity/register/action",
"Microsoft.Capacity/calculatePrice/action",
+ "Microsoft.Capacity/calculateExchange/action",
"Microsoft.Capacity/catalogs/read",
"Microsoft.Capacity/reservationOrders/read",
"Microsoft.Capacity/reservationOrders/write",
"Microsoft.Capacity/reservationOrders/purchase/action",
+ "Microsoft.Capacity/exchange/action",
"Microsoft.Capacity/reservationOrders/reservations/read",
"Microsoft.BillingBenefits/savingsPlanOrderAliases/write",
"Microsoft.BillingBenefits/savingsPlanOrders/read",
+ "Microsoft.BillingBenefits/savingsPlanOrders/write",
"Microsoft.BillingBenefits/savingsPlanOrders/savingsPlans/read",
- "Microsoft.BillingBenefits/savingsPlanOrders/action"
+ "Microsoft.BillingBenefits/savingsPlanOrders/action",
+ "Microsoft.BillingBenefits/validate/action"
],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@arm/CUDly-CrossSubscription/template.json` around lines 45 - 56, The custom
role's "actions" array is missing required RBAC actions which cause 403s for
exchange and savings-plan flows; update the actions list (the JSON property
"actions" in the role definition) to include
Microsoft.Capacity/calculateExchange/action, Microsoft.Capacity/exchange/action,
Microsoft.BillingBenefits/savingsPlanOrders/write, and
Microsoft.BillingBenefits/validate/action so the role covers exchange and
savings-plan operations.
| "scope": "/providers/Microsoft.Capacity", | ||
| "name": "[guid(parameters('servicePrincipalObjectId'), 'cudlyCustomRoleCapacity')]", | ||
| "dependsOn": [ | ||
| "[variables('customRoleDefinitionId')]" | ||
| ], | ||
| "properties": { | ||
| "roleDefinitionId": "[variables('roles').costManagementReader]", | ||
| "roleDefinitionId": "[variables('customRoleDefinitionId')]", | ||
| "principalId": "[parameters('servicePrincipalObjectId')]", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Azure RBAC ARM templates, are Microsoft.Authorization/roleAssignmentsnames unique within their assignment scope, and would reusing the same name at tenant scope/providers/Microsoft.Capacitywith a differentroleDefinitionId across subscriptions cause a conflict or immutable update failure?
💡 Result:
In Azure, Microsoft.Authorization/roleAssignments names must be a globally unique identifier (GUID) [1][2][3]. Crucially, these names must be unique within the Microsoft Entra tenant, regardless of the scope at which the role assignment is applied [1][2][4]. Regarding your scenario: 1. Conflict Potential: If you attempt to use the same GUID (name) for two different role assignments, Azure will interpret this as an update to the existing assignment rather than the creation of a new, separate one [5]. If the properties of the new assignment (e.g., roleDefinitionId, principalId, or scope) differ from the existing one, the deployment will likely fail or cause an immutable update error, depending on the specifics of the change [1][5][6]. 2. Immutable Update Failure: Because a role assignment name is the identifier used to track the resource, changing its core properties while keeping the same name essentially asks Azure to modify an existing resource [5]. Since role assignments are tracked by name across the tenant, you cannot have multiple distinct assignments share the same name [1][2]. To avoid conflicts and ensure idempotency in your ARM templates or Bicep files, it is best practice to generate a deterministic GUID based on the unique combination of the scope, principal ID, and role definition ID (for example, by using the guid function in Bicep or ARM) [4][5][7]. This ensures that for a specific set of parameters, the same assignment name is always generated, preventing accidental duplicates while ensuring that unrelated assignments do not collide [4][7].
Citations:
- 1: https://github.com/MicrosoftDocs/azure-docs/blob/main/articles/role-based-access-control/role-assignments.md
- 2: https://learn.microsoft.com/en-us/azure/role-based-access-control/role-assignments
- 3: https://github.com/MicrosoftDocs/azure-docs/blob/main/articles/role-based-access-control/role-assignments-rest.md
- 4: https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/scenarios-rbac
- 5: https://stackoverflow.com/questions/79498128/use-same-guid-in-bicep-templates-and-azure-portal
- 6: RBAC not getting created uniquely Azure/bicep#4568
- 7: https://learn.microsoft.com/en-us/answers/questions/1007498/microsoft-authorization-roleassignments-devops-ci
Make the provider-scope role assignment name unique per subscription
Microsoft.Authorization/roleAssignments uses the ARM name (GUID) as the Entra tenant identifier for the role assignment; reusing guid(parameters('servicePrincipalObjectId'), 'cudlyCustomRoleCapacity') at scope /providers/Microsoft.Capacity while roleDefinitionId varies per subscription can cause later deployments to target/update the same roleAssignment name and fail (immutable update/resource identifier mismatch).
Suggested patch
- "name": "[guid(parameters('servicePrincipalObjectId'), 'cudlyCustomRoleCapacity')]",
+ "name": "[guid(parameters('servicePrincipalObjectId'), 'cudlyCustomRoleCapacity', subscription().subscriptionId)]",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@arm/CUDly-CrossSubscription/template.json` around lines 88 - 95, The role
assignment `name` GUID at provider scope is not unique per subscription and can
collide across deployments; update the `name` expression for the
Microsoft.Authorization/roleAssignments resource so it includes the subscription
identifier (e.g., subscription().subscriptionId) along with
parameters('servicePrincipalObjectId') and the fixed salt
'cudlyCustomRoleCapacity' to produce a subscription-scoped GUID; locate the
resource that sets "name": "[guid(parameters('servicePrincipalObjectId'),
'cudlyCustomRoleCapacity')]" and replace the GUID inputs to include the
subscription id so each subscription gets a distinct roleAssignment name while
keeping the same salt and principalId/roleDefinitionId usage.
| scope = "/subscriptions/${local.subscription_id}" | ||
| role_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id | ||
| principal_id = azuread_service_principal.cudly.object_id | ||
|
|
||
| depends_on = [azurerm_role_definition.cudly_reservation_purchaser] |
There was a problem hiding this comment.
Add the provider-scope assignment for Microsoft.Capacity.
This only assigns the role at /subscriptions/${local.subscription_id}, but the reservation flow calls provider-root endpoints under /providers/Microsoft.Capacity/.... Without a second assignment at that provider scope, the original 403 on calculatePrice/purchase can still reproduce for Terraform-managed tenants.
Suggested diff
resource "azurerm_role_assignment" "cudly_reservations" {
scope = "/subscriptions/${local.subscription_id}"
role_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id
principal_id = azuread_service_principal.cudly.object_id
depends_on = [azurerm_role_definition.cudly_reservation_purchaser]
}
+
+resource "azurerm_role_assignment" "cudly_reservations_capacity_provider" {
+ scope = "/providers/Microsoft.Capacity"
+ role_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id
+ principal_id = azuread_service_principal.cudly.object_id
+
+ depends_on = [azurerm_role_definition.cudly_reservation_purchaser]
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scope = "/subscriptions/${local.subscription_id}" | |
| role_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id | |
| principal_id = azuread_service_principal.cudly.object_id | |
| depends_on = [azurerm_role_definition.cudly_reservation_purchaser] | |
| resource "azurerm_role_assignment" "cudly_reservations" { | |
| scope = "/subscriptions/${local.subscription_id}" | |
| role_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id | |
| principal_id = azuread_service_principal.cudly.object_id | |
| depends_on = [azurerm_role_definition.cudly_reservation_purchaser] | |
| } | |
| resource "azurerm_role_assignment" "cudly_reservations_capacity_provider" { | |
| scope = "/providers/Microsoft.Capacity" | |
| role_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id | |
| principal_id = azuread_service_principal.cudly.object_id | |
| depends_on = [azurerm_role_definition.cudly_reservation_purchaser] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@iac/federation/azure-target/terraform/main.tf` around lines 97 - 101, Add a
second role assignment at the Microsoft.Capacity provider scope so the service
principal can call provider-root endpoints: create a new azurerm_role_assignment
(e.g., azurerm_role_assignment.cudly_reservation_purchaser_provider_scope) with
scope set to
"/subscriptions/${local.subscription_id}/providers/Microsoft.Capacity",
role_definition_id set to
azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id,
principal_id set to azuread_service_principal.cudly.object_id, and ensure it
depends_on azurerm_role_definition.cudly_reservation_purchaser (and any related
resources) just like the existing subscription-scoped assignment.
… add Reader The host container-app identity was still on the built-in Reservation Purchaser role (which omits reservationOrders/purchase/action, the same bug this PR fixes customer-side). Adds the same custom role plus a subscription-scope Reader, factoring the role definition into a shared module so customer + host stay in lockstep. - New module: terraform/modules/iam/azure/cudly-reservation-role (main.tf / variables.tf / outputs.tf) - single source of truth for the custom role's action list - iac/federation/azure-target/terraform/main.tf: switch from inlined azurerm_role_definition to the shared module (no behavioural change) - terraform/modules/compute/azure/container-apps/main.tf: replace built-in Reservation Purchaser assignment with custom-role module + add subscription-scope Reader for host-account resource enumeration Refs #731.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Why this fix is needed
Live reservation purchases on production Azure subscriptions are failing with 403:
The two-step
calculatePrice->purchaseflow introduced by PR #680 (providers/azure/services/internal/reservations/purchase.go) requiresMicrosoft.Capacity/reservationOrders/purchase/action. The built-in Reservation Purchaser role (currently the only role our IaC assigns) does not include that action; it stops atreservationOrders/write+reservationOrders/read+calculatePrice/action. A custom role with the explicit actions is the tightest fix.Changes
ARM template (
arm/CUDly-CrossSubscription/template.json):Before: two
roleAssignmentsusing the built-in Reservation Purchaser role (one at subscription scope, one at/providers/Microsoft.Capacityscope).After:
Microsoft.Authorization/roleDefinitionsresource (apiVersion: 2022-04-01) —roleName: "CUDly Reservation Purchaser (custom)"with all 11 required actions (see below),assignableScopesincludes both subscription ID and/providers/Microsoft.Capacity.dependsOnpointing to the roleDefinition resource so ARM provisions them in the correct order.Terraform (
iac/federation/azure-target/terraform/main.tf):Before:
azurerm_role_assignment.cudly_reservationswithrole_definition_name = "Reservation Purchaser".After:
azurerm_role_definition.cudly_reservation_purchaserwith the same 11 actions, scoped todata.azurerm_subscription.current.id,assignable_scopesincludes both subscription ID and/providers/Microsoft.Capacity.azurerm_role_assignment.cudly_reservationsto referencerole_definition_id = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id.depends_on = [azurerm_role_definition.cudly_reservation_purchaser]per the project's RBAC propagation lesson (Azure takes up to 10 min; without this the assignment can 403 with RoleDefinitionDoesNotExist on the first apply).Actions granted by the custom role (both bundles):
Test plan
az deployment sub create) to the affected subscription./providers/Microsoft.Capacityscope).terraform applyon a test subscription's azure-target stack; verify no error and role assignment references the custom role.Closes #731
Summary by CodeRabbit