Repository navigation
fix(azure-arm): grant SP the actual purchase actions via custom role (closes #731) - #732
Conversation
The built-in Reservation Purchaser role (f7b75c60) is missing Microsoft.Capacity/calculateprice/action, reservationorders/write, and the Microsoft.BillingBenefits actions needed for savings plans, which caused 403 on live purchases (issue #731). Add a custom role definition "CUDly Reservation and Savings Plan Purchaser" with the exact actions observed in the SDK call sites: Microsoft.Capacity: calculateprice/action, reservationorders/write, reservationorders/read, reservationorders/reservations/read, register/action, catalogs/read Microsoft.BillingBenefits: savingsPlanOrderAliases/write, savingsPlanOrders/read, savingsPlanOrders/savingsPlans/read, savingsPlanOrders/action The built-in Reservation Purchaser and Reader assignments are kept unchanged. The custom role uses a deterministic GUID derived from the subscription ID so re-deployments are idempotent. Validated: az deployment sub validate exits cleanly. Document re-deployment requirement in known-issues.md.
cristim
added
triaged
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR extends an ARM subscription deployment template to create and assign a subscription-scoped custom role that grants Microsoft.Capacity and Microsoft.BillingBenefits actions for reservation and savings-plan purchases, exposes the custom role ID as an output, and updates known-issues documentation with redeployment instructions and an added GCP log excerpt. ChangesARM Custom Role and Savings Plan Purchase Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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: 1
🧹 Nitpick comments (1)
known-issues.md (1)
22-27: ⚡ Quick winConsider adding
--no-promptflag for consistency and automation-friendliness.The deployment command is functionally correct but omits the
--no-promptflag used in the actual setup script. Adding it prevents unexpected interactive prompts during re-deployment, which is especially important for scripted or automated execution contexts.📝 Suggested addition for better operator experience
az deployment sub create \ --location eastus \ --template-file arm/CUDly-CrossSubscription/template.json \ --parameters servicePrincipalObjectId=<SP-object-id> \ - --name CUDly-CrossSubscription + --name CUDly-CrossSubscription \ + --no-promptAs per relevant code snippet from
arm/CUDly-CrossSubscription/setup.sh:123, the setup script includes--no-promptto ensure non-interactive execution.🤖 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 `@known-issues.md` around lines 22 - 27, Update the az deployment sub create invocation shown (the command using --template-file arm/CUDly-CrossSubscription/template.json and --parameters servicePrincipalObjectId=<SP-object-id>) to include the --no-prompt flag so the command becomes non-interactive and matches the setup.sh behavior; this ensures scripted or automated runs won’t hang on prompts during re-deployment.
🤖 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 61-63: The assignableScopes entry uses
subscriptionResourceId('Microsoft.Resources/subscriptions',
subscription().subscriptionId) which expands to the provider-scoped path;
replace that expression with the canonical subscription scope string such as
subscription().id (or concat('/subscriptions/', subscription().subscriptionId))
so assignableScopes contains "/subscriptions/{subscriptionId}" — update the
assignableScopes array value where the template defines assignableScopes.
---
Nitpick comments:
In `@known-issues.md`:
- Around line 22-27: Update the az deployment sub create invocation shown (the
command using --template-file arm/CUDly-CrossSubscription/template.json and
--parameters servicePrincipalObjectId=<SP-object-id>) to include the --no-prompt
flag so the command becomes non-interactive and matches the setup.sh behavior;
this ensures scripted or automated runs won’t hang on prompts during
re-deployment.
🪄 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: 93b51fd5-385d-4c92-a7d9-56ace5b0c7f7
📒 Files selected for processing (2)
arm/CUDly-CrossSubscription/template.jsonknown-issues.md
…rompt to known-issues example
- assignableScopes was using subscriptionResourceId('Microsoft.Resources/subscriptions',
subscription().subscriptionId) which expands to the provider-scoped path
/subscriptions/{id}/providers/Microsoft.Resources/subscriptions/{id} -- not the canonical
subscription scope /subscriptions/{id} that ARM RBAC expects. Switch to subscription().id
which returns exactly the right value. ARM validate returns error:null after the fix.
- known-issues.md redeploy command now includes --no-prompt, matching setup.sh line 122
and preventing interactive hangs in automated/scripted re-deployments.
Addresses CR findings on PR #732 (review 4359641918).
CR fixes (cca72f7)Two findings from the 2026-05-26 CodeRabbit review addressed: Actionable - Nitpick - @coderabbitai review |
|
Triggering a fresh review of the updated commits. ✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
Fixes #731 (P0). The Azure ARM template grants the CUDly SP the built-in
Reservation Purchaserrole (f7b75c60-3036-4b75-91c3-6b41c27c1689), which is misnamed: its actions cover only catalog/recommendations reads +register/action. None ofcalculatePrice/action,reservationOrders/write,reservationOrders/read,reservationOrders/purchase/action(the action that 403'd in the user-reported failure onStandard_D2d_v4) or any of the savings-plan billing-benefits actions are present.Adds a
Microsoft.Authorization/roleDefinitionscustom role that enumerates every concrete action the executors call at runtime, and a role assignment for that custom role at subscription scope. Existing built-in assignments (Reader,Cost Management Reader,Reservation Purchaser, tenant-scope Capacity) are preserved so we still get recommendations reads and so existing tenants degrade gracefully on partial redeploy.Actions enumerated (traced from SDK call sites)
Microsoft.Capacity:
calculateprice/action—internal/reservations/purchase.gostep 1reservationorders/write—internal/reservations/purchase.gostep 2 (POST .../reservationOrders/{id}/purchase)reservationorders/read,reservationorders/reservations/read—providers/azure/services/compute/exchange.goregister/action—ensureCapacityProviderRegisteredcatalogs/read— recommendation discoveryMicrosoft.BillingBenefits (savings plans):
savingsPlanOrderAliases/write—SavingsPlanOrderAliasClient.BeginCreatesavingsPlanOrders/read,savingsPlanOrders/savingsPlans/read— list flowssavingsPlanOrders/action—RPClient.ValidatePurchasefrom PR feat(commitmentopts/azure): probe Savings Plans offerings for live validation #724Tenant upgrade requirement
Existing CUDly users MUST redeploy
arm/CUDly-CrossSubscription/template.jsonto pick up the new custom role.known-issues.mdupdated with an Outstanding entry pointing at #731.Test plan
az deployment sub validatereturnserror: nullagainst a sandbox subscriptionStandard_D2d_v4purchase, confirm no 403Reservation Purchaserassignment still resolves recommendations reads after the custom role is addedSummary by CodeRabbit
New Features
Documentation