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
10 changes: 9 additions & 1 deletion cmd/wfctl/iac_typed_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1274,8 +1274,16 @@ func mapPBActionStatusToInterface(s pb.ActionStatus) (interfaces.ActionStatus, b
return interfaces.ActionStatusError, true
case pb.ActionStatus_ACTION_STATUS_DELETE_FAILED:
return interfaces.ActionStatusDeleteFailed, true
// Phase 2.3 (workflow#698) — engine populates COMPENSATION_FAILED + SKIPPED
// server-side; plugins may emit COMPENSATED if they implement own compensation.
case pb.ActionStatus_ACTION_STATUS_COMPENSATED:
return interfaces.ActionStatusCompensated, true
case pb.ActionStatus_ACTION_STATUS_COMPENSATION_FAILED:
return interfaces.ActionStatusCompensationFailed, true
case pb.ActionStatus_ACTION_STATUS_SKIPPED:
return interfaces.ActionStatusSkipped, true
default:
// UNSPECIFIED (tag 0) and any unknown wire value (tags 4+)
// UNSPECIFIED (tag 0) and any unknown wire value (tags 7+)
// fall here. Caller surfaces the !ok as an explicit error.
return interfaces.ActionStatusUnspecified, false
}
Expand Down
11 changes: 8 additions & 3 deletions cmd/wfctl/iac_typed_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -602,9 +602,10 @@ func TestApplyResultFromPB_RejectsUnknownStatus(t *testing.T) {
}
}

// TestMapPBActionStatusToInterface_ActionableValues pins the three
// actionable tags SUCCESS / ERROR / DELETE_FAILED to their
// interfaces.ActionStatus mirrors with ok=true.
// TestMapPBActionStatusToInterface_ActionableValues pins the six
// actionable tags (Phase 2: SUCCESS/ERROR/DELETE_FAILED + Phase 2.3 workflow#698:
// COMPENSATED/COMPENSATION_FAILED/SKIPPED) to their interfaces.ActionStatus
// mirrors with ok=true.
func TestMapPBActionStatusToInterface_ActionableValues(t *testing.T) {
cases := []struct {
name string
Expand All @@ -614,6 +615,10 @@ func TestMapPBActionStatusToInterface_ActionableValues(t *testing.T) {
{"SUCCESS", pb.ActionStatus_ACTION_STATUS_SUCCESS, interfaces.ActionStatusSuccess},
{"ERROR", pb.ActionStatus_ACTION_STATUS_ERROR, interfaces.ActionStatusError},
{"DELETE_FAILED", pb.ActionStatus_ACTION_STATUS_DELETE_FAILED, interfaces.ActionStatusDeleteFailed},
// Phase 2.3 (workflow#698) — new actionable enum values:
{"COMPENSATED", pb.ActionStatus_ACTION_STATUS_COMPENSATED, interfaces.ActionStatusCompensated},
{"COMPENSATION_FAILED", pb.ActionStatus_ACTION_STATUS_COMPENSATION_FAILED, interfaces.ActionStatusCompensationFailed},
{"SKIPPED", pb.ActionStatus_ACTION_STATUS_SKIPPED, interfaces.ActionStatusSkipped},
}
for _, c := range cases {
got, ok := mapPBActionStatusToInterface(c.in)
Expand Down
89 changes: 72 additions & 17 deletions iac/wfctlhelpers/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,20 +297,28 @@ func applyPlanWithEnvProviderAndHooks(
// pattern below records the outcome unconditionally on every exit
// from the inner func; the surrounding for loop then bubbles
// fatalErr (hook failures, ctx cancellation) to the caller.
var iterErr error // best-effort error: action failed, continue
var fatalErr error // hook/ctx error: stop the whole apply
var iterErr error // best-effort error: action failed, continue
var iterStatus interfaces.ActionStatus // Phase 2.3 (workflow#698): assigned at each error site
var fatalErr error // hook/ctx error: stop the whole apply

func() {
defer func() {
status := mapDispatchErrToStatus(iterErr, action.Action)
// Phase 2.3 (workflow#698): success-path default — if no
// error site assigned a phase-specific status, the action
// completed cleanly. Otherwise iterStatus was set by the
// path that raised iterErr to the correct phase-specific
// value (SKIPPED / Error / DeleteFailed / CompensationFailed).
if iterErr == nil {
iterStatus = interfaces.ActionStatusSuccess
}
errStr := ""
if iterErr != nil {
errStr = iterErr.Error()
}
result.Actions = append(result.Actions, interfaces.ActionOutcome{
//nolint:gosec // ActionIndex is loop counter bound by len(plan.Actions); G115 false positive.
ActionIndex: uint32(i),
Status: status,
Status: iterStatus,
Error: errStr,
})
}()
Expand All @@ -320,9 +328,11 @@ func applyPlanWithEnvProviderAndHooks(
// guarantees apply stops between actions even if a driver
// happens to ignore ctx. The deferred postcondition still runs
// on early return so InputDriftReport is populated even on a
// canceled apply.
// canceled apply. Phase 2.3 (#698): ctx-cancel is pre-dispatch
// — action was never attempted; cloud-side state unchanged.
if err := ctx.Err(); err != nil {
iterErr = err
iterStatus = statusForPreDispatchSkip()
fatalErr = err
return
}
Expand All @@ -335,7 +345,8 @@ func applyPlanWithEnvProviderAndHooks(
// must not reach the driver. The loop continues to the next
// action (best-effort apply contract). os.LookupEnv is the
// production env source; nil-safe inside ResolveSpec — refs that
// only need replaceIDMap / syncedOutputs still resolve.
// only need replaceIDMap / syncedOutputs still resolve. Phase 2.3
// (#698): JIT-fail is pre-dispatch — no driver call yet.
resolved, err := jitsubst.ResolveSpec(action.Resource, result.ReplaceIDMap, syncedOutputs, os.LookupEnv)
if err != nil {
result.Errors = append(result.Errors, interfaces.ActionError{
Expand All @@ -344,9 +355,12 @@ func applyPlanWithEnvProviderAndHooks(
Error: fmt.Sprintf("jit substitution: %v", err),
})
iterErr = fmt.Errorf("jit substitution: %v", err)
iterStatus = statusForPreDispatchSkip()
return
}
action.Resource = resolved
// Phase 2.3 (#698): driver-resolve-fail is pre-dispatch — no
// driver method has been called yet.
d, err := p.ResourceDriver(action.Resource.Type)
if err != nil {
result.Errors = append(result.Errors, interfaces.ActionError{
Expand All @@ -355,6 +369,7 @@ func applyPlanWithEnvProviderAndHooks(
Error: fmt.Sprintf("resolve driver: %v", err),
})
iterErr = fmt.Errorf("resolve driver: %v", err)
iterStatus = statusForPreDispatchSkip()
return
}
// Capture result.Resources length pre-dispatch so we can identify
Expand All @@ -375,22 +390,35 @@ func applyPlanWithEnvProviderAndHooks(
if err := dispatchAction(ctx, d, action, result, actionHooks, deleteHookActive); err != nil {
var hookErr hookDispatchError
if errors.As(err, &hookErr) {
// Phase 2.3 (#698): hookDispatchError wraps a hook
// (typically driver-layer Delete hook) that ran AFTER
// the cloud-side action — cloud-side work IS done;
// hook failure is post-hook semantically.
fatalErr = fmt.Errorf("%s/%s: %w", action.Resource.Type, action.Resource.Name, hookErr.err)
iterErr = hookErr.err
iterStatus = statusForPostHookFailure()
return
}
// Phase 2.3 (#698): generic dispatch error — driver's
// Create/Update/Delete RPC returned err. Action attempted;
// cloud-side state may be partially mutated.
result.Errors = append(result.Errors, interfaces.ActionError{
Resource: action.Resource.Name,
Action: action.Action,
Error: err.Error(),
})
iterErr = err
iterStatus = statusForDispatchError(action.Action)
return
}
if action.Action == "delete" {
// Phase 2.3 (#698): post-delete-hook ran AFTER cloud-side
// delete succeeded — cloud-side work IS done; hook failure
// is post-hook semantically.
if err := actionHooks.OnResourceDeleted(ctx, action); err != nil {
fatalErr = fmt.Errorf("%s/%s: post-delete hook: %w", action.Resource.Type, action.Resource.Name, err)
iterErr = err
iterStatus = statusForPostHookFailure()
return
}
}
Expand All @@ -399,9 +427,13 @@ func applyPlanWithEnvProviderAndHooks(
out = fillMissingOutputIdentity(action.Resource, out)
result.Resources[len(result.Resources)-1] = out
if hooks.OnResourceApplied != nil {
// Phase 2.3 (#698): post-apply-hook ran AFTER cloud-side
// create/update succeeded — cloud-side work IS done;
// hook failure is post-hook semantically.
if err := hooks.OnResourceApplied(ctx, d, action, out); err != nil {
fatalErr = fmt.Errorf("%s/%s: post-apply hook: %w", action.Resource.Type, action.Resource.Name, err)
iterErr = err
iterStatus = statusForPostHookFailure()
return
}
}
Expand Down Expand Up @@ -429,23 +461,46 @@ func applyPlanWithEnvProviderAndHooks(
return result, nil
}

// mapDispatchErrToStatus translates a per-action dispatch error into the
// Phase 2 ActionStatus value (workflow#640). delete actions that fail map
// to ActionStatusDeleteFailed so wfctl downstream can drop the
// state-removal optimistic-update; all other failures map to
// ActionStatusError. Compensation paths (Phase 2.3 — COMPENSATED /
// COMPENSATION_FAILED) are reserved in iac.proto but not yet emitted
// here. nil err means the dispatch (and any post-hooks) succeeded.
func mapDispatchErrToStatus(err error, actionType string) interfaces.ActionStatus {
if err == nil {
return interfaces.ActionStatusSuccess
}
// Phase 2.3 (workflow#698): replaced single mapDispatchErrToStatus with
// 3 phase-specific helpers — each per-action exit path now assigns its
// status directly from the call site (clearer than late-mapping in defer).
// The single-helper version conflated pre-dispatch failures (ctx-cancel /
// JIT-fail / driver-resolve-fail) and post-hook failures with
// dispatch-level errors, breaking ADR 0040 invariant 2 (failed-delete
// preservation requires distinguishing "delete dispatch failed" from
// "delete never attempted").

// statusForPreDispatchSkip returns SKIPPED — action was never attempted
// at the driver. Used for ctx-cancel, JIT-substitution-fail, and
// driver-resolve-fail paths. Cloud-side state unchanged from pre-apply.
// Per workflow#698 Phase 2.3.
func statusForPreDispatchSkip() interfaces.ActionStatus {
return interfaces.ActionStatusSkipped
}

// statusForDispatchError returns Error (non-delete) or DeleteFailed (delete).
// Used when driver's Create/Update/Delete RPC returned an error. Action was
// attempted; cloud-side state may be partially mutated. For delete actions,
// DELETE_FAILED instructs wfctl to preserve state.
// Per workflow#698 Phase 2.3 (extracted from prior mapDispatchErrToStatus).
func statusForDispatchError(actionType string) interfaces.ActionStatus {
if actionType == "delete" {
return interfaces.ActionStatusDeleteFailed
}
return interfaces.ActionStatusError
}

// statusForPostHookFailure returns COMPENSATION_FAILED — driver succeeded
// but wfctl-side hook (OnResourceApplied / OnResourceDeleted) failed (or
// the driver's own hook layer returned hookDispatchError after touching
// cloud-side state). Cloud-side work IS done; operator must verify state;
// may need manual compensation. State preservation required regardless of
// action type.
// Per workflow#698 Phase 2.3.
func statusForPostHookFailure() interfaces.ActionStatus {
return interfaces.ActionStatusCompensationFailed
}

func preflightProviderOwnedReplaceWithDeleteHooks(p interfaces.IaCProvider, plan *interfaces.IaCPlan) error {
for i := range plan.Actions {
action := plan.Actions[i]
Expand Down
28 changes: 26 additions & 2 deletions iac/wfctlhelpers/apply_hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,12 @@ func TestApplyPlanWithHooks_PopulatesActions_PreDispatchDriverError(t *testing.T
if len(result.Actions) != 1 {
t.Fatalf("expected 1 ActionOutcome (length-assert invariant), got %d: %+v", len(result.Actions), result.Actions)
}
if result.Actions[0].Status != interfaces.ActionStatusError {
t.Errorf("driver-resolve-error status: want Error, got %v", result.Actions[0].Status)
// Phase 2.3 (workflow#698) reclassification: driver-resolve-error is
// PRE-DISPATCH — driver's Create/Update/Delete RPC never called; cloud
// state unchanged. Status maps to SKIPPED. Phase 2 mapped this to Error
// which conflated pre-dispatch skip with dispatch-level failure.
if result.Actions[0].Status != interfaces.ActionStatusSkipped {
t.Errorf("driver-resolve-error status: want Skipped (Phase 2.3 pre-dispatch reclassification), got %v", result.Actions[0].Status)
}
if result.Actions[0].ActionIndex != 0 {
t.Errorf("ActionIndex: want 0, got %d", result.Actions[0].ActionIndex)
Expand Down Expand Up @@ -511,3 +515,23 @@ func TestApplyPlanWithHooks_OnPlanComplete_RecoversFromPanic(t *testing.T) {
t.Errorf("expected last result.Errors entry to have Resource=\"<plan-finalize>\"; got: %+v", result.Errors)
}
}

// TestStatusHelpers_Phase23 pins the 3 phase-specific helper functions
// per workflow#698 Phase 2.3 to their plan-spec'd return values.
func TestStatusHelpers_Phase23(t *testing.T) {
if got := statusForPreDispatchSkip(); got != interfaces.ActionStatusSkipped {
t.Errorf("statusForPreDispatchSkip() = %v; want ActionStatusSkipped", got)
}
if got := statusForDispatchError("create"); got != interfaces.ActionStatusError {
t.Errorf("statusForDispatchError(\"create\") = %v; want ActionStatusError", got)
}
if got := statusForDispatchError("update"); got != interfaces.ActionStatusError {
t.Errorf("statusForDispatchError(\"update\") = %v; want ActionStatusError", got)
}
if got := statusForDispatchError("delete"); got != interfaces.ActionStatusDeleteFailed {
t.Errorf("statusForDispatchError(\"delete\") = %v; want ActionStatusDeleteFailed", got)
}
if got := statusForPostHookFailure(); got != interfaces.ActionStatusCompensationFailed {
t.Errorf("statusForPostHookFailure() = %v; want ActionStatusCompensationFailed", got)
}
}
16 changes: 12 additions & 4 deletions interfaces/iac_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,14 @@ type DriftEntry struct {

// ActionStatus categorizes per-action outcomes for wfctl-side hook dispatch.
// Mirrors pb.ActionStatus (plugin/external/proto/iac.proto) for type-safe Go
// boundary use; constant tags 0/1/2/3 MUST stay in lockstep with the proto.
// Per workflow#640 Phase 2 + ADR 0040 invariants 1-2. Tags 4-5 are reserved
// in the proto for Phase 2.3 compensation (ActionStatusCompensated +
// ActionStatusCompensationFailed) and intentionally not declared here yet.
// boundary use; constant tags 0-6 MUST stay in lockstep with the proto.
// Per workflow#640 Phase 2 + workflow#698 Phase 2.3 + ADR 0040 invariants 1-2.
//
// Phase 2.3 reclassification: the engine now distinguishes pre-dispatch
// (Skipped), dispatch (Error/DeleteFailed), and post-hook (CompensationFailed)
// failures. Consumers that previously checked ActionStatusError alone for
// "action did not produce desired end-state" should now check
// {Error, DeleteFailed, CompensationFailed, Skipped}.
type ActionStatus uint8

const (
Expand All @@ -157,6 +161,10 @@ const (
ActionStatusSuccess
ActionStatusError
ActionStatusDeleteFailed
// Phase 2.3 (workflow#698):
ActionStatusCompensated // reserved-for-future-emission; engine v0.56.0 does NOT emit (no auto-compensation); plugins may emit if they implement own compensation
ActionStatusCompensationFailed // post-driver-success hook failed; cloud-side work IS done; operator must verify state; manual compensation may be required
ActionStatusSkipped // action never attempted at driver (ctx-cancel pre-dispatch, JIT-substitution-fail, driver-resolve-fail); cloud-side state unchanged
)

// ActionOutcome mirrors pb.ActionResult. Engine populates one entry per
Expand Down
29 changes: 29 additions & 0 deletions interfaces/iac_state_phase23_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package interfaces

import "testing"

// TestActionStatus_Phase23_ValuesMatchProtoTags asserts ActionStatus iota
// values match plugin/external/proto/iac.proto ActionStatus enum tag numbers.
// Per workflow#698 Phase 2.3.
func TestActionStatus_Phase23_ValuesMatchProtoTags(t *testing.T) {
cases := []struct {
name string
status ActionStatus
wantInt int
}{
{"Unspecified=0", ActionStatusUnspecified, 0},
{"Success=1", ActionStatusSuccess, 1},
{"Error=2", ActionStatusError, 2},
{"DeleteFailed=3", ActionStatusDeleteFailed, 3},
{"Compensated=4", ActionStatusCompensated, 4},
{"CompensationFailed=5", ActionStatusCompensationFailed, 5},
{"Skipped=6", ActionStatusSkipped, 6},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
if int(c.status) != c.wantInt {
t.Errorf("%s: int(%d) != want %d", c.name, c.status, c.wantInt)
}
})
}
}
27 changes: 21 additions & 6 deletions plugin/external/proto/iac.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion plugin/external/proto/iac.proto
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,12 @@ enum ActionStatus {
ACTION_STATUS_SUCCESS = 1;
ACTION_STATUS_ERROR = 2;
ACTION_STATUS_DELETE_FAILED = 3;
reserved 4, 5; // Phase 2.3 compensation (ACTION_STATUS_COMPENSATED / COMPENSATION_FAILED)
// Phase 2.3 (workflow#698): reserved tags 4+5 from Phase 2 now defined +
// tag 6 for "skipped" — replaces misclassification of pre-dispatch
// ctx-cancel/JIT-fail/driver-resolve-fail as Error/DeleteFailed.
ACTION_STATUS_COMPENSATED = 4; // reserved-for-future-emission; engine v0.56.0 does NOT emit (no auto-compensation); plugins may emit if they implement own compensation
ACTION_STATUS_COMPENSATION_FAILED = 5; // post-driver-success hook failed; cloud-side work IS done; operator must verify state; manual compensation may be required
ACTION_STATUS_SKIPPED = 6; // action never attempted at driver (ctx-cancel pre-dispatch, JIT-substitution-fail, driver-resolve-fail); cloud-side state unchanged
}

// ActionResult is the per-action outcome surfacing for Phase 2 v2 hooks.
Expand Down
Loading