Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions internal/config/store_postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -880,13 +880,40 @@ func (s *PostgresStore) DeletePurchasePlan(ctx context.Context, planID string) e
return err
}
}
if err := cancelOpenPlanExecutionsTx(ctx, tx, planID); err != nil {
return err
}
if _, err := tx.Exec(ctx, `DELETE FROM purchase_plans WHERE id = $1`, planID); err != nil {
return fmt.Errorf("failed to delete purchase plan: %w", err)
}
return nil
})
}

// cancelOpenPlanExecutionsTx cancels the plan's step rows that can still be
// approved or run (pending, notified, paused) in the delete's transaction.
// purchase_executions.plan_id is ON DELETE SET NULL (migration 000033), and
// token approval applies the plan's max_step_commitment and the approve-own
// plan-step rule only while plan_id is set (approveConstraintsForSession), so
// a row left open would become approvable past its plan's cap (issue #862).
// The rows' suppressions go with them, as on the other cancel paths.
func cancelOpenPlanExecutionsTx(ctx context.Context, tx pgx.Tx, planID string) error {
const q = `
WITH canceled AS (
UPDATE purchase_executions
SET status = $2, updated_at = NOW()
WHERE plan_id = $1
AND status IN ('pending', 'notified', 'paused')
RETURNING execution_id
)
DELETE FROM purchase_suppressions
WHERE execution_id IN (SELECT execution_id FROM canceled)`
if _, err := tx.Exec(ctx, q, planID, StatusCanceled); err != nil {
return fmt.Errorf("failed to cancel open executions of purchase plan: %w", err)
}
return nil
}

// buildListPlansQuery returns the SQL query and args for ListPurchasePlans.
//
// When accountIDs is empty the query returns all plans without an unassigned
Expand Down
23 changes: 22 additions & 1 deletion internal/config/store_postgres_pgxmock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,10 @@ func TestPGXMock_DeletePurchasePlan_Success(t *testing.T) {
ctx := context.Background()

expectLockedUnmanagedPlan(mock)
mock.ExpectExec("DELETE").WithArgs(pgxmock.AnyArg()).
mock.ExpectExec(`(?s)UPDATE purchase_executions.*WHERE plan_id = \$1.*'pending', 'notified', 'paused'.*DELETE FROM purchase_suppressions`).
WithArgs("plan-id", StatusCanceled).
WillReturnResult(pgxmock.NewResult("DELETE", 0))
mock.ExpectExec("DELETE FROM purchase_plans").WithArgs("plan-id").
WillReturnResult(pgxmock.NewResult("DELETE", 1))
mock.ExpectCommit()

Expand All @@ -658,6 +661,24 @@ func TestPGXMock_DeletePurchasePlan_Success(t *testing.T) {
assert.NoError(t, mock.ExpectationsWereMet())
}

// TestPGXMock_DeletePurchasePlan_CancelFailureKeepsPlan pins issue #862's
// ordering: the plan row is deleted only after its open step rows were
// canceled in the same transaction, so a failed cancel leaves both in place.
func TestPGXMock_DeletePurchasePlan_CancelFailureKeepsPlan(t *testing.T) {
mock := newMock(t)
store := storeWith(mock)
ctx := context.Background()

expectLockedUnmanagedPlan(mock)
mock.ExpectExec("UPDATE purchase_executions").WithArgs("plan-id", StatusCanceled).
WillReturnError(errors.New("db down"))
mock.ExpectRollback()

err := store.DeletePurchasePlan(ctx, "plan-id")
require.ErrorContains(t, err, "cancel open executions")
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestPGXMock_DeletePurchasePlan_NotFound(t *testing.T) {
mock := newMock(t)
store := storeWith(mock)
Expand Down
76 changes: 76 additions & 0 deletions internal/config/store_postgres_plan_delete_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//go:build integration
// +build integration

package config

import (
"context"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/require"
)

// TestDeletePurchasePlan_CancelsOpenStepRows covers issue #862: plan_id is ON
// DELETE SET NULL, and token approval applies the plan's step cap only while
// plan_id is set, so a step row still open when its plan is deleted must not
// survive as an approvable, plan-less row.
func TestDeletePurchasePlan_CancelsOpenStepRows(t *testing.T) {
ctx := context.Background()
f := newRampFixture(ctx, t, 1, 0, 4)
acct := &f.accounts[0]

open := map[string]*PurchaseExecution{}
for _, status := range []string{"pending", "notified", "paused"} {
open[status] = f.row(ctx, t, 1, acct, status)
}
settled := map[string]*PurchaseExecution{}
for _, status := range []string{"scheduled", "approved", StatusCompleted, StatusFailed} {
settled[status] = f.row(ctx, t, 1, acct, status)
}
suppress := func(execID string) {
require.NoError(t, f.store.CreateSuppression(ctx, &PurchaseSuppression{
ExecutionID: execID, AccountID: *acct, Provider: "aws", Service: "ec2",
Region: "us-east-1", ResourceType: "m5.large", SuppressedCount: 1,
ExpiresAt: time.Now().Add(24 * time.Hour),
}))
}
suppress(open["pending"].ExecutionID)
suppress(settled["scheduled"].ExecutionID)

other := &PurchasePlan{Name: "other " + uuid.NewString()[:8], Services: map[string]ServiceConfig{}}
require.NoError(t, f.store.CreatePurchasePlan(ctx, other))
sibling := &PurchaseExecution{
ExecutionID: uuid.NewString(), IdempotencyKey: uuid.NewString(), PlanID: other.ID,
Status: "pending", StepNumber: 1, ScheduledDate: time.Now(),
}
require.NoError(t, f.store.SavePurchaseExecution(ctx, sibling))

require.NoError(t, f.store.DeletePurchasePlan(ctx, f.planID))

for status, exec := range open {
got, err := f.store.GetExecutionByID(ctx, exec.ExecutionID)
require.NoError(t, err)
require.Equal(t, StatusCanceled, got.Status, "%s row of the deleted plan stays approvable", status)
require.Empty(t, got.PlanID, "%s row is detached from the deleted plan", status)
_, err = f.store.TransitionExecutionStatus(ctx, exec.ExecutionID, []string{"pending", "notified", "paused"}, "running", nil)
require.ErrorIs(t, err, ErrExecutionNotInExpectedStatus, "%s row can still be claimed for approval", status)
}
for status, exec := range settled {
got, err := f.store.GetExecutionByID(ctx, exec.ExecutionID)
require.NoError(t, err)
require.Equal(t, status, got.Status, "%s row is not an open step row and keeps its status", status)
}
got, err := f.store.GetExecutionByID(ctx, sibling.ExecutionID)
require.NoError(t, err)
require.Equal(t, "pending", got.Status, "another plan's row is untouched")

var pendingSup, scheduledSup int
require.NoError(t, f.store.db.QueryRow(ctx,
`SELECT COUNT(*) FROM purchase_suppressions WHERE execution_id = $1`, open["pending"].ExecutionID).Scan(&pendingSup))
require.NoError(t, f.store.db.QueryRow(ctx,
`SELECT COUNT(*) FROM purchase_suppressions WHERE execution_id = $1`, settled["scheduled"].ExecutionID).Scan(&scheduledSup))
require.Zero(t, pendingSup, "the canceled row's suppression is released")
require.Equal(t, 1, scheduledSup, "a row that was not canceled keeps its suppression")
}
Loading