From a78a5e7d0318c76fce14d09e4af892aa241e6333 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 03:07:30 +0100 Subject: [PATCH 1/7] chore(deps): pin cloud-commitments-go at 0b677e1 Moves all four cloud-commitments-go modules from c83aef8 to 0b677e1, which adds #706 (Azure Redis dedupe against held reservations), #702 (managedredis is the only Azure Redis recommendation producer), #709 (optional attempt-journal claim reader), #694 (the Azure cache client refuses purchases) and #695 (ladder statuses spelled canceled). The platform changes these need follow in this PR. Co-Authored-By: Claude Opus 5.5 --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index e8945c471..ee1314449 100644 --- a/go.mod +++ b/go.mod @@ -88,10 +88,10 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations v1.1.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.5.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.5.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106191535-c83aef8b18a9 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261106191535-c83aef8b18a9 - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261106191535-c83aef8b18a9 - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261106191535-c83aef8b18a9 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261107014806-0b677e19ae94 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261107014806-0b677e19ae94 + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261107014806-0b677e19ae94 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261107014806-0b677e19ae94 github.com/aws/aws-lambda-go v1.55.1 github.com/aws/aws-sdk-go-v2/service/kms v1.62.1 github.com/aws/aws-sdk-go-v2/service/lambda v1.113.4 diff --git a/go.sum b/go.sum index 60277f83d..17d1e6340 100644 --- a/go.sum +++ b/go.sum @@ -90,14 +90,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.57.0/go.mod h1:dzcEjy1WJ0Q4u9twNR3LcLhNoYMRCrMCMafpxa0TjPQ= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.57.0 h1:RoO5+d7uCmDqovLrHCr2/BuViUXvdcrNxyNM1pN9dDQ= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.57.0/go.mod h1:YqwkQPrWSC7+byyc1VlKbWLBF5JsW5IoL6xUkemYSXk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106191535-c83aef8b18a9 h1:omDAY4B4wshoedBYE368TW05d0PR2HTnebPgu8WqIFw= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261106191535-c83aef8b18a9 h1:z4LNOkM5qTNq7dbzfdh0VdsSW4BTxRuVaXjOZfRADe0= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:uVh1sdSIwKbHCWZ/TA66m2cR9Odb/36sxDHNKLyuMKY= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261106191535-c83aef8b18a9 h1:OISjPdyg7xcy8kCJelHJle4YnTMQycAWzglVi6L15uw= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:X+73EdStvYV6pqEVZZFyMfLG1frORlO9B1IsuK0LfOs= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261106191535-c83aef8b18a9 h1:7Vz6QVVlwe0GG7TT2+4tQp9HBBCZ0YkfEDwFZOF+Ir8= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261106191535-c83aef8b18a9/go.mod h1:kfbQ5CsDr1958iSuZq6edcuB0es62XRCXGn/hFFYBW0= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261107014806-0b677e19ae94 h1:4AYCeaAqeNBKKLs6Q/FeIRWXCWFUe59/RiBrY9O39Xw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261107014806-0b677e19ae94/go.mod h1:l9V+ecJCfAigYr6XiK9kpIUemF9bnXOKHJN1s4TPsM0= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261107014806-0b677e19ae94 h1:J7e8eVErWWG+yP80X2GAv5/dTTXQemejLXC1qYsvBWk= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261107014806-0b677e19ae94/go.mod h1:uVh1sdSIwKbHCWZ/TA66m2cR9Odb/36sxDHNKLyuMKY= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261107014806-0b677e19ae94 h1:55Yb/A/GF5yDCH+AxUnHbVZFMFNKMVjxipkgxgBLwss= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261107014806-0b677e19ae94/go.mod h1:h6EewdM42n97XHBC3/aMxZFAMUOoEla4Y18TDYtfC6w= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261107014806-0b677e19ae94 h1:7k9LzEBsSzF2HQFlUDDm6AMj/hs4VKLbL6MoC3Ghkq4= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261107014806-0b677e19ae94/go.mod h1:kfbQ5CsDr1958iSuZq6edcuB0es62XRCXGn/hFFYBW0= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8= From e2c6cf56c0307dc965ab499388019196b4626f07 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 15:02:00 +0100 Subject: [PATCH 2/7] fix(purchase): buy Azure Redis recommendations through managedredis cloud-commitments-go#694 makes the Azure cache client refuse every purchase: managedredis (ServiceMemoryDB) is now the only Azure service that buys RedisCache reservations. Azure Redis recommendations are still labeled ServiceCache, so once the go pin includes #694 every Azure Redis purchase through the platform would be refused. The purchase now asks the provider for the client that buys the recommendation (purchaseClientService): Azure ServiceCache resolves to ServiceMemoryDB, everything else is unchanged. The recommendation, its history row and suppressions keep their own service label; the managedredis client does not read it. Refs #1801 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 399a832c663b03a27a494409efd1e34499ee896d) --- internal/purchase/azure_redis_routing_test.go | 69 +++++++++++++++++++ internal/purchase/execution.go | 14 +++- internal/purchase/execution_test.go | 10 +-- 3 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 internal/purchase/azure_redis_routing_test.go diff --git a/internal/purchase/azure_redis_routing_test.go b/internal/purchase/azure_redis_routing_test.go new file mode 100644 index 000000000..0d817ceed --- /dev/null +++ b/internal/purchase/azure_redis_routing_test.go @@ -0,0 +1,69 @@ +package purchase + +import ( + "context" + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// #1801: only the managedredis client (ServiceMemoryDB) buys Azure Redis +// reservations; the Azure cache client refuses every purchase +// (cloud-commitments-go#694). A recommendation labeled "cache" must reach the +// right client for its provider, and keep its own label everywhere else. +func purchaseCacheRecThrough(t *testing.T, providerName, region string, want common.ServiceType) *MockProvider { + t.Helper() + ctx := context.Background() + store, sender, factory, prov, svc := new(MockConfigStore), new(MockEmailSender), new(MockProviderFactory), new(MockProvider), new(MockServiceClient) + assertMocksOnCleanup(t, store, sender, factory, prov, svc) + store.SavePurchaseExecutionFn = func(context.Context, *config.PurchaseExecution) error { return nil } + store.On("SavePurchaseHistory", ctx, mock.Anything).Return(nil) + factory.On("CreateAndValidateProvider", mock.Anything, providerName, mock.Anything).Return(prov, nil) + prov.On("GetServiceClient", mock.Anything, want, region).Return(provider.ServiceClient(svc), nil) + svc.On("PurchaseCommitment", mock.Anything, mock.MatchedBy(func(rec common.Recommendation) bool { + return rec.Service == common.ServiceCache + }), mock.Anything).Return(common.PurchaseResult{Success: true, CommitmentID: "order-1"}, nil) + + exec := &config.PurchaseExecution{ExecutionID: "exec-1801-" + providerName, IdempotencyKey: "lineage-1801-" + providerName, + Recommendations: []config.RecommendationRecord{{ID: "rec-1", Provider: providerName, Service: "cache", + ResourceType: "cache.r6g.large", Region: region, Count: 1, Term: 1, Selected: true}}} + m := &Manager{config: store, email: sender, providerFactory: factory} + _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.Config)(nil), "") + require.NoError(t, err) + require.Empty(t, errs) + require.True(t, exec.Recommendations[0].Purchased, "the purchase must reach the %s client", want) + return prov +} + +// [fail-first] On main the Azure lookup asks for ServiceCache, which this +// mock provider does not serve, so the purchase fails. +func TestAzureCachePurchaseUsesTheManagedRedisClient(t *testing.T) { + prov := purchaseCacheRecThrough(t, "azure", "westeurope", common.ServiceMemoryDB) + prov.AssertNotCalled(t, "GetServiceClient", mock.Anything, common.ServiceCache, mock.Anything) +} + +// [guard] AWS "cache" stays on the cache (ElastiCache) client. +func TestAWSCachePurchaseKeepsTheCacheClient(t *testing.T) { + prov := purchaseCacheRecThrough(t, "aws", "us-east-1", common.ServiceCache) + prov.AssertNotCalled(t, "GetServiceClient", mock.Anything, common.ServiceMemoryDB, mock.Anything) +} + +func TestPurchaseClientService(t *testing.T) { + for _, tc := range []struct { + provider string + in, want common.ServiceType + }{ + {"azure", common.ServiceCache, common.ServiceMemoryDB}, + {"azure", common.ServiceMemoryDB, common.ServiceMemoryDB}, + {"azure", common.ServiceCompute, common.ServiceCompute}, + {"aws", common.ServiceCache, common.ServiceCache}, + {"gcp", common.ServiceCache, common.ServiceCache}, + {"", common.ServiceCache, common.ServiceCache}, + } { + require.Equal(t, tc.want, purchaseClientService(tc.provider, tc.in), "%s/%s", tc.provider, tc.in) + } +} diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 1965e0f85..15e98de59 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -1278,7 +1278,7 @@ func (m *Manager) executeSinglePurchase(ctx context.Context, rec config.Recommen // Get the service client for this region t0 = time.Now() - serviceClient, err := cloudProvider.GetServiceClient(recCtx, serviceType, rec.Region) + serviceClient, err := cloudProvider.GetServiceClient(recCtx, purchaseClientService(rec.Provider, serviceType), rec.Region) if err != nil { logging.Errorf("purchase[%s]: service client lookup failed for %s/%s in %s after %s: %v", opts.ExecutionID, rec.Provider, serviceType, rec.Region, time.Since(t0), err) @@ -1417,6 +1417,18 @@ func resolveEC2OfferingClass(s string) (ec2types.OfferingClassType, error) { } } +// purchaseClientService is the service whose client buys a recommendation of +// service. Azure Redis recommendations are labeled ServiceCache, but only the +// managedredis client (ServiceMemoryDB) buys RedisCache reservations; the +// Azure cache client refuses every purchase (cloud-commitments-go#694, #1801). +// Labels in history and suppressions keep the recommendation's own service. +func purchaseClientService(providerName string, service common.ServiceType) common.ServiceType { + if providerName == string(common.ProviderAzure) && service == common.ServiceCache { + return common.ServiceMemoryDB + } + return service +} + // mapServiceSlug returns the common.ServiceType for a canonical or legacy // service slug. // diff --git a/internal/purchase/execution_test.go b/internal/purchase/execution_test.go index eed8fc74a..bd7714b5a 100644 --- a/internal/purchase/execution_test.go +++ b/internal/purchase/execution_test.go @@ -1449,10 +1449,12 @@ func TestExecutePurchase_AzureCanonicalServiceTypes(t *testing.T) { expectedType: common.ServiceCompute, }, { - name: "cache_canonical", - service: "cache", - resource: "Standard_C1", - expectedType: common.ServiceCache, + name: "cache_canonical", + service: "cache", + resource: "Standard_C1", + // Azure Redis is bought by the managedredis client; the cache + // client refuses purchases (cloud-commitments-go#694, #1801). + expectedType: common.ServiceMemoryDB, }, { name: "relational_db_canonical", From 9cf0789f7712a08cbfe4c73d7a0c52af9dcae9c6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 21:15:40 +0100 Subject: [PATCH 3/7] fix(ladder): accept both stored spellings of canceled cloud-commitments-go#554 writes ladder run and tranche statuses as "canceled", and no migration renames ladder_runs or ladder_tranches rows, so legacy "cancelled" rows and new "canceled" rows coexist. - The execution recheck parses the run status (ParseRunStatus), so both spellings refuse and a status the code does not know fails closed. - The amend ceiling's latest-run SQL excludes both spellings. - The ladder timeline loaders normalize both to "canceled", so labels, filters and counts see one value; the OpenAPI enums list both. - Every use of the deprecated Cancelled constants is renamed to Canceled. Tests cover each site with both spellings, including the Retry guard (LockLadderTrancheForRetryTx feeds LadderTrancheSettled). Part of #1792 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 63b70cd5d3318c77fffd6d8c7d94771ce2c096df) --- .../src/__tests__/ladder-timeline-api.test.ts | 12 +++++++- .../__tests__/ladder-timeline-filters.test.ts | 12 ++++---- .../src/__tests__/ladder-timeline.test.ts | 4 +-- frontend/src/api/ladder.ts | 14 +++++++--- frontend/src/ladder-timeline-filters.ts | 8 +++--- ...er_purchases_retry_ladder_canceled_test.go | 28 +++++++++++++++++++ internal/api/openapi.yaml | 9 ++++-- internal/config/interfaces.go | 2 +- internal/config/store_postgres_ladder.go | 6 ++-- .../store_postgres_ladder_amend_ceiling.go | 5 ++-- ...s_ladder_amend_ceiling_integration_test.go | 23 +++++++++------ .../config/store_postgres_ladder_cancel.go | 6 ++-- .../store_postgres_ladder_cancel_test.go | 10 +++---- ...e_postgres_ladder_gate_integration_test.go | 2 +- .../config/store_postgres_ladder_lifecycle.go | 4 +-- ...tgres_ladder_lifecycle_integration_test.go | 16 +++++------ .../store_postgres_ladder_lifecycle_test.go | 12 ++++---- ...ostgres_ladder_netting_integration_test.go | 4 +-- ...ostgres_ladder_settled_integration_test.go | 4 +-- ...es_ladder_settled_plan_integration_test.go | 2 +- internal/config/store_postgres_ladder_test.go | 12 ++++---- internal/purchase/ladder_execution_recheck.go | 13 ++++++++- .../purchase/ladder_execution_recheck_test.go | 7 +++-- ...ndler_ladder_immediate_integration_test.go | 4 +-- 24 files changed, 143 insertions(+), 76 deletions(-) create mode 100644 internal/api/handler_purchases_retry_ladder_canceled_test.go diff --git a/frontend/src/__tests__/ladder-timeline-api.test.ts b/frontend/src/__tests__/ladder-timeline-api.test.ts index bc645984b..70f37d7b0 100644 --- a/frontend/src/__tests__/ladder-timeline-api.test.ts +++ b/frontend/src/__tests__/ladder-timeline-api.test.ts @@ -1,6 +1,6 @@ jest.mock('../api/client', () => ({ apiRequest: jest.fn() })); -import { getLadderEvents, type LadderEvent } from '../api/ladder'; +import { getLadderEvents, getLadderRuns, type LadderEvent, type LadderRun } from '../api/ladder'; import { apiRequest } from '../api/client'; beforeEach(() => jest.resetAllMocks()); @@ -31,3 +31,13 @@ test('OD cross-check compares the displayed od_usd_hr values and tolerates a pre jest.mocked(apiRequest).mockResolvedValueOnce({ total_count: 1, total_usd_hr: '5.000000', events: [legacy] }); expect(await getLadderEvents('account', 'aws')).toHaveLength(1); }); + +// #1792: legacy rows say "cancelled", new ones "canceled"; the loader returns one spelling. +test('loaders normalize both stored spellings of canceled', async () => { + const event: LadderEvent = { id: 'old', config_id: 'config', run_id: 'run', layer_type: 'compute-sp', term: '1yr', payment_option: 'no-upfront', status: 'cancelled', run_status: 'cancelled', revision: 0, amount_usd_hr: '1.000000', scheduled_date: '2030-01-01T00:00:00Z', created_at: '2026-01-01T00:00:00Z' }; + jest.mocked(apiRequest).mockResolvedValueOnce({ total_count: 3, total_usd_hr: '3.000000', events: [event, { ...event, id: 'new', status: 'canceled', run_status: 'canceled' }, { ...event, id: 'live', status: 'scheduled', run_status: 'planned' }] }); + expect((await getLadderEvents('account', 'aws')).map(e => [e.status, e.run_status])).toEqual([['canceled', 'canceled'], ['canceled', 'canceled'], ['scheduled', 'planned']]); + const run = { id: 'r1', config_id: 'config', status: 'cancelled', started_at: '2026-01-01T00:00:00Z', created_at: '2026-01-01T00:00:00Z', baseline_usd_hr: null, target_usd_hr: null, existing_usd_hr: null, actions: [] } as unknown as LadderRun; + jest.mocked(apiRequest).mockResolvedValueOnce({ total_count: 2, runs: [run, { ...run, id: 'r2', status: 'executing' }] }); + expect((await getLadderRuns('account', 'aws')).map(r => r.status)).toEqual(['canceled', 'executing']); +}); diff --git a/frontend/src/__tests__/ladder-timeline-filters.test.ts b/frontend/src/__tests__/ladder-timeline-filters.test.ts index a1af45a2a..85773fb43 100644 --- a/frontend/src/__tests__/ladder-timeline-filters.test.ts +++ b/frontend/src/__tests__/ladder-timeline-filters.test.ts @@ -39,8 +39,8 @@ test('eligibility covers status, execution, deadline, and unavailable revision A test('ladder labels cover every known enum and show unknown codes raw', () => { expect(['ec2-instance-sp', 'compute-sp', 'convertible-ri', 'azure-reservation', 'azure-savings-plan'].map(v => ladderLabel('layer_type', v))) .toEqual(['EC2 Instance Savings Plan', 'Compute Savings Plan', 'Convertible RI', 'Azure Reservation', 'Azure Savings Plan']); - expect(['scheduled', 'fired', 'completed', 'cancelled', 'failed'].map(v => ladderLabel('status', v))) - .toEqual(['Scheduled', 'Fired', 'Completed', 'Cancelled', 'Failed']); + expect(['scheduled', 'fired', 'completed', 'canceled', 'failed'].map(v => ladderLabel('status', v))) + .toEqual(['Scheduled', 'Fired', 'Completed', 'Canceled', 'Failed']); expect(['1yr', '3yr'].map(v => ladderLabel('term', v))).toEqual(['1 Year', '3 Years']); expect(['no-upfront', 'partial-upfront', 'all-upfront'].map(v => ladderLabel('payment_option', v))).toEqual(['No Upfront', 'Partial Upfront', 'All Upfront']); expect(ladderLabel('layer_type', 'new-thing')).toBe('new-thing'); @@ -66,7 +66,7 @@ test('sub-0.0001 amounts are not shown as $0.0000 and edit reasons use readable expect(formatLadderAmount('0.000040')).toBe('<$0.0001/hr'); expect(formatLadderAmount('0.000100')).toBe('$0.0001/hr'); expect(ladderEditReason({ ...event, run_status: 'awaiting_approval' })).toBe('Run is awaiting approval.'); - expect(ladderEditReason({ ...event, status: 'cancelled' })).toBe('Tranche is cancelled.'); + expect(ladderEditReason({ ...event, status: 'canceled' })).toBe('Tranche is canceled.'); }); // FIXTURE: hand-written tranches. @@ -81,15 +81,15 @@ test('ladderCumulative carries each layer forward on shared times and keeps zero expect([...result.rowTotals]).toEqual([['a', 2500000n], ['b', 3500000n], ['c', 3500000n]]); }); -// FIXTURE: hand-written statuses; the gate repro was scheduled 1.0 + cancelled 5.0 + failed 7.0 showing $13.00/hr instead of $1.00/hr. +// FIXTURE: hand-written statuses; the gate repro was scheduled 1.0 + canceled 5.0 + failed 7.0 showing $13.00/hr instead of $1.00/hr. test('ladderCumulative counts only scheduled, fired and completed tranches (#698 gate)', () => { const at = (id: string, status: string, amount: string, day: number) => ({ ...event, id, status, amount_usd_hr: amount, scheduled_date: `2030-01-0${day}T00:00:00Z` }); - const rows = [at('s', 'scheduled', '1.000000', 1), at('c', 'cancelled', '5.000000', 2), at('f', 'failed', '7.000000', 3), at('x', 'weird', '9.000000', 4), at('fi', 'fired', '2.000000', 5), at('d', 'completed', '3.000000', 6)]; + const rows = [at('s', 'scheduled', '1.000000', 1), at('c', 'canceled', '5.000000', 2), at('f', 'failed', '7.000000', 3), at('x', 'weird', '9.000000', 4), at('fi', 'fired', '2.000000', 5), at('d', 'completed', '3.000000', 6)]; const result = ladderCumulative(rows, 0n); expect(result.totals).toEqual([1000000n, 3000000n, 6000000n]); expect([...result.rowTotals]).toEqual([['s', 1000000n], ['c', null], ['f', null], ['x', null], ['fi', 3000000n], ['d', 6000000n]]); expect(rows.map(ladderNotCountedNote)).toEqual([undefined, 'Not counted', 'Not counted', 'Not counted (unknown status)', undefined, undefined]); - expect(ladderCumulative([at('c', 'cancelled', '5.000000', 1)], 0n).times).toEqual([]); + expect(ladderCumulative([at('c', 'canceled', '5.000000', 1)], 0n).times).toEqual([]); }); test('typed UTC filter dates parse strictly and whole days cover both edges (issue #753)', () => { diff --git a/frontend/src/__tests__/ladder-timeline.test.ts b/frontend/src/__tests__/ladder-timeline.test.ts index 190eeb4ec..de0ac5023 100644 --- a/frontend/src/__tests__/ladder-timeline.test.ts +++ b/frontend/src/__tests__/ladder-timeline.test.ts @@ -315,7 +315,7 @@ test('a missing design token fails loudly instead of painting a default color (# // FIXTURE: hand-written tranches reproducing the gate finding (status filter cleared so all statuses are visible). test('cancelled, failed and unknown tranches stay visible but are not counted in chart, summary or table (#698 gate)', async () => { const row = (id: string, status: string, amount: string, day: number) => ({ ...event, id, status, amount_usd_hr: amount, scheduled_date: `2030-01-0${day}T00:00:00Z` }); - jest.mocked(getLadderEvents).mockResolvedValue([row('s', 'scheduled', '1.000000', 1), row('c', 'cancelled', '5.000000', 2), row('f', 'failed', '7.000000', 3), row('x', 'weird', '9.000000', 4)]); + jest.mocked(getLadderEvents).mockResolvedValue([row('s', 'scheduled', '1.000000', 1), row('c', 'canceled', '5.000000', 2), row('f', 'failed', '7.000000', 3), row('x', 'weird', '9.000000', 4)]); await initLadderTimeline(document.getElementById('timeline')!, [], false); const status = document.querySelector('[data-filter="status"]')!; status.value = ''; status.dispatchEvent(new Event('change')); @@ -336,7 +336,7 @@ test('layers are told apart by point shape as well as color, and a selection wit await initLadderTimeline(document.getElementById('timeline')!, [], false); const config = jest.mocked(Chart).mock.calls[0]![1] as { data: { datasets: { pointStyle: string }[] } }; expect(new Set(config.data.datasets.map(d => d.pointStyle)).size).toBe(2); - jest.mocked(getLadderEvents).mockResolvedValue([{ ...event, status: 'cancelled' }]); + jest.mocked(getLadderEvents).mockResolvedValue([{ ...event, status: 'canceled' }]); document.body.innerHTML = '
'; jest.mocked(Chart).mockClear(); await initLadderTimeline(document.getElementById('timeline')!, [], false); diff --git a/frontend/src/api/ladder.ts b/frontend/src/api/ladder.ts index cd184b6d7..eef6be0a4 100644 --- a/frontend/src/api/ladder.ts +++ b/frontend/src/api/ladder.ts @@ -174,12 +174,18 @@ async function completeTimeline( return items; } -export function getLadderEvents(accountID: string, provider: string): Promise { - return completeTimeline('tranches', accountID, provider); +// Rows written before cloud-commitments-go#554 keep the legacy "cancelled"; +// one spelling here keeps labels, filters and counts on a single value (#1792). +const canceledSpelling = (status: string): string => (status === 'cancelled' ? 'canceled' : status); + +export async function getLadderEvents(accountID: string, provider: string): Promise { + const events = await completeTimeline('tranches', accountID, provider); + return events.map(event => ({ ...event, status: canceledSpelling(event.status), run_status: canceledSpelling(event.run_status) })); } -export function getLadderRuns(accountID: string, provider: string): Promise { - return completeTimeline('runs', accountID, provider); +export async function getLadderRuns(accountID: string, provider: string): Promise { + const runs = await completeTimeline('runs', accountID, provider); + return runs.map(run => ({ ...run, status: canceledSpelling(run.status) })); } export interface LadderAmendment { diff --git a/frontend/src/ladder-timeline-filters.ts b/frontend/src/ladder-timeline-filters.ts index f73694dbe..94608d640 100644 --- a/frontend/src/ladder-timeline-filters.ts +++ b/frontend/src/ladder-timeline-filters.ts @@ -62,12 +62,12 @@ const layerLabels: Record = { 'azure-reservation': 'Azure Reservation', 'azure-savings-plan': 'Azure Savings Plan', }; const statusLabels: Record = { - scheduled: 'Scheduled', fired: 'Fired', completed: 'Completed', cancelled: 'Cancelled', failed: 'Failed', + scheduled: 'Scheduled', fired: 'Fired', completed: 'Completed', canceled: 'Canceled', failed: 'Failed', }; const runStatusLabels: Record = { planned: 'planned', awaiting_approval: 'awaiting approval', approved: 'approved', executing: 'executing', - completed: 'completed', failed: 'failed', cancelled: 'cancelled', expired: 'expired', + completed: 'completed', failed: 'failed', canceled: 'canceled', expired: 'expired', }; /** Label for a ladder enum code. An unknown code is shown raw so it stays visible. */ @@ -130,9 +130,9 @@ export function formatLadderUtcInput(value: string): string { return `${date} ${time.endsWith(':00') ? time.slice(0, 5) : time}`; } -/** Statuses that count toward cumulative commitment; cancelled and failed tranches were never bought. */ +/** Statuses that count toward cumulative commitment; canceled and failed tranches were never bought. */ export const ladderCountedStatuses: readonly string[] = ['scheduled', 'fired', 'completed']; -export const LADDER_COUNTED_NOTE = 'Counts scheduled, fired and completed tranches; cancelled, failed and unrecognised statuses are excluded.'; +export const LADDER_COUNTED_NOTE = 'Counts scheduled, fired and completed tranches; canceled, failed and unrecognised statuses are excluded.'; /** Note for a row that is excluded from the cumulative, or undefined when it is counted. */ export function ladderNotCountedNote(event: LadderEvent): string | undefined { diff --git a/internal/api/handler_purchases_retry_ladder_canceled_test.go b/internal/api/handler_purchases_retry_ladder_canceled_test.go new file mode 100644 index 000000000..e00e734e9 --- /dev/null +++ b/internal/api/handler_purchases_retry_ladder_canceled_test.go @@ -0,0 +1,28 @@ +package api + +import ( + "context" + "errors" + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// #1792: LockLadderTrancheForRetryTx returns the stored tranche status raw +// (store_postgres_ladder_reconcile.go). Rows written before +// cloud-commitments-go#554 keep "cancelled", new ones write "canceled"; Retry +// must refuse a canceled tranche under either spelling. +func TestGuardLadderTrancheForRetry_RefusesBothCanceledSpellings(t *testing.T) { + for _, stored := range []string{"canceled", "cancelled"} { //nolint:misspell // legacy stored ladder status value + t.Run(stored, func(t *testing.T) { + store := new(MockConfigStore) + store.On("LockLadderTrancheForRetryTx", mock.Anything, mock.Anything, "tranche-key"). + Return(ladder.TrancheStatus(stored), true, nil) + h := &Handler{config: store} + err := h.guardLadderTrancheForRetry(context.Background(), nil, "tranche-key") + require.True(t, errors.Is(err, errRetryLadderTrancheSettled), "a %q tranche must refuse Retry, got %v", stored, err) + }) + } +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 602a538b6..66bc05a98 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -9337,10 +9337,12 @@ components: enum: [all-upfront, partial-upfront, no-upfront] status: type: string - enum: [scheduled, fired, completed, cancelled, failed] + description: New rows write "canceled"; rows written before cloud-commitments-go#554 keep "cancelled". + enum: [scheduled, fired, completed, canceled, cancelled, failed] run_status: type: string - enum: [planned, awaiting_approval, approved, executing, completed, failed, cancelled, expired] + description: New rows write "canceled"; rows written before cloud-commitments-go#554 keep "cancelled". + enum: [planned, awaiting_approval, approved, executing, completed, failed, canceled, cancelled, expired] execution_id: type: string description: Omitted until the tranche has an execution. @@ -9393,7 +9395,8 @@ components: format: uuid status: type: string - enum: [planned, awaiting_approval, approved, executing, completed, failed, cancelled, expired] + description: New rows write "canceled"; rows written before cloud-commitments-go#554 keep "cancelled". + enum: [planned, awaiting_approval, approved, executing, completed, failed, canceled, cancelled, expired] started_at: type: string format: date-time diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go index a2f03fc70..bb465562f 100644 --- a/internal/config/interfaces.go +++ b/internal/config/interfaces.go @@ -881,7 +881,7 @@ type StoreInterface interface { // current status), so callers can distinguish a race from a hard error. // Statuses are typed ladder.RunStatus so callers cannot pass an arbitrary // string that would never match a stored status. A transition to - // RunStatusCancelled is refused (ErrLadderRunCancelViaTransition). + // RunStatusCanceled is refused (ErrLadderRunCancelViaTransition). // // CancelLadderRun is the only way to cancel a ladder run: a CAS UPDATE // from one of fromStatuses to cancelled that stamps cancelled_by with the diff --git a/internal/config/store_postgres_ladder.go b/internal/config/store_postgres_ladder.go index 90ffed86d..15bd63215 100644 --- a/internal/config/store_postgres_ladder.go +++ b/internal/config/store_postgres_ladder.go @@ -352,7 +352,7 @@ func cancelOverdueLadderTranchesTx(ctx context.Context, tx pgx.Tx, configID *str AND status = $3 AND execution_id IS NULL AND scheduled_date <= $4 - `, string(ladder.TrancheStatusCancelled), *configID, string(ladder.TrancheStatusScheduled), ladderGraceCutoff(asOf)) + `, string(ladder.TrancheStatusCanceled), *configID, string(ladder.TrancheStatusScheduled), ladderGraceCutoff(asOf)) if err != nil { return fmt.Errorf("failed to cancel overdue ladder_tranches config_id=%s: %w", *configID, err) } @@ -438,11 +438,11 @@ func (s *PostgresStore) LatestLadderRunStartedAt(ctx context.Context, configID s // one of fromStatuses to toStatus via a CAS UPDATE. Returns the updated row // on success, or (nil, nil) when zero rows are affected (race lost or // unexpected current status). A hard DB error is returned as a non-nil error. -// A transition to RunStatusCancelled is refused with +// A transition to RunStatusCanceled is refused with // ErrLadderRunCancelViaTransition: cancels go through CancelLadderRun, which // stamps cancelled_by in the same UPDATE. func (s *PostgresStore) TransitionLadderRunStatus(ctx context.Context, id string, fromStatuses []ladder.RunStatus, toStatus ladder.RunStatus) (*LadderRunDB, error) { - if toStatus == ladder.RunStatusCancelled { + if toStatus == ladder.RunStatusCanceled { return nil, ErrLadderRunCancelViaTransition } // Typed public signature; the stringly-typed shape stays at the DB boundary. diff --git a/internal/config/store_postgres_ladder_amend_ceiling.go b/internal/config/store_postgres_ladder_amend_ceiling.go index 0a067f2e1..41d210324 100644 --- a/internal/config/store_postgres_ladder_amend_ceiling.go +++ b/internal/config/store_postgres_ladder_amend_ceiling.go @@ -109,7 +109,8 @@ type ladderCeilingSnapshot struct { } // loadLadderCeiling reads the latest non-canceled run's snapshot joined with the -// config's current coverage. It refuses a run without a full snapshot. +// config's current coverage. It refuses a run without a full snapshot. Rows +// keep either spelling of canceled (#1792), so both are excluded. func loadLadderCeiling(ctx context.Context, tx pgx.Tx, configID string) (ladderCeilingSnapshot, error) { var snap ladderCeilingSnapshot var hasSnapshot bool @@ -117,7 +118,7 @@ func loadLadderCeiling(ctx context.Context, tx pgx.Tx, configID string) (ladderC r.target_usd_hr IS NOT NULL AND r.existing_usd_hr IS NOT NULL AND r.baseline_usd_hr IS NOT NULL, COALESCE(trunc(LEAST(r.target_usd_hr,r.baseline_usd_hr*c.target_coverage/100)-r.existing_usd_hr,6),0)::float8 FROM ladder_runs r JOIN ladder_configs c ON c.id=r.config_id - WHERE r.config_id=$1 AND r.status<>'cancelled' ORDER BY r.started_at DESC,r.created_at DESC,r.id DESC LIMIT 1`, + WHERE r.config_id=$1 AND r.status NOT IN ('canceled','cancelled') ORDER BY r.started_at DESC,r.created_at DESC,r.id DESC LIMIT 1`, configID).Scan(&snap.anchor, &hasSnapshot, &snap.ceiling) if errors.Is(err, pgx.ErrNoRows) { return snap, fmt.Errorf("%w: config has no non-canceled run", ErrLadderAmendInvalid) diff --git a/internal/config/store_postgres_ladder_amend_ceiling_integration_test.go b/internal/config/store_postgres_ladder_amend_ceiling_integration_test.go index 4b841e4eb..9fe030f73 100644 --- a/internal/config/store_postgres_ladder_amend_ceiling_integration_test.go +++ b/internal/config/store_postgres_ladder_amend_ceiling_integration_test.go @@ -160,14 +160,19 @@ func TestLadderAmendCeiling_SnapshotAndCoverage(t *testing.T) { } func TestLadderAmendCeiling_LatestRunDecides(t *testing.T) { - f := newCeilingFixture(t) - _, ids := f.plan(fptr(10), fptr(0), fptr(100), 1, 1) - f.plan(fptr(3), fptr(0), fptr(100), 1) - require.ErrorIs(t, f.amend(ids[0], 2), ErrLadderAmendInvalid, "latest run's ceiling 3 < 4 in flight, older run's 10 must not apply") - runID, _ := f.plan(fptr(50), fptr(0), fptr(100)) - _, err := f.store.db.Exec(f.ctx, "UPDATE ladder_runs SET status='cancelled' WHERE id=$1", runID) - require.NoError(t, err) - require.ErrorIs(t, f.amend(ids[0], 2), ErrLadderAmendInvalid, "a canceled latest run is ignored") + // Both stored spellings of canceled must be skipped (#1792). + for _, canceled := range []string{"canceled", "cancelled"} { //nolint:misspell // legacy stored ladder status value + t.Run(canceled, func(t *testing.T) { + f := newCeilingFixture(t) + _, ids := f.plan(fptr(10), fptr(0), fptr(100), 1, 1) + f.plan(fptr(3), fptr(0), fptr(100), 1) + require.ErrorIs(t, f.amend(ids[0], 2), ErrLadderAmendInvalid, "latest run's ceiling 3 < 4 in flight, older run's 10 must not apply") + runID, _ := f.plan(fptr(50), fptr(0), fptr(100)) + _, err := f.store.db.Exec(f.ctx, "UPDATE ladder_runs SET status=$2 WHERE id=$1", runID, canceled) + require.NoError(t, err) + require.ErrorIs(t, f.amend(ids[0], 2), ErrLadderAmendInvalid, "a canceled latest run is ignored") + }) + } } func TestLadderAmendCeiling_WaitsForPlanner(t *testing.T) { @@ -266,7 +271,7 @@ func TestLadderAmendCeiling_CancelsOverdueBeforeCounting(t *testing.T) { require.NoError(t, f.amend(ids[1], 10), "X is not in flight, so 10 fits the ceiling") var status ladder.TrancheStatus require.NoError(t, f.store.db.QueryRow(f.ctx, "SELECT status FROM ladder_tranches WHERE id=$1", ids[0]).Scan(&status)) - require.Equal(t, ladder.TrancheStatusCancelled, status, "the overdue row must be canceled, not left for a claim with an older asOf") + require.Equal(t, ladder.TrancheStatusCanceled, status, "the overdue row must be canceled, not left for a claim with an older asOf") claimed, err := f.store.ClaimDueLadderTranches(f.ctx, f.cfgID, time.Now().Add(-time.Minute), 10, func(context.Context, pgx.Tx, LadderTrancheDB) (string, error) { return uuid.NewString(), nil }) require.NoError(t, err) diff --git a/internal/config/store_postgres_ladder_cancel.go b/internal/config/store_postgres_ladder_cancel.go index ee684b06d..e52635631 100644 --- a/internal/config/store_postgres_ladder_cancel.go +++ b/internal/config/store_postgres_ladder_cancel.go @@ -17,7 +17,7 @@ import ( var ErrLadderRunCancelActorRequired = errors.New("canceling a ladder run requires the actor's email") // ErrLadderRunCancelViaTransition is returned by TransitionLadderRunStatus for -// a transition to RunStatusCancelled: that path cannot stamp the canceled-by +// a transition to RunStatusCanceled: that path cannot stamp the canceled-by // audit column, so cancels must go through CancelLadderRun (issue #542). var ErrLadderRunCancelViaTransition = errors.New("ladder run cancel must use CancelLadderRun so the actor is recorded") @@ -50,7 +50,7 @@ func ladderRunStatusStrings(statuses []ladder.RunStatus) []string { } // CancelLadderRun atomically transitions a ladder_runs row from one of -// fromStatuses to RunStatusCancelled and stamps the canceled-by audit column +// fromStatuses to RunStatusCanceled and stamps the canceled-by audit column // with the actor's email in the same UPDATE. Returns the updated row on success, or (nil, nil) // when zero rows are affected (race lost or unexpected current status). An // empty actor is rejected with ErrLadderRunCancelActorRequired before any @@ -60,7 +60,7 @@ func (s *PostgresStore) CancelLadderRun(ctx context.Context, id string, fromStat if actor == "" { return nil, ErrLadderRunCancelActorRequired } - row := s.db.QueryRow(ctx, ladderRunCancelQuery, id, string(ladder.RunStatusCancelled), ladderRunStatusStrings(fromStatuses), actor) + row := s.db.QueryRow(ctx, ladderRunCancelQuery, id, string(ladder.RunStatusCanceled), ladderRunStatusStrings(fromStatuses), actor) result, err := scanLadderRun(row) if err != nil { if errors.Is(err, pgx.ErrNoRows) { diff --git a/internal/config/store_postgres_ladder_cancel_test.go b/internal/config/store_postgres_ladder_cancel_test.go index 2b0ca7c9a..debae71ef 100644 --- a/internal/config/store_postgres_ladder_cancel_test.go +++ b/internal/config/store_postgres_ladder_cancel_test.go @@ -39,9 +39,9 @@ func TestCancelLadderRun_StampsCancelledBy(t *testing.T) { now := time.Now().UTC() actor := "alice@example.com" pool.ExpectQuery(regexp.QuoteMeta(ladderRunCancelQuery)). - WithArgs("run-1", string(ladder.RunStatusCancelled), []string{"awaiting_approval", "approved"}, actor). + WithArgs("run-1", string(ladder.RunStatusCanceled), []string{"awaiting_approval", "approved"}, actor). WillReturnRows(pgxmock.NewRows(ladderRunMockColumns).AddRow( - "run-1", nil, now, nil, ladder.RunStatusCancelled, nil, nil, + "run-1", nil, now, nil, ladder.RunStatusCanceled, nil, nil, nil, nil, nil, nil, []byte(`{}`), 0.0, 0.0, 0.0, nil, nil, @@ -53,7 +53,7 @@ func TestCancelLadderRun_StampsCancelledBy(t *testing.T) { []ladder.RunStatus{ladder.RunStatusAwaitingApproval, ladder.RunStatusApproved}, " "+actor+" ") require.NoError(t, err) require.NotNil(t, run) - assert.Equal(t, ladder.RunStatusCancelled, run.Status) + assert.Equal(t, ladder.RunStatusCanceled, run.Status) require.NotNil(t, run.CancelledBy, "a canceled run must record who canceled it") assert.Equal(t, actor, *run.CancelledBy) require.NoError(t, pool.ExpectationsWereMet()) @@ -62,7 +62,7 @@ func TestCancelLadderRun_StampsCancelledBy(t *testing.T) { func TestCancelLadderRun_CASLostReturnsNil(t *testing.T) { store, pool := newLadderCancelMockStore(t) pool.ExpectQuery(regexp.QuoteMeta(ladderRunCancelQuery)). - WithArgs("run-1", string(ladder.RunStatusCancelled), []string{"approved"}, "bob@example.com"). + WithArgs("run-1", string(ladder.RunStatusCanceled), []string{"approved"}, "bob@example.com"). WillReturnRows(pgxmock.NewRows(ladderRunMockColumns)) run, err := store.CancelLadderRun(context.Background(), "run-1", @@ -86,7 +86,7 @@ func TestCancelLadderRun_RequiresActor(t *testing.T) { func TestTransitionLadderRunStatus_RefusesCancel(t *testing.T) { store, pool := newLadderCancelMockStore(t) run, err := store.TransitionLadderRunStatus(context.Background(), "run-1", - []ladder.RunStatus{ladder.RunStatusExecuting}, ladder.RunStatusCancelled) + []ladder.RunStatus{ladder.RunStatusExecuting}, ladder.RunStatusCanceled) require.ErrorIs(t, err, ErrLadderRunCancelViaTransition) assert.Nil(t, run) require.NoError(t, pool.ExpectationsWereMet(), "a cancel must not bypass the canceled-by stamp") diff --git a/internal/config/store_postgres_ladder_gate_integration_test.go b/internal/config/store_postgres_ladder_gate_integration_test.go index c45b01b4a..0e129ad30 100644 --- a/internal/config/store_postgres_ladder_gate_integration_test.go +++ b/internal/config/store_postgres_ladder_gate_integration_test.go @@ -57,7 +57,7 @@ func TestGetLadderExecutionGate(t *testing.T) { g, err := store.GetLadderExecutionGate(ctx, tr.ID) require.NoError(t, err) require.NotNil(t, g.RunStatus) - assert.Equal(t, string(ladder.RunStatusCancelled), *g.RunStatus) + assert.Equal(t, string(ladder.RunStatusCanceled), *g.RunStatus) }) t.Run("deleted config and run come back nil, not false", func(t *testing.T) { diff --git a/internal/config/store_postgres_ladder_lifecycle.go b/internal/config/store_postgres_ladder_lifecycle.go index 7fd703640..f0dadb582 100644 --- a/internal/config/store_postgres_ladder_lifecycle.go +++ b/internal/config/store_postgres_ladder_lifecycle.go @@ -30,7 +30,7 @@ func ladderGraceCutoff(asOf time.Time) time.Time { // unknown stays fired (and keeps netting) until it is resolved to completed or // failed. var ladderTrancheEdges = map[ladder.TrancheStatus][]ladder.TrancheStatus{ - ladder.TrancheStatusScheduled: {ladder.TrancheStatusFired, ladder.TrancheStatusCancelled}, + ladder.TrancheStatusScheduled: {ladder.TrancheStatusFired, ladder.TrancheStatusCanceled}, ladder.TrancheStatusFired: {ladder.TrancheStatusCompleted, ladder.TrancheStatusFailed}, } @@ -70,7 +70,7 @@ func (s *PostgresStore) TransitionLadderTrancheTx(ctx context.Context, tx pgx.Tx if from == ladder.TrancheStatusScheduled && to == ladder.TrancheStatusFired { return false, fmt.Errorf("ladder tranche %s: scheduled -> fired must go through ClaimDueLadderTranches", id) } - if reason == "" && (to == ladder.TrancheStatusFailed || to == ladder.TrancheStatusCancelled) { + if reason == "" && (to == ladder.TrancheStatusFailed || to == ladder.TrancheStatusCanceled) { return false, fmt.Errorf("ladder tranche %s: a %s transition needs a status reason", id, to) } tag, err := tx.Exec(ctx, ` diff --git a/internal/config/store_postgres_ladder_lifecycle_integration_test.go b/internal/config/store_postgres_ladder_lifecycle_integration_test.go index ae237fff8..a6294a3d2 100644 --- a/internal/config/store_postgres_ladder_lifecycle_integration_test.go +++ b/internal/config/store_postgres_ladder_lifecycle_integration_test.go @@ -125,7 +125,7 @@ func TestLadderGraceBoundary_ScheduledRowsAreCountedOrCancelledNeverNeither(t *t for _, r := range rows { got := trancheStatus(ctx, t, store, r.tr.ID) if r.sched { - canceled := got == ladder.TrancheStatusCancelled + canceled := got == ladder.TrancheStatusCanceled assert.NotEqual(t, r.counted, canceled, "scheduled row at %v: counted=%v status=%s (must be counted XOR canceled)", r.tr.ScheduledDate.Sub(asOf), r.counted, got) } else { @@ -225,8 +225,8 @@ func TestClaimDueLadderTranches_NeverClaimsOnDemandOrUnknownBasisRows(t *testing later := asOf.Add(LadderFireGrace) _, err = store.SaveLadderRunWithTranches(ctx, &LadderRunDB{ConfigID: &cfgID, StartedAt: later, Status: ladder.RunStatusPlanned}, nil) require.NoError(t, err) - assert.Equal(t, ladder.TrancheStatusCancelled, trancheStatus(ctx, t, store, unknown.ID), "past grace the sweep cancels it") - assert.Equal(t, ladder.TrancheStatusCancelled, trancheStatus(ctx, t, store, od.ID), "past grace the sweep cancels it") + assert.Equal(t, ladder.TrancheStatusCanceled, trancheStatus(ctx, t, store, unknown.ID), "past grace the sweep cancels it") + assert.Equal(t, ladder.TrancheStatusCanceled, trancheStatus(ctx, t, store, od.ID), "past grace the sweep cancels it") } func TestClaimDueLadderTranches_CallbackFailureRollsBackClaimAndExecution(t *testing.T) { @@ -281,10 +281,10 @@ func TestTransitionLadderTranche_EveryLegalEdgeAndCompareAndSet(t *testing.T) { require.NoError(t, store.SaveLadderTranches(ctx, []LadderTrancheDB{canceled, toCompleted, toFailed})) require.Equal(t, 7.0, inFlight(ctx, t, store, cfgID, asOf)) - ok, err := store.TransitionLadderTranche(ctx, canceled.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled, "test") + ok, err := store.TransitionLadderTranche(ctx, canceled.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled, "test") require.NoError(t, err) assert.True(t, ok) - ok, err = store.TransitionLadderTranche(ctx, canceled.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled, "test") + ok, err = store.TransitionLadderTranche(ctx, canceled.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled, "test") require.NoError(t, err) assert.False(t, ok, "second apply loses the compare-and-set") assert.Equal(t, 6.0, inFlight(ctx, t, store, cfgID, asOf), "canceled releases netting") @@ -293,7 +293,7 @@ func TestTransitionLadderTranche_EveryLegalEdgeAndCompareAndSet(t *testing.T) { require.NoError(t, err) assert.Equal(t, 6.0, inFlight(ctx, t, store, cfgID, asOf), "fired still nets") - ok, err = store.TransitionLadderTranche(ctx, toCompleted.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled, "test") + ok, err = store.TransitionLadderTranche(ctx, toCompleted.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled, "test") require.NoError(t, err) assert.False(t, ok, "a claimed tranche can no longer be canceled from scheduled") ok, err = store.TransitionLadderTranche(ctx, toCompleted.ID, ladder.TrancheStatusFired, ladder.TrancheStatusCompleted, "test") @@ -388,7 +388,7 @@ func TestClaimVsCancelAndOverdueSweep_ExactlyOneOutcome(t *testing.T) { }() go func() { defer wg.Done() - cancelWon, cancelErr = store.TransitionLadderTranche(ctx, tr.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled, "test") + cancelWon, cancelErr = store.TransitionLadderTranche(ctx, tr.ID, ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled, "test") }() go func() { defer wg.Done() @@ -404,7 +404,7 @@ func TestClaimVsCancelAndOverdueSweep_ExactlyOneOutcome(t *testing.T) { switch final { case ladder.TrancheStatusFired: assert.True(t, claimWon && !cancelWon, "iteration %d: fired but claimWon=%v cancelWon=%v", i, claimWon, cancelWon) - case ladder.TrancheStatusCancelled: + case ladder.TrancheStatusCanceled: assert.True(t, cancelWon && !claimWon, "iteration %d: canceled but claimWon=%v cancelWon=%v", i, claimWon, cancelWon) default: t.Fatalf("iteration %d: unexpected final status %s", i, final) diff --git a/internal/config/store_postgres_ladder_lifecycle_test.go b/internal/config/store_postgres_ladder_lifecycle_test.go index 5dc7c1c02..37a70db80 100644 --- a/internal/config/store_postgres_ladder_lifecycle_test.go +++ b/internal/config/store_postgres_ladder_lifecycle_test.go @@ -11,16 +11,16 @@ import ( var allLadderTrancheStatuses = []ladder.TrancheStatus{ ladder.TrancheStatusScheduled, ladder.TrancheStatusFired, ladder.TrancheStatusCompleted, - ladder.TrancheStatusCancelled, ladder.TrancheStatusFailed, + ladder.TrancheStatusCanceled, ladder.TrancheStatusFailed, } // Pure unit test (no database): the edge table is the whole state machine. func TestLadderTrancheEdges_OnlyDocumentedTransitionsAreLegal(t *testing.T) { legal := map[[2]ladder.TrancheStatus]bool{ - {ladder.TrancheStatusScheduled, ladder.TrancheStatusFired}: true, - {ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled}: true, - {ladder.TrancheStatusFired, ladder.TrancheStatusCompleted}: true, - {ladder.TrancheStatusFired, ladder.TrancheStatusFailed}: true, + {ladder.TrancheStatusScheduled, ladder.TrancheStatusFired}: true, + {ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled}: true, + {ladder.TrancheStatusFired, ladder.TrancheStatusCompleted}: true, + {ladder.TrancheStatusFired, ladder.TrancheStatusFailed}: true, } for _, from := range allLadderTrancheStatuses { for _, to := range allLadderTrancheStatuses { @@ -60,7 +60,7 @@ func TestTransitionLadderTranche_RequiresReasonForFailedAndCanceled(t *testing.T s := &PostgresStore{} for _, c := range []struct{ from, to ladder.TrancheStatus }{ {ladder.TrancheStatusFired, ladder.TrancheStatusFailed}, - {ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled}, + {ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled}, } { applied, err := s.TransitionLadderTrancheTx(context.Background(), nil, "id", c.from, c.to, "") assert.ErrorContains(t, err, "needs a status reason", "%s -> %s", c.from, c.to) diff --git a/internal/config/store_postgres_ladder_netting_integration_test.go b/internal/config/store_postgres_ladder_netting_integration_test.go index 9af689cd8..778395256 100644 --- a/internal/config/store_postgres_ladder_netting_integration_test.go +++ b/internal/config/store_postgres_ladder_netting_integration_test.go @@ -86,7 +86,7 @@ func TestLadderNetting_SelectionBoundaries(t *testing.T) { add(ladder.TrancheStatusCompleted, 32, lockTestAsOf, &anchor, "") add(ladder.TrancheStatusCompleted, 64, lockTestAsOf, &before, "") add(ladder.TrancheStatusFailed, 128, lockTestAsOf, nil, "") - add(ladder.TrancheStatusCancelled, 256, lockTestAsOf, nil, "") + add(ladder.TrancheStatusCanceled, 256, lockTestAsOf, nil, "") other := seedLadderConfigWithExtID(ctx, t, store, "fixture-other") require.NoError(t, store.SaveLadderTranches(ctx, []LadderTrancheDB{lifecycleTranche(other, 512, ladder.TrancheStatusFired, lockTestAsOf)})) _, err := store.PlanAndSaveLadderRun(ctx, cfg, lockTestAsOf, func(rows []LadderInFlightRow) (*LadderRunDB, []LadderTrancheDB, error) { @@ -95,7 +95,7 @@ func TestLadderNetting_SelectionBoundaries(t *testing.T) { }) require.NoError(t, err) var overdue int - require.NoError(t, store.db.QueryRow(ctx, "SELECT count(*) FROM ladder_tranches WHERE config_id=$1 AND status='cancelled' AND amount_usd_hr IN (4,8)", cfg).Scan(&overdue)) + require.NoError(t, store.db.QueryRow(ctx, "SELECT count(*) FROM ladder_tranches WHERE config_id=$1 AND status=$2 AND amount_usd_hr IN (4,8)", cfg, string(ladder.TrancheStatusCanceled)).Scan(&overdue)) require.Equal(t, 2, overdue) } diff --git a/internal/config/store_postgres_ladder_settled_integration_test.go b/internal/config/store_postgres_ladder_settled_integration_test.go index ddd264107..8b11994a1 100644 --- a/internal/config/store_postgres_ladder_settled_integration_test.go +++ b/internal/config/store_postgres_ladder_settled_integration_test.go @@ -57,7 +57,7 @@ func TestPostgresStoreDB_LadderSettledAt(t *testing.T) { _, err = store.TransitionLadderTranche(ctx, failed, ladder.TrancheStatusFired, ladder.TrancheStatusFailed, "boom") require.NoError(t, err) - _, err = store.TransitionLadderTranche(ctx, skipped, ladder.TrancheStatusScheduled, ladder.TrancheStatusCancelled, "no") + _, err = store.TransitionLadderTranche(ctx, skipped, ladder.TrancheStatusScheduled, ladder.TrancheStatusCanceled, "no") require.NoError(t, err) assert.Nil(t, settledAt(failed)) assert.Nil(t, settledAt(skipped)) @@ -106,7 +106,7 @@ func TestPostgresStoreDB_LadderSettledAt(t *testing.T) { add(ladder.TrancheStatusCompleted, 4, &after) assert.InDelta(t, 4, sum(), 1e-9, "one microsecond after counts") add(ladder.TrancheStatusFailed, 8, nil) - add(ladder.TrancheStatusCancelled, 16, nil) + add(ladder.TrancheStatusCanceled, 16, nil) add(ladder.TrancheStatusFired, 32, nil) add(ladder.TrancheStatusScheduled, 64, nil) assert.InDelta(t, 4, sum(), 1e-9, "only completed rows count") diff --git a/internal/config/store_postgres_ladder_settled_plan_integration_test.go b/internal/config/store_postgres_ladder_settled_plan_integration_test.go index 4cc26279f..715d0799c 100644 --- a/internal/config/store_postgres_ladder_settled_plan_integration_test.go +++ b/internal/config/store_postgres_ladder_settled_plan_integration_test.go @@ -55,7 +55,7 @@ func TestPostgresStoreDB_PlanAndSaveSettledTerm(t *testing.T) { add(ladder.TrancheStatusCompleted, 8, at(ladderSettleSkewMargin-time.Second)) assert.InDelta(t, 12, planned(), 1e-9, "settled within the skew margin before asOf counts") add(ladder.TrancheStatusFailed, 16, nil) - add(ladder.TrancheStatusCancelled, 32, nil) + add(ladder.TrancheStatusCanceled, 32, nil) assert.InDelta(t, 12, planned(), 1e-9, "failed and canceled never count") add(ladder.TrancheStatusFired, 64, nil) assert.InDelta(t, 76, planned(), 1e-9, "fired still counts as in-flight, added to the settled term") diff --git a/internal/config/store_postgres_ladder_test.go b/internal/config/store_postgres_ladder_test.go index 485535f40..47413a162 100644 --- a/internal/config/store_postgres_ladder_test.go +++ b/internal/config/store_postgres_ladder_test.go @@ -391,7 +391,7 @@ func TestPostgresStore_CancelLadderRun_StampsCancelledBy(t *testing.T) { require.NoError(t, err) refused, err := store.TransitionLadderRunStatus(ctx, run.ID, - []ladder.RunStatus{ladder.RunStatusAwaitingApproval}, ladder.RunStatusCancelled) + []ladder.RunStatus{ladder.RunStatusAwaitingApproval}, ladder.RunStatusCanceled) require.ErrorIs(t, err, ErrLadderRunCancelViaTransition) assert.Nil(t, refused) @@ -404,12 +404,12 @@ func TestPostgresStore_CancelLadderRun_StampsCancelledBy(t *testing.T) { []ladder.RunStatus{ladder.RunStatusAwaitingApproval}, "alice@example.com") require.NoError(t, err) require.NotNil(t, updated) - assert.Equal(t, ladder.RunStatusCancelled, updated.Status) + assert.Equal(t, ladder.RunStatusCanceled, updated.Status) got, err := store.GetLadderRun(ctx, run.ID) require.NoError(t, err) require.NotNil(t, got) - assert.Equal(t, ladder.RunStatusCancelled, got.Status) + assert.Equal(t, ladder.RunStatusCanceled, got.Status) require.NotNil(t, got.CancelledBy, "ladder_runs.cancelled_by must be stamped on cancel") assert.Equal(t, "alice@example.com", *got.CancelledBy) } @@ -575,7 +575,7 @@ func TestPostgresStore_InFlightLadderRows(t *testing.T) { insertTranche(t, &cfgID, 3.0, ladder.TrancheStatusScheduled) // included insertTranche(t, &cfgID, 2.0, ladder.TrancheStatusFired) // included (unresolved purchase) insertTranche(t, &cfgID, 5.0, ladder.TrancheStatusCompleted) // excluded (already in E) - insertTranche(t, &cfgID, 1.0, ladder.TrancheStatusCancelled) // excluded (terminal) + insertTranche(t, &cfgID, 1.0, ladder.TrancheStatusCanceled) // excluded (terminal) insertTranche(t, &cfgID, 4.0, ladder.TrancheStatusFailed) // excluded (terminal) // Still 'scheduled' but past the grace window: never fired, so never bought // (issue #118). Excluded, or past plans would net out of every later gap. @@ -731,12 +731,12 @@ func TestPostgresStore_SaveLadderRunWithTranches_CancelsOverdue(t *testing.T) { // 12 day-0 tranches + 10 two-week tranches past the grace window; the // 11th is due exactly at start (within grace, still netted/fireable) and // the 12th is in the future. - assert.Equal(t, 22, countStatus(t, cfgID, ladder.TrancheStatusCancelled), "overdue scheduled rows must be marked canceled, not deleted") + assert.Equal(t, 22, countStatus(t, cfgID, ladder.TrancheStatusCanceled), "overdue scheduled rows must be marked canceled, not deleted") assert.Equal(t, 1, countStatus(t, cfgID, ladder.TrancheStatusFired), "an overdue fired row must be left alone") assert.Equal(t, 1, countStatus(t, cfgID, ladder.TrancheStatusCompleted), "an overdue completed row must be left alone") var linkedStatus string require.NoError(t, store.db.QueryRow(ctx, `SELECT status FROM ladder_tranches WHERE id = $1`, linked.ID).Scan(&linkedStatus)) assert.Equal(t, string(ladder.TrancheStatusScheduled), linkedStatus, "an overdue row linked to an execution must be left alone") assert.Equal(t, 1, countStatus(t, otherID, ladder.TrancheStatusScheduled), "another config's tranches must be untouched") - assert.Equal(t, 0, countStatus(t, otherID, ladder.TrancheStatusCancelled)) + assert.Equal(t, 0, countStatus(t, otherID, ladder.TrancheStatusCanceled)) } diff --git a/internal/purchase/ladder_execution_recheck.go b/internal/purchase/ladder_execution_recheck.go index 44c29b82e..21ca8a161 100644 --- a/internal/purchase/ladder_execution_recheck.go +++ b/internal/purchase/ladder_execution_recheck.go @@ -74,7 +74,18 @@ func ladderGateRefusal(gate *config.LadderExecutionGate) error { return fmt.Errorf("refusing to execute ladder purchase: ladder config mode is %q, not auto_approve", *gate.ConfigMode) case gate.RunStatus == nil: return errors.New("refusing to execute ladder purchase: its ladder run no longer exists") - case ladder.RunStatus(*gate.RunStatus) == ladder.RunStatusCancelled: + } + return ladderRunRefusal(*gate.RunStatus) +} + +// ladderRunRefusal parses the stored run status, so both stored spellings of +// canceled refuse (#1792), and a status the code does not know fails closed. +func ladderRunRefusal(status string) error { + run, err := ladder.ParseRunStatus(status) + if err != nil { + return fmt.Errorf("refusing to execute ladder purchase: its ladder run status %q is unknown", status) + } + if run == ladder.RunStatusCanceled { return errors.New("refusing to execute ladder purchase: its ladder run was canceled") } return nil diff --git a/internal/purchase/ladder_execution_recheck_test.go b/internal/purchase/ladder_execution_recheck_test.go index 8a640b6e2..aa3f2382e 100644 --- a/internal/purchase/ladder_execution_recheck_test.go +++ b/internal/purchase/ladder_execution_recheck_test.go @@ -80,12 +80,15 @@ func TestLadderRecheck_RefusesBeforeProviderCall(t *testing.T) { {"planning flag off", openGate(), nil, ladderGlobal(false, true), nil, "planning is switched off"}, {"config disabled", gateWith(func(g *config.LadderExecutionGate) { g.ConfigEnabled = ptr(false) }), nil, ladderGlobal(true, true), nil, "config is disabled"}, {"config mode email_approval", gateWith(func(g *config.LadderExecutionGate) { g.ConfigMode = ptr("email_approval") }), nil, ladderGlobal(true, true), nil, "not auto_approve"}, - {"run canceled", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = ptr(string(ladder.RunStatusCancelled)) }), nil, ladderGlobal(true, true), nil, "run was canceled"}, + {"run canceled", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = ptr("canceled") }), nil, ladderGlobal(true, true), nil, "run was canceled"}, + {"run cancelled (legacy row)", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = ptr("cancelled") }), nil, ladderGlobal(true, true), nil, "run was canceled"}, //nolint:misspell // legacy stored ladder status value + {"run status not known to the code", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = ptr("bogus") }), nil, ladderGlobal(true, true), nil, `run status "bogus" is unknown`}, {"config deleted", gateWith(func(g *config.LadderExecutionGate) { g.ConfigEnabled, g.ConfigMode = nil, nil }), nil, ladderGlobal(true, true), nil, "config no longer exists"}, {"run deleted", gateWith(func(g *config.LadderExecutionGate) { g.RunStatus = nil }), nil, ladderGlobal(true, true), nil, "run no longer exists"}, {"tranche failed by operator", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusFailed)) }), nil, ladderGlobal(true, true), nil, "its tranche is failed"}, {"tranche completed", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCompleted)) }), nil, ladderGlobal(true, true), nil, "its tranche is completed"}, - {"tranche cancelled", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr(string(ladder.TrancheStatusCancelled)) }), nil, ladderGlobal(true, true), nil, "its tranche is cancelled"}, //nolint:misspell // "cancelled" is the stored ladder status value + {"tranche cancelled (legacy row)", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr("cancelled") }), nil, ladderGlobal(true, true), nil, "its tranche is cancelled"}, //nolint:misspell // legacy stored ladder status value + {"tranche canceled", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr("canceled") }), nil, ladderGlobal(true, true), nil, "its tranche is canceled"}, {"tranche status not known to the code", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = ptr("bogus") }), nil, ladderGlobal(true, true), nil, "its tranche is bogus"}, {"tranche status unknown", gateWith(func(g *config.LadderExecutionGate) { g.TrancheStatus = nil }), nil, ladderGlobal(true, true), nil, "tranche status is unknown"}, {"tranche lookup DB error", nil, errors.New("connection reset"), ladderGlobal(true, true), nil, "tranche lookup failed"}, diff --git a/internal/server/handler_ladder_immediate_integration_test.go b/internal/server/handler_ladder_immediate_integration_test.go index 521c00f4e..34ac67bbf 100644 --- a/internal/server/handler_ladder_immediate_integration_test.go +++ b/internal/server/handler_ladder_immediate_integration_test.go @@ -202,8 +202,8 @@ func TestLadderB1_NextRunNetsImmediateRowWithinGrace(t *testing.T) { }{ {"1h", time.Hour, 0, string(pkgladder.TrancheStatusScheduled)}, {"grace minus 1us", config.LadderFireGrace - time.Microsecond, 0, string(pkgladder.TrancheStatusScheduled)}, - {"exactly grace", config.LadderFireGrace, 5, string(pkgladder.TrancheStatusCancelled)}, - {"grace plus 1us", config.LadderFireGrace + time.Microsecond, 5, string(pkgladder.TrancheStatusCancelled)}, + {"exactly grace", config.LadderFireGrace, 5, string(pkgladder.TrancheStatusCanceled)}, + {"grace plus 1us", config.LadderFireGrace + time.Microsecond, 5, string(pkgladder.TrancheStatusCanceled)}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { From bbd13f7e44ef2348912f9e5767a4bdbe09c2d8e9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 03:15:21 +0100 Subject: [PATCH 4/7] test(purchase): create the routing tests' mock provider in the test TestNoUnfailableMockAssertions resolves a mock receiver only from new(T) or &T{} in the same scope. The Azure Redis routing tests got their MockProvider back from a helper, so the guard could not check their AssertNotCalled calls. Each test now creates the provider and passes it in. Co-Authored-By: Claude Opus 5.5 --- internal/purchase/azure_redis_routing_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/purchase/azure_redis_routing_test.go b/internal/purchase/azure_redis_routing_test.go index 0d817ceed..91a8cb0e6 100644 --- a/internal/purchase/azure_redis_routing_test.go +++ b/internal/purchase/azure_redis_routing_test.go @@ -15,10 +15,10 @@ import ( // reservations; the Azure cache client refuses every purchase // (cloud-commitments-go#694). A recommendation labeled "cache" must reach the // right client for its provider, and keep its own label everywhere else. -func purchaseCacheRecThrough(t *testing.T, providerName, region string, want common.ServiceType) *MockProvider { +func purchaseCacheRecThrough(t *testing.T, prov *MockProvider, providerName, region string, want common.ServiceType) { t.Helper() ctx := context.Background() - store, sender, factory, prov, svc := new(MockConfigStore), new(MockEmailSender), new(MockProviderFactory), new(MockProvider), new(MockServiceClient) + store, sender, factory, svc := new(MockConfigStore), new(MockEmailSender), new(MockProviderFactory), new(MockServiceClient) assertMocksOnCleanup(t, store, sender, factory, prov, svc) store.SavePurchaseExecutionFn = func(context.Context, *config.PurchaseExecution) error { return nil } store.On("SavePurchaseHistory", ctx, mock.Anything).Return(nil) @@ -36,19 +36,20 @@ func purchaseCacheRecThrough(t *testing.T, providerName, region string, want com require.NoError(t, err) require.Empty(t, errs) require.True(t, exec.Recommendations[0].Purchased, "the purchase must reach the %s client", want) - return prov } // [fail-first] On main the Azure lookup asks for ServiceCache, which this // mock provider does not serve, so the purchase fails. func TestAzureCachePurchaseUsesTheManagedRedisClient(t *testing.T) { - prov := purchaseCacheRecThrough(t, "azure", "westeurope", common.ServiceMemoryDB) + prov := new(MockProvider) + purchaseCacheRecThrough(t, prov, "azure", "westeurope", common.ServiceMemoryDB) prov.AssertNotCalled(t, "GetServiceClient", mock.Anything, common.ServiceCache, mock.Anything) } // [guard] AWS "cache" stays on the cache (ElastiCache) client. func TestAWSCachePurchaseKeepsTheCacheClient(t *testing.T) { - prov := purchaseCacheRecThrough(t, "aws", "us-east-1", common.ServiceCache) + prov := new(MockProvider) + purchaseCacheRecThrough(t, prov, "aws", "us-east-1", common.ServiceCache) prov.AssertNotCalled(t, "GetServiceClient", mock.Anything, common.ServiceMemoryDB, mock.Anything) } From b02337c7cefee4733a160288bc444525f293be50 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 03:23:55 +0100 Subject: [PATCH 5/7] fix(scheduler): keep Azure Redis recommendations stored as cache Since cloud-commitments-go#702 the Azure provider labels Redis recommendations memorydb. The platform keys recommendation IDs, suppressions and per-account service config on cache for Azure, so a changed label would let an existing suppression stop matching and drop the account config. convertRecommendations now stores an Azure memorydb recommendation as cache through config.StoredRecommendationService. AWS memorydb is unchanged. Purchases already route Azure cache to the managedredis client. Co-Authored-By: Claude Opus 5.5 --- internal/config/types.go | 12 +++ internal/config/types_test.go | 21 +++++ .../scheduler/azure_redis_service_test.go | 84 +++++++++++++++++++ internal/scheduler/scheduler.go | 2 +- 4 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 internal/scheduler/azure_redis_service_test.go diff --git a/internal/config/types.go b/internal/config/types.go index e55729b5c..3b089bf3c 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -860,6 +860,18 @@ func CanonicalSuppressionService(provider, service string) string { return service } +// StoredRecommendationService is the service slug the platform stores and keys +// a provider recommendation under. go#702 relabelled Azure Redis recs from +// cache to memorydb; the platform's IDs, suppressions and per-account config +// key Azure Redis as cache, so it maps back here. AWS memorydb is a distinct +// service and is unchanged. +func StoredRecommendationService(provider string, service common.ServiceType) string { + if provider == string(common.ProviderAzure) && service == common.ServiceMemoryDB { + return string(common.ServiceCache) + } + return string(service) +} + // AppliesTo reports whether the suppression counts against a recommendation // on the same 6-tuple whose variant is v. A row without a variant (legacy) // applies to every variant, and a nil v (the rec's Details could not be read) diff --git a/internal/config/types_test.go b/internal/config/types_test.go index 99ea86a1a..c43101b22 100644 --- a/internal/config/types_test.go +++ b/internal/config/types_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -494,3 +495,23 @@ func TestIsUSDCommitmentCurrency(t *testing.T) { } } } + +func TestStoredRecommendationService(t *testing.T) { + cases := []struct { + provider string + service common.ServiceType + want string + }{ + {"azure", common.ServiceMemoryDB, "cache"}, + {"azure", common.ServiceCache, "cache"}, + {"azure", common.ServiceCompute, string(common.ServiceCompute)}, + {"aws", common.ServiceMemoryDB, "memorydb"}, + {"aws", common.ServiceCache, "cache"}, + {"aws", common.ServiceEC2, "ec2"}, + {"gcp", common.ServiceMemoryDB, "memorydb"}, + {"gcp", common.ServiceCache, "cache"}, + } + for _, c := range cases { + assert.Equal(t, c.want, StoredRecommendationService(c.provider, c.service), "%s/%s", c.provider, c.service) + } +} diff --git a/internal/scheduler/azure_redis_service_test.go b/internal/scheduler/azure_redis_service_test.go new file mode 100644 index 000000000..b3f855235 --- /dev/null +++ b/internal/scheduler/azure_redis_service_test.go @@ -0,0 +1,84 @@ +package scheduler + +import ( + "context" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func redisRec(provider common.ProviderType, service common.ServiceType, account string) common.Recommendation { + return common.Recommendation{ + Provider: provider, Account: account, Service: service, Region: "westeurope", + ResourceType: "C1", Count: 3, Term: "1yr", PaymentOption: "all-upfront", + } +} + +// go#702 emits Azure Redis as memorydb from both the Consumption and Advisor +// paths; it must be stored and keyed exactly like the former cache rec. +func TestConvertRecommendations_AzureMemoryDBStoredAsCacheWithSameID(t *testing.T) { + s := &Scheduler{} + want := s.convertRecommendations([]common.Recommendation{redisRec(common.ProviderAzure, common.ServiceCache, "sub-1")}, "azure") + require.Len(t, want, 1) + for _, name := range []string{"consumption", "advisor"} { + rec := redisRec(common.ProviderAzure, common.ServiceMemoryDB, "sub-1") + got := s.convertRecommendations([]common.Recommendation{rec}, "azure") + require.Len(t, got, 1, name) + assert.Equal(t, "cache", got[0].Service, name) + assert.Equal(t, want[0].ID, got[0].ID, name) + } +} + +func TestConvertRecommendations_AWSMemoryDBUnchanged(t *testing.T) { + s := &Scheduler{} + got := s.convertRecommendations([]common.Recommendation{redisRec(common.ProviderAWS, common.ServiceMemoryDB, "")}, "aws") + require.Len(t, got, 1) + assert.Equal(t, "memorydb", got[0].Service) + assert.Equal(t, "aws||memorydb|westeurope|C1||1|all-upfront", got[0].ID) +} + +func azureMemoryDBStored(t *testing.T, acct string) config.RecommendationRecord { + t.Helper() + s := &Scheduler{} + out := s.convertRecommendations([]common.Recommendation{redisRec(common.ProviderAzure, common.ServiceMemoryDB, "")}, "azure") + require.Len(t, out, 1) + out[0].CloudAccountID = strPtr(acct) + return out[0] +} + +// The no-re-buy case: a suppression written under azure/cache still hides a +// memorydb-sourced Azure rec through the real ListRecommendations path. +func TestApplySuppressions_AzureCacheSuppressionHidesMemoryDBSourcedRec(t *testing.T) { + rec := azureMemoryDBStored(t, "acct-1") + store := &mockSuppressionStore{ + recs: []config.RecommendationRecord{rec}, + sups: []config.PurchaseSuppression{{ + ExecutionID: "e1", AccountID: "acct-1", Provider: "azure", Service: "cache", + Region: rec.Region, ResourceType: rec.ResourceType, SuppressedCount: rec.Count, + ExpiresAt: time.Now().Add(24 * time.Hour), + }}, + } + s := &Scheduler{config: store} + got, err := s.ListRecommendations(context.Background(), config.RecommendationFilter{}) + require.NoError(t, err) + assert.Empty(t, got, "fully suppressed rec must not be offered again") +} + +// azure/cache per-account toggles still apply to a memorydb-sourced rec. +func TestFilterRecsByResolvedConfigs_AzureCacheConfigAppliesToMemoryDBSourcedRec(t *testing.T) { + rec := azureMemoryDBStored(t, "acct-1") + key := config.AccountConfigKey("acct-1", "azure", "cache") + for name, cfg := range map[string]*config.ServiceConfig{ + "disabled": {Enabled: false}, + "min_count": {Enabled: true, MinCount: rec.Count + 1}, + } { + got := filterRecsByResolvedConfigs([]config.RecommendationRecord{rec}, map[string]*config.ServiceConfig{key: cfg}) + assert.Empty(t, got, name) + } + got := filterRecsByResolvedConfigs([]config.RecommendationRecord{rec}, map[string]*config.ServiceConfig{key: {Enabled: true, MinCount: rec.Count}}) + assert.Len(t, got, 1) +} diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 3d1b89525..11ca764b1 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -1824,7 +1824,7 @@ func (s *Scheduler) convertRecommendations(recs []common.Recommendation, provide record := config.RecommendationRecord{ SourceAccountID: sourceAccountID, Provider: providerName, - Service: string(rec.Service), + Service: config.StoredRecommendationService(providerName, rec.Service), Region: rec.Region, ResourceType: rec.ResourceType, Engine: engine, From d9a4f7820ce1940b720175d2b5b576e41d5243e4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 03:26:28 +0100 Subject: [PATCH 6/7] test(api): reword a comment the misspell linter flags The Retry guard test comment named the legacy double-l status spelling literally; the code lines already carry nolint markers. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_purchases_retry_ladder_canceled_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/handler_purchases_retry_ladder_canceled_test.go b/internal/api/handler_purchases_retry_ladder_canceled_test.go index e00e734e9..888278637 100644 --- a/internal/api/handler_purchases_retry_ladder_canceled_test.go +++ b/internal/api/handler_purchases_retry_ladder_canceled_test.go @@ -12,7 +12,7 @@ import ( // #1792: LockLadderTrancheForRetryTx returns the stored tranche status raw // (store_postgres_ladder_reconcile.go). Rows written before -// cloud-commitments-go#554 keep "cancelled", new ones write "canceled"; Retry +// cloud-commitments-go#554 keep the double-l spelling, new ones write "canceled"; Retry // must refuse a canceled tranche under either spelling. func TestGuardLadderTrancheForRetry_RefusesBothCanceledSpellings(t *testing.T) { for _, stored := range []string{"canceled", "cancelled"} { //nolint:misspell // legacy stored ladder status value From 3930c149c97bd432ebcdbd55fafd74d8b8200773 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 7 Nov 2026 04:31:20 +0100 Subject: [PATCH 7/7] fix(suppressions): match Azure Redis suppressions across the engine change cloud-commitments-go#702 gives Advisor-sourced Azure Redis recommendations an engine ("redis"); before the bump they had none. The engine is part of the suppression key, so a suppression written for such a recommendation before the bump would stop matching it afterwards, and the recommendation could be offered again at full count. CanonicalSuppressionEngine treats "" and "redis" as the same engine for azure/cache: - in the suppression bucket written at purchase time; - on the stored and recommendation sides of aggregateSuppressions and applySuppressionIndex; - through SuppressionEngineSpellings, in the paged recommendations query's suppression keys. Recommendation IDs and RecIdentityKey keep the raw engine, so Advisor-only Azure Redis recommendations change ID once. The stored-error allowlist also covers the ladder recheck's "its ladder run status ... is unknown" refusal. Co-Authored-By: Claude Opus 5.5 --- internal/config/recommendations_page.go | 10 +++ internal/config/suppressions_build.go | 2 +- internal/config/suppressions_build_test.go | 14 ++++ internal/config/types.go | 10 +++ internal/purchase/errnotes.go | 2 +- internal/purchase/errnotes_test.go | 4 ++ .../scheduler/azure_redis_service_test.go | 66 ++++++++++++++++--- internal/scheduler/scheduler.go | 6 +- internal/scheduler/scheduler_page.go | 4 +- 9 files changed, 103 insertions(+), 15 deletions(-) diff --git a/internal/config/recommendations_page.go b/internal/config/recommendations_page.go index d213aad71..2174a28b2 100644 --- a/internal/config/recommendations_page.go +++ b/internal/config/recommendations_page.go @@ -99,6 +99,16 @@ func SuppressionTupleKey(accountID, provider, service, region, resourceType, eng return accountID + sep + provider + sep + service + sep + region + sep + resourceType + sep + engine } +// SuppressionEngineSpellings returns every engine spelling a stored rec can +// carry that CanonicalSuppressionEngine maps to canonical. +func SuppressionEngineSpellings(provider, service, canonical string) []string { + out := []string{canonical} + if CanonicalSuppressionEngine(provider, service, "") == canonical && canonical != "" { + out = append(out, "") + } + return out +} + // SuppressionServiceSpellings returns every service spelling a stored rec // can carry that CanonicalSuppressionService maps to canonical for provider: // canonical itself (when it maps to itself) and its legacy and dash-free diff --git a/internal/config/suppressions_build.go b/internal/config/suppressions_build.go index 5051cb31c..439aac763 100644 --- a/internal/config/suppressions_build.go +++ b/internal/config/suppressions_build.go @@ -34,7 +34,7 @@ func suppressionBucketOf(rec *RecommendationRecord) suppressionBucket { service: CanonicalSuppressionService(rec.Provider, rec.Service), region: rec.Region, resourceType: rec.ResourceType, - engine: rec.Engine, + engine: CanonicalSuppressionEngine(rec.Provider, CanonicalSuppressionService(rec.Provider, rec.Service), rec.Engine), } v, err := rec.SuppressionVariant() if err != nil { diff --git a/internal/config/suppressions_build_test.go b/internal/config/suppressions_build_test.go index 2215f404f..bf23018ec 100644 --- a/internal/config/suppressions_build_test.go +++ b/internal/config/suppressions_build_test.go @@ -340,3 +340,17 @@ func TestCanonicalSuppressionTenancy(t *testing.T) { assert.Equal(t, "Dedicated", canonicalSuppressionTenancy("Dedicated")) assert.Equal(t, "default", canonicalSuppressionTenancy("default")) } + +func TestCanonicalSuppressionEngine(t *testing.T) { + for _, tc := range []struct{ provider, service, engine, want string }{ + {"azure", "cache", "", "redis"}, + {"azure", "cache", "redis", "redis"}, + {"azure", "cache", "valkey", "valkey"}, + {"aws", "cache", "", ""}, + {"azure", "compute", "", ""}, + } { + assert.Equal(t, tc.want, CanonicalSuppressionEngine(tc.provider, tc.service, tc.engine), tc.provider+"/"+tc.service+"/"+tc.engine) + } + assert.Equal(t, []string{"redis", ""}, SuppressionEngineSpellings("azure", "cache", "redis")) + assert.Equal(t, []string{""}, SuppressionEngineSpellings("aws", "cache", "")) +} diff --git a/internal/config/types.go b/internal/config/types.go index 3b089bf3c..7f964cccd 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -860,6 +860,16 @@ func CanonicalSuppressionService(provider, service string) string { return service } +// CanonicalSuppressionEngine reads an Azure cache engine "" as "redis" so a +// suppression and a rec match across go#702, which gave Advisor-sourced Azure +// Redis recs an engine (they had none before). Other tuples are unchanged. +func CanonicalSuppressionEngine(provider, service, engine string) string { + if provider == string(common.ProviderAzure) && service == string(common.ServiceCache) && engine == "" { + return "redis" + } + return engine +} + // StoredRecommendationService is the service slug the platform stores and keys // a provider recommendation under. go#702 relabelled Azure Redis recs from // cache to memorydb; the platform's IDs, suppressions and per-account config diff --git a/internal/purchase/errnotes.go b/internal/purchase/errnotes.go index 561045750..77e86b5ed 100644 --- a/internal/purchase/errnotes.go +++ b/internal/purchase/errnotes.go @@ -27,7 +27,7 @@ var serverNotePatterns = []string{ `plan step refused by commitment cap(?:: plan has no max_step_commitment set \(auto-purchase of steps with recommendations is refused until one is set\)|: step total commitment \d+\.\d{2} exceeds max_step_commitment \d+\.\d{2})?`, `scheduled purchase skipped: it was due too long ago(?:: it was due at \d{4}-\d{2}-\d{2}T[0-9:.]+Z, [0-9a-z.]+ ago, which is more than the [0-9a-z.]+ limit \(SCHEDULED_PURCHASE_MAX_OVERDUE\); nothing was submitted, retry it if the purchase is still wanted)?`, `purchase refused after claim: the claimed execution no longer passes the authorization checks`, - `refusing to execute ladder purchase: (?:global config unavailable|ladder planning is switched off|ladder execution is switched off|its tranche status is unknown|its tranche is [a-z_]+|its ladder config no longer exists|its ladder config is disabled|ladder config mode is "[a-z_]*", not auto_approve|its ladder run no longer exists|its ladder run was canceled)`, + `refusing to execute ladder purchase: (?:global config unavailable|ladder planning is switched off|ladder execution is switched off|its tranche status is unknown|its tranche is [a-z_]+|its ladder config no longer exists|its ladder config is disabled|ladder config mode is "[a-z_]*", not auto_approve|its ladder run no longer exists|its ladder run was canceled|its ladder run status "[a-z_]{1,32}" is unknown)`, `scheduled plan step reached its scheduled date without recommendations; no purchase was submitted`, `no current recommendation matched plan ` + uuidPattern + ` step \d+ by \d{4}-\d{2}-\d{2} \(\d+-day grace after the plan date\)(?:; ` + skippedNotePattern + `)?`, `target already met: target \d+, committed \d+(?:; ` + skippedNotePattern + `)?; \d+ failed row\(s\) of unknown outcome counted as committed`, diff --git a/internal/purchase/errnotes_test.go b/internal/purchase/errnotes_test.go index 562f9cf2e..c0fc9afe9 100644 --- a/internal/purchase/errnotes_test.go +++ b/internal/purchase/errnotes_test.go @@ -39,6 +39,7 @@ func TestSanitizeStoredError_AllowlistedNotesPassThroughVerbatim(t *testing.T) { dueRowAgeRefusal(&config.PurchaseExecution{ScheduledDate: time.Now().Add(-100 * time.Hour)}, time.Hour, time.Now()).Error(), ErrClaimedRowRefused.Error(), "refusing to execute ladder purchase: its ladder run was canceled", + `refusing to execute ladder purchase: its ladder run status "weird_state" is unknown`, "refusing to execute ladder purchase: its tranche is paused", "refusing to execute: ladder tranche lookup failed", "refusing to execute ladder purchase: global config unreadable", @@ -235,8 +236,11 @@ func TestSanitizeStoredError_SlotsRejectWhitespaceAndControls(t *testing.T) { errAmbiguousAccountScope.Error() + ": 1 selected recommendation(s) carry no cloud_account_id while 2 target account %s", "EC2 recommendation for %s has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation", `refusing to execute retry attempt 1: provider "%s" is not known to reject a duplicate purchase, so re-driving this could buy a second commitment`, + `refusing to execute ladder purchase: its ladder run status "%s" is unknown`, resolve, } + quoted := `refusing to execute ladder purchase: its ladder run status "a\"b" is unknown` + assert.NotEqual(t, quoted, SanitizeStoredError(quoted)) for _, bad := range []string{"a b", "a\nb", "a\rb", "a\tb", "a\vb", "a\fb", "a\u0085b", "a\u00a0b", "a\u2028b", "a\u200bb", "a\x00b"} { for _, slot := range slots { if slot == resolve && bad == "a b" { diff --git a/internal/scheduler/azure_redis_service_test.go b/internal/scheduler/azure_redis_service_test.go index b3f855235..4316a7db5 100644 --- a/internal/scheduler/azure_redis_service_test.go +++ b/internal/scheduler/azure_redis_service_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/managedredis" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,18 +19,63 @@ func redisRec(provider common.ProviderType, service common.ServiceType, account } } -// go#702 emits Azure Redis as memorydb from both the Consumption and Advisor -// paths; it must be stored and keyed exactly like the former cache rec. -func TestConvertRecommendations_AzureMemoryDBStoredAsCacheWithSameID(t *testing.T) { +// advisorRedisRecs returns the Advisor-sourced Azure Redis rec in its real +// pre-bump shape (cache, no Details) and post-bump shape (memorydb with +// managedredis.Details, go#702 providers/azure/recommendations.go:654-656). +func advisorRedisRecs() (before, after common.Recommendation) { + before = redisRec(common.ProviderAzure, common.ServiceCache, "sub-1") + after = redisRec(common.ProviderAzure, common.ServiceMemoryDB, "sub-1") + after.Details = managedredis.Details(after.ResourceType) + return before, after +} + +// go#702 emits Azure Redis as memorydb; the Consumption rec keeps its former +// cache ID, and an Advisor-only rec shifts ID once because it gains engine +// "redis" (accepted; suppressions are engine-canonicalised instead). +func TestConvertRecommendations_AzureMemoryDBStoredAsCache(t *testing.T) { s := &Scheduler{} - want := s.convertRecommendations([]common.Recommendation{redisRec(common.ProviderAzure, common.ServiceCache, "sub-1")}, "azure") + consumption := redisRec(common.ProviderAzure, common.ServiceMemoryDB, "sub-1") + consumption.Details = managedredis.Details(consumption.ResourceType) + _, advisor := advisorRedisRecs() + want := s.convertRecommendations([]common.Recommendation{consumption}, "azure") + got := s.convertRecommendations([]common.Recommendation{advisor}, "azure") require.Len(t, want, 1) - for _, name := range []string{"consumption", "advisor"} { - rec := redisRec(common.ProviderAzure, common.ServiceMemoryDB, "sub-1") - got := s.convertRecommendations([]common.Recommendation{rec}, "azure") - require.Len(t, got, 1, name) - assert.Equal(t, "cache", got[0].Service, name) - assert.Equal(t, want[0].ID, got[0].ID, name) + require.Len(t, got, 1) + assert.Equal(t, "cache", got[0].Service) + assert.Equal(t, "redis", got[0].Engine) + assert.Equal(t, want[0].ID, got[0].ID) +} + +func TestApplySuppressions_AdvisorRedisEngineStableAcrossGoBump(t *testing.T) { + s := &Scheduler{} + before, after := advisorRedisRecs() + oldRec := s.convertRecommendations([]common.Recommendation{before}, "azure")[0] + newRec := s.convertRecommendations([]common.Recommendation{after}, "azure")[0] + require.Equal(t, "", oldRec.Engine) + require.Equal(t, "redis", newRec.Engine) + for _, tc := range []struct { + name string + supEngine string + rec config.RecommendationRecord + }{ + {"old suppression hides new rec", oldRec.Engine, newRec}, + {"new suppression hides old rec", newRec.Engine, oldRec}, + {"same-engine control", newRec.Engine, newRec}, + } { + rec := tc.rec + rec.CloudAccountID = strPtr("acct-1") + store := &mockSuppressionStore{ + recs: []config.RecommendationRecord{rec}, + sups: []config.PurchaseSuppression{{ + ExecutionID: "e1", AccountID: "acct-1", Provider: "azure", Service: "cache", + Region: rec.Region, ResourceType: rec.ResourceType, Engine: tc.supEngine, + SuppressedCount: 1, ExpiresAt: time.Now().Add(24 * time.Hour), + }}, + } + got, err := (&Scheduler{config: store}).ListRecommendations(context.Background(), config.RecommendationFilter{}) + require.NoError(t, err, tc.name) + require.Len(t, got, 1, tc.name) + assert.Equal(t, rec.Count-1, got[0].Count, tc.name) } } diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 11ca764b1..964386b81 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -1517,8 +1517,9 @@ func aggregateSuppressions(sups []config.PurchaseSuppression) map[suppressionKey k := suppressionKey{ accountID: sup.AccountID, provider: sup.Provider, service: config.CanonicalSuppressionService(sup.Provider, sup.Service), - region: sup.Region, resourceType: sup.ResourceType, engine: sup.Engine, + region: sup.Region, resourceType: sup.ResourceType, } + k.engine = config.CanonicalSuppressionEngine(k.provider, k.service, sup.Engine) index[k] = append(index[k], sup) } return index @@ -1563,8 +1564,9 @@ func applySuppressionIndex(recs []config.RecommendationRecord, index map[suppres k := suppressionKey{ accountID: accountID, provider: rec.Provider, service: config.CanonicalSuppressionService(rec.Provider, rec.Service), - region: rec.Region, resourceType: rec.ResourceType, engine: rec.Engine, + region: rec.Region, resourceType: rec.ResourceType, } + k.engine = config.CanonicalSuppressionEngine(k.provider, k.service, rec.Engine) a := matchingSuppressions(&rec, index[k]) if a == nil { out = append(out, rec) diff --git a/internal/scheduler/scheduler_page.go b/internal/scheduler/scheduler_page.go index eae378f99..2b6c1dc85 100644 --- a/internal/scheduler/scheduler_page.go +++ b/internal/scheduler/scheduler_page.go @@ -135,7 +135,9 @@ func suppressionPageKeys(index map[suppressionKey][]config.PurchaseSuppression) var keys []string for k := range index { for _, service := range config.SuppressionServiceSpellings(k.provider, k.service) { - keys = append(keys, config.SuppressionTupleKey(k.accountID, k.provider, service, k.region, k.resourceType, k.engine)) + for _, engine := range config.SuppressionEngineSpellings(k.provider, k.service, k.engine) { + keys = append(keys, config.SuppressionTupleKey(k.accountID, k.provider, service, k.region, k.resourceType, engine)) + } } } sort.Strings(keys)