[cristim]

Summary

This is PR B of the #272 slice. It is stacked on #467 (PR A), so merge #467 first, then retarget this PR to main so CI runs.

Since #207, Azure savings plan inventory has failed closed. GetExistingCommitments errors for every plan whose billingScopeId is not the client subscription. A tenant that buys savings plans at billing-account scope (EA or MCA) therefore gets no inventory and no duplicate check. This slice handles the case that can be attributed without guessing: a plan billed to a billing account whose applied scope is Single and names exactly one subscription.

How each billing scope and applied scope is handled (scope.go):

Billing scope	Applied scope	Result
/subscriptions/<sub>	any	Unchanged from #40/#207: owned if it is the client sub, otherwise skipped. A malformed scope is an error.
EA /providers/Microsoft.Billing/billingAccounts/{id}, or MCA .../billingAccounts/{id}/billingProfiles/{id}	Single on the client sub (or an RG in it)	Attributed to the client sub
EA / MCA	Single on another sub	Skipped. It cannot cover the client sub.
EA / MCA	Single with missing or blank scope, a sub/RG disagreement, or a non-blank management group	Error
EA / MCA	Shared, ManagementGroup, nil, or any unknown value	Error. Fails closed as before.
anything else	any	Error

• Billing-scope matching. Exactly the two documented shapes are accepted. Each literal segment is compared case-insensitively. Empty segments, a trailing slash, any other suffix, whitespace and control characters are rejected. MCA ids may contain : and _.
• Subscription and RG parsing. SubscriptionID must be exactly /subscriptions/<sub>. ResourceGroupID must be exactly /subscriptions/<sub>/resourceGroups/<rg>. A blank SubscriptionID is treated as unset. When both are present they must agree. TenantID is ignored, because a subscription GUID belongs to one tenant.
• Logging. Errors and logs never contain the billing-account id. When N > 0, one aggregate line per call reports the N skipped plans.

Out of scope.

• Shared billing-account plans stay fail-closed. Attributing them is a product decision on feat(azure): attribute billing-account-scoped Shared savings plans instead of failing inventory #272.
• Plans billed to another subscription but applied to ours are a separate pre-existing gap with duplicate-buy risk. It is tracked in fix(azure): duplicate check misses savings plans billed to another subscription but applied to ours #466 and noted in code.

Refs #272, #466

Verification (macOS, Go 1.26.9)

Commands

• (cd providers/azure && GOWORK=off go test -race -short ./...) passes.
• make build and make vet pass.
• golangci-lint v2.10.1 (the CI pin) reports 0 issues.
• gofmt and gocyclo are clean.

Tests

• TestPlanOwnedBy_BillingAccountScopes covers both shapes, the suffix, trailing-slash and empty-profile rejections, blank-sub-with-RG, nil properties, an unknown enum, a management group, a sub/RG disagreement, and mixed-case literals. It also asserts that no error contains the account id.
• TestGetExistingCommitments_SDKBillingAccountSingleScope runs SDK-shaped JSON through the real pager into DuplicateChecker for five cases: ea-ours, mca-rg-ours, ea-other, single-no-subscription and shared.
• TestGetExistingCommitments_AggregateSkipLog is non-parallel and restores the log writer.
• In client_test.go, only account-single-ours and account-single-other changed expectation.

Fail-before. These fail on PR A's code: those two rows, 10 scope subtests, and the SDK cases ea-ours, mca-rg-ours and ea-other. The independent reviewer re-ran this after the tests were compacted.

Deletion probes. The reviewer ran these in scratch copies, and each fails a test:

• drop the sub/RG disagreement check
• drop the fail-closed arm
• drop the blank-as-nil handling
• drop the management-group check
• accept any billing-account suffix
• make the billingProfiles, resourceGroups or account-prefix comparison case-sensitive

Review. The plan had two passes and the diff had two passes, all by the same non-author reviewer. Pass 1 added strict scope shapes, the sub/RG rules and the aggregate log, and filed #466. Diff pass 1 added the mixed-case cases. No actionable findings remain on d7296ee.

This uses SDK-shaped fixtures only. No Azure call or purchase was made.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 16 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 68 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 95058b7b-ef18-4e46-bdbe-16f7b91de501

📥 Commits

Reviewing files that changed from the base of the PR and between 926d1d5 and c6457a8.

📒 Files selected for processing: 4

• providers/azure/services/savingsplans/client.go
• providers/azure/services/savingsplans/client_test.go
• providers/azure/services/savingsplans/scope.go
• providers/azure/services/savingsplans/scope_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent adversarial review: CLEAN at d7296ee0a278d3848dfa7b809dc031c1e9e71e0d

Scope: this PR's own delta over #467 (e1d3708..d7296ee). #467 is reviewed separately. Reviewer: an independent agent that did not write the change (Sonnet, high effort), cold brief; the PR body was treated as a claim.

Blocking findings: none.

What was attacked (against armbillingbenefits v2, constants.go:31-37, models.go:15-30):

• AppliedScopeType: only Single is accepted. Shared, ManagementGroup, nil, unknown values and a lowercase single all error (fail closed, so no duplicate-buy path).
• AppliedScopeProperties: a non-blank management group errors; nil properties error; a blank SubscriptionID is treated as unset; a sub/RG disagreement errors (case-insensitive); a malformed RG id errors, including a trailing slash and extra or empty segments.
• Billing scope: exactly the EA (1 segment) and MCA billingProfiles (3 segments) shapes, with literal segments compared case-insensitively. A trailing slash, empty segments and extra segments fall through to an error. Client-sub comparison is EqualFold on both paths.
• Single scope on another subscription is skipped. That is correct, since the plan cannot cover this subscription. Plans billed to another subscription but applied to ours remain the pre-existing gap tracked in fix(azure): duplicate check misses savings plans billed to another subscription but applied to ours #466.
• Logging: the only new log line is an aggregate count, and all new error strings are static. No billing scope or account id reaches a log or error.
• fix(azure): cap the savings-plan inventory pager and honor ctx cancellation #459 (c2b1511, page cap in the same client.go) is already in the refactor(azure): move savings-plan ownership checks into scope.go #467 base; no semantic conflict with this delta.
• Mutation probes (scratch copy, reverted): dropping the sub/RG disagreement check, or letting Shared past the Single guard, each makes TestPlanOwnedBy_BillingAccountScopes fail.

Non-blocking: a whitespace-only SubscriptionID next to a valid RG id is treated as unset and the RG id drives attribution. This is safe.

Local verification (macOS, Go 1.27.2, exact SHA d7296ee)

Fixture-based evidence (SDK-shaped JSON through the real pager; no Azure call):

• Fail-before (behaviour level): with client.go and scope.go from e1d3708 and the new scope_test.go set aside (it references the new API), TestGetExistingCommitments_SubscriptionOwnership/account-single-ours and /account-single-other FAIL.
• Pass-after: (cd providers/azure && GOWORK=off go test -race -short ./...) exit 0 (13 packages ok); make build and make vet pass.
• Real-account gap: not exercised against a live EA/MCA tenant; not required to merge per the review gate.

Labels mirror #272. CodeRabbit is rate-limited; if that holds at merge time, the merge note will carry the quota waiver.

Next: after #467 merges, this PR is retargeted to main and rebased. If git range-diff shows the commit unchanged, this verdict carries over; CI then runs on the new head.

[cristim]

#467 merged as 926d1d5. Retargeted this PR to main and rebased its own commit onto it (git rebase --onto origin/main e1d3708). New head c6457a84e4fa8ca8c8d5453461c00a88ef6bd386.

git range-diff e1d3708..d7296ee origin/main..c6457a8:

1:  d7296ee = 1:  c6457a8 fix(azure): attribute single-scope savings plans billed to a billing account

The commit is identical, so the CLEAN verdict for d7296ee carries over to c6457a8. On the new head, make build passes and go test -race ./services/savingsplans/ (providers/azure, GOWORK=off) is ok. CI is now running against main.

[cristim]

CI is green on head c6457a84 after the retarget to main: pre-commit 38969601546 and Build & Test 38969601718.

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Pull request is closed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Merged by squash at the reviewed head c6457a84e4fa8ca8c8d5453461c00a88ef6bd386 as 011be8f. Gates met on that SHA:

• Independent adversarial review CLEAN for d7296ee; carried over to c6457a8 by an identical git range-diff after retargeting to main once refactor(azure): move savings-plan ownership checks into scope.go #467 merged.
• Local fixture verification: fail-before and pass-after.
• CI green: pre-commit 38969601546, Build & Test 38969601718.
• Labels mirrored from feat(azure): attribute billing-account-scoped Shared savings plans instead of failing inventory #272. No unresolved threads.

CR waived: quota, adversarial review + local verification + green CI. #272 stays open: shared-scope billing-account plans are a product decision, and #466 covers plans billed to another subscription. A retrospective CodeRabbit review will run when quota returns.