From 4ad7c9e57fdcfa209a807916acfbfd06fa22cd27 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 00:49:15 +0100 Subject: [PATCH 01/10] chore(providers): pin pkg to the CostKnown commit Re-pin providers/aws and providers/azure to pkg 8d06fde536cf (#689) so the producers can set Commitment.CostKnown. Azure also moves aws-sdk-go-v2 and smithy-go via MVS. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/aws/go.mod | 2 +- providers/aws/go.sum | 4 ++-- providers/azure/go.mod | 6 +++--- providers/azure/go.sum | 6 ++++++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/providers/aws/go.mod b/providers/aws/go.mod index 4e090755..32b5fcb2 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.10 require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106140354-6b6d481e8298 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf github.com/aws/aws-sdk-go-v2 v1.47.5 github.com/aws/aws-sdk-go-v2/config v1.33.13 github.com/aws/aws-sdk-go-v2/credentials v1.20.13 diff --git a/providers/aws/go.sum b/providers/aws/go.sum index a45abbbf..6d4e9a1c 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-20261106140354-6b6d481e8298 h1:BsbLH7s1R9FXHHdih54yw3IDsWcsoaZNEKj9yxtTuf0= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106140354-6b6d481e8298/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf h1:uKewFUeWwrvgoBaFeZMepJXs+xsQdqTVRomAc6tYMX8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= github.com/aws/aws-sdk-go-v2 v1.47.5 h1:5yA+A+BiXbDq1ZLbYwPViwhEGDMdR0MEhL8Jh2LnLS4= github.com/aws/aws-sdk-go-v2 v1.47.5/go.mod h1:iNIkogrRvFwm+1OQIUOP2lPBtlXZboOIUWiCa+pHEMw= github.com/aws/aws-sdk-go-v2/config v1.33.13 h1:u52x6ISQOx172tpwM0VRxhJzEsTJYpMcEYUGM2A1wrw= diff --git a/providers/azure/go.mod b/providers/azure/go.mod index a30de384..9c1e9e5d 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-20261104063122-3efa339480ac + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf github.com/stretchr/testify v1.12.1 golang.org/x/sync v0.24.0 ) @@ -23,8 +23,8 @@ require ( require ( github.com/Azure/azure-sdk-for-go/sdk/internal v1.13.0 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1 // indirect - github.com/aws/aws-sdk-go-v2 v1.47.4 // indirect - github.com/aws/smithy-go v1.28.6 // indirect + github.com/aws/aws-sdk-go-v2 v1.47.5 // indirect + github.com/aws/smithy-go v1.28.7 // indirect github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/uuid v1.6.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect diff --git a/providers/azure/go.sum b/providers/azure/go.sum index cfd17080..64de652b 100644 --- a/providers/azure/go.sum +++ b/providers/azure/go.sum @@ -38,10 +38,16 @@ github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1 h1:0O6j18nQDo github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1/go.mod h1:xdYAf5bjkOpsd3auA8riiJW4vneBubt9caavL7626jQ= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac h1:EvEUCt1uH5oBtPcJJPt1mX6XYQWAKtL9QBSo9CfeaOw= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac/go.mod h1:pseCFRgm1rzBKomktjw6E0ayxmVE7XfHHiOmew4ka+g= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf h1:uKewFUeWwrvgoBaFeZMepJXs+xsQdqTVRomAc6tYMX8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= github.com/aws/aws-sdk-go-v2 v1.47.4 h1:bTvk6UdSSSvaQsfLVj1JRDEEqj/PDRwGf1KanaRzau0= github.com/aws/aws-sdk-go-v2 v1.47.4/go.mod h1:XkrjAZvr705Nytv6PkPUhoSB2W9+vVHBp5oAWpkh8Ak= +github.com/aws/aws-sdk-go-v2 v1.47.5 h1:5yA+A+BiXbDq1ZLbYwPViwhEGDMdR0MEhL8Jh2LnLS4= +github.com/aws/aws-sdk-go-v2 v1.47.5/go.mod h1:iNIkogrRvFwm+1OQIUOP2lPBtlXZboOIUWiCa+pHEMw= github.com/aws/smithy-go v1.28.6 h1:Pj+/sUviNYjow8ELRtNwKHZ9ECeF+7pATX3+shjLIrw= github.com/aws/smithy-go v1.28.6/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/aws/smithy-go v1.28.7 h1:h/n1SHrwVtaDYLz8mR1/Kq4Xh4RaFbFW7bOYckg9mfE= +github.com/aws/smithy-go v1.28.7/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= From c3505c37245d4db63d56dba7ba2625043a5eca80 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 00:53:30 +0100 Subject: [PATCH 02/10] fix(aws/ladder): mark RI and SP commitment costs as known riToCommitment and spToCommitment compute Cost from provider data, so set CostKnown. A zero cost is now distinguishable from unknown. EC2 listing stays unknown, pinned by a test. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/aws/ladder/commitments.go | 2 ++ providers/aws/ladder/commitments_test.go | 31 +++++++++++++++++++++++ providers/aws/services/ec2/client_test.go | 24 ++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 providers/aws/ladder/commitments_test.go diff --git a/providers/aws/ladder/commitments.go b/providers/aws/ladder/commitments.go index 589bc6c7..3129edbd 100644 --- a/providers/aws/ladder/commitments.go +++ b/providers/aws/ladder/commitments.go @@ -90,6 +90,7 @@ func riToCommitment(ri *ec2svc.ConvertibleRI, accountID, region string) common.C StartDate: ri.Start, EndDate: ri.End, Cost: totalHourlyCost, + CostKnown: true, } } @@ -140,5 +141,6 @@ func spToCommitment(sp *ActiveSP, accountID string) common.Commitment { StartDate: sp.StartDate, EndDate: sp.EndDate, Cost: sp.HourlyCommitmentUSD, + CostKnown: true, } } diff --git a/providers/aws/ladder/commitments_test.go b/providers/aws/ladder/commitments_test.go new file mode 100644 index 00000000..a1885316 --- /dev/null +++ b/providers/aws/ladder/commitments_test.go @@ -0,0 +1,31 @@ +package ladder + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" +) + +func TestRIToCommitment_ZeroComputedCostIsKnown(t *testing.T) { + t.Parallel() + // Fully-upfront RI with no duration, recurring or usage price: the + // computed hourly cost is a real 0, not an unknown one. + ri := &ec2svc.ConvertibleRI{ReservedInstanceID: "ri-1", InstanceCount: 2} + + got := riToCommitment(ri, "123456789012", "us-east-1") + + assert.True(t, got.CostKnown) + assert.Zero(t, got.Cost) +} + +func TestSPToCommitment_ZeroHourlyCommitmentIsKnown(t *testing.T) { + t.Parallel() + sp := &ActiveSP{PlanID: "sp-1", PlanType: spPlanTypeCompute, HourlyCommitmentUSD: 0} + + got := spToCommitment(sp, "123456789012") + + assert.True(t, got.CostKnown) + assert.Zero(t, got.Cost) +} diff --git a/providers/aws/services/ec2/client_test.go b/providers/aws/services/ec2/client_test.go index 6099ae1e..d7c0ad0f 100644 --- a/providers/aws/services/ec2/client_test.go +++ b/providers/aws/services/ec2/client_test.go @@ -278,6 +278,30 @@ func TestClient_GetExistingCommitments(t *testing.T) { } } +func TestClient_GetExistingCommitments_CostIsUnknown(t *testing.T) { + t.Parallel() + mockClient := &MockEC2Client{} + mockClient.On("DescribeReservedInstances", mock.Anything, mock.Anything). + Return(&ec2.DescribeReservedInstancesOutput{ + ReservedInstances: []types.ReservedInstances{{ + ReservedInstancesId: aws.String("ri-123"), + InstanceType: types.InstanceTypeT3Micro, + InstanceCount: aws.Int32(1), + State: types.ReservedInstanceStateActive, + Duration: aws.Int64(31536000), + Start: aws.Time(time.Now()), + }}, + }, nil).Once() + client := &Client{client: mockClient, region: "us-east-1"} + + result, err := client.GetExistingCommitments(context.Background()) + + assert.NoError(t, err) + if assert.Len(t, result, 1) { + assert.False(t, result[0].CostKnown) + } +} + func TestClient_GetValidResourceTypes(t *testing.T) { t.Parallel() tests := []struct { From 0d854281bbea0192c3f863d4fb1c7cf14e18e68e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:09:15 +0100 Subject: [PATCH 03/10] fix(azure/savingsplans): mark a reported commitment amount as known convertSavingsPlan assigns Cost from Commitment.Amount, so set CostKnown in that branch. A reported 0 is now distinguishable from an absent amount. Refs #74 Co-Authored-By: Claude Opus 5.5 --- .../azure/services/savingsplans/client.go | 1 + .../services/savingsplans/client_test.go | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/providers/azure/services/savingsplans/client.go b/providers/azure/services/savingsplans/client.go index 62701d4c..a8f9e3e6 100644 --- a/providers/azure/services/savingsplans/client.go +++ b/providers/azure/services/savingsplans/client.go @@ -304,6 +304,7 @@ func convertSavingsPlan(sp *armbillingbenefits.SavingsPlanModel, subscriptionID if props.Commitment != nil && props.Commitment.Amount != nil { commitment.Cost = *props.Commitment.Amount + commitment.CostKnown = true } } diff --git a/providers/azure/services/savingsplans/client_test.go b/providers/azure/services/savingsplans/client_test.go index 4dfebd3b..a97b5401 100644 --- a/providers/azure/services/savingsplans/client_test.go +++ b/providers/azure/services/savingsplans/client_test.go @@ -907,3 +907,52 @@ func TestPurchaseCommitment_NonCurrencyErrorIsNotCurrencyRefusal(t *testing.T) { require.Error(t, err) assert.NotErrorIs(t, err, common.ErrCurrencyNotAccepted) } + +func TestConvertSavingsPlan_CostKnown(t *testing.T) { + t.Parallel() + now := time.Now().UTC() + zero, rate := 0.0, 2.5 + plan := func(c *armbillingbenefits.Commitment) *armbillingbenefits.SavingsPlanModel { + return &armbillingbenefits.SavingsPlanModel{ + ID: toPtr("/subscriptions/sub/savingsPlans/sp1"), + SKU: &armbillingbenefits.SKU{Name: toPtr("Compute_Savings_Plan")}, + Properties: &armbillingbenefits.SavingsPlanModelProperties{ + EffectiveDateTime: &now, + Commitment: c, + }, + } + } + tests := []struct { + name string + commit *armbillingbenefits.Commitment + wantKnown bool + wantCost float64 + }{ + {"zero amount is known", &armbillingbenefits.Commitment{Amount: &zero}, true, 0}, + {"reported amount is known", &armbillingbenefits.Commitment{Amount: &rate}, true, 2.5}, + {"nil amount is unknown", &armbillingbenefits.Commitment{}, false, 0}, + {"nil commitment is unknown", nil, false, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := convertSavingsPlan(plan(tt.commit), "sub") + require.NoError(t, err) + assert.Equal(t, tt.wantKnown, got.CostKnown) + assert.Equal(t, tt.wantCost, got.Cost) + }) + } +} + +func TestConvertSavingsPlan_NilPropertiesIsRejected(t *testing.T) { + t.Parallel() + sp := &armbillingbenefits.SavingsPlanModel{ + ID: toPtr("/subscriptions/sub/savingsPlans/sp1"), + SKU: &armbillingbenefits.SKU{Name: toPtr("Compute_Savings_Plan")}, + } + + got, err := convertSavingsPlan(sp, "sub") + + require.Error(t, err) + assert.Nil(t, got, "no commitment is returned, so no cost can be claimed known") +} From cdc2d1386dd86d89c011c7d55052a92f23c0d83c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:10:41 +0100 Subject: [PATCH 04/10] fix(azure/cache): stop fabricating region and state The consumption ReservationDetail API reports neither, so the cache inventory converter left them to the client region and a hardcoded active. Leave them empty (unknown). Adds a shared RequireUnknownInventory test helper. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/azure/services/cache/client.go | 2 -- providers/azure/services/cache/client_test.go | 30 +++++++++++++++++++ .../reservationstest/inventory.go | 22 ++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 providers/azure/services/internal/reservations/reservationstest/inventory.go diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index acb1035a..3d0de922 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -186,8 +186,6 @@ func (c *Client) convertRedisReservation(detail *armconsumption.ReservationDetai Account: c.subscriptionID, CommitmentType: common.CommitmentReservedInstance, Service: common.ServiceCache, - Region: c.region, - State: common.CommitmentStateActive, } if props.ReservationID != nil { diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go index 3ca58d62..5f546397 100644 --- a/providers/azure/services/cache/client_test.go +++ b/providers/azure/services/cache/client_test.go @@ -16,6 +16,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" azpricing "github.com/LeanerCloud/cloud-commitments-go/providers/azure/internal/pricing" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/internal/reservations/reservationstest" ) // MockReservationsDetailsPager mocks the ReservationsDetailsPager interface. @@ -861,3 +862,32 @@ func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { require.Error(t, err, "nil-fallback client must reject IMDS connections") assert.Contains(t, err.Error(), "blocked") } + +func TestCacheClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testing.T) { + t.Parallel() + for _, region := range []string{"eastus", "westeurope"} { + t.Run(region, func(t *testing.T) { + t.Parallel() + client := NewClient(nil, "test-subscription", region) + reservationID, skuName := "res-1", "redis-premium-p1" + client.SetReservationsPager(&MockReservationsDetailsPager{ + pages: []armconsumption.ReservationsDetailsClientListResponse{{ + ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{{ + Properties: &armconsumption.ReservationDetailProperties{ + ReservationID: &reservationID, + SKUName: &skuName, + }, + }}, + }, + }}, + }) + + commitments, err := client.GetExistingCommitments(context.Background()) + + require.NoError(t, err) + require.Len(t, commitments, 1) + reservationstest.RequireUnknownInventory(t, commitments[0]) + }) + } +} diff --git a/providers/azure/services/internal/reservations/reservationstest/inventory.go b/providers/azure/services/internal/reservations/reservationstest/inventory.go new file mode 100644 index 00000000..d982b86a --- /dev/null +++ b/providers/azure/services/internal/reservations/reservationstest/inventory.go @@ -0,0 +1,22 @@ +package reservationstest + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" +) + +// RequireUnknownInventory asserts that c carries only what the consumption +// ReservationDetail API reports: no region, state, price, quantity or start +// date. Producers built on that API must not invent these from the client. +func RequireUnknownInventory(t *testing.T, c common.Commitment) { + t.Helper() + assert.Empty(t, c.Region, "region is not in ReservationDetail") + assert.Empty(t, c.State, "state is not in ReservationDetail") + assert.False(t, c.CostKnown) + assert.Zero(t, c.Cost) + assert.Zero(t, c.Count) + assert.True(t, c.StartDate.IsZero()) +} From 67bba811f826778e8b166b5cc0131a0421ed1f36 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:11:22 +0100 Subject: [PATCH 05/10] fix(azure/cosmosdb,database): stop fabricating region and state Same as the cache converter: ReservationDetail reports neither, so leave Region and State empty instead of the client region and active. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/azure/services/cosmosdb/client.go | 2 -- .../azure/services/cosmosdb/client_test.go | 30 +++++++++++++++++++ providers/azure/services/database/client.go | 2 -- .../azure/services/database/client_test.go | 30 +++++++++++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index 66eb07e7..0a29c544 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -285,8 +285,6 @@ func (c *Client) convertCosmosReservation(detail *armconsumption.ReservationDeta Account: c.subscriptionID, CommitmentType: common.CommitmentReservedInstance, Service: common.ServiceNoSQL, - Region: c.region, - State: common.CommitmentStateActive, } if props.ReservationID != nil { diff --git a/providers/azure/services/cosmosdb/client_test.go b/providers/azure/services/cosmosdb/client_test.go index 06a846bf..f4b5e9aa 100644 --- a/providers/azure/services/cosmosdb/client_test.go +++ b/providers/azure/services/cosmosdb/client_test.go @@ -21,6 +21,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/internal/reservations/reservationstest" ) // MockRecommendationsPager mocks the RecommendationsPager interface. @@ -1768,3 +1769,32 @@ func TestGetCosmosPricing_LegitimateSKUBuildsExactFilter(t *testing.T) { assert.Equal(t, "serviceName eq 'Azure Cosmos DB' and productName eq 'Azure Cosmos DB' and armRegionName eq 'Global' and armSkuName eq 'Cosmos_DB_100_RUs' and priceType eq 'Reservation'", req.URL.Query().Get("$filter")) mockHTTP.AssertExpectations(t) } + +func TestCosmosDBClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testing.T) { + t.Parallel() + for _, region := range []string{"eastus", "westeurope"} { + t.Run(region, func(t *testing.T) { + t.Parallel() + client := NewClient(nil, "test-subscription", region) + reservationID, skuName := "res-1", "cosmos-db-standard" + client.SetReservationsPager(&MockReservationsDetailsPager{ + pages: []armconsumption.ReservationsDetailsClientListResponse{{ + ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{{ + Properties: &armconsumption.ReservationDetailProperties{ + ReservationID: &reservationID, + SKUName: &skuName, + }, + }}, + }, + }}, + }) + + commitments, err := client.GetExistingCommitments(context.Background()) + + require.NoError(t, err) + require.Len(t, commitments, 1) + reservationstest.RequireUnknownInventory(t, commitments[0]) + }) + } +} diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index da0b56c1..74db42f8 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -341,8 +341,6 @@ func (c *Client) convertSQLReservation(detail *armconsumption.ReservationDetail) Account: c.subscriptionID, CommitmentType: common.CommitmentReservedInstance, Service: common.ServiceRelationalDB, - Region: c.region, - State: common.CommitmentStateActive, } if props.ReservationID != nil { diff --git a/providers/azure/services/database/client_test.go b/providers/azure/services/database/client_test.go index bbf7ee93..ca2b58e4 100644 --- a/providers/azure/services/database/client_test.go +++ b/providers/azure/services/database/client_test.go @@ -22,6 +22,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" azpricing "github.com/LeanerCloud/cloud-commitments-go/providers/azure/internal/pricing" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/internal/reservations/reservationstest" ) // MockRecommendationsPager mocks the RecommendationsPager interface. @@ -2196,3 +2197,32 @@ func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { require.Error(t, err, "nil-fallback client must reject IMDS connections") assert.Contains(t, err.Error(), "blocked") } + +func TestDatabaseClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testing.T) { + t.Parallel() + for _, region := range []string{"eastus", "westeurope"} { + t.Run(region, func(t *testing.T) { + t.Parallel() + client := NewClient(nil, "test-subscription", region) + reservationID, skuName := "res-1", "sql-standard-s0" + client.SetReservationsPager(&MockReservationsDetailsPager{ + pages: []armconsumption.ReservationsDetailsClientListResponse{{ + ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{{ + Properties: &armconsumption.ReservationDetailProperties{ + ReservationID: &reservationID, + SKUName: &skuName, + }, + }}, + }, + }}, + }) + + commitments, err := client.GetExistingCommitments(context.Background()) + + require.NoError(t, err) + require.Len(t, commitments, 1) + reservationstest.RequireUnknownInventory(t, commitments[0]) + }) + } +} From 8e62d60c98df6fc7d572a5ccc976cab092cbfeee Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:11:46 +0100 Subject: [PATCH 06/10] fix(azure/search,synapse): stop fabricating region and state Same as the cache converter: leave Region and State empty because ReservationDetail does not report them. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/azure/services/search/client.go | 2 -- .../azure/services/search/client_test.go | 30 +++++++++++++++++++ providers/azure/services/synapse/client.go | 2 -- .../azure/services/synapse/client_test.go | 30 +++++++++++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go index d79deb48..720bc99f 100644 --- a/providers/azure/services/search/client.go +++ b/providers/azure/services/search/client.go @@ -204,8 +204,6 @@ func (c *Client) convertSearchReservation(detail *armconsumption.ReservationDeta Account: c.subscriptionID, CommitmentType: common.CommitmentReservedInstance, Service: common.ServiceSearch, - Region: c.region, - State: common.CommitmentStateActive, } if props.ReservationID != nil { diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go index cfda5ecd..8a82aca2 100644 --- a/providers/azure/services/search/client_test.go +++ b/providers/azure/services/search/client_test.go @@ -21,6 +21,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/internal/pricing" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/internal/reservations/reservationstest" ) // MockRecommendationsPager mocks the RecommendationsPager interface. @@ -1589,3 +1590,32 @@ func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { require.Error(t, err, "nil-fallback client must reject IMDS connections") assert.Contains(t, err.Error(), "blocked") } + +func TestSearchClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testing.T) { + t.Parallel() + for _, region := range []string{"eastus", "westeurope"} { + t.Run(region, func(t *testing.T) { + t.Parallel() + client := NewClient(nil, "test-subscription", region) + reservationID, skuName := "res-1", "search-standard-s1" + client.SetReservationsPager(&MockReservationsDetailsPager{ + pages: []armconsumption.ReservationsDetailsClientListResponse{{ + ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{{ + Properties: &armconsumption.ReservationDetailProperties{ + ReservationID: &reservationID, + SKUName: &skuName, + }, + }}, + }, + }}, + }) + + commitments, err := client.GetExistingCommitments(context.Background()) + + require.NoError(t, err) + require.Len(t, commitments, 1) + reservationstest.RequireUnknownInventory(t, commitments[0]) + }) + } +} diff --git a/providers/azure/services/synapse/client.go b/providers/azure/services/synapse/client.go index 0d79c14f..45a6d55d 100644 --- a/providers/azure/services/synapse/client.go +++ b/providers/azure/services/synapse/client.go @@ -268,8 +268,6 @@ func (c *Client) convertSynapseReservation(detail *armconsumption.ReservationDet Account: c.subscriptionID, CommitmentType: common.CommitmentReservedInstance, Service: common.ServiceDataWarehouse, - Region: c.region, - State: common.CommitmentStateActive, } if props.ReservationID != nil { commitment.CommitmentID = *props.ReservationID diff --git a/providers/azure/services/synapse/client_test.go b/providers/azure/services/synapse/client_test.go index 9b1c85bc..4a7dacfa 100644 --- a/providers/azure/services/synapse/client_test.go +++ b/providers/azure/services/synapse/client_test.go @@ -23,6 +23,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/internal/reservations" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/internal/reservations/reservationstest" ) // ---- credential mock ------------------------------------------------------- @@ -1333,3 +1334,32 @@ func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { require.Error(t, err, "nil-fallback client must reject IMDS connections") assert.Contains(t, err.Error(), "blocked") } + +func TestSynapseClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testing.T) { + t.Parallel() + for _, region := range []string{"eastus", "westeurope"} { + t.Run(region, func(t *testing.T) { + t.Parallel() + client := &Client{subscriptionID: "sub-123", region: region} + reservationID, skuName := "res-1", "DW1000c" + client.SetReservationsPager(&fakeReservationsPager{ + pages: []armconsumption.ReservationsDetailsClientListResponse{{ + ReservationDetailsListResult: armconsumption.ReservationDetailsListResult{ + Value: []*armconsumption.ReservationDetail{{ + Properties: &armconsumption.ReservationDetailProperties{ + ReservationID: &reservationID, + SKUName: &skuName, + }, + }}, + }, + }}, + }) + + commitments, err := client.GetExistingCommitments(context.Background()) + + require.NoError(t, err) + require.Len(t, commitments, 1) + reservationstest.RequireUnknownInventory(t, commitments[0]) + }) + } +} From f37ef20b849bad29904fb016c3988a938974a1af Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:12:21 +0100 Subject: [PATCH 07/10] docs(azure): describe unknown cost via CostKnown Drop the stale issue #62 reference and the no-consumer-reads-Cost claim. Cost is unknown here, CostKnown stays false, and zero is not free. Refs #74 Co-Authored-By: Claude Opus 5.5 --- .../azure/services/internal/reservations/inventory.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/providers/azure/services/internal/reservations/inventory.go b/providers/azure/services/internal/reservations/inventory.go index 80994f3e..e08eb5c2 100644 --- a/providers/azure/services/internal/reservations/inventory.go +++ b/providers/azure/services/internal/reservations/inventory.go @@ -224,11 +224,8 @@ func appliedOrderIDs(resp armreservations.AzureReservationAPIClientGetAppliedRes // The SDK exposes PurchaseDate at day precision, so this cannot establish // whether a purchase yesterday occurred within the last 24 hours. // -// Cost stays at zero. The reservations API carries no per-reservation price, -// so zero here means "unknown", not "free". The shared common.Commitment.Cost -// scalar has no absent-value representation (tracked by issue #62); the only -// in-repo consumer of GetExistingCommitments (the pkg/recfilter -// DuplicateChecker) never reads Cost, so the zero value is harmless there. +// Cost is unknown: the reservations API carries no per-reservation price, so +// CostKnown stays false and a zero Cost must not be read as free. func CommitmentFromReservation(r *armreservations.ReservationResponse, account string, service common.ServiceType, wantType armreservations.ReservedResourceType, now time.Time) *common.Commitment { if r == nil || r.Properties == nil { return nil From 19f230ef5e84cd2055693b24a8877024a0c30dfc Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:35:31 +0100 Subject: [PATCH 08/10] fix(aws/ladder): only claim a known RI cost when Duration is set riHourlyCost skips upfront amortization without a Duration, so an RI with missing pricing data got a partial cost yet claimed CostKnown. Gate the marker on Duration > 0 and replace the misleading zero-cost test. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/aws/ladder/commitments.go | 5 ++++- providers/aws/ladder/commitments_test.go | 24 +++++++++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/providers/aws/ladder/commitments.go b/providers/aws/ladder/commitments.go index 3129edbd..6eb5f44f 100644 --- a/providers/aws/ladder/commitments.go +++ b/providers/aws/ladder/commitments.go @@ -74,6 +74,9 @@ func (a *AWSLadder) listRICommitments(ctx context.Context) ([]common.Commitment, // FixedPrice) are per-instance, so the per-instance hourly rate is multiplied // by InstanceCount. See riHourlyCost for the formula and semantics (matches the // repo's canonical monthlyCostFromConvertibleRI in internal/api/handler_ri_exchange.go). +// +// CostKnown requires a Duration: riHourlyCost skips upfront amortization without +// one, so the total would be incomplete. func riToCommitment(ri *ec2svc.ConvertibleRI, accountID, region string) common.Commitment { totalHourlyCost := riHourlyCost(ri) @@ -90,7 +93,7 @@ func riToCommitment(ri *ec2svc.ConvertibleRI, accountID, region string) common.C StartDate: ri.Start, EndDate: ri.End, Cost: totalHourlyCost, - CostKnown: true, + CostKnown: ri.Duration > 0, } } diff --git a/providers/aws/ladder/commitments_test.go b/providers/aws/ladder/commitments_test.go index a1885316..991fd7d4 100644 --- a/providers/aws/ladder/commitments_test.go +++ b/providers/aws/ladder/commitments_test.go @@ -8,16 +8,30 @@ import ( ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" ) -func TestRIToCommitment_ZeroComputedCostIsKnown(t *testing.T) { +func TestRIToCommitment_FullyUpfrontWithDurationIsKnown(t *testing.T) { t.Parallel() - // Fully-upfront RI with no duration, recurring or usage price: the - // computed hourly cost is a real 0, not an unknown one. - ri := &ec2svc.ConvertibleRI{ReservedInstanceID: "ri-1", InstanceCount: 2} + ri := &ec2svc.ConvertibleRI{ + ReservedInstanceID: "ri-1", + InstanceCount: 2, + FixedPrice: 8760, + Duration: 31536000, // one year in seconds + } got := riToCommitment(ri, "123456789012", "us-east-1") assert.True(t, got.CostKnown) - assert.Zero(t, got.Cost) + assert.Greater(t, got.Cost, 0.0) +} + +func TestRIToCommitment_NoDurationIsUnknown(t *testing.T) { + t.Parallel() + // Without a duration the upfront part cannot be amortized, so the + // computed cost is incomplete and must not be claimed as known. + ri := &ec2svc.ConvertibleRI{ReservedInstanceID: "ri-1", InstanceCount: 2, FixedPrice: 8760} + + got := riToCommitment(ri, "123456789012", "us-east-1") + + assert.False(t, got.CostKnown) } func TestSPToCommitment_ZeroHourlyCommitmentIsKnown(t *testing.T) { From 89919c2ecdd27b8a27d717391f523f631b196199 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 01:37:28 +0100 Subject: [PATCH 09/10] refactor(azure): rename RequireUnknownInventory to AssertUnknownInventory The helper uses assert, so it does not stop the test; name it accordingly. Refs #74 Co-Authored-By: Claude Opus 5.5 --- providers/azure/services/cache/client_test.go | 2 +- providers/azure/services/cosmosdb/client_test.go | 2 +- providers/azure/services/database/client_test.go | 2 +- .../internal/reservations/reservationstest/inventory.go | 4 ++-- providers/azure/services/search/client_test.go | 2 +- providers/azure/services/synapse/client_test.go | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go index 5f546397..698a929d 100644 --- a/providers/azure/services/cache/client_test.go +++ b/providers/azure/services/cache/client_test.go @@ -887,7 +887,7 @@ func TestCacheClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testin require.NoError(t, err) require.Len(t, commitments, 1) - reservationstest.RequireUnknownInventory(t, commitments[0]) + reservationstest.AssertUnknownInventory(t, commitments[0]) }) } } diff --git a/providers/azure/services/cosmosdb/client_test.go b/providers/azure/services/cosmosdb/client_test.go index f4b5e9aa..c78d9577 100644 --- a/providers/azure/services/cosmosdb/client_test.go +++ b/providers/azure/services/cosmosdb/client_test.go @@ -1794,7 +1794,7 @@ func TestCosmosDBClient_GetExistingCommitments_DoesNotInventRegionOrState(t *tes require.NoError(t, err) require.Len(t, commitments, 1) - reservationstest.RequireUnknownInventory(t, commitments[0]) + reservationstest.AssertUnknownInventory(t, commitments[0]) }) } } diff --git a/providers/azure/services/database/client_test.go b/providers/azure/services/database/client_test.go index ca2b58e4..77ed011c 100644 --- a/providers/azure/services/database/client_test.go +++ b/providers/azure/services/database/client_test.go @@ -2222,7 +2222,7 @@ func TestDatabaseClient_GetExistingCommitments_DoesNotInventRegionOrState(t *tes require.NoError(t, err) require.Len(t, commitments, 1) - reservationstest.RequireUnknownInventory(t, commitments[0]) + reservationstest.AssertUnknownInventory(t, commitments[0]) }) } } diff --git a/providers/azure/services/internal/reservations/reservationstest/inventory.go b/providers/azure/services/internal/reservations/reservationstest/inventory.go index d982b86a..dcfc88c7 100644 --- a/providers/azure/services/internal/reservations/reservationstest/inventory.go +++ b/providers/azure/services/internal/reservations/reservationstest/inventory.go @@ -8,10 +8,10 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" ) -// RequireUnknownInventory asserts that c carries only what the consumption +// AssertUnknownInventory asserts that c carries only what the consumption // ReservationDetail API reports: no region, state, price, quantity or start // date. Producers built on that API must not invent these from the client. -func RequireUnknownInventory(t *testing.T, c common.Commitment) { +func AssertUnknownInventory(t *testing.T, c common.Commitment) { t.Helper() assert.Empty(t, c.Region, "region is not in ReservationDetail") assert.Empty(t, c.State, "state is not in ReservationDetail") diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go index 8a82aca2..48178b85 100644 --- a/providers/azure/services/search/client_test.go +++ b/providers/azure/services/search/client_test.go @@ -1615,7 +1615,7 @@ func TestSearchClient_GetExistingCommitments_DoesNotInventRegionOrState(t *testi require.NoError(t, err) require.Len(t, commitments, 1) - reservationstest.RequireUnknownInventory(t, commitments[0]) + reservationstest.AssertUnknownInventory(t, commitments[0]) }) } } diff --git a/providers/azure/services/synapse/client_test.go b/providers/azure/services/synapse/client_test.go index 4a7dacfa..93e85ba4 100644 --- a/providers/azure/services/synapse/client_test.go +++ b/providers/azure/services/synapse/client_test.go @@ -1359,7 +1359,7 @@ func TestSynapseClient_GetExistingCommitments_DoesNotInventRegionOrState(t *test require.NoError(t, err) require.Len(t, commitments, 1) - reservationstest.RequireUnknownInventory(t, commitments[0]) + reservationstest.AssertUnknownInventory(t, commitments[0]) }) } } From 93a29ee5dd9899509255c5d262bedb5c7aece4c8 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 03:13:05 +0100 Subject: [PATCH 10/10] chore(azure): tidy go.sum after the pkg re-pin Drops the go.sum lines for the old pkg, aws-sdk-go-v2 and smithy-go versions that the rebased pin no longer needs. Co-Authored-By: Claude Opus 5.5 --- providers/azure/go.sum | 6 ------ 1 file changed, 6 deletions(-) diff --git a/providers/azure/go.sum b/providers/azure/go.sum index 64de652b..b3d56161 100644 --- a/providers/azure/go.sum +++ b/providers/azure/go.sum @@ -36,16 +36,10 @@ 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.10.1 h1:0O6j18nQDoIff6/mtosT5LNU2WehEwp+LqfX6699oqY= github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1/go.mod h1:xdYAf5bjkOpsd3auA8riiJW4vneBubt9caavL7626jQ= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac h1:EvEUCt1uH5oBtPcJJPt1mX6XYQWAKtL9QBSo9CfeaOw= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac/go.mod h1:pseCFRgm1rzBKomktjw6E0ayxmVE7XfHHiOmew4ka+g= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf h1:uKewFUeWwrvgoBaFeZMepJXs+xsQdqTVRomAc6tYMX8= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106184548-8d06fde536cf/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= -github.com/aws/aws-sdk-go-v2 v1.47.4 h1:bTvk6UdSSSvaQsfLVj1JRDEEqj/PDRwGf1KanaRzau0= -github.com/aws/aws-sdk-go-v2 v1.47.4/go.mod h1:XkrjAZvr705Nytv6PkPUhoSB2W9+vVHBp5oAWpkh8Ak= github.com/aws/aws-sdk-go-v2 v1.47.5 h1:5yA+A+BiXbDq1ZLbYwPViwhEGDMdR0MEhL8Jh2LnLS4= github.com/aws/aws-sdk-go-v2 v1.47.5/go.mod h1:iNIkogrRvFwm+1OQIUOP2lPBtlXZboOIUWiCa+pHEMw= -github.com/aws/smithy-go v1.28.6 h1:Pj+/sUviNYjow8ELRtNwKHZ9ECeF+7pATX3+shjLIrw= -github.com/aws/smithy-go v1.28.6/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/aws/smithy-go v1.28.7 h1:h/n1SHrwVtaDYLz8mR1/Kq4Xh4RaFbFW7bOYckg9mfE= github.com/aws/smithy-go v1.28.7/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY=