feat(plans): settle a failed-with-submission row so a managed plan can be deleted

internal/api/handler_execution_settle.go

@@ -0,0 +1,159 @@
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"strings"
+	"time"
+	"unicode/utf8"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-lambda-go/events"
+)
+
+const (
+	executionSettleReasonMin = 10
+	executionSettleReasonMax = 300
+	executionSettleBodyMax   = 2048
+)
+
+// executionSettleRequest is the body of POST /api/purchases/{id}/settle.
+type executionSettleRequest struct {
+	Reason      string `json:"reason"`
+	Acknowledge bool   `json:"acknowledge_unknown_outcome"`
+}
+
+type executionSettleResponse struct {
+	ExecutionID string `json:"execution_id"`
+	Status      string `json:"status"`
+	SettledAt   string `json:"outcome_settled_at"`
+	SettledBy   string `json:"outcome_settled_by"`
+	Settlement  string `json:"outcome_settlement"`
+	Replayed    bool   `json:"replayed"`
+}
+
+// executionSettleStore is the part of the store the endpoint needs.
+type executionSettleStore interface {
+	SettleFailedExecution(ctx context.Context, req config.ExecutionSettleRequest) (*config.ExecutionSettlement, error)
+}
+
+// settleExecution records an operator's audited settlement of a failed plan
+// execution whose outcome is unknown (issue #999), so it stops blocking the
+// delete of its editor-managed plan. The row stays failed and keeps counting
+// as possibly bought; it can no longer be retried. Gated like a purchase:
+// execute:purchases, account access to the execution, and the grant's account
+// constraints with strict scope.
+func (h *Handler) settleExecution(ctx context.Context, req *events.LambdaFunctionURLRequest, id string) (any, error) {
+	if err := validateUUID(id); err != nil {
+		return nil, err
+	}
+	session, err := h.requirePermission(ctx, req, auth.ActionExecute, auth.ResourcePurchases)
+	if err != nil {
+		return nil, err
+	}
+	body, err := decodeExecutionSettle(req.Body)
+	if err != nil {
+		return nil, err
+	}
+	if authErr := h.authorizeExecutionSettle(ctx, session, id); authErr != nil {
+		return nil, authErr
+	}
+	store, ok := h.config.(executionSettleStore)
+	if !ok {
+		return nil, NewClientError(503, "Execution settle storage is unavailable")
+	}
+	res, err := store.SettleFailedExecution(ctx, config.ExecutionSettleRequest{
+		ExecutionID: id, Actor: session.UserID, Reason: body.Reason, Acknowledge: body.Acknowledge,
+	})
+	if err != nil {
+		return nil, mapExecutionSettleError(err)
+	}
+	if !res.Replayed {
+		logging.Infof("purchase execution settled by operator: execution=%s actor=%q via=%s acknowledged_unknown_outcome=true settlement=%q",
+			id, session.UserID, credentialKind(ctx, session), res.Settlement)
+	}
+	return executionSettleResponse{
+		ExecutionID: res.ExecutionID, Status: res.Status, SettledAt: res.SettledAt.UTC().Format(time.RFC3339),
+		SettledBy: res.SettledBy, Settlement: res.Settlement, Replayed: res.Replayed,
+	}, nil
+}
+
+// authorizeExecutionSettle checks account access to the execution, then the
+// execute:purchases constraints against what the execution would have bought
+// (its recommendations' providers, services, regions and total commitment)
+// and its accounts. StrictScope makes a dimension the execution leaves
+// unknown, such as the provider of a step with no recommendations, require an
+// unconstrained grant.
+func (h *Handler) authorizeExecutionSettle(ctx context.Context, session *Session, id string) error {
+	if err := h.requireExecutionAccess(ctx, session, id); err != nil {
+		return err
+	}
+	execution, err := h.config.GetExecutionByID(ctx, id)
+	if errors.Is(err, config.ErrNotFound) || (err == nil && execution == nil) {
+		return errNotFound
+	}
+	if err != nil {
+		return fmt.Errorf("failed to get execution: %w", err)
+	}
+	accounts, err := h.executionAccounts(ctx, execution, nil)
+	if err != nil {
+		return err
+	}
+	ids := make([]string, 0, len(accounts))
+	for i := range accounts {
+		ids = append(ids, accounts[i].ID)
+	}
+	sets := purchaseConstraintSets(execution.Recommendations)
+	if len(sets) == 0 {
+		sets = []auth.PermissionConstraints{{}}
+	}
+	for i := range sets {
+		sets[i].AccountIDs, sets[i].StrictScope = ids, true
+	}
+	return h.requirePermissionConstraints(ctx, session, auth.ActionExecute, auth.ResourcePurchases, sets)
+}
+
+func mapExecutionSettleError(err error) error {
+	switch {
+	case errors.Is(err, config.ErrExecutionSettleNotFound):
+		return errNotFound
+	case errors.Is(err, config.ErrExecutionSettleInvalid):
+		return NewClientError(400, err.Error())
+	case errors.Is(err, config.ErrExecutionSettleConflict):
+		return NewClientError(409, err.Error())
+	case errors.Is(err, config.ErrExecutionSettleNotReady):
+		return NewClientError(503, err.Error())
+	}
+	return err
+}
+
+func decodeExecutionSettle(raw string) (executionSettleRequest, error) {
+	var body executionSettleRequest
+	if len(raw) > executionSettleBodyMax {
+		return body, NewClientError(400, "Settle body is too large")
+	}
+	decoder := json.NewDecoder(strings.NewReader(raw))
+	decoder.DisallowUnknownFields()
+	if err := decoder.Decode(&body); err != nil {
+		return body, NewClientError(400, "Invalid settle body")
+	}
+	if err := decoder.Decode(new(any)); err != io.EOF {
+		return body, NewClientError(400, "Expected one settle body")
+	}
+	body.Reason = strings.TrimSpace(body.Reason)
+	if !body.Acknowledge {
+		return body, NewClientError(400, "acknowledge_unknown_outcome must be true: this purchase may have been submitted; check the cloud account first")
+	}
+	if n := utf8.RuneCountInString(body.Reason); n < executionSettleReasonMin || n > executionSettleReasonMax {
+		return body, NewClientError(400, fmt.Sprintf("reason must be %d to %d characters", executionSettleReasonMin, executionSettleReasonMax))
+	}
+	if hasControlRune(body.Reason) {
+		return body, NewClientError(400, "reason must not contain control characters")
+	}
+	return body, nil
+}

internal/api/handler_execution_settle_integration_test.go

@@ -0,0 +1,112 @@
+//go:build integration
+
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+const settleBody = `{"reason":"checked the subscription: nothing was bought","acknowledge_unknown_outcome":true}`
+
+var purchaserPerms = []auth.Permission{
+	{Action: auth.ActionAdmin, Resource: auth.ResourceAll},
+	{Action: auth.ActionExecute, Resource: auth.ResourcePurchases},
+}
+
+func (f *createConcurrencyFixture) settledAt(ctx context.Context, t *testing.T, id string) *time.Time {
+	t.Helper()
+	var at *time.Time
+	require.NoError(t, f.pool.QueryRow(ctx, `SELECT outcome_settled_at FROM purchase_executions WHERE execution_id = $1`, id).Scan(&at))
+	return at
+}
+
+// The issue #999 scenario through the HTTP surface: a failed row with an
+// unknown outcome makes the managed delete 409 until a purchaser settles it.
+func TestSettleUnblocksManagedPlanDeleteOverHTTP(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	setPlanManaged(ctx, t, f, true)
+	id := f.saveRow(ctx, t, "failed", false, time.Now(), true)
+	requireLiveExecutionsRefusal(t, f.deletePlan(ctx, f.handler))
+
+	h := limitedHandler(f, purchaserPerms, nil)
+	h.auth.(*MockAuthService).On("ValidateCSRFToken", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+	req := managedGuardRequest(settleBody)
+	req.Headers["X-CSRF-Token"] = "test-csrf"
+	req.RequestContext.HTTP.Method = "POST"
+	req.RequestContext.HTTP.Path = "/api/purchases/" + id + "/settle"
+	resp, err := h.HandleRequest(ctx, req)
+	require.NoError(t, err)
+	require.Equal(t, http.StatusOK, resp.StatusCode, resp.Body)
+	var got executionSettleResponse
+	require.NoError(t, json.Unmarshal([]byte(resp.Body), &got))
+	require.Equal(t, id, got.ExecutionID)
+	require.Equal(t, "failed", got.Status)
+	require.Equal(t, "00000000-0000-0000-0000-000000000001", got.SettledBy)
+	require.Contains(t, got.Settlement, `reason="checked the subscription: nothing was bought"`)
+	require.False(t, got.Replayed)
+
+	require.NoError(t, f.deletePlan(ctx, f.handler))
+	row, err := f.store.GetExecutionByID(ctx, id)
+	require.NoError(t, err)
+	require.Equal(t, "failed", row.Status)
+	require.NotNil(t, f.settledAt(ctx, t, id))
+}
+
+func TestSettleAuthorizationAndValidation(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	setPlanManaged(ctx, t, f, true)
+	id := f.saveRow(ctx, t, "failed", false, time.Now(), true)
+	completed := f.saveRow(ctx, t, "completed", false, time.Now(), true)
+	var acct string
+	require.NoError(t, f.pool.QueryRow(ctx, `SELECT account_id::text FROM plan_accounts WHERE plan_id = $1`, f.planID).Scan(&acct))
+	constrained := func(c auth.PermissionConstraints) []auth.Permission {
+		return []auth.Permission{{Action: auth.ActionExecute, Resource: auth.ResourcePurchases, Constraints: &c}}
+	}
+	cases := []struct {
+		name   string
+		perms  []auth.Permission
+		scoped []string
+		id     string
+		body   string
+		status int
+	}{
+		{"admin without execute", []auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}, nil, id, settleBody, http.StatusForbidden},
+		{"out of account scope", purchaserPerms, []string{"other-account"}, id, settleBody, http.StatusNotFound},
+		{"execute constrained to another account", constrained(auth.PermissionConstraints{AccountIDs: []string{"other-account"}}), nil, id, settleBody, http.StatusForbidden},
+		{"execute constrained to another provider", constrained(auth.PermissionConstraints{AccountIDs: []string{acct}, Providers: []string{"aws"}}), nil, id, settleBody, http.StatusForbidden},
+		{"no acknowledgement", purchaserPerms, nil, id, `{"reason":"checked the subscription"}`, http.StatusBadRequest},
+		{"short reason", purchaserPerms, nil, id, `{"reason":"ok","acknowledge_unknown_outcome":true}`, http.StatusBadRequest},
+		{"unknown field", purchaserPerms, nil, id, `{"reason":"checked the subscription","acknowledge_unknown_outcome":true,"force":true}`, http.StatusBadRequest},
+		{"completed row", purchaserPerms, nil, completed, settleBody, http.StatusConflict},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			h := limitedHandler(f, tc.perms, tc.scoped)
+			_, err := h.settleExecution(ctx, managedGuardRequest(tc.body), tc.id)
+			var ce *clientError
+			if errors.As(err, &ce) {
+				require.Equal(t, tc.status, ce.code, ce.message)
+			} else {
+				require.True(t, IsNotFoundError(err) && tc.status == http.StatusNotFound, "got %v", err)
+			}
+			require.Nil(t, f.settledAt(ctx, t, tc.id), "a refused settle writes nothing")
+		})
+	}
+
+	t.Run("constrained to the plan's account", func(t *testing.T) {
+		h := limitedHandler(f, constrained(auth.PermissionConstraints{AccountIDs: []string{acct}}), nil)
+		_, err := h.settleExecution(ctx, managedGuardRequest(settleBody), id)
+		require.NoError(t, err)
+	})
+}

internal/api/handler_execution_settle_test.go

@@ -0,0 +1,45 @@
+package api
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDecodeExecutionSettle(t *testing.T) {
+	ok, err := decodeExecutionSettle(`{"reason":"  checked the account  ","acknowledge_unknown_outcome":true}`)
+	require.NoError(t, err)
+	require.Equal(t, "checked the account", ok.Reason)
+
+	for name, body := range map[string]string{
+		"empty":         ``,
+		"no ack":        `{"reason":"checked the account"}`,
+		"ack false":     `{"reason":"checked the account","acknowledge_unknown_outcome":false}`,
+		"short":         `{"reason":"  short  ","acknowledge_unknown_outcome":true}`,
+		"long":          `{"reason":"` + strings.Repeat("x", executionSettleReasonMax+1) + `","acknowledge_unknown_outcome":true}`,
+		"control":       `{"reason":"checked the\naccount","acknowledge_unknown_outcome":true}`,
+		"unknown field": `{"reason":"checked the account","acknowledge_unknown_outcome":true,"x":1}`,
+		"two bodies":    `{"reason":"checked the account","acknowledge_unknown_outcome":true}{}`,
+		"too large":     strings.Repeat(" ", executionSettleBodyMax+1),
+	} {
+		_, err := decodeExecutionSettle(body)
+		var ce *clientError
+		require.True(t, errors.As(err, &ce), name)
+		require.Equal(t, 400, ce.code, name)
+	}
+}
+
+func TestMapExecutionSettleError(t *testing.T) {
+	require.True(t, IsNotFoundError(mapExecutionSettleError(config.ErrExecutionSettleNotFound)))
+	for sentinel, code := range map[error]int{config.ErrExecutionSettleInvalid: 400, config.ErrExecutionSettleConflict: 409} {
+		var ce *clientError
+		require.True(t, errors.As(mapExecutionSettleError(fmt.Errorf("%w: detail", sentinel)), &ce))
+		require.Equal(t, code, ce.code)
+	}
+	other := errors.New("db down")
+	require.Equal(t, other, mapExecutionSettleError(other), "an internal error is not a client error")
+}

internal/api/handler_purchases.go

@@ -2848,7 +2848,7 @@ func (h *Handler) persistRetrySuccessorTx(ctx context.Context, failedExec, newEx
 			map[string]any{})
 	case errors.Is(err, errRetryLinkRaceLost):
 		return NewClientErrorWithDetails(409,
-			fmt.Sprintf("execution %s was already retried; act on its descendant instead", failedExec.ExecutionID),
+			fmt.Sprintf("execution %s was already retried or settled; reload it", failedExec.ExecutionID),
 			map[string]any{})
 	case errors.As(err, &dup):
 		return NewClientErrorWithDetails(409,