diff --git a/internal/commitmentopts/probe_azure.go b/internal/commitmentopts/probe_azure.go index d982c80ab..2ebddddd6 100644 --- a/internal/commitmentopts/probe_azure.go +++ b/internal/commitmentopts/probe_azure.go @@ -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" @@ -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. @@ -101,10 +108,23 @@ 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) @@ -112,7 +132,7 @@ func (p *AzureSPProber) ProbeAzure(ctx context.Context, cred azcore.TokenCredent 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 @@ -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. // @@ -142,6 +171,7 @@ func (p *AzureSPProber) probeCombo( client AzureSPValidateAPI, term armbillingbenefits.Term, billingPlan *armbillingbenefits.BillingPlan, + currencyCode string, ) (bool, error) { subscriptionID := azureProbeSubscriptionID billingScopeID := fmt.Sprintf("/subscriptions/%s", subscriptionID) @@ -149,7 +179,6 @@ func (p *AzureSPProber) probeCombo( appliedScope := armbillingbenefits.AppliedScopeTypeShared hourlyAmount := azureProbeHourlyCommitment displayName := "cudly-probe" - currencyCode := "USD" planType := "Compute" props := &armbillingbenefits.SavingsPlanOrderAliasProperties{ diff --git a/internal/commitmentopts/probe_azure_test.go b/internal/commitmentopts/probe_azure_test.go index 2ba15f03e..d85c5aa39 100644 --- a/internal/commitmentopts/probe_azure_test.go +++ b/internal/commitmentopts/probe_azure_test.go @@ -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) @@ -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) @@ -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) } @@ -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") } @@ -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") } @@ -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) } @@ -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") } @@ -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) } @@ -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 { @@ -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)") @@ -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} { @@ -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 // ---------------------------------------------------------------------------