diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cb9ef17..660b0dbf 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) 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/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/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 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.