Repository navigation
Conversation
GetExistingCommitments lists savings plans with the tenant-wide ListAll endpoint (/providers/Microsoft.BillingBenefits/savingsPlans) and stamped every returned plan with the client's subscription. In a tenant with several subscriptions, each subscription reported every other subscription's plans as its own, exposing their IDs and amounts and double-counting committed spend. Keep only plans whose billingScopeId equals /subscriptions/<id> (case-insensitive), the same scope PurchaseCommitment buys under. A plan is billed to exactly one scope even when its applied scope is Shared, so each plan is now reported once, by the subscription that pays for it. Plans billed elsewhere or with no billing scope are skipped and counted in a warning log line. Closes #40
cristim
added
triaged
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughThe Azure Savings Plans client filters tenant-wide results to plans attributable to its subscription. It logs skipped plans, with details capped at 20 entries, and uses a shared helper to build subscription billing scopes for purchase and validation requests. ChangesAzure Savings Plan subscription filtering
Priority: ⬆️ High Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: High Merge Risk: ⚪ Minimal · up to No actionable issue is established from the supplied evidence; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
…ling account
ListAll responses on EA and MCA tenants report billingScopeId as a
billing account (/providers/Microsoft.Billing/billingAccounts/...), not
/subscriptions/{id}; see the SavingsPlansList.json, SavingsPlansInOrderList.json
and SavingsPlanItemGet.json examples in azure-rest-api-specs
billingbenefits 2022-11-01. Matching on the billing scope alone hid every
such plan, including from the duplicate-purchase check.
A plan is now ours when its billing scope is /subscriptions/{id}, or when
it is billed to a non-subscription scope, its applied scope type is
Single and appliedScopeProperties.subscriptionId is /subscriptions/{id}
(both compared case-insensitively). A plan billed to another subscription
stays that subscription's even when applied to ours. Shared plans billed
to a billing account cannot be attributed to one subscription and are
still skipped.
The skip warning now carries the total count and, for up to 20 plans,
each plan ID with its billing and applied scope.
|
Independent source/contract review of exact head
Proposed minimal correction: distinguish attributable rows, definitively foreign subscription billing, and unresolved attribution. Preserve safe foreign exclusion; return an error rather than a successful partial inventory on unresolved rows. Exercise the actual Azure SDK client through DuplicateChecker, with behavioral baseline/mutation evidence and a reachable purchase consumer proving zero purchase calls on that error. This does not implement full Shared-plan attribution or prove every direct purchase caller is protected. I am preparing the scoped plan and local verification while readiness #488 remains with its existing owner. Please flag overlapping ownership before edits to this PR branch. No source changes, tests, cloud calls or merges have been made for this review. Independent Astra replaces the unavailable exact reviewer under the user's instruction; the separate real-account acceptance standard is awaiting clarification. |
Return an error and no inventory when subscription ownership cannot be established. Preserve proven foreign subscription filtering and exercise the real SDK pager through public inventory and duplicate checking. Refs #40
1 participant
Azure Savings Plan inventory now returns only plans billed to the client's subscription and returns an error when ownership cannot be resolved. Previously, unresolved rows could disappear from a successful inventory result, weakening duplicate-purchase checks.
Subscription billing scope determines ownership. Applied benefit scope does not establish who pays. Known foreign subscription rows are excluded; missing, malformed, unknown or billing-account scopes fail the entire inventory rather than returning a partial list. Owned rows require a nonblank plan ID. Purchase and validation use the same subscription billing-scope helper.
Published candidate:
a1f9ad611ad32958fffa7c36ff52630bd0636006. The complete feature diff is three files, 263 changed lines. The candidate integrates mainac0f2696; the subsequent six-path delta to observed maineaa9f740was independently checked for compatibility.Local verification:
CI is running on the published commit. These are offline synthetic fixtures, not real Azure account or purchase acceptance. Real-account acceptance and the exact final-HEAD reviewer gate remain required; keep this PR open. Issue #40's broader purchase-orchestration acceptance remains unresolved.
Refs #40