Skip to content

fix(azure): commitments fabricate State "active" and Region from fields the SDK omits #74

Open
Open
fix(azure): commitments fabricate State "active" and Region from fields the SDK omits#74
Labels
effort/mDaysimpact/manyAffects most userspriority/p1Next up; this sprintseverity/highSignificant harmstatus/needs-infoAwaiting clarification from the askertriagedItem has been triagedtype/bugDefecturgency/this-sprintWithin the current sprint

Description

@cristim
cristim
opened on Sep 2, 2026

Summary

The same six Azure converters stamp a state of active and the client's own region onto every commitment, but the SDK response type carries neither field. The reservation listing is subscription-wide and unfiltered by region, so a reservation in one region is labelled with whichever region the client was constructed for and matched against recommendations it does not discount. The unconditional active state means an expired or cancelled reservation still counts as live coverage. The grouped sibling finding is that the same converters leave count, start date, end date and cost unset, so coverage arithmetic sees zero committed capacity and every Azure commitment looks long expired.

Location

  • providers/azure/services/cache/client.go:251 (identical in cosmosdb/client.go:252, database/client.go:282, search/client.go:197, synapse/client.go:223, managedredis/client.go:223)
  • providers/azure/services/cache/client.go:260 (identical in the five sibling converters) (A08b-007)

All at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd

Failure scenario

ReservationDetailProperties has no state and no region field, so both values are invented. The reservation listing is subscription-wide and unfiltered by region, so a westeurope reservation returned to a client constructed for eastus is stamped Region: "eastus" and matched against eastus recommendations that it does not discount. State: "active" is unconditional, so an expired or cancelled reservation still counts as live coverage and suppresses a needed repurchase.

The same defect appears at:

  • A08b-007: Azure commitments never populate Count, StartDate, EndDate or Cost

Evidence

commitment := &common.Commitment{
	Provider:       common.ProviderAzure,
	Account:        c.subscriptionID,
	CommitmentType: common.CommitmentReservedInstance,
	Service:        common.ServiceCache,
	Region:         c.region,
	State:          "active",
}

Suggested fix

Source region and state from an inventory API that reports them (armreservations.ReservationResponse has Location and ProvisioningState), and leave them empty rather than guessing when unavailable.

Found by the 2026-09-02 codebase audit, finding A08b-006, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Activity

  1. cristim
    added
    triagedItem has been triaged
    priority/p1Next up; this sprint
    severity/highSignificant harm
    urgency/this-sprintWithin the current sprint
    impact/manyAffects most users
    effort/mDays
    type/bugDefect
    on Sep 2, 2026
  2. cristim
    added
    triagedItem has been triaged
    urgency/this-sprintWithin the current sprint
    priority/p1Next up; this sprint
    severity/highSignificant harm
    on Sep 27, 2026
  3. cristim
    transferred this issue fromLeanerCloud/cloud-commitments-clion Sep 27, 2026
  4. cristim
    added
    impact/manyAffects most users
    effort/mDays
    type/bugDefect
    on Sep 27, 2026

  5. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    Source investigation also confirmed the same fabricated region/active-state and missing quantity/date fields in the Compute converter at main 9962786. Filed #190 for that sibling path so a six-converter repair does not leave Compute behind. This is source evidence, not live subscription acceptance; shared inventory scoping and cost semantics still need to be established before implementation.

  6. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    Further source and contract review for #74 and #190 found that replacing the converters alone would leave incorrect matching.

    • The cached armreservations/v4 v4.0.0 SDK exposes actual child ID, location, SKU quantity, provisioning state, benefit start, purchase timestamp, precise expiry, and applied scope. EffectiveDateTime is the start of the current reservation revision, not the original purchase. Current pkg/recfilter/dedupe.go filters recent purchases using StartDate before invoking the provider-specific filter, so those timestamps cannot be conflated.
    • Tenant-wide reservation listing is not subscription inventory. Billing scope identifies the payer, not the subscriptions receiving benefits. Applied-reservation discovery returns order IDs; split children still require their own scope checks. Resource-group, shared, and management-group capacity must not each be counted as unrestricted capacity in every client subscription.
    • Child inventory has no historical price. The existing shared Cost scalar has hourly-cost consumers, so assigning an order-level total or inventing zero would not fix the missing-cost contract. The complete repair needs explicit missing-cost semantics without silently changing existing consumers.
    • Search and Managed Redis service/SKU discrimination is still unverified. The current extracted CLI factory constructs AWS clients only; its inventory path is not evidence of an existing Azure end-to-end consumer.

    Sources: cached SDK v4 models.go (Properties, ReservationResponse), timestamp decoding in models_serde.go, and the current shared pkg/recfilter/dedupe.go at main 9962786e06951a20d58980954e8024fbf49e2d0c. Official contracts: tenant reservation listing, applied reservation discovery, and benefit scopes.

    No implementation or live subscription verification is claimed. These findings remain part of the open issue: establish the full scoped inventory/consumer contract, separate purchase and benefit timestamps, settle missing cost and service identity, then verify SDK decoding through the actual consumer with pre-fix failures and independent review. Compute remains tracked in #190.

  7. cristim
    added 3 commits that reference this issue on Oct 8, 2026

    fix(azure/compute): source reservation inventory fields from the rese…

    443b6f7

    fix(azure/compute): source reservation inventory fields from the rese…

    110d4be

    refactor(azure): move strict reservation inventory checks to the shar…

    56db459

  8. cristim commented on Oct 17, 2026

    @cristim
    cristim
    on Oct 17, 2026
    MemberAuthor

    Decision needed (one line): for armreservations-sourced commitments with no historical price, is Cost = 0 acceptable, or should Commitment gain an explicit cost-known marker? Context: convertSavingsPlan is fixed (#494, #498). The six reservation converters (cache, cosmosdb, database, search, synapse, managedredis) still stamp Region: c.region and State: Active from the consumption ReservationDetail API, which has no state, location or dates. The real fix moves them to armreservations, as compute already does through internal/reservations/strict.go, after #423 and #439 merge. Safe default: an explicit missing-cost marker, never an order total in Cost. First slice: cache plus managedredis, about 300 lines. Marking status/needs-info.

  9. cristim
    added
    status/needs-infoAwaiting clarification from the asker
    on Oct 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort/mDaysimpact/manyAffects most userspriority/p1Next up; this sprintseverity/highSignificant harmstatus/needs-infoAwaiting clarification from the askertriagedItem has been triagedtype/bugDefecturgency/this-sprintWithin the current sprint

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions