From 33b8044f628d53fee4cb590ab9f521f79db77483 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 00:15:37 +0100 Subject: [PATCH] feat(ladder): refuse unannounced, 4-eyes and non-convertible RI ladder purchases at execution The execute-time ladder recheck now also refuses, before any provider call, a ladder execution when: - notification_sent is not stamped (no production path stamps it yet, so ladder purchases stay refused until one does); - RequireDifferentApprover is on, since a ladder purchase has no second approver; - its tranche is on the convertible RI layer and the global offering class is neither empty nor convertible, because the Manager buys the global class. GetLadderExecutionGate now also reads the tranche layer_type. Each refusal is a fixed server-authored note, added to the stored-error allowlist, and ends the row failed with no_submission like the other ladder refusals. Non-ladder executions are unaffected. pre-commit run with SKIP=cfn-lint: cfn-lint cannot install on Python 3.15 and no CloudFormation file is touched. Part of #70 Co-Authored-By: Claude Sonnet 5.5 --- ...e_postgres_ladder_gate_integration_test.go | 2 + .../config/store_postgres_ladder_reconcile.go | 6 +- internal/purchase/errnotes.go | 2 +- internal/purchase/errnotes_test.go | 3 + internal/purchase/ladder_execution_recheck.go | 42 ++++++-- ...dder_execution_recheck_integration_test.go | 102 ++++++++++++++++++ .../purchase/ladder_execution_recheck_test.go | 61 ++++++++++- 7 files changed, 207 insertions(+), 11 deletions(-) create mode 100644 internal/purchase/ladder_execution_recheck_integration_test.go diff --git a/internal/config/store_postgres_ladder_gate_integration_test.go b/internal/config/store_postgres_ladder_gate_integration_test.go index 0e129ad30..00a6f4ec7 100644 --- a/internal/config/store_postgres_ladder_gate_integration_test.go +++ b/internal/config/store_postgres_ladder_gate_integration_test.go @@ -27,6 +27,7 @@ func TestGetLadderExecutionGate(t *testing.T) { require.NoError(t, err) tr := lifecycleTranche(cfgID, 1, ladder.TrancheStatusFired, time.Now().UTC()) tr.RunID = &run.ID + tr.LayerType = ladder.LayerConvertibleRI // not lifecycleTranche's default, so a hard-coded layer fails require.NoError(t, store.SaveLadderTranches(ctx, []LadderTrancheDB{tr})) t.Run("ladder key returns the config switches and run status", func(t *testing.T) { @@ -41,6 +42,7 @@ func TestGetLadderExecutionGate(t *testing.T) { assert.Equal(t, "executing", *g.RunStatus) require.NotNil(t, g.TrancheStatus) assert.Equal(t, "fired", *g.TrancheStatus) + assert.Equal(t, tr.LayerType, g.LayerType) }) t.Run("non-ladder keys are not an error", func(t *testing.T) { diff --git a/internal/config/store_postgres_ladder_reconcile.go b/internal/config/store_postgres_ladder_reconcile.go index 0969b027a..6e847ca8d 100644 --- a/internal/config/store_postgres_ladder_reconcile.go +++ b/internal/config/store_postgres_ladder_reconcile.go @@ -169,6 +169,8 @@ type LadderExecutionGate struct { ConfigMode *string RunStatus *string TrancheStatus *string + // LayerType is the tranche's own layer (NOT NULL in the schema). + LayerType ladder.LayerType } // GetLadderExecutionGate finds the ladder tranche whose id equals an @@ -183,11 +185,11 @@ func (s *PostgresStore) GetLadderExecutionGate(ctx context.Context, idempotencyK } var g LadderExecutionGate err := s.db.QueryRow(ctx, ` - SELECT c.enabled, c.mode, r.status, t.status + SELECT c.enabled, c.mode, r.status, t.status, t.layer_type FROM ladder_tranches t LEFT JOIN ladder_configs c ON c.id = t.config_id LEFT JOIN ladder_runs r ON r.id = t.run_id - WHERE t.id::text = $1`, idempotencyKey).Scan(&g.ConfigEnabled, &g.ConfigMode, &g.RunStatus, &g.TrancheStatus) + WHERE t.id::text = $1`, idempotencyKey).Scan(&g.ConfigEnabled, &g.ConfigMode, &g.RunStatus, &g.TrancheStatus, &g.LayerType) if errors.Is(err, pgx.ErrNoRows) { return nil, nil } diff --git a/internal/purchase/errnotes.go b/internal/purchase/errnotes.go index cbf71e610..7f6198bbd 100644 --- a/internal/purchase/errnotes.go +++ b/internal/purchase/errnotes.go @@ -28,7 +28,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}|` + boundGroupCapNotePatterns + `)?`, `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|its ladder run status "[a-z_]{1,32}" is unknown)`, + `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|its scheduled-purchase notice was not sent|purchases need a second approver|RI ladder purchases need the convertible offering class)`, `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 8d66ea704..44729acf4 100644 --- a/internal/purchase/errnotes_test.go +++ b/internal/purchase/errnotes_test.go @@ -44,6 +44,9 @@ func TestSanitizeStoredError_AllowlistedNotesPassThroughVerbatim(t *testing.T) { "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", + ladderNoticeNotSentNote, + ladderNeedsSecondApproverNote, + ladderRINeedsConvertibleNote, "refusing to execute: ladder tranche lookup failed", "refusing to execute ladder purchase: global config unreadable", "scheduled plan step reached its scheduled date without recommendations; no purchase was submitted", diff --git a/internal/purchase/ladder_execution_recheck.go b/internal/purchase/ladder_execution_recheck.go index 21ca8a161..629114bd6 100644 --- a/internal/purchase/ladder_execution_recheck.go +++ b/internal/purchase/ladder_execution_recheck.go @@ -37,29 +37,57 @@ func (m *Manager) ladderExecutionRefusal(ctx context.Context, exec *config.Purch if gate == nil { return nil } - if err := m.ladderFlagsRefusal(ctx); err != nil { + global, err := m.ladderFlagsRefusal(ctx) + if err != nil { + return err + } + if err := ladderGateRefusal(gate); err != nil { return err } - return ladderGateRefusal(gate) + return ladderApprovalRefusal(exec, gate, global) } -func (m *Manager) ladderFlagsRefusal(ctx context.Context) error { +func (m *Manager) ladderFlagsRefusal(ctx context.Context) (*config.GlobalConfig, error) { global, err := m.config.GetGlobalConfig(ctx) if err != nil { - return fmt.Errorf("refusing to execute ladder purchase: global config unreadable: %w", err) + return nil, fmt.Errorf("refusing to execute ladder purchase: global config unreadable: %w", err) } if global == nil { - return errors.New("refusing to execute ladder purchase: global config unavailable") + return nil, errors.New("refusing to execute ladder purchase: global config unavailable") } if !global.LadderingEnabled { - return errors.New("refusing to execute ladder purchase: ladder planning is switched off") + return nil, errors.New("refusing to execute ladder purchase: ladder planning is switched off") } if !global.LadderExecutionEnabled { - return errors.New("refusing to execute ladder purchase: ladder execution is switched off") + return nil, errors.New("refusing to execute ladder purchase: ladder execution is switched off") + } + return global, nil +} + +// ladderApprovalRefusal covers what a ladder purchase must have beyond the +// switches. The scheduled-purchase notice must have gone out (no production +// path stamps notification_sent yet, so this refuses until one does). 4-eyes +// mode is refused because a ladder purchase has no second approver. The +// Manager buys the global offering class, so an RI-layer tranche cannot run +// while that class is anything but convertible. +func ladderApprovalRefusal(exec *config.PurchaseExecution, gate *config.LadderExecutionGate, global *config.GlobalConfig) error { + switch { + case exec.NotificationSent == nil: + return errors.New(ladderNoticeNotSentNote) + case global.RequireDifferentApprover: + return errors.New(ladderNeedsSecondApproverNote) + case gate.LayerType == ladder.LayerConvertibleRI && global.OfferingClass != "" && global.OfferingClass != "convertible": + return errors.New(ladderRINeedsConvertibleNote) } return nil } +const ( + ladderNoticeNotSentNote = "refusing to execute ladder purchase: its scheduled-purchase notice was not sent" + ladderNeedsSecondApproverNote = "refusing to execute ladder purchase: purchases need a second approver" + ladderRINeedsConvertibleNote = "refusing to execute ladder purchase: RI ladder purchases need the convertible offering class" +) + func ladderGateRefusal(gate *config.LadderExecutionGate) error { switch { case gate.TrancheStatus == nil: diff --git a/internal/purchase/ladder_execution_recheck_integration_test.go b/internal/purchase/ladder_execution_recheck_integration_test.go new file mode 100644 index 000000000..c1224f51f --- /dev/null +++ b/internal/purchase/ladder_execution_recheck_integration_test.go @@ -0,0 +1,102 @@ +//go:build integration + +package purchase + +// Real PostgreSQL for the execute-time ladder gates added in platform#70 S8c1: +// a ladder execution is fired through fireOneDue against the real store and +// the counting fake provider (labeled fixture; nothing reaches a cloud API). + +import ( + "context" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// ladderFireRow seeds a ladder config, run and tranche of the given layer, plus +// a scheduled execution keyed by the tranche id, with the ladder switches on. +func ladderFireRow(ctx context.Context, t *testing.T, f *rampStepFixture, layer ladder.LayerType, stamped bool) *config.PurchaseExecution { + t.Helper() + f.repairAccounts(ctx, t) + global, err := f.store.GetGlobalConfig(ctx) + require.NoError(t, err) + global.LadderingEnabled, global.LadderExecutionEnabled = true, true + global.RequireDifferentApprover, global.OfferingClass = false, "" + require.NoError(t, f.store.SaveGlobalConfig(ctx, global)) + + cfg, err := f.store.UpsertLadderConfig(ctx, &config.LadderConfigDB{ + CloudAccountID: f.accounts[0].ID, Provider: "aws", Enabled: true, Mode: "auto_approve", Cadence: "daily", + TargetCoverage: 80, BufferFraction: 0.1, BaselinePercentile: 5, LookbackDays: 30, + BufferUtilizationThreshold: 50, MaxActionsPerRun: 5, RampSchedule: []byte(`{"steps":[{"after_days":0,"fraction":1.0}]}`)}) + require.NoError(t, err) + run, err := f.store.SaveLadderRun(ctx, &config.LadderRunDB{ConfigID: &cfg.ID, StartedAt: time.Now().UTC(), Status: ladder.RunStatusExecuting}) + require.NoError(t, err) + basis, rate := config.LadderAmountCommitment, "1" + tr := config.LadderTrancheDB{ID: uuid.New().String(), ConfigID: &cfg.ID, RunID: &run.ID, LayerType: layer, + Term: ladder.Term1Year, PaymentOption: ladder.PaymentNoUpfront, Status: ladder.TrancheStatusScheduled, + AmountUSDHr: 5, ScheduledDate: time.Now().Add(-time.Hour), AmountBasis: &basis, CommitmentRate: &rate} + require.NoError(t, f.store.SaveLadderTranches(ctx, []config.LadderTrancheDB{tr})) + + row := &config.PurchaseExecution{ + PlanID: f.plan.ID, ExecutionID: uuid.New().String(), Status: "scheduled", StepNumber: 3, IdempotencyKey: tr.ID, + ScheduledDate: time.Now().Add(-time.Minute), CloudAccountID: &f.accounts[0].ID, + Recommendations: []config.RecommendationRecord{withAccount(rampStepRecommendation()[0], f.accounts[0].ID)}, + } + if stamped { + now := time.Now().Add(-time.Hour) + row.NotificationSent = &now + } + require.NoError(t, f.store.SavePurchaseExecution(ctx, row)) + return row +} + +func TestLadderFire_RefusesUnannouncedFourEyesAndNonConvertible(t *testing.T) { + ctx := context.Background() + f := newRampStepFixture(ctx, t) + f.email.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything).Return(nil).Maybe() + cases := []struct { + name string + layer ladder.LayerType + stamped bool + mutate func(*config.GlobalConfig) + want string + }{ + {"notification_sent NULL", ladder.LayerComputeSP, false, func(*config.GlobalConfig) {}, ladderNoticeNotSentNote}, + {"4-eyes on", ladder.LayerComputeSP, true, func(g *config.GlobalConfig) { g.RequireDifferentApprover = true }, ladderNeedsSecondApproverNote}, + {"RI layer, class switched to standard", ladder.LayerConvertibleRI, true, func(g *config.GlobalConfig) { g.OfferingClass = "standard" }, ladderRINeedsConvertibleNote}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + row := ladderFireRow(ctx, t, f, tc.layer, tc.stamped) + global, err := f.store.GetGlobalConfig(ctx) + require.NoError(t, err) + tc.mutate(global) + require.NoError(t, f.store.SaveGlobalConfig(ctx, global)) + + before := f.purchaseCount() + assert.Equal(t, fireErrored, f.manager.fireOneDue(ctx, rowStatusCopy(row))) + assert.Equal(t, before, f.purchaseCount(), "a refused ladder execution must reach no provider") + got := rowStatus(ctx, t, f, row.ExecutionID) + assert.Equal(t, "failed", got.Status) + assert.True(t, got.NoSubmission, "the refusal must stamp no_submission") + assert.Contains(t, got.Error, tc.want) + }) + } + + t.Run("stamped, 4-eyes off, convertible RI still buys", func(t *testing.T) { + row := ladderFireRow(ctx, t, f, ladder.LayerConvertibleRI, true) + global, err := f.store.GetGlobalConfig(ctx) + require.NoError(t, err) + global.RequireDifferentApprover, global.OfferingClass = false, "convertible" + require.NoError(t, f.store.SaveGlobalConfig(ctx, global)) + before := f.purchaseCount() + f.manager.fireOneDue(ctx, rowStatusCopy(row)) + assert.Equal(t, 1, f.purchaseCount()-before) + }) +} diff --git a/internal/purchase/ladder_execution_recheck_test.go b/internal/purchase/ladder_execution_recheck_test.go index aa3f2382e..86960f41c 100644 --- a/internal/purchase/ladder_execution_recheck_test.go +++ b/internal/purchase/ladder_execution_recheck_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "testing" + "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" @@ -25,7 +26,7 @@ func ptr[T any](v T) *T { return &v } // openGate is a gate that allows execution; each case flips one field. func openGate() *config.LadderExecutionGate { - return &config.LadderExecutionGate{ConfigEnabled: ptr(true), ConfigMode: ptr("auto_approve"), RunStatus: ptr("executing"), TrancheStatus: ptr("fired")} + return &config.LadderExecutionGate{ConfigEnabled: ptr(true), ConfigMode: ptr("auto_approve"), RunStatus: ptr("executing"), TrancheStatus: ptr("fired"), LayerType: ladder.LayerComputeSP} } func ladderGlobal(planning, execution bool) *config.GlobalConfig { @@ -38,6 +39,8 @@ func ladderHarness(t *testing.T, key string, gate *config.LadderExecutionGate, g t.Helper() exec := armedExecution("aws", "savingsplans", 0, "notified") exec.IdempotencyKey = key + notified := time.Now().Add(-time.Hour) + exec.NotificationSent = ¬ified mgr, store, rec := armedHarness(t, common.ServiceSavingsPlansAll) kept := store.ExpectedCalls[:0] for _, c := range store.ExpectedCalls { @@ -182,3 +185,59 @@ func TestLadderRecheck_EveryExecutorEntryRefuses(t *testing.T) { assert.Equal(t, 0, rec.count()) }) } + +// S8c1: an announced, single-approver purchase of the class the Manager will +// actually buy. Each case flips one input; the pass cases prove the refusal is +// not blanket. +func TestLadderRecheck_ApprovalAndClassGate(t *testing.T) { + ri := func(g *config.LadderExecutionGate) { g.LayerType = ladder.LayerConvertibleRI } + withGlobal := func(mut func(*config.GlobalConfig)) *config.GlobalConfig { + g := ladderGlobal(true, true) + mut(g) + return g + } + cases := []struct { + name string + mutGt func(*config.LadderExecutionGate) + mutEx func(*config.PurchaseExecution) + global *config.GlobalConfig + want string // empty = allowed + }{ + {"notice not sent", nil, func(e *config.PurchaseExecution) { e.NotificationSent = nil }, ladderGlobal(true, true), ladderNoticeNotSentNote}, + {"4-eyes on", nil, nil, withGlobal(func(g *config.GlobalConfig) { g.RequireDifferentApprover = true }), ladderNeedsSecondApproverNote}, + {"RI layer, standard class", ri, nil, withGlobal(func(g *config.GlobalConfig) { g.OfferingClass = "standard" }), ladderRINeedsConvertibleNote}, + {"RI layer, empty class", ri, nil, ladderGlobal(true, true), ""}, + {"RI layer, convertible class", ri, nil, withGlobal(func(g *config.GlobalConfig) { g.OfferingClass = "convertible" }), ""}, + {"non-RI layer, standard class", nil, nil, withGlobal(func(g *config.GlobalConfig) { g.OfferingClass = "standard" }), ""}, + {"stamped, 4-eyes off", nil, nil, ladderGlobal(true, true), ""}, + {"gate refusal wins over a missing notice", func(g *config.LadderExecutionGate) { g.RunStatus = ptr("cancelled") }, func(e *config.PurchaseExecution) { e.NotificationSent = nil }, ladderGlobal(true, true), "refusing to execute ladder purchase: its ladder run was canceled"}, //nolint:misspell // legacy stored ladder status value + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gate := openGate() + if tc.mutGt != nil { + tc.mutGt(gate) + } + mgr, store, rec, exec := ladderHarness(t, ladderTrancheKey, gate, nil, tc.global, nil) + if tc.mutEx != nil { + tc.mutEx(exec) + } + var saved []config.PurchaseExecution + store.SavePurchaseExecutionFn = func(_ context.Context, e *config.PurchaseExecution) error { + saved = append(saved, *e) + return nil + } + runViaClaimFunnel(t, mgr, store, exec) + if tc.want == "" { + assert.Equal(t, 1, rec.count()) + return + } + assert.Equal(t, 0, rec.count(), "a refused ladder execution must reach no provider") + require.NotEmpty(t, saved) + final := saved[len(saved)-1] + assert.Equal(t, "failed", final.Status) + assert.True(t, final.NoSubmission) + assert.Contains(t, final.Error, tc.want) + }) + } +}