Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 27 additions & 33 deletions arm/CUDly-CrossSubscription/template.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
"variables": {
"roles": {
"reader": "/providers/Microsoft.Authorization/roleDefinitions/acdd72a7-3385-48ef-bd42-f606fba81ae7",
"costManagementReader": "/providers/Microsoft.Authorization/roleDefinitions/72fafb9e-0641-4937-9268-a91bfd8191a3",
"reservationPurchaser": "/providers/Microsoft.Authorization/roleDefinitions/f7b75c60-3036-4b75-91c3-6b41c27c1689"
"costManagementReader": "/providers/Microsoft.Authorization/roleDefinitions/72fafb9e-0641-4937-9268-a91bfd8191a3"
},
"customRoleName": "[guid(subscription().subscriptionId, 'cudly-reservation-purchaser')]",
"customRoleDefinitionId": "[subscriptionResourceId('Microsoft.Authorization/roleDefinitions', guid(subscription().subscriptionId, 'cudly-reservation-purchaser'))]"
Expand All @@ -38,28 +37,32 @@
"apiVersion": "2022-04-01",
"name": "[variables('customRoleName')]",
"properties": {
"roleName": "CUDly Reservation and Savings Plan Purchaser",
"description": "Custom role granting CUDly exactly the Microsoft.Capacity and Microsoft.BillingBenefits actions it calls at runtime. Complements the built-in Reservation Purchaser assignment (which covers catalog reads and recommendations) by adding the purchase and calculatePrice actions that the built-in role omits.",
"roleName": "CUDly Reservation Purchaser (custom)",
"description": "Custom role granting CUDly exactly the Microsoft.Capacity and Microsoft.BillingBenefits actions required by the calculatePrice -> purchase flow. Replaces the built-in Reservation Purchaser, which lacks reservationOrders/purchase/action.",
"type": "CustomRole",
"permissions": [
{
"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"
Comment on lines 45 to 56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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
done

Repository: 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/action
  • Microsoft.Capacity/exchange/action
  • Microsoft.BillingBenefits/savingsPlanOrders/write
  • Microsoft.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.

],
"notActions": []
"notActions": [],
"dataActions": [],
"notDataActions": []
}
],
"assignableScopes": [
"[subscription().id]"
"[concat('/subscriptions/', subscription().subscriptionId)]",
"/providers/Microsoft.Capacity"
]
}
},
Expand All @@ -75,56 +78,47 @@
"roleDefinitionId": "[variables('customRoleDefinitionId')]",
"principalId": "[parameters('servicePrincipalObjectId')]",
"principalType": "ServicePrincipal",
"description": "CUDly — purchase reservations and savings plans via custom role that enumerates every required action explicitly"
"description": "CUDly — subscription-scope assignment of custom role; grants calculatePrice + purchase/action required by the two-step reservation purchase flow"
}
},

{
"type": "Microsoft.Authorization/roleAssignments",
"apiVersion": "2022-04-01",
"name": "[guid(parameters('servicePrincipalObjectId'), 'reader', subscription().subscriptionId)]",
"properties": {
"roleDefinitionId": "[variables('roles').reader]",
"principalId": "[parameters('servicePrincipalObjectId')]",
"principalType": "ServicePrincipal",
"description": "CUDly — enumerate subscription resources and locations"
}
},

{
"type": "Microsoft.Authorization/roleAssignments",
"apiVersion": "2022-04-01",
"name": "[guid(parameters('servicePrincipalObjectId'), 'costManagementReader', subscription().subscriptionId)]",
"scope": "/providers/Microsoft.Capacity",
"name": "[guid(parameters('servicePrincipalObjectId'), 'cudlyCustomRoleCapacity')]",
"dependsOn": [
"[variables('customRoleDefinitionId')]"
],
"properties": {
"roleDefinitionId": "[variables('roles').costManagementReader]",
"roleDefinitionId": "[variables('customRoleDefinitionId')]",
"principalId": "[parameters('servicePrincipalObjectId')]",
Comment on lines +88 to 95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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:


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.

"principalType": "ServicePrincipal",
"description": "CUDly — read cost and reservation utilisation data"
"description": "CUDly — tenant-capacity-scope assignment of custom role; required for reservationOrders/purchase/action at /providers/Microsoft.Capacity scope"
}
},

{
"type": "Microsoft.Authorization/roleAssignments",
"apiVersion": "2022-04-01",
"name": "[guid(parameters('servicePrincipalObjectId'), 'reservationPurchaser', subscription().subscriptionId)]",
"name": "[guid(parameters('servicePrincipalObjectId'), 'reader', subscription().subscriptionId)]",
"properties": {
"roleDefinitionId": "[variables('roles').reservationPurchaser]",
"roleDefinitionId": "[variables('roles').reader]",
"principalId": "[parameters('servicePrincipalObjectId')]",
"principalType": "ServicePrincipal",
"description": "CUDly — reservation catalog reads and recommendations via built-in Reservation Purchaser role (kept in addition to the custom role)"
"description": "CUDly — enumerate subscription resources and locations"
}
},

{
"type": "Microsoft.Authorization/roleAssignments",
"apiVersion": "2022-04-01",
"scope": "/providers/Microsoft.Capacity",
"name": "[guid(parameters('servicePrincipalObjectId'), 'reservationPurchaserCapacity')]",
"name": "[guid(parameters('servicePrincipalObjectId'), 'costManagementReader', subscription().subscriptionId)]",
"properties": {
"roleDefinitionId": "[variables('roles').reservationPurchaser]",
"roleDefinitionId": "[variables('roles').costManagementReader]",
"principalId": "[parameters('servicePrincipalObjectId')]",
"principalType": "ServicePrincipal",
"description": "CUDly — read + purchase Azure Reservations at tenant capacity scope. Supersets Reservation Reader (582fc458-8989-419f-a480-75249a578f9d), which is not provisioned in every tenant and would otherwise fail with RoleDefinitionDoesNotExist on those tenants."
"description": "CUDly — read cost and reservation utilisation data"
}
}
],
Expand Down
25 changes: 21 additions & 4 deletions iac/federation/azure-target/terraform/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,26 @@ resource "azuread_application_federated_identity_credential" "cudly" {
subject = var.cudly_federated_subject
}

# Reservation Purchaser is the built-in Azure role for purchasing and managing reservations.
# Custom role: grants the exact Microsoft.Capacity and Microsoft.BillingBenefits actions
# used by the calculatePrice -> purchase flow (introduced in PR #680). The built-in
# Reservation Purchaser role lacks reservationOrders/purchase/action, which causes 403 on
# production reservation purchases.
#
# The role definition is factored into a shared module so the customer-side (here) and
# host-side (terraform/modules/compute/azure/container-apps) definitions stay in lockstep.
module "cudly_reservation_role" {
source = "../../../../terraform/modules/iam/azure/cudly-reservation-role"
scope = data.azurerm_subscription.current.id
name_suffix = local.subscription_id
}

# Assign the custom role at subscription scope.
# depends_on ensures the role definition is fully propagated before the assignment
# is created (Azure RBAC propagation can take up to 10 minutes).
resource "azurerm_role_assignment" "cudly_reservations" {
scope = "/subscriptions/${local.subscription_id}"
role_definition_name = "Reservation Purchaser"
principal_id = azuread_service_principal.cudly.object_id
scope = "/subscriptions/${local.subscription_id}"
role_definition_id = module.cudly_reservation_role.role_definition_resource_id
principal_id = azuread_service_principal.cudly.object_id

depends_on = [module.cudly_reservation_role]
}
31 changes: 25 additions & 6 deletions terraform/modules/compute/azure/container-apps/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -249,16 +249,35 @@ resource "azurerm_role_assignment" "cost_management_reader" {
principal_id = azurerm_user_assigned_identity.container_app.principal_id
}

# Reservation Purchaser: allows writing reservationOrders so CUDly can
# purchase/exchange Azure reservations on behalf of users.
# Note: "Reservations Reader" does not exist as a built-in Azure role.
# Read access to reservations is covered by the Reservation Purchaser role itself.
resource "azurerm_role_assignment" "reservations_purchaser" {
# Reader: allows the host container-app identity to enumerate VMs, Redis,
# Cosmos, Search, SQL, and compute SKUs in the host subscription when the host
# account is ingested as a "Self" account. Without this, every per-service SDK
# list call against the host subscription returns 403.
resource "azurerm_role_assignment" "subscription_reader" {
scope = data.azurerm_subscription.current.id
role_definition_name = "Reservation Purchaser"
role_definition_name = "Reader"
principal_id = azurerm_user_assigned_identity.container_app.principal_id
}

# Custom reservation-purchaser role: same role as customer-side but scoped to
# the host subscription. The built-in Reservation Purchaser lacks
# reservationOrders/purchase/action (same gap fixed customer-side by PR #744).
# The role definition is factored into a shared module so both sides stay in
# lockstep with the ARM template.
module "cudly_reservation_role" {
source = "../../../iam/azure/cudly-reservation-role"
scope = data.azurerm_subscription.current.id
name_suffix = data.azurerm_subscription.current.subscription_id
}

resource "azurerm_role_assignment" "reservations_purchaser" {
scope = data.azurerm_subscription.current.id
role_definition_id = module.cudly_reservation_role.role_definition_resource_id
principal_id = azurerm_user_assigned_identity.container_app.principal_id

depends_on = [module.cudly_reservation_role]
}

# Key Vault Crypto User: allows the container app's managed identity
# to call Sign + GetKey on the OIDC signing key. The key never leaves
# the vault — the app only receives signatures computed inside Azure.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions terraform/modules/iam/azure/cudly-reservation-role/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
terraform {
required_version = ">= 1.5"

required_providers {
azurerm = {
source = "hashicorp/azurerm"
version = ">= 3.0"
}
}
}

# Custom role: grants the exact Microsoft.Capacity and Microsoft.BillingBenefits
# actions used by the calculatePrice -> purchase flow. The built-in Reservation
# Purchaser role lacks reservationOrders/purchase/action, which causes 403 on
# production reservation purchases.
#
# Used by both:
# - customer-side IaC (iac/federation/azure-target/terraform) for the
# customer SP that CUDly authenticates with via workload identity federation
# - host-side IaC (terraform/modules/compute/azure/container-apps) for the
# container-app user-assigned identity running the CUDly process itself
#
# Keep the actions list here in sync with arm/CUDly-CrossSubscription/template.json.
resource "azurerm_role_definition" "cudly_reservation_purchaser" {
name = "CUDly Reservation Purchaser (custom) - ${var.name_suffix}"
scope = var.scope
description = "Custom role granting CUDly exactly the Microsoft.Capacity and Microsoft.BillingBenefits actions required by the calculatePrice -> purchase flow. Replaces the built-in Reservation Purchaser, which lacks reservationOrders/purchase/action."

permissions {
actions = [
"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",
]
not_actions = []
}

assignable_scopes = [
var.scope,
"/providers/Microsoft.Capacity",
]
}
4 changes: 4 additions & 0 deletions terraform/modules/iam/azure/cudly-reservation-role/outputs.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
output "role_definition_resource_id" {
description = "Full ARM resource ID of the custom role definition. Use as role_definition_id in azurerm_role_assignment blocks."
value = azurerm_role_definition.cudly_reservation_purchaser.role_definition_resource_id
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
variable "scope" {
description = "Subscription resource ID used as the role definition scope and the base of assignable_scopes (e.g. data.azurerm_subscription.current.id)."
type = string
}

variable "name_suffix" {
description = "Suffix appended to the role display name to keep customer and host role definitions distinct within the same Azure AD tenant (e.g. the subscription ID)."
type = string
}
Loading