diff --git a/CHANGELOG.md b/CHANGELOG.md index 652afd34..6b9d9a8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- **`azure/savingsplans`:** the commitment currency is no longer hardcoded to + USD. `SavingsPlanDetails.Currency` (new, additive) is sent in the purchase and + validate bodies and returned by `GetOfferingDetails`; empty or malformed + values (anything but 3 ASCII letters; format-only, not ISO 4217 membership) + fail closed (#46). + - **`pkg/insurance`:** hardened vendor-text handling in `Client` errors (#286). The sanitizer now also strips format, line and paragraph separator characters (Unicode Cf, Zl, Zp) from vendor text; other invisible characters diff --git a/ci_cd_sanity_tests/go.mod b/ci_cd_sanity_tests/go.mod index c157d216..5053eccc 100644 --- a/ci_cd_sanity_tests/go.mod +++ b/ci_cd_sanity_tests/go.mod @@ -41,7 +41,7 @@ require ( require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 github.com/aws/aws-sdk-go-v2/service/sts v1.33.17 ) diff --git a/ci_cd_sanity_tests/go.sum b/ci_cd_sanity_tests/go.sum index d13721f8..a6427626 100644 --- a/ci_cd_sanity_tests/go.sum +++ b/ci_cd_sanity_tests/go.sum @@ -20,8 +20,8 @@ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 h1:/m3hxkSHGd8AJlhBaPTttRDwtwC1ycYH7IbRkRKR9bM= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= diff --git a/providers/aws/go.mod b/providers/aws/go.mod index 2abe838e..faff0db6 100644 --- a/providers/aws/go.mod +++ b/providers/aws/go.mod @@ -3,7 +3,7 @@ module github.com/LeanerCloud/cloud-commitments-go/providers/aws go 1.26.9 require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 github.com/aws/aws-sdk-go-v2 v1.41.5 github.com/aws/aws-sdk-go-v2/config v1.29.12 github.com/aws/aws-sdk-go-v2/credentials v1.17.65 diff --git a/providers/aws/go.sum b/providers/aws/go.sum index 46b71fdb..d8cae52c 100644 --- a/providers/aws/go.sum +++ b/providers/aws/go.sum @@ -1,5 +1,5 @@ -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 h1:/m3hxkSHGd8AJlhBaPTttRDwtwC1ycYH7IbRkRKR9bM= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= diff --git a/providers/azure/go.mod b/providers/azure/go.mod index fd327825..601eabf7 100644 --- a/providers/azure/go.mod +++ b/providers/azure/go.mod @@ -15,7 +15,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions v1.3.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/search/armsearch v1.4.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql v1.2.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 github.com/stretchr/testify v1.11.1 golang.org/x/sync v0.23.0 ) diff --git a/providers/azure/go.sum b/providers/azure/go.sum index 0a259fa4..0119b3ba 100644 --- a/providers/azure/go.sum +++ b/providers/azure/go.sum @@ -38,8 +38,8 @@ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 h1:/m3hxkSHGd8AJlhBaPTttRDwtwC1ycYH7IbRkRKR9bM= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng= diff --git a/providers/azure/services/savingsplans/client.go b/providers/azure/services/savingsplans/client.go index 346cf8da..87c6a6ed 100644 --- a/providers/azure/services/savingsplans/client.go +++ b/providers/azure/services/savingsplans/client.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/url" + "regexp" "strings" "time" @@ -123,6 +124,12 @@ func (c *Client) GetRegion() string { // GetRecommendations returns an empty slice. // +// Contract for the future implementation: when this is backed by the Cost +// Management benefitRecommendations API, it must set +// SavingsPlanDetails.Currency from that response's properties.currencyCode. +// There is no billing-account lookup, and an empty currency is never treated +// as USD (PurchaseCommitment, ValidateOffering and GetOfferingDetails reject it). +// // Azure does not have a stable public API for Savings Plan purchase // recommendations (the Benefits Recommendations API is still in preview). // This mirrors the AWS Savings Plans client which also returns empty here and @@ -277,6 +284,20 @@ func convertSavingsPlan(sp *armbillingbenefits.SavingsPlanModel, subscriptionID return &commitment } +var currencyCodePattern = regexp.MustCompile(`^[A-Za-z]{3}$`) + +// commitmentCurrency returns the upper-cased 3-letter currency code of the +// commitment amount. It fails closed on empty or malformed input (no trimming, +// no USD default). The check is format-only, not ISO 4217 membership; the +// pattern runs before upper-casing so Unicode case folds such as "ınr" or "ſek" +// are rejected. Azure needs the code to match the billing account's currency. +func commitmentCurrency(d *common.SavingsPlanDetails) (string, error) { + if !currencyCodePattern.MatchString(d.Currency) { + return "", fmt.Errorf("azure savings plan commitment currency %q is not a 3-letter currency code", d.Currency) + } + return strings.ToUpper(d.Currency), nil +} + // PurchaseCommitment creates an Azure Savings Plan via the OrderAlias API. // // Idempotency: the SavingsPlanOrderAlias is created with an HTTP PUT on the @@ -312,6 +333,11 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati billingScopeID := c.billingScopeID() appliedScope := armbillingbenefits.AppliedScopeTypeShared hourlyAmount := spDetails.HourlyCommitment + currency, err := commitmentCurrency(spDetails) + if err != nil { + result.Error = err + return result, result.Error + } displayName := fmt.Sprintf("cudly-%s-%s", spDetails.PlanType, rec.Term) body := armbillingbenefits.SavingsPlanOrderAliasModel{ @@ -323,7 +349,7 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati AppliedScopeType: &appliedScope, Commitment: &armbillingbenefits.Commitment{ Amount: &hourlyAmount, - CurrencyCode: toPtr("USD"), + CurrencyCode: ¤cy, Grain: &grain, }, }, @@ -404,6 +430,10 @@ func (c *Client) buildValidateBody(rec common.Recommendation) (armbillingbenefit billingScopeID := c.billingScopeID() appliedScope := armbillingbenefits.AppliedScopeTypeShared hourlyAmount := spDetails.HourlyCommitment + currency, err := commitmentCurrency(spDetails) + if err != nil { + return armbillingbenefits.SavingsPlanPurchaseValidateRequest{}, err + } displayName := "cudly-validate" return armbillingbenefits.SavingsPlanPurchaseValidateRequest{ @@ -417,7 +447,7 @@ func (c *Client) buildValidateBody(rec common.Recommendation) (armbillingbenefit AppliedScopeType: &appliedScope, Commitment: &armbillingbenefits.Commitment{ Amount: &hourlyAmount, - CurrencyCode: toPtr("USD"), + CurrencyCode: ¤cy, Grain: &grain, }, }, @@ -463,6 +493,11 @@ func (c *Client) GetOfferingDetails(ctx context.Context, rec common.Recommendati return nil, fmt.Errorf("invalid service details for Azure Savings Plans") } + currency, err := commitmentCurrency(spDetails) + if err != nil { + return nil, err + } + // Calculate term hours. azTerm, err := toAzureTerm(rec.Term) if err != nil { @@ -507,7 +542,7 @@ func (c *Client) GetOfferingDetails(ctx context.Context, rec common.Recommendati RecurringCost: recurringCost, TotalCost: totalCost, EffectiveHourlyRate: spDetails.HourlyCommitment, - Currency: "USD", + Currency: currency, }, nil } diff --git a/providers/azure/services/savingsplans/client_test.go b/providers/azure/services/savingsplans/client_test.go index 2d6b173a..2f425b4c 100644 --- a/providers/azure/services/savingsplans/client_test.go +++ b/providers/azure/services/savingsplans/client_test.go @@ -48,14 +48,18 @@ type mockOrderAliasClient struct { // capturedName records the alias name passed to BeginCreate so tests can // assert the deterministic idempotency-derived name. capturedName string + capturedBody armbillingbenefits.SavingsPlanOrderAliasModel + calls int } func (m *mockOrderAliasClient) BeginCreate( _ context.Context, savingsPlanOrderAliasName string, - _ armbillingbenefits.SavingsPlanOrderAliasModel, + body armbillingbenefits.SavingsPlanOrderAliasModel, _ *armbillingbenefits.SavingsPlanOrderAliasClientBeginCreateOptions, ) (SavingsPlanOrderAliasPoller, error) { + m.calls++ + m.capturedBody = body m.capturedName = savingsPlanOrderAliasName if m.createErr != nil { return nil, m.createErr @@ -75,15 +79,19 @@ func (p *mockPoller) PollUntilDone(_ context.Context, _ *PollOptions) (armbillin // mockRPValidate is a struct-based mock for RPValidateAPI. type mockRPValidate struct { - resp armbillingbenefits.RPClientValidatePurchaseResponse - err error + resp armbillingbenefits.RPClientValidatePurchaseResponse + err error + captured armbillingbenefits.SavingsPlanPurchaseValidateRequest + calls int } func (m *mockRPValidate) ValidatePurchase( _ context.Context, - _ armbillingbenefits.SavingsPlanPurchaseValidateRequest, + req armbillingbenefits.SavingsPlanPurchaseValidateRequest, _ *armbillingbenefits.RPClientValidatePurchaseOptions, ) (armbillingbenefits.RPClientValidatePurchaseResponse, error) { + m.calls++ + m.captured = req return m.resp, m.err } @@ -274,6 +282,7 @@ func makeRec(term string) common.Recommendation { Details: &common.SavingsPlanDetails{ PlanType: "Compute", HourlyCommitment: 1.5, + Currency: "USD", }, } } @@ -451,7 +460,7 @@ func TestGetOfferingDetails_1yr_NoUpfront(t *testing.T) { rec := common.Recommendation{ Term: "1yr", PaymentOption: "No Upfront", - Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0}, + Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0, Currency: "USD"}, } details, err := c.GetOfferingDetails(context.Background(), rec) @@ -472,7 +481,7 @@ func TestGetOfferingDetails_3yr_AllUpfront(t *testing.T) { rec := common.Recommendation{ Term: "3yr", PaymentOption: "All Upfront", - Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0}, + Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0, Currency: "USD"}, } details, err := c.GetOfferingDetails(context.Background(), rec) @@ -489,7 +498,7 @@ func TestGetOfferingDetails_5yr_AllUpfront(t *testing.T) { rec := common.Recommendation{ Term: "5yr", PaymentOption: "All Upfront", - Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0}, + Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0, Currency: "USD"}, } details, err := c.GetOfferingDetails(context.Background(), rec) @@ -507,7 +516,7 @@ func TestGetOfferingDetails_BadTerm(t *testing.T) { rec := common.Recommendation{ Term: "99yr", PaymentOption: "No Upfront", - Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0}, + Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0, Currency: "USD"}, } _, err := c.GetOfferingDetails(context.Background(), rec) @@ -520,7 +529,7 @@ func TestGetOfferingDetails_BadPaymentOption(t *testing.T) { rec := common.Recommendation{ Term: "1yr", PaymentOption: "Quarterly", - Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0}, + Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1.0, Currency: "USD"}, } _, err := c.GetOfferingDetails(context.Background(), rec) @@ -714,3 +723,64 @@ func TestAzureSavingsPlanState(t *testing.T) { assert.Equal(t, want, azureSavingsPlanState(ps), "provisioning state %q", ps) } } + +// --- commitment currency (go#46) --- + +func currencyRec(currency string) common.Recommendation { + rec := makeRec("1yr") + rec.Details = &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 12, Currency: currency} + return rec +} + +func TestCommitmentCurrency_SameAcrossPurchaseValidateOffering(t *testing.T) { + rec := currencyRec("JPY") + orderID := "order1" + alias := &mockOrderAliasClient{createPoller: &mockPoller{resp: armbillingbenefits.SavingsPlanOrderAliasClientCreateResponse{ + SavingsPlanOrderAliasModel: armbillingbenefits.SavingsPlanOrderAliasModel{ID: &orderID}, + }}} + rp := &mockRPValidate{} + c := NewClient(nil, "sub", "eastus") + c.SetOrderAliasClient(alias) + c.SetRPValidateClient(rp) + + _, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{}) + require.NoError(t, err) + require.NoError(t, c.ValidateOffering(context.Background(), rec)) + details, err := c.GetOfferingDetails(context.Background(), rec) + require.NoError(t, err) + + commitment := alias.capturedBody.Properties.Commitment + assert.Equal(t, "JPY", *commitment.CurrencyCode) + assert.InDelta(t, 12.0, *commitment.Amount, 0.001) + assert.Equal(t, "JPY", *rp.captured.Benefits[0].Properties.Commitment.CurrencyCode) + assert.Equal(t, "JPY", details.Currency) +} + +func TestCommitmentCurrency_FailsClosed(t *testing.T) { + for _, bad := range []string{"", " eur", "EURO", "E1R", "us", "ınr", "ſek"} { + t.Run(bad, func(t *testing.T) { + alias := &mockOrderAliasClient{} + rp := &mockRPValidate{} + c := NewClient(nil, "sub", "eastus") + c.SetOrderAliasClient(alias) + c.SetRPValidateClient(rp) + rec := currencyRec(bad) + + _, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{}) + require.Error(t, err) + assert.Zero(t, alias.calls) + require.Error(t, c.ValidateOffering(context.Background(), rec)) + assert.Zero(t, rp.calls) + _, err = c.GetOfferingDetails(context.Background(), rec) + require.Error(t, err) + }) + } +} + +func TestCommitmentCurrency_LowercaseNormalised(t *testing.T) { + rp := &mockRPValidate{} + c := NewClient(nil, "sub", "eastus") + c.SetRPValidateClient(rp) + require.NoError(t, c.ValidateOffering(context.Background(), currencyRec("eur"))) + assert.Equal(t, "EUR", *rp.captured.Benefits[0].Properties.Commitment.CurrencyCode) +} diff --git a/providers/gcp/go.mod b/providers/gcp/go.mod index 729bbf63..1cdb1020 100644 --- a/providers/gcp/go.mod +++ b/providers/gcp/go.mod @@ -8,7 +8,7 @@ require ( cloud.google.com/go/redis v1.18.3 cloud.google.com/go/resourcemanager v1.10.7 cloud.google.com/go/storage v1.59.2 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 github.com/googleapis/gax-go/v2 v2.21.0 github.com/stretchr/testify v1.11.1 golang.org/x/oauth2 v0.36.0 diff --git a/providers/gcp/go.sum b/providers/gcp/go.sum index d7927ce2..32e56b49 100644 --- a/providers/gcp/go.sum +++ b/providers/gcp/go.sum @@ -36,8 +36,8 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db h1:d/eTZP31KewqE049NlQM3nZIUygRudHJ25Im2ibJk58= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261009181525-30bf383795db/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668 h1:/m3hxkSHGd8AJlhBaPTttRDwtwC1ycYH7IbRkRKR9bM= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012192523-3838efd56668/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng=