fix(azure/savingsplans): send Compute_Savings_Plan as sku.name on purchase and validate

[IAmJSD]

Root cause

savingsplans.Client.PurchaseCommitment and buildValidateBody copied SavingsPlanDetails.PlanType straight into sku.name. CUDly's PlanType is "Compute" (from GetRecommendations / benefit recommendations, #371 / #373, and GetValidResourceTypes). Azure names the SKU Compute_Savings_Plan. That is the armSkuName the Benefit Recommendations API returns (this package already checks it as armSkuComputeSavingsPlan), and the armbillingbenefits v1.0.0 SDK examples generated from the REST spec (SavingsPlanOrderAliasCreate.json, SavingsPlanOrderAliasCreateSingleScope.json, SavingsPlansPurchaseValidate.json) send it as sku.name. So every Azure savings plan purchase and validate request sent sku.name: "Compute".

Fix

The issue asked for a decision on the vocabulary. This PR maps at the request boundary, so the PlanType vocabulary stays as it is:

• New savingsPlanSKUName(planType) in client.go: "Compute" and "Compute_Savings_Plan" both map to "Compute_Savings_Plan". Any other value returns an error.
• PurchaseCommitment and buildValidateBody (used by ValidateOffering) use it. An unmapped PlanType now fails before any Azure call (BeginCreate / ValidatePurchase are never reached), instead of sending a sku.name the API would reject. The match is exact: no case folding and no trimming, the same strictness as commitmentCurrency.
• displayName (cudly-Compute-1yr), GetOfferingDetails.ResourceType and recommendation output are unchanged.

Closes #387

Verification

All of this is unit / mock / fixture evidence. No real Azure API was called and nothing was purchased. Tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because this machine's default AWS profile is invalid. That has no effect on the Azure module.

New tests in providers/azure/services/savingsplans/sku_name_test.go:

• TestSKUName_ComputePlanTypeSendsARMSKU: checks the sku.name captured from both the BeginCreate body and the ValidatePurchase body, for PlanType Compute and Compute_Savings_Plan.
• TestSKUName_RecommendationRoundTrip: takes a recommendation from GetRecommendations on the existing 2024-08-01 benefit-recommendation fixture row (armSkuName: Compute_Savings_Plan), purchases and validates it, and checks that the same SKU is sent.
• TestSKUName_UnknownPlanTypeFailsClosed: "", MachineLearning, compute, " Compute" and "Compute_Savings_Plan " each return an error, and the alias and RP mocks record zero calls.

Fail before (fix stashed, on 7383c3a):

$ cd providers/azure && go test ./services/savingsplans/ -run 'TestSKUName_(ComputePlanType|RecommendationRoundTrip)' -count=1
--- FAIL: TestSKUName_ComputePlanTypeSendsARMSKU   expected: "Compute_Savings_Plan"  actual: "Compute"
--- FAIL: TestSKUName_RecommendationRoundTrip      expected: "Compute_Savings_Plan"  actual: "Compute"
FAIL

(Before the fix, TestSKUName_UnknownPlanTypeFailsClosed also failed, because the purchase went through to the mock.)

Pass after:

$ cd providers/azure && go test ./services/savingsplans/ -run TestSKUName -count=1 -v   # all 3 tests + subtests PASS
$ cd providers/azure && go test -race -short ./... -count=1                            # ok (whole azure module)
$ cd providers/azure && go build ./... && go vet ./...                                 # clean
$ cd providers/azure && golangci-lint run --timeout=5m --allow-parallel-runners ./...  # v2.10.1: 0 issues
$ gocyclo -over 10 providers/azure/services/savingsplans/client.go                     # silent
$ bash scripts/gofmt-hook.sh <changed files>; make tidy-check                          # clean
$ bash scripts/gosec-hook.sh <changed files>                                           # gosec v2.28.0: clean
$ bash scripts/test-git-secrets-allowlist.sh                                           # 44 PASS, 0 FAIL
$ git secrets --scan   (fresh git init copy of tracked files, isolated HOME)           # clean

Mutation probes. Each one was applied by hand, the tests were run, and the change was reverted:

• validate body sends PlanType again: ComputePlanType and RoundTrip fail
• purchase body sends PlanType again: ComputePlanType and RoundTrip fail
• map unknown PlanTypes to Compute_Savings_Plan instead of failing: UnknownPlanTypeFailsClosed fails
• case-insensitive or trimmed match (compute, " Compute"): UnknownPlanTypeFailsClosed fails

Real-cloud gap: the expected sku.name comes from the SDK's spec-generated examples and from the Benefit Recommendations armSkuName. It has not been checked against a live SavingsPlanOrderAlias PUT or validate call, and no purchase was made. The first real purchase or validate run would confirm it.

Out of scope (deliberately not changed)

• GetValidResourceTypes still lists "MachineLearning". I don't know of a billingbenefits SKU name for it, so a MachineLearning PlanType now fails closed with a clear error instead of sending sku.name: "MachineLearning". Whether to remove it from the list, or map it if Azure has such a SKU, is a separate decision.
• fetchOnDemandRate puts the plan type into a Retail Prices armSkuName eq '<planType>' filter. It has no non-test callers, and it is unclear whether the right value there is a VM SKU or the savings plan SKU. Left alone.
• client_test.go is already over the 500-line guideline, so the new tests went into their own file instead of being added to it.

Maintainer: please mirror the issue's triage labels on this PR: priority/p2, severity/medium, urgency/this-quarter, impact/few, effort/s, type/bug, triaged.

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

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

[cristim]

Independent review verdict: CLEAN at 3251bd6

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

What was verified:

• SDK source of truth:
  • In armbillingbenefits v1.0.0, SKU.Name is a free string with no constant (models.go:371).
  • The spec-generated examples send Compute_Savings_Plan: savingsplanorderalias_client_example_test.go:45,87, rp_client_example_test.go:49,68 and savingsplan_client_example_test.go:150.
  • So the mapping is correct. It reuses the existing armSkuComputeSavingsPlan constant (benefit_recommendations.go:35), so there is no duplicate mapping.
• Only two places send sku.name (purchase and buildValidateBody), and both use savingsPlanSKUName. displayName (client.go:348) and OfferingDetails.ResourceType (client.go:549) still use the raw PlanType and are unchanged. The idempotency alias comes from opts.IdempotencyToken, not from PlanType.
• Producers:
  • The only Azure producer of SavingsPlanDetails.PlanType is benefit_recommendations.go:251, which sets Compute.
  • cloud-commitments-cli only reads PlanType.
  • cloud-commitments-platform treats SavingsPlanDetails generically and builds no Azure PlanType.
  • So no real producer yields a value that now errors.
• Fail-closed: the mapping error returns at client.go:343-347 (purchase) and :444-447 (validate). Both are before any client or poller creation and before BeginCreate or ValidatePurchase. result.Success stays false and nothing is half-written. The tests assert zero mock calls.

Non-blocking notes:

• GetValidResourceTypes (client.go:591) still lists MachineLearning, which now fails closed at purchase and validate. Before, the request went out but Azure would have rejected it. Dropping or mapping it is a follow-up, as the body says.
• The expected SKU name is backed by the spec and SDK examples only. It has not been confirmed with a live call, as the body discloses.

What was attacked and how:

• cd providers/azure && go test -race -short ./... -count=1: all packages pass. golangci-lint v2.10.1 reports 0 issues.
• Mutation probes, each reverted:
  • purchase mapping reverted to the raw PlanType: 3 tests fail;
  • validate mapping reverted: 3 tests fail;
  • case-insensitive match: 2 tests fail;
  • unknown PlanType mapped to Compute: UnknownPlanTypeFailsClosed fails.
• Fail-before: with the parent client.go and the PR's tests, all 3 new tests fail.

Gate:

• Labels need mirroring from fix(azure/savingsplans): purchase path sends PlanType "Compute" as sku.name; the API expects Compute_Savings_Plan #387: priority/p2, severity/medium, urgency/this-quarter, impact/few, effort/s, type/bug, triaged.
• When I checked, Unit and Integration Tests were still pending, and every other check had passed.
• The draft needs to be marked ready, and CodeRabbit must review it then.

[cristim]

Independent adversarial review: CLEAN at 3251bd64fc005fb80b91d3f077632f8f98965825

Reviewer: an independent agent that did not write this change (Sonnet, high effort), cold brief, full diff. The PR body was treated as a claim.

Blocking findings: none.

SDK evidence (armbillingbenefits@v1.0.0): sku.name is Compute_Savings_Plan.

• Alias create: savingsplanorderalias_client_example_test.go:45 and :87.
• Validate purchase: rp_client_example_test.go:49 and :68.
• Plan get: savingsplan_client_example_test.go:150.

So main sends a SKU name the API does not use on every Azure savings plan purchase and validate.

Coverage and ordering:

• Only two builders set SKU{Name}: PurchaseCommitment (client.go ~351) and buildValidateBody (~453). Both now use savingsPlanSKUName, and neither passes PlanType raw.
• savingsPlanSKUName runs after commitmentCurrency and before OrderIDFromToken (fix(azure/savingsplans)!: refuse a malformed idempotency token before the purchase PUT #495 on main), the alias client, BeginCreate and ValidatePurchase. The tests assert zero alias and RP calls on an unmapped plan type.
• Matching is exact: compute, Compute and Compute_Savings_Plan fail, and the tests cover them.

Against current main (4e7ef1e): git merge-tree is clean. On the merged tree, the savings plan tests pass (178), and make build and make vet are clean. #494's savingsPlanResourceType (read side, EqualFold) and this PR's write-side map share the armSkuComputeSavingsPlan constant, so they cannot disagree on the ARM name. A single shared table would be tidier but is optional (non-blocking).

Mutation probes (scratch copy, reverted):

• Passing PlanType through raw in both builders fails TestSKUName_ComputePlanTypeSendsARMSKU/Compute and TestSKUName_RecommendationRoundTrip.
• Accepting any value fails TestSKUName_UnknownPlanTypeFailsClosed.

Local verification (macOS)

Mock evidence: the real request body structs are captured at the mock BeginCreate/ValidatePurchase clients. No Azure call and no purchase were made.

• Fail-before: with PlanType sent raw, the two SKU tests above FAIL.
• Pass-after at 3251bd6: (cd providers/azure && GOWORK=off go test -race -short ./services/savingsplans/...) passes (159 tests). make build and make vet pass. Fork CI is green on this head (pre-commit 39025997109, Build & Test 39025997108).

Labels mirror #387.

Needed before merge. @IAmJSD:

1. Mark the PR ready for review.
2. Merge main (it merges cleanly).

[cristim]

Correction to my verdict above: that review ran on Sonnet. This repo's review gate (CLAUDE.md:161-162) requires the independent review on Opus 5.5 (claude-opus-5-5), so the verdict above does not satisfy the gate. Treat its findings as advisory, and do not merge on it. An Opus review naming the head SHA is still required. (claude-cc-reviewer)

[cristim]

Retraction of my correction above: it was wrong. I read the review gate from a stale local checkout. On origin/main, the gate has had no model pin since #279 (2026-10-08): "an independent adversarial review of the full PR diff ... names the exact head SHA. No model pin". The Sonnet verdict above therefore stands as a valid independent review. Sorry for the noise. (claude-cc-reviewer)

[cristim]

Merge driver status: draft; waiting on the author to mark it ready. The independent verdict is posted above. Once it is un-drafted, I will update the branch with main (as the author agreed on #423), wait for CI and merge.