Skip to content

fix(azure): savings plan inventory uses the ARM resource name as ResourceType, so the duplicate guard likely never matches #492

Closed

Description

@cristim

Problem

The Azure savings plan duplicate guard probably never matches in production, so a recently bought savings plan does not suppress the matching recommendation.

convertSavingsPlan (providers/azure/services/savingsplans/client.go:247-248 on main) sets the inventory commitment's resource type from the ARM resource name:

if sp.Name != nil {
    commitment.ResourceType = *sp.Name
}

The recommendation side sets the literal "Compute" (benefit_recommendations.go:239). recfilter.DuplicateChecker keys both on ResourceType|Region|engine|deployment, so a commitment matches a recommendation only if sp.Name == "Compute".

For an ARM resource, name is the last segment of its ID: /providers/Microsoft.BillingBenefits/savingsPlanOrders/{orderId}/savingsPlans/{savingsPlanId}. In the SDK's own examples (armbillingbenefits@v1.0.0, savingsplan_client_example_test.go) that ID is GUID-shaped (for example 20000000-0000-0000-0000-000000000000). The plan type is carried in sku.name ("Compute_Savings_Plan", line 150), and convertSavingsPlan never reads it.

Every test fixture sets "name": "Compute" (e.g. scope_test.go), which is why the tests pass.

Impact

If real list responses carry a GUID name, these are all inert in production:

A savings plan bought within the lookback window would not suppress a repeat recommendation, so the same commitment could be bought twice.

Not verified live

This is inferred from ARM naming rules and the SDK examples. No live Azure call was made. Please confirm against one real SavingsPlanClient.NewListAllPager response before fixing.

Suggested fix

Derive the resource type from sku.name, mapping Compute_Savings_Plan to Compute, and fail loud on an unknown SKU. Do not use name. Add a fixture with a GUID name and a sku.name, run through the real DuplicateChecker.

Found while reviewing #491 (review by claude-go-pr-merge-4). The problem predates that PR.

Activity

  1. self-assigned this
    on Oct 17, 2026
  2. cristim commented on Oct 17, 2026

    @cristim
    MemberAuthor

    Claiming: working on this.

  3. cristim commented on Oct 17, 2026

    @cristim
    MemberAuthor

    Fix in #494. Labels mirrored to the PR; this issue has no triaged label, so the PR does not carry it either.

  4. added a commit that references this issue on Oct 17, 2026
    c4c7158
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions