Skip to content
Open
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
24 changes: 22 additions & 2 deletions internal/purchase/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,12 @@ func (m *Manager) executeForAccount(ctx context.Context, baseExec *config.Purcha
if err != nil {
acctExec.Status = "failed"
acctExec.Error = err.Error()
_ = m.config.SavePurchaseExecution(ctx, &acctExec)
return false, fmt.Errorf("credential resolution failed for account %s: %w", account.ID, err)
credErr := fmt.Errorf("credential resolution failed for account %s: %w", account.ID, err)
// Audit-trail save: the failed row is as much a part of the audit
// trail as a success row, so a save failure must surface (AUDIT LOSS)
// instead of being silently dropped. Extracted to a helper to keep
// executeForAccount under the gocyclo:10 budget.
return false, m.persistFailedAccountExecution(ctx, &acctExec, account, credErr)
}

accountID := account.ExternalID
Expand Down Expand Up @@ -288,6 +292,22 @@ func (m *Manager) executeForAccount(ctx context.Context, baseExec *config.Purcha
return committed, nil
}

// persistFailedAccountExecution writes a per-account execution row that the
// caller has already stamped with Status="failed" + Error, and joins any save
// failure with the originating cause via errors.Join so neither error is
// silently dropped (the row is as much audit trail as a success row; issue
// #1184 / COR-10). Extracted from executeForAccount so the new branch keeps
// that function under the gocyclo:10 budget.
func (m *Manager) persistFailedAccountExecution(ctx context.Context, acctExec *config.PurchaseExecution, account config.CloudAccount, cause error) error {
if saveErr := m.config.SavePurchaseExecution(ctx, acctExec); saveErr != nil {
return errors.Join(
fmt.Errorf("AUDIT LOSS: failed to save execution record for account %s: %w", account.ID, saveErr),
cause,
)
}
return cause
}

// resolveSingleAccountProvider derives per-account credentials for the
// single-account execution path. Returns (nil, "", nil) when no account can be
// identified (ambient credentials are used in that case), or when no recs are
Expand Down
65 changes: 65 additions & 0 deletions internal/purchase/execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,71 @@ func TestExecuteForAccount_CredentialFailure_MarksFailed(t *testing.T) {
}
}

// TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced is the regression
// guard for COR-10 (#1184): when credential resolution fails AND persisting
// the per-account "failed" row also fails, the save failure must surface as an
// AUDIT LOSS error alongside the credential error instead of being silently
// discarded. The failed row is as much audit trail as a success row.
func TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockEmail := new(MockEmailSender)
mockFactory := new(MockProviderFactory)

// No credentials stored — credential resolution fails for the account.
credStore := &MockCredentialStore{
LoadRawFn: func(_ context.Context, _, _ string) ([]byte, error) {
return nil, nil
},
}

// Persisting the per-account "failed" row also fails.
saveErr := errors.New("db write failed")
mockStore.SavePurchaseExecutionFn = func(_ context.Context, _ *config.PurchaseExecution) error {
return saveErr
}

account := config.CloudAccount{
ID: "aaaaaaaa-0000-0000-0000-000000000001", Name: "aws-failing",
Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys",
}
plan := &config.PurchasePlan{
ID: "plan-credfail-save",
Name: "Credential Failure Save Error Plan",
RampSchedule: config.RampSchedule{CurrentStep: 0, TotalSteps: 1},
}
baseExec := &config.PurchaseExecution{
ExecutionID: "exec-credfail-save",
PlanID: "plan-credfail-save",
Status: "pending",
Recommendations: []config.RecommendationRecord{
{
Provider: "aws", Service: "ec2", ResourceType: "m5.large",
Region: "us-east-1", Count: 1, Selected: true,
},
},
}

manager := &Manager{
config: mockStore,
email: mockEmail,
providerFactory: mockFactory,
credStore: credStore,
}

committed, err := manager.executeForAccount(ctx, baseExec, plan, account)
assert.False(t, committed)
require.Error(t, err)
assert.Contains(t, err.Error(), "AUDIT LOSS: failed to save execution record for account "+account.ID)
assert.ErrorIs(t, err, saveErr, "the underlying save error must stay inspectable via errors.Is")
assert.Contains(t, err.Error(), "credential resolution failed for account "+account.ID,
"the original credential failure must not be masked by the save failure")

// No provider may ever be constructed when credentials fail to resolve.
mockFactory.AssertNotCalled(t, "CreateAndValidateProvider")
mockStore.AssertExpectations(t)
}

// TestExecuteMultiAccount_PartialFailure_IsolatesAccounts is the regression
// guard for spec E-2: when one account (account-I) has invalid credentials and
// another (account-V) has valid credentials, account-V's execution must
Expand Down
Loading