From ce224e9abebe5633a66a6e94b639aa9ef5c8a07e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 14 Oct 2026 16:46:54 +0200 Subject: [PATCH 1/3] fix(api): restore guarded planned purchase creation --- internal/api/handler_plans.go | 108 ++++++++++++--- ...dler_plans_concurrency_integration_test.go | 90 ++++++++++-- internal/api/handler_plans_test.go | 27 ++-- internal/api/handler_test.go | 17 +-- internal/api/openapi.yaml | 6 +- internal/api/plan_step_no_recs_test.go | 131 +++++++++++++++--- 6 files changed, 301 insertions(+), 78 deletions(-) diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go index 2e6ca4a6..82882b36 100644 --- a/internal/api/handler_plans.go +++ b/internal/api/handler_plans.go @@ -11,6 +11,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/planstep" "github.com/aws/aws-lambda-go/events" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -432,7 +433,6 @@ func (h *Handler) deletePlan(ctx context.Context, req *events.LambdaFunctionURLR return map[string]string{"status": "deleted"}, nil } -//nolint:unparam // the response is nil only while steps cannot carry recommendations (platform#609); tier B returns it again func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.LambdaFunctionURLRequest, planID string) (*CreatePlannedPurchasesResponse, error) { if err := validateUUID(planID); err != nil { return nil, err @@ -453,9 +453,8 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La return nil, err } - // Parsed only so malformed input still gets its specific 400 before the - // tier-A refusal below. - if _, _, err = h.parseCreatePurchasesRequest(httpReq.Body); err != nil { + req, startDate, err := h.parseCreatePurchasesRequest(httpReq.Body) + if err != nil { return nil, err } @@ -463,15 +462,19 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La return nil, err } - // Issue #609 (tier A): nothing attaches recommendations to a plan step, so - // every step this endpoint minted was a bare row that could never buy - // anything yet showed up as healthy. Refuse until steps can carry - // recommendations (tier B). createPlannedPurchasesTx and - // createPurchaseExecutionsTx stay in place for that change; the handler no - // longer reaches them, so the creator stamp (#950), the single transaction - // and the ramp lock (#1861) are exercised through them directly in tests. - return nil, NewClientError(409, - "plan steps cannot yet carry recommendations (tracked in platform#631), so no purchases can be added to a plan; purchase from Opportunities instead") + created := 0 + err = h.config.WithTx(ctx, func(tx pgx.Tx) error { + var txErr error + created, txErr = h.createPlannedPurchasesTx(ctx, tx, planID, req.Count, startDate, resolveCreatorUserID(session), session) + return txErr + }) + if err != nil { + if _, ok := IsClientError(err); ok || errors.Is(err, errNotFound) { + return nil, err + } + return nil, mapCreatePlanStorageError(err, "plan not found", "failed to schedule purchases", "createPlannedPurchases: transaction failed: %v", err) + } + return &CreatePlannedPurchasesResponse{Created: created}, nil } // createPlannedPurchasesTx is the transactional body of createPlannedPurchases: @@ -485,7 +488,7 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La // two creates both see the step free and each mints a root row for it, and // approving both re-fans-out over accounts that already bought. CompletePlanStep // takes this same lock, so a completion cannot interleave either. -func (h *Handler) createPlannedPurchasesTx(ctx context.Context, tx pgx.Tx, planID string, count int, startDate time.Time, creator *string) (int, error) { +func (h *Handler) createPlannedPurchasesTx(ctx context.Context, tx pgx.Tx, planID string, count int, startDate time.Time, creator *string, session *Session) (int, error) { plan, err := h.config.LockPurchasePlanTx(ctx, tx, planID) if err != nil { logging.Errorf("createPlannedPurchases: LockPurchasePlanTx failed (plan=%s): %v", planID, err) @@ -494,6 +497,12 @@ func (h *Handler) createPlannedPurchasesTx(ctx context.Context, tx pgx.Tx, planI if plan == nil { return 0, NewClientError(404, "plan not found") } + if err := h.requirePlanMutationAccess(ctx, session, planID); err != nil { + return 0, err + } + if err := h.validatePurchaseScheduling(ctx, plan, count, startDate); err != nil { + return 0, err + } if refuseErr := h.refuseOccupiedRampSteps(ctx, tx, plan, planID, count); refuseErr != nil { return 0, refuseErr @@ -509,6 +518,65 @@ func (h *Handler) createPlannedPurchasesTx(ctx context.Context, tx pgx.Tx, planI return created, nil } +func (h *Handler) validatePurchaseScheduling(ctx context.Context, plan *config.PurchasePlan, count int, startDate time.Time) error { + if !plan.Enabled { + return NewClientError(409, "plan must be enabled before purchases can be scheduled") + } + if err := validateScheduledRamp(plan, count); err != nil { + return err + } + accounts, err := h.config.GetPlanAccounts(ctx, plan.ID) + if err != nil { + return err + } + if len(accounts) == 0 { + return NewClientError(409, "plan must have target accounts before purchases can be scheduled") + } + if plan.AutoPurchase { + if err := h.requireAutomaticScheduling(ctx, plan); err != nil { + return err + } + } + if !startDate.After(time.Now()) { + return NewClientError(400, "start_date must be in the future") + } + return nil +} + +func validateScheduledRamp(plan *config.PurchasePlan, count int) error { + if err := plan.Validate(); err != nil { + return NewClientError(409, fmt.Sprintf("plan cannot be scheduled: %v", err)) + } + if err := planstep.Precheck(plan); err != nil { + return NewClientError(409, err.Error()) + } + if err := plan.RequireVetoWindow(); err != nil { + return NewClientError(409, err.Error()) + } + ramp := plan.RampSchedule + if count > ramp.TotalSteps-ramp.CurrentStep || (ramp.Type == "immediate" && ramp.TotalSteps != 1) { + return NewClientError(409, "count exceeds the plan's remaining ramp steps") + } + if ramp.Type != "immediate" && ramp.StepIntervalDays <= 0 { + return NewClientError(409, "step interval must be positive for scheduled ramp purchases") + } + return nil +} + +func (h *Handler) requireAutomaticScheduling(ctx context.Context, plan *config.PurchasePlan) error { + if plan.MaxStepCommitment == nil { + return NewClientError(409, "automatic plans require max_step_commitment") + } + global, err := h.config.GetGlobalConfig(ctx) + if err != nil { + return err + } + if global == nil || !global.AutoPurchaseEnabled { + return NewClientError(409, "automatic purchasing is disabled globally") + } + return nil +} + // refuseOccupiedRampSteps blocks a create whose steps overlap one that is // already covered (issue #1861). Must be called with the plan's ramp lock held // for the same transaction, which createPlannedPurchasesTx does. @@ -608,9 +676,6 @@ func (h *Handler) getPlanForPurchaseCreation(ctx context.Context, planID string) // reachable only by admin / update-any holders. func (h *Handler) createPurchaseExecutionsTx(ctx context.Context, tx pgx.Tx, plan *config.PurchasePlan, planID string, count int, startDate time.Time, creator *string) (int, error) { intervalDays := plan.RampSchedule.StepIntervalDays - if intervalDays == 0 { - intervalDays = 7 // Default to weekly if not set - } created := 0 for i := 0; i < count; i++ { @@ -642,11 +707,10 @@ func (h *Handler) createPurchaseExecutionsTx(ctx context.Context, tx pgx.Tx, pla // inside the caller's transaction so it commits atomically with the // SavePurchaseExecutionTx writes from createPurchaseExecutionsTx. func (h *Handler) updatePlanNextExecutionDateTx(ctx context.Context, tx pgx.Tx, plan *config.PurchasePlan, startDate time.Time) error { - if plan.NextExecutionDate == nil || plan.NextExecutionDate.After(startDate) { - plan.NextExecutionDate = &startDate - if err := h.config.UpdatePurchasePlanTx(ctx, tx, plan); err != nil { - return fmt.Errorf("failed to update plan: %w", err) - } + plan.RampSchedule.StartDate = startDate.AddDate(0, 0, -plan.RampSchedule.CurrentStep*plan.RampSchedule.StepIntervalDays) + plan.NextExecutionDate = &startDate + if err := h.config.UpdatePurchasePlanTx(ctx, tx, plan); err != nil { + return fmt.Errorf("failed to update plan: %w", err) } return nil } diff --git a/internal/api/handler_plans_concurrency_integration_test.go b/internal/api/handler_plans_concurrency_integration_test.go index 6107b2ca..9a8b0f8d 100644 --- a/internal/api/handler_plans_concurrency_integration_test.go +++ b/internal/api/handler_plans_concurrency_integration_test.go @@ -53,8 +53,10 @@ func newCreateConcurrencyFixture(ctx context.Context, t *testing.T) *createConcu store := config.NewPostgresStore(container.DB) plan := &config.PurchasePlan{ - Name: "Concurrent Create Plan", - Services: map[string]config.ServiceConfig{}, + Name: "Concurrent Create Plan", + Enabled: true, + NotificationDaysBefore: 2, + Services: map[string]config.ServiceConfig{"azure:compute": {Provider: "azure", Service: "compute", Enabled: true, Term: 1, Payment: "monthly", Coverage: 80}}, RampSchedule: config.RampSchedule{ Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7, CurrentStep: 2, TotalSteps: 4, StartDate: time.Now().AddDate(0, 0, -14), @@ -63,7 +65,7 @@ func newCreateConcurrencyFixture(ctx context.Context, t *testing.T) *createConcu require.NoError(t, store.CreatePurchasePlan(ctx, plan)) acct := &config.CloudAccount{ - Name: "acct-a", Provider: "aws", ExternalID: "333333333333", Enabled: true, + Name: "acct-a", Provider: "azure", ExternalID: "fixture-subscription-a", Enabled: true, } require.NoError(t, store.CreateCloudAccount(ctx, acct)) require.NoError(t, store.SetPlanAccounts(ctx, plan.ID, []string{acct.ID})) @@ -91,22 +93,80 @@ func newCreateConcurrencyFixture(ctx context.Context, t *testing.T) *createConcu } } -// create creates `count` steps through createPlannedPurchasesTx, the path -// POST /plans/{id}/purchases used to reach. The handler itself refuses -// unconditionally while plan steps cannot carry recommendations (platform#609, -// tier A), so the ramp lock and guard are driven here directly. +// create exercises authentication and scheduling against the real store. func (f *createConcurrencyFixture) create(ctx context.Context, count int) (*CreatePlannedPurchasesResponse, error) { - start := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC) - created := 0 - err := f.store.WithTx(ctx, func(tx pgx.Tx) error { - n, txErr := f.handler.createPlannedPurchasesTx(ctx, tx, f.planID, count, start, nil) - created = n - return txErr - }) + body, err := json.Marshal(CreatePlannedPurchasesRequest{Count: count, StartDate: time.Now().AddDate(0, 0, 7).Format("2006-01-02")}) if err != nil { return nil, err } - return &CreatePlannedPurchasesResponse{Created: created}, nil + return f.handler.createPlannedPurchases(ctx, &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}, Body: string(body)}, f.planID) +} + +type membershipChangingStore struct { + config.StoreInterface + beforeLock func() error +} + +func (s membershipChangingStore) LockPurchasePlanTx(ctx context.Context, tx pgx.Tx, id string) (*config.PurchasePlan, error) { + if err := s.beforeLock(); err != nil { + return nil, err + } + return s.StoreInterface.LockPurchasePlanTx(ctx, tx, id) +} + +func TestCreatePlannedPurchases_MembershipChangesBeforeLock(t *testing.T) { + ctx := context.Background() + f := newCreateConcurrencyFixture(ctx, t) + accounts, err := f.store.GetPlanAccounts(ctx, f.planID) + require.NoError(t, err) + inaccessible := &config.CloudAccount{Name: "acct-b", Provider: "azure", ExternalID: "fixture-subscription-b", Enabled: true} + require.NoError(t, f.store.CreateCloudAccount(ctx, inaccessible)) + var userID string + require.NoError(t, f.pool.QueryRow(ctx, `SELECT id FROM users WHERE email='creator@example.com'`).Scan(&userID)) + auth := new(MockAuthService) + auth.On("ValidateSession", mock.Anything, "admin-token").Return(&Session{UserID: userID}, nil) + auth.grantScoped(accounts[0].ID) + f.handler.auth = auth + f.handler.config = membershipChangingStore{StoreInterface: f.store, beforeLock: func() error { + return f.store.SetPlanAccounts(ctx, f.planID, []string{inaccessible.ID}) + }} + _, err = f.create(ctx, 1) + require.ErrorIs(t, err, errNotFound) + assert.Zero(t, f.executionsForStep(ctx, t, 3)) +} + +func TestCreatePlannedPurchases_RoutedDatesAndRollback(t *testing.T) { + ctx := context.Background() + f := newCreateConcurrencyFixture(ctx, t) + auth := f.handler.auth.(*MockAuthService) + auth.On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).Return(nil) + first := time.Now().AddDate(0, 0, 7).UTC().Truncate(24 * time.Hour) + request := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token", "X-CSRF-Token": "test-csrf", "Content-Type": "application/json"}, + Body: `{"count":2,"start_date":"` + first.Format("2006-01-02") + `"}`, + RequestContext: events.LambdaFunctionURLRequestContext{HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{Method: "POST", Path: "/api/plans/" + f.planID + "/purchases"}}} + // Fail the second insert in this dedicated test database, proving atomic rollback. + _, err := f.pool.Exec(ctx, `CREATE FUNCTION fail_step_four() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN IF NEW.step_number = 4 THEN RAISE EXCEPTION 'injected failure'; END IF; RETURN NEW; END $$; CREATE TRIGGER fail_step_four BEFORE INSERT ON purchase_executions FOR EACH ROW EXECUTE FUNCTION fail_step_four()`) + require.NoError(t, err) + response, err := f.handler.HandleRequest(ctx, request) + require.NoError(t, err) + require.Equal(t, 500, response.StatusCode) + assert.Zero(t, f.executionsForStep(ctx, t, 3)) + _, err = f.pool.Exec(ctx, `ALTER TABLE purchase_executions DISABLE TRIGGER fail_step_four`) + require.NoError(t, err) + response, err = f.handler.HandleRequest(ctx, request) + require.NoError(t, err) + require.Equal(t, 200, response.StatusCode) + assert.JSONEq(t, `{"created":2}`, response.Body) + var scheduled time.Time + var creator *string + require.NoError(t, f.pool.QueryRow(ctx, `SELECT scheduled_date, created_by_user_id FROM purchase_executions WHERE plan_id=$1 AND step_number=3`, f.planID).Scan(&scheduled, &creator)) + assert.True(t, first.Equal(scheduled)) + require.NotNil(t, creator) + plan, err := f.store.GetPurchasePlan(ctx, f.planID) + require.NoError(t, err) + assert.True(t, first.Equal(*plan.NextExecutionDate)) + plan.RampSchedule.CurrentStep++ + assert.True(t, plan.RampSchedule.GetNextPurchaseDate().Equal(first.AddDate(0, 0, 7))) } // executionsForStep counts the plan's execution rows stamped with step. diff --git a/internal/api/handler_plans_test.go b/internal/api/handler_plans_test.go index 083124d7..07902b65 100644 --- a/internal/api/handler_plans_test.go +++ b/internal/api/handler_plans_test.go @@ -721,17 +721,28 @@ func TestHandler_updatePlan_InvalidBody(t *testing.T) { // MockAuthService is a mock implementation of AuthServiceInterface -// createStepsViaTx drives the transactional step-creation path -// (createPlannedPurchasesTx) that POST /plans/{id}/purchases used to reach. -// The handler now refuses unconditionally (platform#609, tier A), so the -// behavior these tests pin (creator stamping #950, single transaction, ramp -// guard #1861) is exercised here until tier B restores the endpoint. +// createStepsViaTx isolates transactional creator, rollback and occupied-step behavior. func createStepsViaTx(ctx context.Context, h *Handler, count int, creator *string) (int, error) { const planID = "11111111-1111-1111-1111-111111111111" - start := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC) + plan, err := h.config.GetPurchasePlan(ctx, planID) + if err != nil { + return 0, err + } + plan.Enabled = true + plan.NotificationDaysBefore = 1 + plan.RampSchedule.Type = "weekly" + plan.RampSchedule.PercentPerStep = 25 + plan.RampSchedule.TotalSteps = 52 + plan.Services = map[string]config.ServiceConfig{"azure:compute": {Provider: "azure", Service: "compute", Enabled: true, Term: 1, Payment: "monthly", Coverage: 80}} + if store, ok := h.config.(*MockConfigStore); ok { + store.GetPlanAccountsFn = func(context.Context, string) ([]config.CloudAccount, error) { + return []config.CloudAccount{{Provider: "azure"}}, nil + } + } + start := time.Now().AddDate(0, 0, 7) created := 0 - err := h.config.WithTx(ctx, func(tx pgx.Tx) error { - n, txErr := h.createPlannedPurchasesTx(ctx, tx, planID, count, start, creator) + err = h.config.WithTx(ctx, func(tx pgx.Tx) error { + n, txErr := h.createPlannedPurchasesTx(ctx, tx, planID, count, start, creator, &Session{UserID: apiKeyAdminUserID}) created = n return txErr }) diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go index 6b4dd45b..4b17780c 100644 --- a/internal/api/handler_test.go +++ b/internal/api/handler_test.go @@ -1364,13 +1364,14 @@ func TestHandler_HandleRequest_CreatePlannedPurchases(t *testing.T) { mockAuth.grantAdmin() mockAuth.On("ValidateCSRFToken", ctx, mock.Anything, mock.Anything).Return(nil) - plan := &config.PurchasePlan{ - ID: "11111111-1111-1111-1111-111111111111", - Name: "Test Plan", - RampSchedule: config.RampSchedule{StepIntervalDays: 7}, - } + plan := bareStepAPIPlan() + plan.ID = "11111111-1111-1111-1111-111111111111" + plan.AutoPurchase = false mockStore.On("GetPurchasePlan", mock.Anything, "11111111-1111-1111-1111-111111111111").Return(plan, nil) + mockStore.On("GetPlanAccounts", mock.Anything, plan.ID).Return([]config.CloudAccount{{Provider: "azure"}}, nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Times(2) + mockStore.On("UpdatePurchasePlan", mock.Anything, mock.Anything).Return(nil) handler := &Handler{config: mockStore, auth: mockAuth, corsAllowedOrigin: "*", apiKey: "test-key"} @@ -1381,7 +1382,7 @@ func TestHandler_HandleRequest_CreatePlannedPurchases(t *testing.T) { "Authorization": "Bearer test-token", "X-CSRF-Token": "test-csrf", }, - Body: `{"count": 2, "start_date": "2024-12-01"}`, + Body: `{"count": 2, "start_date": "` + time.Now().AddDate(0, 0, 7).Format("2006-01-02") + `"}`, RequestContext: events.LambdaFunctionURLRequestContext{ HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{ Method: "POST", @@ -1392,8 +1393,8 @@ func TestHandler_HandleRequest_CreatePlannedPurchases(t *testing.T) { resp, err := handler.HandleRequest(ctx, req) require.NoError(t, err) - // Plan steps cannot carry recommendations yet (#609), so the route refuses. - assert.Equal(t, 409, resp.StatusCode) + assert.Equal(t, 200, resp.StatusCode) + assert.JSONEq(t, `{"created":2}`, resp.Body) } // Tests for edge cases in getPlan. diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 346144b8..ce3838ed 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -446,8 +446,10 @@ paths: $ref: '#/components/responses/NotFound' '409': description: > - Plan steps cannot yet carry recommendations (platform#631), so no - purchases can be added to a plan. Purchase from Opportunities instead. + The plan is disabled, has unsupported scheduling settings, no target + accounts or insufficient remaining steps, or the requested steps are + occupied. Automatic plans also require a valid commitment cap and + the global automatic purchasing switch. content: application/json: schema: diff --git a/internal/api/plan_step_no_recs_test.go b/internal/api/plan_step_no_recs_test.go index 6339b183..922d2775 100644 --- a/internal/api/plan_step_no_recs_test.go +++ b/internal/api/plan_step_no_recs_test.go @@ -2,8 +2,9 @@ package api import ( "context" - "errors" + "fmt" "testing" + "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" @@ -13,9 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -// Issue #609 (tier A). Plan steps carry no recommendations, so a step can -// never buy anything. These tests use the reported plan: Azure compute, 1 year, -// monthly payment, Auto-Purchase on. +// The reported plan uses Azure compute, one year, monthly payment and a weekly ramp. const ( bareStepAPIPlanID = "11111111-1111-1111-1111-111111111609" @@ -24,18 +23,19 @@ const ( func bareStepAPIPlan() *config.PurchasePlan { return &config.PurchasePlan{ - ID: bareStepAPIPlanID, - Name: "Azure compute 80% weekly", - Enabled: true, - AutoPurchase: true, + ID: bareStepAPIPlanID, + Name: "Azure compute 80% weekly", + Enabled: true, + AutoPurchase: true, + NotificationDaysBefore: 2, Services: map[string]config.ServiceConfig{ - "azure:compute": {Provider: "azure", Service: "compute", Term: 1, Payment: "monthly"}, + "azure:compute": {Provider: "azure", Service: "compute", Enabled: true, Term: 1, Payment: "monthly", Coverage: 80}, }, - RampSchedule: config.RampSchedule{CurrentStep: 0, TotalSteps: 4, StepIntervalDays: 7}, + RampSchedule: config.RampSchedule{Type: "weekly", PercentPerStep: 25, CurrentStep: 0, TotalSteps: 4, StepIntervalDays: 7}, } } -func TestAddPurchasesToPlanIsRefusedWhileStepsCannotCarryRecommendations(t *testing.T) { +func TestAddPurchasesToPlanCreatesFutureRoots(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) @@ -46,26 +46,111 @@ func TestAddPurchasesToPlanIsRefusedWhileStepsCannotCarryRecommendations(t *test Email: "admin@example.com", }, nil) mockAuth.grantAdmin() - mockStore.On("GetPurchasePlan", ctx, bareStepAPIPlanID).Return(bareStepAPIPlan(), nil) - // Nothing may be written: reaching the transaction at all is the defect. - mockStore.On("WithTx", ctx, mock.AnythingOfType("func(pgx.Tx) error")). - Return(errors.New("transaction opened: the guard did not fire")).Maybe() + plan := bareStepAPIPlan() + plan.AutoPurchase = false + mockStore.On("GetPurchasePlan", ctx, bareStepAPIPlanID).Return(plan, nil) + mockStore.On("GetPlanAccounts", ctx, bareStepAPIPlanID).Return([]config.CloudAccount{{ID: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", Provider: "azure"}}, nil) + mockStore.On("SavePurchaseExecution", ctx, mock.Anything).Return(nil).Times(4) + mockStore.On("UpdatePurchasePlan", ctx, mock.Anything).Return(nil) handler := &Handler{config: mockStore, auth: mockAuth} req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{"Authorization": "Bearer admin-token"}, - Body: `{"count": 4, "start_date": "2026-10-07"}`, + Body: `{"count": 4, "start_date": "` + time.Now().AddDate(0, 0, 7).Format("2006-01-02") + `"}`, } result, err := handler.createPlannedPurchases(ctx, req, bareStepAPIPlanID) - require.Error(t, err, "Add Purchases must not create steps that cannot buy anything") - assert.Nil(t, result) - ce, ok := IsClientError(err) - require.True(t, ok, "expected a client error, got %v", err) - assert.Equal(t, 409, ce.code) - assert.Contains(t, ce.Error(), "#631") - mockStore.AssertNotCalled(t, "SavePurchaseExecutionTx") + require.NoError(t, err) + require.Equal(t, 4, result.Created) + mockStore.AssertExpectations(t) +} + +func TestAddPurchasesValidatesLockedPlan(t *testing.T) { + tests := []struct { + name string + change func(*config.PurchasePlan) + switchOn bool + want string + }{ + {"disabled", func(p *config.PurchasePlan) { p.Enabled = false }, true, "enabled"}, + {"missing auto cap", func(p *config.PurchasePlan) { p.MaxStepCommitment = nil }, true, "max_step_commitment"}, + {"invalid cap", func(p *config.PurchasePlan) { v := 1.005; p.MaxStepCommitment = &v }, true, "decimal"}, + {"switch off", func(*config.PurchasePlan) {}, false, "disabled globally"}, + {"complete", func(p *config.PurchasePlan) { p.RampSchedule.CurrentStep = 4 }, true, "remaining"}, + {"negative step", func(p *config.PurchasePlan) { p.RampSchedule.CurrentStep = -1 }, true, "negative"}, + {"no interval", func(p *config.PurchasePlan) { p.RampSchedule.StepIntervalDays = 0 }, true, "positive"}, + {"no veto", func(p *config.PurchasePlan) { p.NotificationDaysBefore = 0 }, true, "veto"}, + {"ignored filter", func(p *config.PurchasePlan) { + s := p.Services["azure:compute"] + s.IncludeRegions = []string{"eastus"} + p.Services["azure:compute"] = s + }, true, "include_regions"}, + {"automatic", func(*config.PurchasePlan) {}, true, ""}, + {"manual switch off", func(p *config.PurchasePlan) { p.AutoPurchase = false; p.MaxStepCommitment = nil }, false, ""}, + {"immediate", func(p *config.PurchasePlan) { + p.RampSchedule = config.RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1} + }, true, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + plan := bareStepAPIPlan() + cap := 100.0 + plan.MaxStepCommitment = &cap + tt.change(plan) + store.On("GetPurchasePlan", ctx, plan.ID).Return(plan, nil) + store.On("GetPlanAccounts", ctx, plan.ID).Return([]config.CloudAccount{{ID: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", Provider: "azure"}}, nil).Maybe() + store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{AutoPurchaseEnabled: tt.switchOn}, nil).Maybe() + if tt.want == "" { + store.On("SavePurchaseExecution", ctx, mock.Anything).Return(nil).Once() + store.On("UpdatePurchasePlan", ctx, mock.Anything).Return(nil).Once() + } + h := &Handler{config: store} + first := time.Now().AddDate(0, 0, 7) + n, err := h.createPlannedPurchasesTx(ctx, nil, plan.ID, 1, first, nil, &Session{UserID: apiKeyAdminUserID}) + if tt.want != "" { + require.ErrorContains(t, err, tt.want) + assert.Zero(t, n) + store.AssertNotCalled(t, "SavePurchaseExecutionTx") + } else { + require.NoError(t, err) + assert.Equal(t, 1, n) + require.True(t, first.Equal(*plan.NextExecutionDate)) + } + }) + } +} + +func TestAddPurchasesAnchorsPartlyCompletedRamp(t *testing.T) { + for _, step := range []int{0, 2} { + t.Run(fmt.Sprint(step), func(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + plan := bareStepAPIPlan() + plan.AutoPurchase = false + plan.RampSchedule.CurrentStep = step + older := time.Now().AddDate(0, 0, 1) + plan.NextExecutionDate = &older + first := time.Now().AddDate(0, 0, 10) + store.On("GetPurchasePlan", ctx, plan.ID).Return(plan, nil) + store.On("GetPlanAccounts", ctx, plan.ID).Return([]config.CloudAccount{{Provider: "azure"}}, nil) + var rows []config.PurchaseExecution + store.On("SavePurchaseExecution", ctx, mock.Anything).Run(func(a mock.Arguments) { rows = append(rows, *a.Get(1).(*config.PurchaseExecution)) }).Return(nil).Times(2) + store.On("UpdatePurchasePlan", ctx, mock.Anything).Return(nil) + h := &Handler{config: store} + _, err := h.createPlannedPurchasesTx(ctx, nil, plan.ID, 2, first, nil, &Session{UserID: apiKeyAdminUserID}) + require.NoError(t, err) + require.Len(t, rows, 2) + assert.True(t, rows[0].ScheduledDate.Equal(first)) + assert.True(t, rows[1].ScheduledDate.Equal(first.AddDate(0, 0, 7))) + assert.Equal(t, step+1, rows[0].StepNumber) + assert.True(t, first.Equal(*plan.NextExecutionDate)) + plan.RampSchedule.CurrentStep++ + assert.True(t, plan.RampSchedule.GetNextPurchaseDate().Equal(first.AddDate(0, 0, 7))) + }) + } } func TestRetryOfBarePlanStepIsRefusedAndCreatesNothing(t *testing.T) { From 6a2966f794a1c6f43331357a986b142d17d12a45 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 14 Oct 2026 17:29:24 +0200 Subject: [PATCH 2/3] fix(api): satisfy planned purchase lint checks --- internal/api/handler_plans.go | 8 ++++++-- .../api/handler_plans_concurrency_integration_test.go | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go index 82882b36..79dec1cc 100644 --- a/internal/api/handler_plans.go +++ b/internal/api/handler_plans.go @@ -458,7 +458,7 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La return nil, err } - if _, err := h.getPlanForPurchaseCreation(ctx, planID); err != nil { + if _, err = h.getPlanForPurchaseCreation(ctx, planID); err != nil { return nil, err } @@ -469,7 +469,11 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La return txErr }) if err != nil { - if _, ok := IsClientError(err); ok || errors.Is(err, errNotFound) { + clientErr, isClient := IsClientError(err) + if isClient { + return nil, clientErr + } + if errors.Is(err, errNotFound) { return nil, err } return nil, mapCreatePlanStorageError(err, "plan not found", "failed to schedule purchases", "createPlannedPurchases: transaction failed: %v", err) diff --git a/internal/api/handler_plans_concurrency_integration_test.go b/internal/api/handler_plans_concurrency_integration_test.go index 9a8b0f8d..1cd4a0c0 100644 --- a/internal/api/handler_plans_concurrency_integration_test.go +++ b/internal/api/handler_plans_concurrency_integration_test.go @@ -200,7 +200,7 @@ func TestCreatePlannedPurchases_ConcurrentCreatesMintOneStepEach(t *testing.T) { go func() { defer wg.Done() <-start - _, err := f.create(ctx, 1) + _, err := f.create(ctx, 2) errs <- err }() } @@ -226,6 +226,8 @@ func TestCreatePlannedPurchases_ConcurrentCreatesMintOneStepEach(t *testing.T) { assert.Equal(t, 1, f.executionsForStep(ctx, t, 3), "ramp step 3 must end with exactly one execution row, not one per racer") + assert.Equal(t, 1, f.executionsForStep(ctx, t, 4), + "the same winning transaction must also create step 4 exactly once") } // TestCreatePlannedPurchases_BlocksOnTheRampLockAndSeesTheWinner forces the From e25ae316057f67162b6ff0c3e2164fa22ac20024 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 15 Oct 2026 01:16:43 +0200 Subject: [PATCH 3/3] test(api): pin execution fixture to AWS credentials (#875) --- internal/api/handler_purchases_pause_claim_integration_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/api/handler_purchases_pause_claim_integration_test.go b/internal/api/handler_purchases_pause_claim_integration_test.go index cd3df35a..f327b5e4 100644 --- a/internal/api/handler_purchases_pause_claim_integration_test.go +++ b/internal/api/handler_purchases_pause_claim_integration_test.go @@ -102,6 +102,8 @@ func newPauseClaimFixture(t *testing.T) *pauseClaimFixture { account.Enabled = true account.AWSAuthMode = "access_keys" require.NoError(t, f.store.UpdateCloudAccount(ctx, &account)) + _, err = f.pool.Exec(ctx, `UPDATE cloud_accounts SET provider = 'aws', external_id = '333333333333' WHERE id = $1`, account.ID) + require.NoError(t, err) var creator string require.NoError(t, f.pool.QueryRow(ctx, "SELECT id::text FROM users WHERE email='creator@example.com'").Scan(&creator)) plan, err = f.store.GetPurchasePlan(ctx, plan.ID) @@ -111,6 +113,7 @@ func newPauseClaimFixture(t *testing.T) *pauseClaimFixture { persistedAccount, err := f.store.GetCloudAccount(ctx, account.ID) require.NoError(t, err) require.True(t, persistedAccount.Enabled) + require.Equal(t, "aws", persistedAccount.Provider) require.Equal(t, "access_keys", persistedAccount.AWSAuthMode) f.service = &pauseClaimService{entered: make(chan struct{}), release: make(chan struct{})} f.manager = purchase.NewManager(purchase.ManagerConfig{