Skip to content

fix(commitmentopts/azure): probe Savings Plans in the caller's billing currency #1091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
Oct 20, 2026
Merged

fix(commitmentopts/azure): probe Savings Plans in the caller's billing currency #1091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions internal/commitmentopts/probe_azure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package commitmentopts
import (
"context"
"fmt"
"regexp"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/billingbenefits/armbillingbenefits"
Expand All @@ -27,6 +29,11 @@ const azureProbeSubscriptionID = "00000000-0000-0000-0000-000000000000"
// amount), but zero is rejected server-side so we use the documented minimum.
const azureProbeHourlyCommitment float64 = 0.001

// azureCurrencyCodePattern matches a 3-letter ISO 4217 currency code. It
// mirrors the check the cloud-commitments-go Azure Savings Plans provider
// applies to SavingsPlanDetails.Currency before purchase or validation.
var azureCurrencyCodePattern = regexp.MustCompile(`^[A-Za-z]{3}$`)

// azureCandidateCombos lists every (term, billingPlan) pair that the Azure
// Savings Plans API publishes. The probe calls ValidatePurchase for each one
// and retains only those the API accepts as structurally valid.
Expand Down Expand Up @@ -101,18 +108,31 @@ func (p *AzureSPProber) Service() string { return azureSpService }
// ProbeAzure probes the Azure ValidatePurchase endpoint for each candidate
// (term, billingPlan) combo and returns Combos for those the API accepts.
//
// currencyCode is the billing currency of the account the probe runs
// against (a 3-letter ISO 4217 code such as "USD" or "EUR"). Azure
// evaluates a ValidatePurchase request against the billing account's
// currency, so a probe sent in any other currency yields a verdict about a
// commitment the account could never buy. The caller must resolve the
// account's real currency; there is no USD default. An empty or malformed
// code fails closed before any client is built or any request is sent.
//
// The method signature differs from the AWS Prober interface because Azure
// authentication uses azcore.TokenCredential rather than aws.Config. Callers
// use this method directly; Service.probeAndPersistAzure wires it up.
func (p *AzureSPProber) ProbeAzure(ctx context.Context, cred azcore.TokenCredential) ([]Combo, error) {
func (p *AzureSPProber) ProbeAzure(ctx context.Context, cred azcore.TokenCredential, currencyCode string) ([]Combo, error) {
currency, err := azureProbeCurrency(currencyCode)
if err != nil {
return nil, err
}

client, err := p.client(cred)
if err != nil {
return nil, fmt.Errorf("savingsplans: create validate client: %w", err)
}

var combos []Combo
for _, c := range azureCandidateCombos {
ok, err := p.probeCombo(ctx, client, c.azureTerm, c.azurePlan)
ok, err := p.probeCombo(ctx, client, c.azureTerm, c.azurePlan, currency)
if err != nil {
// Treat non-validation errors (auth, network, 5xx) as probe
// failures — they prevent us from knowing whether the combo
Expand All @@ -131,6 +151,15 @@ func (p *AzureSPProber) ProbeAzure(ctx context.Context, cred azcore.TokenCredent
return combos, nil
}

// azureProbeCurrency validates and upper-cases the caller-supplied billing
// currency. It never substitutes a default: an unknown currency is an error.
func azureProbeCurrency(code string) (string, error) {
if !azureCurrencyCodePattern.MatchString(code) {
return "", fmt.Errorf("savingsplans: probe currency %q is not a 3-letter currency code; the billing account's currency must be resolved before probing", code)
}
return strings.ToUpper(code), nil
}

// probeCombo calls ValidatePurchase for a single (term, billingPlan) pair and
// returns true if the API considers it a valid offering.
//
Expand All @@ -142,14 +171,14 @@ func (p *AzureSPProber) probeCombo(
client AzureSPValidateAPI,
term armbillingbenefits.Term,
billingPlan *armbillingbenefits.BillingPlan,
currencyCode string,
) (bool, error) {
subscriptionID := azureProbeSubscriptionID
billingScopeID := fmt.Sprintf("/subscriptions/%s", subscriptionID)
grain := armbillingbenefits.CommitmentGrainHourly
appliedScope := armbillingbenefits.AppliedScopeTypeShared
hourlyAmount := azureProbeHourlyCommitment
displayName := "cudly-probe"
currencyCode := "USD"
planType := "Compute"

props := &armbillingbenefits.SavingsPlanOrderAliasProperties{
Expand Down
88 changes: 77 additions & 11 deletions internal/commitmentopts/probe_azure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestAzureSPProber_AllValid(t *testing.T) {
return validTrue(), nil
})

got, err := p.ProbeAzure(context.Background(), nil)
got, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)
assert.Len(t, got, 6)

Expand Down Expand Up @@ -110,7 +110,7 @@ func TestAzureSPProber_SomeValid(t *testing.T) {
return validTrue(), nil
})

got, err := p.ProbeAzure(context.Background(), nil)
got, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)
// 2 terms x 2 payment plans = 4 combos (P5Y x2 dropped)
assert.Len(t, got, 4)
Expand All @@ -130,7 +130,7 @@ func TestAzureSPProber_NoneValid(t *testing.T) {
return validFalse("not available"), nil
})

got, err := p.ProbeAzure(context.Background(), nil)
got, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)
assert.Empty(t, got)
}
Expand All @@ -142,7 +142,7 @@ func TestAzureSPProber_EmptyBenefitsSlice(t *testing.T) {
return armbillingbenefits.RPClientValidatePurchaseResponse{}, nil
})

got, err := p.ProbeAzure(context.Background(), nil)
got, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)
assert.Len(t, got, 6, "empty benefits slice treated as valid")
}
Expand All @@ -159,7 +159,7 @@ func TestAzureSPProber_NilValidFlag(t *testing.T) {
}, nil
})

got, err := p.ProbeAzure(context.Background(), nil)
got, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)
assert.Len(t, got, 6, "nil Valid flag treated as valid")
}
Expand All @@ -177,7 +177,7 @@ func TestAzureSPProber_APIError_Propagates(t *testing.T) {
return armbillingbenefits.RPClientValidatePurchaseResponse{}, boom
})

_, err := p.ProbeAzure(context.Background(), nil)
_, err := p.ProbeAzure(context.Background(), nil, "USD")
require.Error(t, err)
assert.ErrorIs(t, err, boom)
}
Expand All @@ -192,7 +192,7 @@ func TestAzureSPProber_APIError_FirstComboFails(t *testing.T) {
return armbillingbenefits.RPClientValidatePurchaseResponse{}, boom
})

_, err := p.ProbeAzure(context.Background(), nil)
_, err := p.ProbeAzure(context.Background(), nil, "USD")
require.Error(t, err)
assert.Equal(t, 1, calls, "prober must stop after first error")
}
Expand All @@ -207,7 +207,7 @@ func TestAzureSPProber_ClientBuildError(t *testing.T) {
},
}

_, err := p.ProbeAzure(context.Background(), nil)
_, err := p.ProbeAzure(context.Background(), nil, "USD")
require.Error(t, err)
assert.ErrorIs(t, err, buildErr)
}
Expand All @@ -223,7 +223,7 @@ func TestAzureSPProber_ComboFields(t *testing.T) {
return validTrue(), nil
})

got, err := p.ProbeAzure(context.Background(), nil)
got, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)

sort.Slice(got, func(i, j int) bool {
Expand Down Expand Up @@ -256,7 +256,7 @@ func TestAzureSPProber_RequestContainsTerm(t *testing.T) {
return validTrue(), nil
})

_, err := p.ProbeAzure(context.Background(), nil)
_, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)

assert.Equal(t, 2, seenTerms[armbillingbenefits.TermP1Y], "P1Y must appear twice (upfront + monthly)")
Expand Down Expand Up @@ -284,7 +284,7 @@ func TestAzureSPProber_RequestBillingPlan(t *testing.T) {
return validTrue(), nil
})

_, err := p.ProbeAzure(context.Background(), nil)
_, err := p.ProbeAzure(context.Background(), nil, "USD")
require.NoError(t, err)

for _, term := range []armbillingbenefits.Term{armbillingbenefits.TermP1Y, armbillingbenefits.TermP3Y, armbillingbenefits.TermP5Y} {
Expand All @@ -293,6 +293,72 @@ func TestAzureSPProber_RequestBillingPlan(t *testing.T) {
}
}

// ---------------------------------------------------------------------------
// Billing currency (issue #814)
// ---------------------------------------------------------------------------

func TestAzureSPProber_RequestUsesCallerCurrency(t *testing.T) {
// The probe must validate in the billing account's currency, not a
// hardcoded USD. Lowercase input is normalized to upper case, matching
// the cloud-commitments-go Azure provider.
cases := []struct {
in string
want string
}{
{"EUR", "EUR"},
{"gbp", "GBP"},
{"USD", "USD"},
}
for _, tc := range cases {
t.Run(tc.in, func(t *testing.T) {
var seen []string
p := newFakeProber(func(body armbillingbenefits.SavingsPlanPurchaseValidateRequest) (armbillingbenefits.RPClientValidatePurchaseResponse, error) {
require.Len(t, body.Benefits, 1)
require.NotNil(t, body.Benefits[0].Properties)
require.NotNil(t, body.Benefits[0].Properties.Commitment)
require.NotNil(t, body.Benefits[0].Properties.Commitment.CurrencyCode)
seen = append(seen, *body.Benefits[0].Properties.Commitment.CurrencyCode)
return validTrue(), nil
})

_, err := p.ProbeAzure(context.Background(), nil, tc.in)
require.NoError(t, err)

require.Len(t, seen, len(azureCandidateCombos), "every candidate combo must be probed")
for _, got := range seen {
assert.Equal(t, tc.want, got)
}
})
}
}

func TestAzureSPProber_UnknownCurrency_FailsClosed(t *testing.T) {
// With no resolvable billing currency the probe must refuse to run
// rather than assume USD: no client is built and no request is sent.
for _, in := range []string{"", "US", "EURO", "U$D", "12A", " usd"} {
t.Run(in, func(t *testing.T) {
clientBuilt := false
validateCalls := 0
p := &AzureSPProber{
NewClient: func(_ azcore.TokenCredential) (AzureSPValidateAPI, error) {
clientBuilt = true
return &fakeAzureSPValidate{fn: func(_ armbillingbenefits.SavingsPlanPurchaseValidateRequest) (armbillingbenefits.RPClientValidatePurchaseResponse, error) {
validateCalls++
return validTrue(), nil
}}, nil
},
}

got, err := p.ProbeAzure(context.Background(), nil, in)
require.Error(t, err)
assert.Contains(t, err.Error(), "currency")
assert.Nil(t, got)
assert.False(t, clientBuilt, "client must not be built without a valid currency")
assert.Zero(t, validateCalls, "ValidatePurchase must not be called without a valid currency")
})
}
}

// ---------------------------------------------------------------------------
// DefaultAzureProbers
// ---------------------------------------------------------------------------
Expand Down
Loading