From 19829bee7d0c445893ac0d092c986b5b5cf94b8d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 05:43:02 +0100 Subject: [PATCH 1/2] fix(azure): set CommitmentCurrency on Azure recommendations Consumption-path variants (compute, database, cache, cosmosdb, managedredis, synapse, search) take their commitment from a Pricer-checked USD price, and savings plan benefit rows are rejected unless currencyCode is USD, so both now record USD. Advisor and legacy paths carry no checked commitment amounts and stay empty. Closes #623 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../internal/recommendations/consumption.go | 4 +++ .../recommendations/consumption_test.go | 25 +++++++++++++++++++ .../savingsplans/benefit_recommendations.go | 1 + .../benefit_recommendations_test.go | 10 ++++++++ 5 files changed, 41 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cb9ef17..ebc14559 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -222,6 +222,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- **`azure`:** recommendations from the Consumption path (compute, database, cache, cosmosdb, managedredis, synapse, search) and the Savings Plan benefit path now set `CommitmentCurrency` to `USD`, because their commitment amounts come from sources already required to be USD. The Advisor and legacy paths carry no checked commitment amounts and stay empty (#623). - **AWS/OpenSearch:** the STS account-ID lookup used for post-purchase tagging caches only a successful result, so one throttled or timed-out `GetCallerIdentity` no longer disables tagging for the client's lifetime (#53). - **AWS/recommendations:** `normalizeRegionName` maps the current `Europe (Ireland|Frankfurt|London|Paris|Stockholm)` Cost Explorer display names to region codes, so coverage lookups keyed by region code match those recommendations (#53). - **AWS/recommendations:** `normalizeRegionName` also maps `Asia Pacific (Malaysia|Thailand)`, `Canada West (Calgary)` and `Mexico (Central)` to `ap-southeast-5`, `ap-southeast-7`, `ca-west-1` and `mx-central-1` (#53). diff --git a/providers/azure/internal/recommendations/consumption.go b/providers/azure/internal/recommendations/consumption.go index 354510a3..178b4469 100644 --- a/providers/azure/internal/recommendations/consumption.go +++ b/providers/azure/internal/recommendations/consumption.go @@ -282,6 +282,10 @@ func ExpandConsumptionVariants(ctx context.Context, base common.Recommendation, savingsPct = base.EstimatedSavings / base.OnDemandCost * 100 } + // Pricer.resolve accepted the price only as USD, so both payment legs + // are USD amounts. + base.CommitmentCurrency = currencyUSD + upfront := base upfront.PaymentOption = "upfront" upfront.CommitmentCost = total diff --git a/providers/azure/internal/recommendations/consumption_test.go b/providers/azure/internal/recommendations/consumption_test.go index b3a0cee9..86af885c 100644 --- a/providers/azure/internal/recommendations/consumption_test.go +++ b/providers/azure/internal/recommendations/consumption_test.go @@ -156,6 +156,31 @@ func TestExpandConsumptionVariants_ScreenshotWorkedExample(t *testing.T) { } } +func TestExpandConsumptionVariants_CommitmentCurrency(t *testing.T) { + t.Parallel() + base := extractedBase(t, consumptionRecs(7, "P1Y")["modern"]) + require.Empty(t, base.CommitmentCurrency, "the base carries no commitment amounts yet") + + t.Run("USD price row labels both payment legs", func(t *testing.T) { + variants, err := ExpandConsumptionVariants(context.Background(), base, NewPricer("compute", newStub().lookup)) + require.NoError(t, err) + require.Len(t, variants, 2) + for _, v := range variants { + assert.Equal(t, "USD", v.CommitmentCurrency, v.PaymentOption) + } + }) + for _, cur := range []string{"", "EUR"} { + t.Run("price row currency "+cur+" yields no recommendation", func(t *testing.T) { + lookup := func(context.Context, string, string, int) (ReservationPrice, error) { + return ReservationPrice{Total: 494, Currency: cur}, nil + } + variants, err := ExpandConsumptionVariants(context.Background(), base, NewPricer("compute", lookup)) + require.Error(t, err) + assert.Empty(t, variants) + }) + } +} + func TestExpandConsumptionVariants_ThreeYearUpfrontExceedsOneYear(t *testing.T) { t.Parallel() for _, days := range []int{7, 30, 60} { diff --git a/providers/azure/services/savingsplans/benefit_recommendations.go b/providers/azure/services/savingsplans/benefit_recommendations.go index 1741ac8a..bcb53444 100644 --- a/providers/azure/services/savingsplans/benefit_recommendations.go +++ b/providers/azure/services/savingsplans/benefit_recommendations.go @@ -263,6 +263,7 @@ func convertBenefitRecommendation(r *benefitRecommendation, subscriptionID strin PaymentOption: "monthly", OnDemandCost: *p.CostWithoutBenefit * monthly, CommitmentCost: 0, + CommitmentCurrency: "USD", // validateBenefitRow required a USD currencyCode EstimatedSavings: *d.SavingsAmount * monthly, RecurringMonthlyCost: &recurring, SourceRecommendation: r.ID, diff --git a/providers/azure/services/savingsplans/benefit_recommendations_test.go b/providers/azure/services/savingsplans/benefit_recommendations_test.go index f3324c1b..f0dd5b38 100644 --- a/providers/azure/services/savingsplans/benefit_recommendations_test.go +++ b/providers/azure/services/savingsplans/benefit_recommendations_test.go @@ -72,6 +72,7 @@ func TestGetRecommendations_MapsRow(t *testing.T) { assert.Equal(t, "1yr", r.Term) assert.Equal(t, "monthly", r.PaymentOption) assert.Equal(t, 0.0, r.CommitmentCost) + assert.Equal(t, "USD", r.CommitmentCurrency) require.NotNil(t, r.RecurringMonthlyCost) assert.InDelta(t, 1.5*hoursPerMonth, *r.RecurringMonthlyCost, 1e-9) assert.InDelta(t, 292*hoursPerMonth/1460, r.EstimatedSavings, 1e-9) @@ -254,6 +255,15 @@ func TestGetRecommendations_NonUSDRowIsSkippedAndSurfaced(t *testing.T) { } } +func TestConvertBenefitRecommendation_MissingCurrencyIsRejectedNotLabelled(t *testing.T) { + t.Parallel() + var row benefitRecommendation + require.NoError(t, json.Unmarshal([]byte(strings.Replace(benefitRow("P1Y", nil), `"USD"`, `""`, 1)), &row)) + rec, err := convertBenefitRecommendation(&row, "sub-1") + assert.Nil(t, rec) + assert.Error(t, err) +} + func TestConvertBenefitRecommendation_NonUSDWrapsSentinel(t *testing.T) { t.Parallel() var row benefitRecommendation From b400cbc3c497cee588f868a18f6e83ecba3b2ca4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 05:48:51 +0100 Subject: [PATCH 2/2] test(azure): pin empty CommitmentCurrency on Advisor and search paths The Advisor payload has no commitment amounts and search zeroes them, so neither may be labelled USD. Drop search from the changelog list. Refs #623 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- providers/azure/recommendations_advisor_test.go | 14 ++++++++++++++ providers/azure/services/search/client_test.go | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc14559..660b0dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -222,7 +222,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed -- **`azure`:** recommendations from the Consumption path (compute, database, cache, cosmosdb, managedredis, synapse, search) and the Savings Plan benefit path now set `CommitmentCurrency` to `USD`, because their commitment amounts come from sources already required to be USD. The Advisor and legacy paths carry no checked commitment amounts and stay empty (#623). +- **`azure`:** recommendations from the Consumption path (compute, database, cache, cosmosdb, managedredis, synapse) and the Savings Plan benefit path now set `CommitmentCurrency` to `USD`, because their commitment amounts come from sources already required to be USD. The Advisor and legacy paths carry no checked commitment amounts and stay empty (#623). - **AWS/OpenSearch:** the STS account-ID lookup used for post-purchase tagging caches only a successful result, so one throttled or timed-out `GetCallerIdentity` no longer disables tagging for the client's lifetime (#53). - **AWS/recommendations:** `normalizeRegionName` maps the current `Europe (Ireland|Frankfurt|London|Paris|Stockholm)` Cost Explorer display names to region codes, so coverage lookups keyed by region code match those recommendations (#53). - **AWS/recommendations:** `normalizeRegionName` also maps `Asia Pacific (Malaysia|Thailand)`, `Canada West (Calgary)` and `Mexico (Central)` to `ap-southeast-5`, `ap-southeast-7`, `ca-west-1` and `mx-central-1` (#53). diff --git a/providers/azure/recommendations_advisor_test.go b/providers/azure/recommendations_advisor_test.go index e00b74b4..44b286f5 100644 --- a/providers/azure/recommendations_advisor_test.go +++ b/providers/azure/recommendations_advisor_test.go @@ -200,3 +200,17 @@ func TestConvertAdvisor_RejectsBadSavingsAmounts(t *testing.T) { require.NotNil(t, zero, "a real zero saving is kept") assert.Equal(t, 0.0, zero.EstimatedSavings) } + +// The Advisor payload carries only a USD-checked savings figure, no commitment +// amounts, so no payment leg is proven USD and the currency stays unknown. +func TestConvertAdvisor_LeavesCommitmentCurrencyEmpty(t *testing.T) { + t.Parallel() + a := &RecommendationsClientAdapter{subscriptionID: "sub"} + rec := a.convertAdvisorRecommendation(context.Background(), advisorRec(subscriptionScope, reservationExt(nil))) + require.NotNil(t, rec) + variants := azrecs.ExpandPaymentVariants(*rec) + require.NotEmpty(t, variants) + for _, v := range variants { + assert.Empty(t, v.CommitmentCurrency, v.PaymentOption) + } +} diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go index b245df57..b839cc1a 100644 --- a/providers/azure/services/search/client_test.go +++ b/providers/azure/services/search/client_test.go @@ -1019,6 +1019,7 @@ func TestSearchClient_ConvertAzureSearchRecommendation_PopulatesAllFields(t *tes assert.Equal(t, common.CommitmentReservedInstance, rec.CommitmentType) assert.Equal(t, "1yr", rec.Term) assert.Equal(t, "upfront", rec.PaymentOption) + assert.Empty(t, rec.CommitmentCurrency, "amounts are zeroed here, so no currency is proven") } // MockTokenCredential for testing PurchaseCommitment.