fix(azure): Legacy (EA) recommendations pair the normalized SKU with the un-normalized quantity

[cristim]

Reviewed commit: be11bdcb5 (origin/main), from the 2026-07-28 full-repo review.

Severity: CRITICAL. Azure EA customers buy a fraction of the recommended capacity while the dashboard reports the recommendation as fully actioned.

Where

• providers/azure/internal/recommendations/converter.go:146 and :151-152, via resolveLegacyResourceType at :228-230
• Consumed by every service purchase body, e.g. providers/azure/services/compute/client.go:421-426

What

extractLegacy sets ResourceType from props.NormalizedSize (preferred first in the ladder) but sets Count from props.RecommendedQuantity.

Azure's armconsumption model carries two distinct quantity fields: RecommendedQuantity (units of the ACTUAL SKU, i.e. SKUProperties[SKUName]) and RecommendedQuantityNormalized (units of NormalizedSize). See armconsumption@v1.1.0/models.go:715-721. The code mixes the normalized size with the un-normalized count.

The Modern path (:243-247) prefers SKUName first and is therefore self-consistent. Only the Legacy/EA path is mismatched.

Failure scenario

EA subscription. Azure recommends 4 x Standard_D8s_v3 with NormalizedSize="Standard_D2s_v3", RecommendedQuantity=4, RecommendedQuantityNormalized=16.

The converter yields ResourceType="Standard_D2s_v3", Count=4. buildReservationBody sends sku.name="Standard_D2s_v3", quantity=4. The customer buys one quarter of the recommended capacity and keeps paying on-demand for the rest, while the dashboard reports the recommendation as fully actioned.

The error scales with the family's instance-flexibility ratio, up to 16x for a large-to-smallest normalization.

Scope (added after investigation, PR #1771)

Seven services consume this pairing through the one shared recommendations.Extract, not just the compute example above: compute, cache, cosmosdb, database, managedredis, search, synapse.

Count also feeds provider-agnostic logic in cmd/helpers.go, so the wrong count reached sizing math and duplicate suppression as well as the purchase body:

• applySizing / ApplyCoverage scale by newCount / rec.Count
• ApplyTargetCoverage anchors on rec.Count
• --max-instances clamps on it
• the duplicate checker compares existingCount >= rec.Count and subtracts

Verified it reaches a purchase, not only a display. The pair becomes sku.name and quantity on the calculatePrice and purchase request bodies, and Azure accepts it because both values are individually valid. The only quantity guard on the path is rec.Count <= 0.

Direction: under-buy — fails safe on commitment risk, unsafe on reported savings. Nobody is locked into capacity they cannot use, but EstimatedSavings is Azure's figure for the full recommended quantity and rides along unchanged, so the savings number shown to whoever approves the purchase is wrong. A decision-quality defect rather than a spend defect.

Checked negative: Modern/MCA's primary path is NOT affected

resolveModernResourceType prefers the top-level SKUName, which RecommendedQuantity counts, so the rung real MCA payloads take is self-consistent. Confirmed by running the converter, not assumed. Do not "fix" that rung.

(PR #1771 does pair Modern's fallback rung, which had the same mismatch when SKUName is absent, while leaving the SKUName rung byte-identical.)

Fix direction

In extractLegacy, either pair NormalizedSize with RecommendedQuantityNormalized, or make resolveLegacyResourceType prefer SKUProperties[SKUName] (matching the Modern ladder) so that RecommendedQuantity is in the right units. Assert the pairing in a converter test.

Note

Neither providers/azure nor providers/gcp is covered by CI (#1478), so nothing in this package has been vetted by go vet, golangci-lint, or the unit/integration suites.

[cristim]

Investigation before implementation — premise confirmed by execution, scope wider than filed

Verified against origin/main @ 9bfd6822c in a throwaway worktree. Every claim below is from a run, not a read.

1. Still true on main? Yes.

Drove the unmodified converter with the issue's exact scenario (Azure recommends 4 × Standard_D8s_v3, NormalizedSize="Standard_D2s_v3", RecommendedQuantity=4, RecommendedQuantityNormalized=16):

LEGACY                      ResourceType="Standard_D2s_v3"  Count=4     <-- mismatched
MODERN                      ResourceType="Standard_D8s_v3"  Count=4     <-- self-consistent
LEGACY (no NormalizedSize)  ResourceType="Standard_D8s_v3"  Count=4     <-- self-consistent

Both halves of the issue's claim hold: Legacy mixes the normalized SKU with the un-normalized count, and Modern does not.

RecommendedQuantityNormalized appears nowhere in the repo — not in providers/, not in the fixture builders. The field is never read, so no path currently produces the correct normalized pair.

The issue's line references are still accurate: converter.go:146 (ResourceType), :151-152 (Count), :228-230 (resolveLegacyResourceType).

2. Does it reach a purchase, or only a display? A purchase.

Carried the same payload through the compute converter into buildReservationBody and printed the bytes that go to calculatePrice and purchase:

{"location":"eastus",
 "properties":{"appliedScopeType":"Shared","billingPlan":"Upfront",
   "displayName":"vm-eastus-Standard_D2s_v3-4x-1yr-allup-...",
   "quantity":4,"renew":false,"reservedResourceType":"VirtualMachines","term":"P1Y"},
 "sku":{"name":"Standard_D2s_v3"}}

sku.name is the normalized SKU and quantity is the un-normalized count, on the wire. Azure has no reason to reject it: Standard_D2s_v3 is a real SKU and 4 is a valid quantity. The purchase succeeds and the customer is committed for the term.

The only quantity guard on the path is rec.Count <= 0 (compute/client.go:488). Nothing anywhere validates that the SKU and the count are in the same units.

And the money figures ride along unchanged. Same run:

common.Recommendation: ResourceType="Standard_D2s_v3" Count=4
                       OnDemandCost=1000 CommitmentCost=600 EstimatedSavings=400

EstimatedSavings is Azure's saving for the full recommended quantity — the converter says so itself at :161-167 ("NetSavings is the savings from buying the full recommended quantity"). So the dashboard reports $400/mo against a purchase that delivers roughly a quarter of it. That is the substance of "reported as fully actioned", and it is verifiable in-repo rather than inferred.

3. Which direction does it fail? Under-buy — fails safe on commitment risk, unsafe on reported savings.

The code pairs the smaller SKU with the smaller count, so it buys less capacity than intended. It cannot over-commit the customer into unusable capacity.

Strictly, the direction is decided by Azure's data, not by the code: under-buy iff NormalizedSize ≤ the actual SKU. The armconsumption@v1.1.0 doc comments are terse ("The recommended Quantity Normalized") and do not state units, so that half rests on Azure's documented normalize-to-base-size behaviour rather than on anything in this repo. In-repo corroboration: every one of the 34 Legacy fixture call sites treats NormalizedSize as the small/base size — Standard_D2s_v3, GeneralPurpose_Gen5_2, 100RU, DW500c, Premium_P1.

So: the customer spends real money on a 1–3 year commitment sized at a fraction of intent, keeps paying on-demand for the remainder, and sees the full-size savings number. Bad, but not the worse direction.

4. Does the same logic serve other paths? Yes — considerably wider than the issue states.

The issue says "every service purchase body, e.g. compute/client.go:421-426". It is seven services, all through the one shared recommendations.Extract:

compute, cache, cosmosdb, database, managedredis, search, synapse.

The Modern/MCA path is not affected — resolveModernResourceType prefers the top-level SKUName, so it is self-consistent (confirmed in the run above). This is a Legacy/EA-only defect, as filed.

Not mentioned in the issue, and worth adding: Count also feeds provider-agnostic CLI logic in cmd/helpers.go —

• applySizing / ApplyCoverage scale by newCount / rec.Count (:173-176),
• ApplyTargetCoverage anchors on rec.Count (:207, :397-398),
• --max-instances clamps on it (:528),
• the duplicate checker compares existingCount >= rec.Count and subtracts (:686-698).

None of that is gated on provider, so an Azure EA recommendation carries the wrong count into sizing math and into duplicate suppression as well as into the purchase body.

5. Fix shape and size

Two options, both named in the issue. Recommending A:

Option A — make resolveLegacyResourceType prefer SKUProperties[SKUName], matching the Modern ladder. RecommendedQuantity is then already in the right units.

• Legacy and Modern end up with one semantics — today an EA and an MCA customer with identical usage get different ResourceType.
• Uses RecommendedQuantity (*float64) rather than RecommendedQuantityNormalized (*float32), avoiding a precision question on a money path.
• Introduces no new "field present, its partner absent" state.
• Test blast radius measured, not guessed: of 35 Legacy fixture call sites, 34 set NormalizedSize only (so the ladder still returns it and they are unchanged) and exactly one sets both — converter_test.go:101, TestExtract_ResourceTypePrefersNormalizedSize, which inverts.

Option B — keep the NormalizedSize preference and source Count from RecommendedQuantityNormalized. Preserves the SKU shown today, but needs resolveLegacyResourceType to report which rung it used, a new fixture helper, and a decision for "normalized size present, normalized quantity absent". Note there is no empty-ResourceType guard anywhere in providers/azure, so a fall-through in that state would put an empty sku.name on the wire. Larger, and it adds a failure mode.

Files: providers/azure/internal/recommendations/converter.go (~4 lines), providers/azure/internal/recommendations/converter_test.go (invert one test, add pairing coverage both directions). Option B additionally needs providers/azure/mocks/recommendation_fixtures.go. effort/s is right for A.

Flag for the PR, not to be slipped in silently: either option changes what gets purchased for EA customers. Under A the purchased SKU changes from the normalized size to the actual size for every Legacy recommendation that carries both fields — which is the common real-world payload shape.

Triage

Premise confirmed, severity/critical defensible, labels unchanged. Two corrections to the issue text: the consumer count is seven services rather than one example, and the blast radius includes provider-agnostic CLI sizing and duplicate suppression in cmd/helpers.go, which the issue does not mention.

Module state

providers/azure baseline on main before any change: go build ./..., go vet ./..., go test -race -count=1 ./... all exit 0 — 12 ok + 2 no-test-files = 14 packages, 0 failures. No pre-existing breakage to work around, now that #1755 has the module running in CI.