diff --git a/internal/api/handler_purchases_run_now_integration_test.go b/internal/api/handler_purchases_run_now_integration_test.go index 2c236d58..fc14ceea 100644 --- a/internal/api/handler_purchases_run_now_integration_test.go +++ b/internal/api/handler_purchases_run_now_integration_test.go @@ -21,12 +21,20 @@ import ( // runNowService buys every recommendation except the call numbered failOn. type runNowService struct { fanoutServiceClient - calls atomic.Int32 - failOn int32 + calls atomic.Int32 + failOn int32 + onPurchase func(context.Context) } func (s *runNowService) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) { - if s.calls.Add(1) == s.failOn { + call := s.calls.Add(1) + if s.onPurchase != nil { + s.onPurchase(ctx) + } + if err := ctx.Err(); err != nil { + return common.PurchaseResult{}, err + } + if call == s.failOn { return common.PurchaseResult{}, errors.New("provider rejected the purchase") } return s.fanoutServiceClient.PurchaseCommitment(ctx, rec, opts) @@ -145,3 +153,52 @@ func TestRunNow_LostClaimIsConflict(t *testing.T) { assert.Nil(t, row.ExecutedAt) assert.Zero(t, service.calls.Load()) } + +func TestRunNow_CanceledRequestAfterClaimPersistsTerminalOutcome(t *testing.T) { + for _, tc := range []struct { + name string + failOn int32 + status string + history int + }{ + {name: "completed", status: "completed", history: 1}, + {name: "failed", failOn: 1, status: "failed"}, + } { + t.Run(tc.name, func(t *testing.T) { + f, service := newRunNowFixture(t, tc.failOn, fanoutRecs()) + requestCtx, cancel := context.WithCancel(f.ctx) + defer cancel() + var claimStatus string + var claimErr error + service.onPurchase = func(context.Context) { + row, err := f.store.GetExecutionByID(f.ctx, f.execution.ExecutionID) + claimErr = err + if row != nil { + claimStatus = row.Status + } + cancel() + } + _, runErr := f.handler.runPlannedPurchase(requestCtx, f.request, f.execution.ExecutionID) + require.NoError(t, claimErr) + require.Equal(t, "running", claimStatus) + require.ErrorIs(t, requestCtx.Err(), context.Canceled) + if tc.failOn == 0 { + require.NoError(t, runErr) + } else { + require.Error(t, runErr) + } + verifyCtx, verifyCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer verifyCancel() + row, err := f.store.GetExecutionByID(verifyCtx, f.execution.ExecutionID) + require.NoError(t, err) + require.Equal(t, tc.status, row.Status) + var histories int + require.NoError(t, f.pool.QueryRow(verifyCtx, "SELECT count(*) FROM purchase_history WHERE plan_id=$1", f.planID).Scan(&histories)) + require.Equal(t, tc.history, histories) + freshRequest := *f.request + _, retryErr := f.handler.runPlannedPurchase(verifyCtx, &freshRequest, f.execution.ExecutionID) + require.Error(t, retryErr) + assert.EqualValues(t, 1, service.calls.Load()) + }) + } +} diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index 0e6b7770..9ceee183 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -6674,9 +6674,9 @@ func TestHandler_approvePurchaseViaSession_FourEyesOn_DifferentApproverSucceeds( // refused at execution (#609), and this test is about the 4-eyes check. approved := &config.PurchaseExecution{ExecutionID: execID, Status: "approved"} mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil) - mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil) + mockConfig.On("GetGlobalConfig", mock.Anything).Return(fourEyesCfgOn(), nil) mockConfig.On("TransitionExecutionStatus", ctx, execID, []string{"pending", "notified"}, "running", &approverID).Return(approved, nil) - mockConfig.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockConfig.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) mockAuth := new(MockAuthService) mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{UserID: approverID, Email: approverEmail}, nil) diff --git a/internal/purchase/approvals_test.go b/internal/purchase/approvals_test.go index 5df9312b..6176c4b2 100644 --- a/internal/purchase/approvals_test.go +++ b/internal/purchase/approvals_test.go @@ -546,7 +546,7 @@ func TestManager_ApproveAndExecute_FourEyesOn_AllowsDifferentApprover(t *testing Recommendations: approvalTestRecs(), } - store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil) + store.On("GetGlobalConfig", mock.Anything).Return(fourEyesCfgOnForManager(), nil) store.On("GetExecutionByID", ctx, "exec-direct-diff").Return(execution, nil) store.On("TransitionExecutionStatus", ctx, "exec-direct-diff", approveFromStatuses, "running", &approverUUID).Return(updated, nil) stubExecuteChain(t, store, sender, "plan-fourEyes") @@ -714,7 +714,7 @@ func TestManager_ApproveExecution_FourEyesOn_DifferentActor_Allowed(t *testing.T } store.On("GetExecutionByID", ctx, "exec-sqs-diff").Return(execution, nil) - store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil) + store.On("GetGlobalConfig", mock.Anything).Return(fourEyesCfgOnForManager(), nil) store.On("GetUserEmailByID", ctx, creatorID).Return("creator@example.com", nil) store.On("TransitionExecutionStatus", ctx, "exec-sqs-diff", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil) stubExecuteChain(t, store, sender, "plan-fourEyes") diff --git a/internal/purchase/claim_guard_test.go b/internal/purchase/claim_guard_test.go index 9f6e74b9..1a5014eb 100644 --- a/internal/purchase/claim_guard_test.go +++ b/internal/purchase/claim_guard_test.go @@ -105,7 +105,8 @@ func TestApprove_ClaimsRunningAndStampsGuarded(t *testing.T) { store.On("GetExecutionByID", mock.Anything, "exec-g").Return(&config.PurchaseExecution{ExecutionID: "exec-g", PlanID: "plan-g", Status: "pending"}, nil).Maybe() store.On("TransitionExecutionStatus", ctx, "exec-g", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil) stubExecuteChain(t, store, sender, "plan-g") - store.On("SaveExecutionIfStatus", ctx, mock.Anything, []string{"running"}).Return(nil) + store.On("SaveExecutionIfStatus", ctx, mock.Anything, []string{"running"}).Return(nil).Once() + store.On("SaveExecutionIfStatus", mock.Anything, mock.Anything, []string{"running"}).Return(nil).Once() _, _, err := manager.ApproveAndExecute(ctx, "exec-g", "approver@example.invalid", nil) require.NoError(t, err) diff --git a/internal/purchase/claim_pin_test.go b/internal/purchase/claim_pin_test.go index faf99749..92d5532a 100644 --- a/internal/purchase/claim_pin_test.go +++ b/internal/purchase/claim_pin_test.go @@ -159,12 +159,12 @@ func TestClaimPin_ApprovedRowSkipsCapRecheck(t *testing.T) { ctx := context.Background() exec := dueRecExec("approved", upfrontRec(1e9)) store := new(MockConfigStore) - store.On("GetPurchasePlan", ctx, exec.PlanID).Return(autoPlan(nil), nil) - store.On("GetPlanAccounts", ctx, exec.PlanID).Return(nil, errors.New("accounts unavailable")) + store.On("GetPurchasePlan", mock.Anything, exec.PlanID).Return(autoPlan(nil), nil) + store.On("GetPlanAccounts", mock.Anything, exec.PlanID).Return(nil, errors.New("accounts unavailable")) claimed := exec claimed.Status = "running" store.On("TransitionExecutionStatusPinned", ctx, exec.ExecutionID, mock.Anything, "running", (*string)(nil), false).Return(&claimed, nil) - store.On("SavePurchaseExecution", ctx, mock.Anything).Return(nil) + store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) manager := &Manager{config: store, email: new(MockEmailSender), providerFactory: new(MockProviderFactory)} didClaim, err := manager.claimAndExecute(ctx, &exec) @@ -209,11 +209,12 @@ func TestClaimPin_RecLessRowStillFailsInsteadOfBuying(t *testing.T) { store, factory := new(MockConfigStore), new(MockProviderFactory) exec := dueRecExec("pending") manager := newPinManager(ctx, store, factory, exec, autoPlan(nil), autoPlan(nil)) + store.On("GetPurchasePlan", mock.Anything, exec.PlanID).Return(autoPlan(nil), nil) claimed := exec claimed.Status = "running" store.On("TransitionExecutionStatusPinned", ctx, "exec-gate", mock.Anything, "running", (*string)(nil), true).Return(&claimed, nil) var saved *config.PurchaseExecution - store.On("SavePurchaseExecutionTx", ctx, mock.Anything, mock.Anything). + store.On("SavePurchaseExecutionTx", mock.Anything, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { saved = args.Get(2).(*config.PurchaseExecution) }).Return(nil) result, err := manager.ProcessScheduledPurchases(ctx) require.NoError(t, err) diff --git a/internal/purchase/coverage_extra_test.go b/internal/purchase/coverage_extra_test.go index 71ec647e..e7654c46 100644 --- a/internal/purchase/coverage_extra_test.go +++ b/internal/purchase/coverage_extra_test.go @@ -240,12 +240,12 @@ func TestHandleExecutePurchase_ApprovedStatus(t *testing.T) { runningExec.Status = "running" mockStore.On("TransitionExecutionStatus", ctx, "exec-approved", []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&runningExec, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-approved").Return(plan, nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) - mockStore.On("CompletePlanStep", ctx, "plan-approved", 1).Return(nil) - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockSTS.On("GetCallerIdentity", ctx, mock.Anything).Return(nil, errors.New("sts error")) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-approved").Return(plan, nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockStore.On("CompletePlanStep", mock.Anything, "plan-approved", 1).Return(nil) + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockSTS.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(nil, errors.New("sts error")) mockProv := new(MockProvider) mockSvc := new(MockServiceClient) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProv, nil) @@ -326,16 +326,17 @@ func TestHandleExecutePurchase_SaveError(t *testing.T) { Recommendations: []config.RecommendationRecord{rec}, } - mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil).Once() + mockStore.On("GetGlobalConfig", mock.Anything).Return(autoPurchaseOnCfg(), nil) mockStore.On("GetExecutionByID", ctx, "exec-save-err").Return(exec, nil) // claimAndExecute CASes the row to "running" before executing (issue #1013). runningExec := *exec runningExec.Status = "running" mockStore.On("TransitionExecutionStatus", ctx, "exec-save-err", []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&runningExec, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-save-err").Return(plan, nil) - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(errors.New("save failed")) - mockSTS.On("GetCallerIdentity", ctx, mock.Anything).Return(nil, errors.New("sts error")) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-save-err").Return(plan, nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(errors.New("save failed")) + mockSTS.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(nil, errors.New("sts error")) // Provider factory returns error → purchase fails mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(nil, errors.New("provider error")) @@ -409,10 +410,10 @@ func TestProcessMessage_ApproveHappyPath(t *testing.T) { // regression that drops it (e.g. passes "" or exec.ExecutionID by // mistake) would mismatch the mock expectation and fail the test. plan := &config.PurchasePlan{ID: planID, Name: "test-plan"} - mockStore.On("GetPurchasePlan", ctx, planID).Return(plan, nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.Anything).Return(nil) - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) - mockStore.On("CompletePlanStep", ctx, planID, 1).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, planID).Return(plan, nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockStore.On("CompletePlanStep", mock.Anything, planID, 1).Return(nil) manager := &Manager{ config: mockStore, @@ -514,14 +515,14 @@ func TestProcessMessage_ApproveFourEyesOn_DifferentApproverSucceeds(t *testing.T // row TransitionExecutionStatus returns) in place. mockStore.On("GetExecutionByID", ctx, "exec-appv-diff").Return(exec, nil).Times(3) mockStore.On("GetCloudAccount", ctx, accountID).Return(account, nil) - mockStore.On("GetGlobalConfig", ctx).Return(fourEyesCfgOnForManager(), nil) + mockStore.On("GetGlobalConfig", mock.Anything).Return(fourEyesCfgOnForManager(), nil) mockStore.On("GetUserEmailByID", ctx, creatorID).Return("owner@example.com", nil) mockStore.On("TransitionExecutionStatus", ctx, "exec-appv-diff", []string{"pending", "notified"}, "running", (*string)(nil)).Return(approved, nil) plan := &config.PurchasePlan{ID: planID, Name: "test-plan"} - mockStore.On("GetPurchasePlan", ctx, planID).Return(plan, nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.Anything).Return(nil) - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) - mockStore.On("CompletePlanStep", ctx, planID, 1).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, planID).Return(plan, nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockStore.On("CompletePlanStep", mock.Anything, planID, 1).Return(nil) manager := &Manager{ config: mockStore, diff --git a/internal/purchase/execute_detached_test.go b/internal/purchase/execute_detached_test.go new file mode 100644 index 00000000..023c93b7 --- /dev/null +++ b/internal/purchase/execute_detached_test.go @@ -0,0 +1,174 @@ +package purchase + +import ( + "context" + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/sts" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// detachFixture wires a direct single-account AWS purchase (issue #706). +type detachFixture struct { + manager *Manager + exec *config.PurchaseExecution + svc *MockServiceClient + saved []string + history int + saveCtx context.Context + saveRemaining time.Duration + histCtx context.Context + historyRemaining time.Duration +} + +func newDetachFixture(t *testing.T) *detachFixture { + t.Helper() + f := &detachFixture{svc: new(MockServiceClient)} + store := new(MockConfigStore) + factory := new(MockProviderFactory) + prov := new(MockProvider) + stsMock := new(MockSTSClient) + f.exec = &config.PurchaseExecution{ + ExecutionID: "exec-detach", StepNumber: 1, Status: "running", + Recommendations: []config.RecommendationRecord{ + {Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "us-east-1", Count: 1, UpfrontCost: 500, Selected: true}, + }, + } + store.SavePurchaseExecutionFn = func(ctx context.Context, e *config.PurchaseExecution) error { + f.saveCtx = ctx + if deadline, ok := ctx.Deadline(); ok { + f.saveRemaining = time.Until(deadline) + } + if err := ctx.Err(); err != nil { + return err + } + f.saved = append(f.saved, e.Status) + return nil + } + store.On("SavePurchaseHistory", mock.Anything, mock.Anything).Run(func(a mock.Arguments) { + f.history++ + f.histCtx = a.Get(0).(context.Context) + if deadline, ok := f.histCtx.Deadline(); ok { + f.historyRemaining = time.Until(deadline) + } + }).Return(nil) + email := new(MockEmailSender) + email.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil) + stsMock.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(&stsOut, nil) + factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(prov, nil) + prov.On("GetServiceClient", mock.Anything, common.ServiceEC2, "us-east-1").Return(f.svc, nil) + f.manager = &Manager{config: store, email: email, stsClient: stsMock, providerFactory: factory, dashboardURL: "https://d.example.com"} + return f +} + +func TestExecuteAndFinalize_SlowRunGetsFreshTerminalSaveBudget(t *testing.T) { + t.Setenv("CUDLY_MAX_ACCOUNT_PARALLELISM", "1") + f := newDetachFixture(t) + f.exec.Recommendations = append(f.exec.Recommendations, f.exec.Recommendations[0]) + f.exec.Recommendations[1].ResourceType = "m5.xlarge" + var active atomic.Int32 + var overlapped atomic.Bool + var providerCanceled atomic.Bool + f.svc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { + if active.Add(1) != 1 { + overlapped.Store(true) + } + defer active.Add(-1) + ctx := args.Get(0).(context.Context) + timer := time.NewTimer(16 * time.Second) + defer timer.Stop() + select { + case <-timer.C: + case <-ctx.Done(): + providerCanceled.Store(true) + } + }).Return(common.PurchaseResult{Success: true, CommitmentID: "ri-slow"}, nil).Twice() + started := time.Now() + err := f.manager.executeAndFinalize(context.Background(), f.exec) + assert.Greater(t, time.Since(started), 30*time.Second) + assert.False(t, overlapped.Load()) + assert.False(t, providerCanceled.Load(), "provider call must fit its own budget") + f.svc.AssertNumberOfCalls(t, "PurchaseCommitment", 2) + assert.Equal(t, 2, f.history) + require.NoError(t, err) + assert.Equal(t, []string{"completed"}, f.saved) + assert.Greater(t, f.saveRemaining, 29*time.Second) + assert.LessOrEqual(t, f.saveRemaining, 30*time.Second) +} + +var stsOut = sts.GetCallerIdentityOutput{Account: aws.String("123456789012")} + +// A committed purchase must remain auditable after request cancellation. +func TestExecuteAndFinalize_ClientDisconnectMidPurchase_StillRecorded(t *testing.T) { + f := newDetachFixture(t) + ctx, cancel := context.WithCancel(context.Background()) + f.svc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). + Run(func(mock.Arguments) { cancel() }). + Return(common.PurchaseResult{Success: true, CommitmentID: "ri-1"}, nil).Once() + + err := f.manager.executeAndFinalize(ctx, f.exec) + + require.NoError(t, err) + f.svc.AssertNumberOfCalls(t, "PurchaseCommitment", 1) + assert.Equal(t, []string{"completed"}, f.saved) + assert.Equal(t, 1, f.history) +} + +// Test B: a caller ctx whose deadline already passed (30s request timeout) +// must not reach the provider or the save; both get their own bounded budget. +func TestExecuteAndFinalize_ExpiredCallerDeadline_UsesOwnBudgets(t *testing.T) { + f := newDetachFixture(t) + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + var provDeadline time.Time + var provContextErr error + f.svc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). + Run(func(a mock.Arguments) { + c := a.Get(0).(context.Context) + provContextErr = c.Err() + provDeadline, _ = c.Deadline() + }). + Return(common.PurchaseResult{Success: true, CommitmentID: "ri-2"}, nil).Once() + + require.NoError(t, f.manager.executeAndFinalize(ctx, f.exec)) + require.NoError(t, provContextErr) + + // per-rec cap (30s) still bounds the provider call inside the 4m run budget. + assert.WithinDuration(t, time.Now(), provDeadline, 30*time.Second) + _, ok := f.saveCtx.Deadline() + require.True(t, ok, "terminal save needs a deadline") + // Literal durations on purpose: comparing against the constants would + // pass whatever their values are. + assert.LessOrEqual(t, f.saveRemaining, 30*time.Second) + assert.Greater(t, f.saveRemaining, 29*time.Second) + // SavePurchaseHistory runs inside executePurchase, so it sees the run ctx. + _, ok = f.histCtx.Deadline() + require.True(t, ok, "run ctx needs a deadline") + assert.LessOrEqual(t, f.historyRemaining, 4*time.Minute) + assert.Greater(t, f.historyRemaining, 3*time.Minute+59*time.Second) + assert.Equal(t, []string{"completed"}, f.saved) +} + +// Test C: a failed purchase with a canceled caller still persists "failed". +func TestExecuteAndFinalize_ProviderErrorWithCanceledCaller_PersistsFailed(t *testing.T) { + f := newDetachFixture(t) + ctx, cancel := context.WithCancel(context.Background()) + f.svc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). + Run(func(mock.Arguments) { cancel() }). + Return(common.PurchaseResult{}, errors.New("provider rejected")).Once() + + err := f.manager.executeAndFinalize(ctx, f.exec) + + require.Error(t, err) + assert.NotErrorIs(t, err, config.ErrAuditLoss) + assert.Equal(t, []string{"failed"}, f.saved) +} diff --git a/internal/purchase/execution_test.go b/internal/purchase/execution_test.go index 7a13ee8b..9037222d 100644 --- a/internal/purchase/execution_test.go +++ b/internal/purchase/execution_test.go @@ -1788,11 +1788,11 @@ func TestManager_ExecuteAndFinalize_HistorySaveFailure_StaysVisible(t *testing.T } saveErr := errors.New("insert failed: monthly_cost violates not-null constraint") - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(saveErr) + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(saveErr) // The execution record itself must still be persisted (with the audit marker). - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) - mockSTS.On("GetCallerIdentity", ctx, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockSTS.On("GetCallerIdentity", mock.Anything, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ Account: aws.String("123456789012"), }, nil) mockFactory.On("CreateAndValidateProvider", mock.MatchedBy(hasPerRecDeadline(30*time.Second)), "aws", mock.Anything).Return(mockProviderInst, nil) @@ -1862,9 +1862,9 @@ func TestManager_ExecuteAndFinalize_EmptyCommitmentID_CompletesWithAuditGap(t *t } // SavePurchaseHistory is deliberately not expected on the mock: the guard // must reject the record before it reaches the store. - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) - mockSTS.On("GetCallerIdentity", ctx, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockSTS.On("GetCallerIdentity", mock.Anything, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ Account: aws.String("123456789012"), }, nil) mockFactory.On("CreateAndValidateProvider", mock.MatchedBy(hasPerRecDeadline(30*time.Second)), "aws", mock.Anything).Return(mockProviderInst, nil) @@ -1918,14 +1918,14 @@ func TestManager_ExecuteAndFinalize_SingleAccount_PartialSuccess(t *testing.T) { } // History row must be saved ONLY for the rec that succeeded (m5.large). - mockStore.On("SavePurchaseHistory", ctx, mock.MatchedBy(func(r *config.PurchaseHistoryRecord) bool { + mockStore.On("SavePurchaseHistory", mock.Anything, mock.MatchedBy(func(r *config.PurchaseHistoryRecord) bool { return r.ResourceType == "m5.large" })).Return(nil).Once() - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) // Confirmation must still go out (the issue's core complaint). - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil).Once() + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil).Once() // Ambient STS only consulted as a fallback (no account resolved here). - mockSTS.On("GetCallerIdentity", ctx, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ + mockSTS.On("GetCallerIdentity", mock.Anything, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ Account: aws.String("123456789012"), }, nil).Maybe() diff --git a/internal/purchase/manager.go b/internal/purchase/manager.go index 132afc0b..524a9019 100644 --- a/internal/purchase/manager.go +++ b/internal/purchase/manager.go @@ -227,6 +227,17 @@ func isMultiAccountAckable(execErr error) bool { return errors.As(execErr, &partial) } +const ( + // Combined execution and persistence budgets require lambda_timeout >= 270s. + purchaseExecutionTimeout = 4 * time.Minute + terminalSaveTimeout = 30 * time.Second +) + +// detachedTimeout keeps ctx values but drops its cancellation and deadline. +func detachedTimeout(ctx context.Context, d time.Duration) (context.Context, context.CancelFunc) { + return context.WithTimeout(context.WithoutCancel(ctx), d) +} + // executeAndFinalize runs a purchase and handles status updates, record saving, and progress. // // The root execution row is ALWAYS saved with its finalized status — including @@ -239,17 +250,23 @@ func isMultiAccountAckable(execErr error) bool { // claimAndExecute claims the root to "running" first (issue #1013), would strand // the root row in "running" until the reaper failed it. func (m *Manager) executeAndFinalize(ctx context.Context, exec *config.PurchaseExecution) error { + // A disconnected caller must not cancel a purchase after the claim. + // Its terminal outcome must survive an exhausted execution budget too. + execCtx, cancelExec := detachedTimeout(ctx, purchaseExecutionTimeout) + defer cancelExec() // Last line of defense before money moves (issue #1718). Every executor // entry point funnels through here, so one check covers all of them. - execErr := m.preExecutionRefusal(ctx, exec) + execErr := m.preExecutionRefusal(execCtx, exec) if execErr == nil { - execErr = m.executePurchase(ctx, exec) + execErr = m.executePurchase(execCtx, exec) } m.finalizeExecution(exec, execErr) if execErr != nil { logging.Errorf("Failed to execute purchase %s: %v", exec.ExecutionID, execErr) } - if err := m.saveOrRecoverTerminal(ctx, exec, execErr); err != nil { + saveCtx, cancelSave := detachedTimeout(ctx, terminalSaveTimeout) + defer cancelSave() + if err := m.saveOrRecoverTerminal(saveCtx, exec, execErr); err != nil { logging.Errorf("AUDIT LOSS: failed to save execution status: %v", err) // Wrap with ErrAuditLoss regardless of whether executePurchase itself // failed. When execErr != nil (provider/partial error), finalizeExecution @@ -266,8 +283,8 @@ func (m *Manager) executeAndFinalize(ctx context.Context, exec *config.PurchaseE } } if execErr == nil { - if err := m.updatePlanProgress(ctx, exec); err != nil { - m.recordRampAdvanceRefusal(ctx, exec, err) + if err := m.updatePlanProgress(saveCtx, exec); err != nil { + m.recordRampAdvanceRefusal(saveCtx, exec, err) } } return execErr diff --git a/internal/purchase/manager_test.go b/internal/purchase/manager_test.go index 0e42fa3e..05971928 100644 --- a/internal/purchase/manager_test.go +++ b/internal/purchase/manager_test.go @@ -213,17 +213,18 @@ func TestManager_ProcessScheduledPurchases_DuePurchase(t *testing.T) { claimedExec.Status = "running" mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) - mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil).Once() + mockStore.On("GetGlobalConfig", mock.Anything).Return(autoPurchaseOnCfg(), nil).Maybe() // executableByScheduler calls GetPurchasePlan first (AutoPurchase gate), then // executePurchase calls it again to build purchase options — two calls total. - mockStore.On("GetPurchasePlan", ctx, "plan-456").Return(plan, nil) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-456").Return(plan, nil) mockStore.On("TransitionExecutionStatus", ctx, "exec-123", []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&claimedExec, nil) - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) - mockStore.On("CompletePlanStep", ctx, "plan-456", 1).Return(nil) - mockSTS.On("GetCallerIdentity", ctx, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockStore.On("CompletePlanStep", mock.Anything, "plan-456", 1).Return(nil) + mockSTS.On("GetCallerIdentity", mock.Anything, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ Account: aws.String("123456789012"), }, nil) @@ -316,15 +317,16 @@ func TestManager_ProcessScheduledPurchases_ExecutionFails(t *testing.T) { claimedExec.Status = "running" mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) - mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil).Once() + mockStore.On("GetGlobalConfig", mock.Anything).Return(autoPurchaseOnCfg(), nil).Maybe() // First call: executableByScheduler gate check (AutoPurchase=true -> proceed). // Second call: executePurchase fetches the plan config and returns error to // simulate a purchase failure — this is what drives the row to "failed". - mockStore.On("GetPurchasePlan", ctx, "plan-456").Return(autoPurchasePlan, nil).Once() - mockStore.On("GetPurchasePlan", ctx, "plan-456").Return(nil, errors.New("plan not found")).Once() + mockStore.On("GetPurchasePlan", mock.Anything, "plan-456").Return(autoPurchasePlan, nil).Once() + mockStore.On("GetPurchasePlan", mock.Anything, "plan-456").Return(nil, errors.New("plan not found")).Once() mockStore.On("TransitionExecutionStatus", ctx, "exec-123", []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&claimedExec, nil) - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) // updatePlanProgress is NOT called when execution fails manager := &Manager{ @@ -622,15 +624,15 @@ func TestManager_RecoverStrandedApprovals_AWSOnlyRedrives(t *testing.T) { // CAS claim: approved -> running. The re-drive proceeds only after winning this. mockStore.On("TransitionExecutionStatus", ctx, "exec-aws-stranded", []string{"approved"}, "running", (*string)(nil)). Return(&runningRow, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-aws-456").Return(plan, nil).Once() - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-aws-456").Return(plan, nil).Once() + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) var saved *config.PurchaseExecution - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")). + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchaseExecution) }). Return(nil) - mockStore.On("CompletePlanStep", ctx, "plan-aws-456", 1).Return(nil) - mockSTS.On("GetCallerIdentity", ctx, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ + mockStore.On("CompletePlanStep", mock.Anything, "plan-aws-456", 1).Return(nil) + mockSTS.On("GetCallerIdentity", mock.Anything, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ Account: aws.String("123456789012"), }, nil) @@ -712,14 +714,14 @@ func TestManager_RecoverStrandedApprovals_AzureReservationRedrives(t *testing.T) // CAS claim: approved -> running before re-drive. mockStore.On("TransitionExecutionStatus", ctx, "exec-azure-res-stranded", []string{"approved"}, "running", (*string)(nil)). Return(&runningRow, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-azure-res").Return(plan, nil).Once() - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-azure-res").Return(plan, nil).Once() + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) var saved *config.PurchaseExecution - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")). + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchaseExecution) }). Return(nil) - mockStore.On("CompletePlanStep", ctx, "plan-azure-res", 1).Return(nil) + mockStore.On("CompletePlanStep", mock.Anything, "plan-azure-res", 1).Return(nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.Anything).Return(mockProvider, nil) mockProvider.On("GetServiceClient", mock.Anything, common.ServiceCompute, "eastus").Return(mockServiceClient, nil) @@ -794,14 +796,14 @@ func TestManager_RecoverStrandedApprovals_GCPRedrives(t *testing.T) { // CAS claim: approved -> running before re-drive. mockStore.On("TransitionExecutionStatus", ctx, "exec-gcp-stranded", []string{"approved"}, "running", (*string)(nil)). Return(&runningRow, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-gcp").Return(plan, nil).Once() - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-gcp").Return(plan, nil).Once() + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) var saved *config.PurchaseExecution - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")). + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchaseExecution) }). Return(nil) - mockStore.On("CompletePlanStep", ctx, "plan-gcp", 1).Return(nil) + mockStore.On("CompletePlanStep", mock.Anything, "plan-gcp", 1).Return(nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "gcp", mock.Anything).Return(mockProvider, nil) mockProvider.On("GetServiceClient", mock.Anything, common.ServiceCompute, "us-central1").Return(mockServiceClient, nil) @@ -1122,14 +1124,14 @@ func TestManager_RecoverStrandedApprovals_AWSRedrive_PersistenceFailurePropagate // CAS claim: approved -> running. This succeeds -- the row is now "running". mockStore.On("TransitionExecutionStatus", ctx, "exec-aws-persist-fail", []string{"approved"}, "running", (*string)(nil)). Return(&runningRow, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-aws-persist-456").Return(plan, nil).Once() - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, "plan-aws-persist-456").Return(plan, nil).Once() + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) // SavePurchaseExecution fails AFTER the CAS-to-running succeeded. - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")). + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). Return(saveErr) - mockSTS.On("GetCallerIdentity", ctx, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ + mockSTS.On("GetCallerIdentity", mock.Anything, mock.AnythingOfType("*sts.GetCallerIdentityInput")).Return(&sts.GetCallerIdentityOutput{ Account: aws.String("123456789012"), }, nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProvider, nil) @@ -1198,9 +1200,9 @@ func TestManager_RecoverStrandedApprovals_AWSRedrive_ExecAndPersistBothFail(t *t mockStore.On("TransitionExecutionStatus", ctx, "exec-aws-both-fail", []string{"approved"}, "running", (*string)(nil)). Return(&runningRow, nil) // executePurchase calls GetPurchasePlan; make it fail so execErr != nil. - mockStore.On("GetPurchasePlan", ctx, "plan-aws-both-fail").Return(nil, planErr).Once() + mockStore.On("GetPurchasePlan", mock.Anything, "plan-aws-both-fail").Return(nil, planErr).Once() // SavePurchaseExecution also fails: the row remains "running" in the DB. - mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")). + mockStore.On("SavePurchaseExecution", mock.Anything, mock.AnythingOfType("*config.PurchaseExecution")). Return(saveErr) manager := &Manager{ diff --git a/internal/purchase/money_path_regression_test.go b/internal/purchase/money_path_regression_test.go index f58f35a1..db392f42 100644 --- a/internal/purchase/money_path_regression_test.go +++ b/internal/purchase/money_path_regression_test.go @@ -315,7 +315,8 @@ func TestSQSRedeliveryDoesNotDoubleExecute(t *testing.T) { // pre-fix nothing CASes it to running before the cloud call, so a redelivery // re-reads a still-claimable row). Each GetExecutionByID returns a FRESH // pending copy, faithfully modeling the real double-delivery scenario. - mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil).Once() + mockStore.On("GetGlobalConfig", mock.Anything).Return(autoPurchaseOnCfg(), nil).Maybe() mockStore.On("GetExecutionByID", ctx, "exec-dup").Return(newPending(), nil).Once() mockStore.On("GetExecutionByID", ctx, "exec-dup").Return(newPending(), nil).Once() @@ -332,9 +333,9 @@ func TestSQSRedeliveryDoesNotDoubleExecute(t *testing.T) { Return(nil, fmt.Errorf("%w: row already running", config.ErrExecutionNotInExpectedStatus)).Once() mockStore.SavePurchaseExecutionFn = func(_ context.Context, _ *config.PurchaseExecution) error { return nil } - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) - mockStore.On("GetPurchasePlan", ctx, mock.Anything).Return(&config.PurchasePlan{Name: "p", AutoPurchase: true, MaxStepCommitment: capOf(1e9)}, nil).Maybe() + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("GetPurchasePlan", mock.Anything, mock.Anything).Return(&config.PurchasePlan{Name: "p", AutoPurchase: true, MaxStepCommitment: capOf(1e9)}, nil).Maybe() mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, mock.Anything).Return(mockServiceClient, nil) @@ -393,13 +394,14 @@ func TestMultiAccountPartialSuccessIsAcked(t *testing.T) { } plan := &config.PurchasePlan{ID: "plan-x", Name: "Plan X", AutoPurchase: true, MaxStepCommitment: capOf(1e9)} - mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil).Once() + mockStore.On("GetGlobalConfig", mock.Anything).Return(autoPurchaseOnCfg(), nil).Maybe() mockStore.On("GetExecutionByID", ctx, "root-partial").Return(exec, nil) running := *exec running.Status = "running" mockStore.On("TransitionExecutionStatus", ctx, "root-partial", []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&running, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-x").Return(plan, nil).Maybe() + mockStore.On("GetPurchasePlan", mock.Anything, "plan-x").Return(plan, nil).Maybe() // GetPlanAccounts is served by the Fn hook, not a testify expectation. mockStore.GetPlanAccountsFn = func(_ context.Context, _ string) ([]config.CloudAccount, error) { return accounts, nil @@ -417,8 +419,8 @@ func TestMultiAccountPartialSuccessIsAcked(t *testing.T) { } return nil } - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, mock.Anything).Return(mockServiceClient, nil) @@ -494,13 +496,14 @@ func TestExecuteForAccount_CommittedButUnsavedIsAcked(t *testing.T) { } plan := &config.PurchasePlan{ID: "plan-y", Name: "Plan Y", AutoPurchase: true, MaxStepCommitment: capOf(1e9)} - mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) + mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil).Once() + mockStore.On("GetGlobalConfig", mock.Anything).Return(autoPurchaseOnCfg(), nil).Maybe() mockStore.On("GetExecutionByID", ctx, "root-unsaved").Return(exec, nil) running := *exec running.Status = "running" mockStore.On("TransitionExecutionStatus", ctx, "root-unsaved", []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&running, nil) - mockStore.On("GetPurchasePlan", ctx, "plan-y").Return(plan, nil).Maybe() + mockStore.On("GetPurchasePlan", mock.Anything, "plan-y").Return(plan, nil).Maybe() mockStore.GetPlanAccountsFn = func(_ context.Context, _ string) ([]config.CloudAccount, error) { return accounts, nil } @@ -521,7 +524,7 @@ func TestExecuteForAccount_CommittedButUnsavedIsAcked(t *testing.T) { } return nil } - mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockStore.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) diff --git a/internal/purchase/plan_step_no_recs_test.go b/internal/purchase/plan_step_no_recs_test.go index 83a0ffdb..d260377a 100644 --- a/internal/purchase/plan_step_no_recs_test.go +++ b/internal/purchase/plan_step_no_recs_test.go @@ -93,7 +93,7 @@ func scheduledBarePlanStepFails(t *testing.T, notificationDaysBefore int) { mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil) mockStore.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) - mockStore.On("GetPurchasePlan", ctx, bareStepPlanID).Return(plan, nil) + mockStore.On("GetPurchasePlan", mock.Anything, bareStepPlanID).Return(plan, nil) mockStore.On("TransitionExecutionStatus", ctx, bareStepExecID, []string{"approved", "pending", "notified"}, "running", (*string)(nil)).Return(&claimed, nil) // A two-account plan: the failure must be recorded once on the root row, diff --git a/internal/purchase/scheduled_fire_test.go b/internal/purchase/scheduled_fire_test.go index 8e2bb717..de936376 100644 --- a/internal/purchase/scheduled_fire_test.go +++ b/internal/purchase/scheduled_fire_test.go @@ -195,23 +195,23 @@ func TestFireScheduledDelayedPurchases_EndToEnd(t *testing.T) { store.On("TransitionExecutionStatus", ctx, "exec-scheduled-fire", []string{"scheduled"}, "running", (*string)(nil)). Return(&approved, nil) - store.On("GetPurchasePlan", ctx, "plan-1").Return(plan, nil) + store.On("GetPurchasePlan", mock.Anything, "plan-1").Return(plan, nil) store.GetPlanAccountsFn = func(_ context.Context, _ string) ([]config.CloudAccount, error) { return nil, nil // no plan-level accounts -> single-account path } - store.On("GetCloudAccount", ctx, acctID).Return(account, nil) - store.On("CompletePlanStep", ctx, "plan-1", 1).Return(nil) + store.On("GetCloudAccount", mock.Anything, acctID).Return(account, nil) + store.On("CompletePlanStep", mock.Anything, "plan-1", 1).Return(nil) var savedFinal *config.PurchaseExecution // The audit stamp and the terminal save are both guarded on running; an // unguarded SavePurchaseExecution would be an unexpected call here. - store.On("SaveExecutionIfStatus", ctx, mock.Anything, []string{"running"}). + store.On("SaveExecutionIfStatus", mock.Anything, mock.Anything, []string{"running"}). Run(func(args mock.Arguments) { c := *args.Get(1).(*config.PurchaseExecution) savedFinal = &c }).Return(nil) - store.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + store.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, mock.Anything).Return(mockServiceClient, nil) @@ -275,12 +275,12 @@ func TestFireScheduledDelayedPurchases_AuditGapAfterCASStillFires(t *testing.T) store.On("TransitionExecutionStatus", ctx, "exec-auditgap-fire", []string{"scheduled"}, "running", (*string)(nil)). Return(&approved, nil) - store.On("GetPurchasePlan", ctx, "plan-1").Return(plan, nil) + store.On("GetPurchasePlan", mock.Anything, "plan-1").Return(plan, nil) store.GetPlanAccountsFn = func(_ context.Context, _ string) ([]config.CloudAccount, error) { return nil, nil } - store.On("GetCloudAccount", ctx, acctID).Return(account, nil) - store.On("CompletePlanStep", ctx, "plan-1", 1).Return(nil) + store.On("GetCloudAccount", mock.Anything, acctID).Return(account, nil) + store.On("CompletePlanStep", mock.Anything, "plan-1", 1).Return(nil) // The approved_by audit-stamp save (the FIRST SavePurchaseExecution call, // right after the CAS) fails; the terminal save after executeAndFinalize @@ -293,8 +293,8 @@ func TestFireScheduledDelayedPurchases_AuditGapAfterCASStillFires(t *testing.T) } return nil } - store.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) - mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + store.On("SavePurchaseHistory", mock.Anything, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.AnythingOfType("email.NotificationData")).Return(nil) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, mock.Anything).Return(mockServiceClient, nil)