proposal: WorkflowAction CRD for controller-performed workflow actions

# proposal: WorkflowAction CRD for controller-performed workflow actions

## Summary

Introduce a new namespaced CRD, `WorkflowAction`, as the transport for imperative
workflow lifecycle actions: Stop, Terminate, Suspend and Resume, with Retry as a
follow-up. Clients (the argo-server, the CLI, or any `kubectl` user) create a
`WorkflowAction`; the workflow controller performs the action on the target
Workflow inside its reconciliation loop and reports the outcome on the action's
`status`.

The argo-server stops mutating Workflows to perform actions. In particular it stops
writing Workflow status (today: resume, stop-with-node-selector, and retry all
rewrite `status.nodes` from the server), ending the two-writer problem described in
#2942 and enabling #12538's controller-side retry.

## Motivation

1. **Only the controller should mutate a workflow.** #2942 has been open since
2020: *"Only the workflow controller should be able to change a workflow… We
could have a new operation queue that takes operations to workflows and ensures
they are applied sequentially."* This proposal is that operation queue. Today
`ResumeWorkflow`, `updateSuspendedNode` (stop/resume with node field selector)
and `FormulateRetryWorkflow` perform node-status surgery from the server,
outside the reconcile loop. This is the root cause of a family of corruption
bugs: #16450 (retry resets a TaskGroup nothing will finish), #10918
(stop-then-retry corrupts the node tree), #12156, #16055, #16638.
2. **Actions get observable outcomes.** Today resume can return 200 while the write
is lost or ineffective (#12863). A `WorkflowAction` has its own `status`
(`Pending → Succeeded | Failed` plus a reason), so "accepted" and "applied" are
distinguishable by construction. The server preserves synchronous API semantics
by watching the action to a terminal phase (see *Sync contract*).
3. **Controller-side retry** (#12538, maintainer-endorsed): retry's pod deletions
and status surgery move to the controller, `delete pods` is removed from the
server role, and retry works with plain `kubectl`. This proposal ships the
transport; retry itself is the follow-up commit (same release).
4. **Actions without the server.** All actions become standard Kubernetes
resources: `kubectl create` a `WorkflowAction` and the controller does the rest,
with standard Kubernetes RBAC and audit logging (#14163 partially, #14102).
5. **Actor attribution** (#14102): the action records who requested it, and the
controller stamps the existing `workflows.argoproj.io/actor` / action labels
onto the Workflow when applying it.

Prior art: this is the concrete write-up of the `*Request` CRD idea proposed in
[#6490 (comment)](https://github.com/argoproj/argo-workflows/issues/6490#issuecomment-1961246329)
(precedent: `CertificateSigningRequest`, cert-manager `CertificateRequest`, Velero
operations), adapted to a single kind (see *Alternatives considered*).

## Non-goals

- **Fine-grained per-verb RBAC.** A single kind means Kubernetes RBAC cannot
distinguish "may Resume" from "may Terminate" (that was the per-kind `*Request`
variant's selling point). We consider this secondary: `create workflowactions` is
already strictly narrower than today's `patch workflows`, and per-verb policy is
expressible with an in-tree ValidatingAdmissionPolicy, with no webhook and no
policy engine (worked example below). This deliberately avoids introducing new
authz machinery (c.f. the objection in
[#6490 (comment)](https://github.com/argoproj/argo-workflows/issues/6490#issuecomment-1954941505)).
- **Status subresource on the Workflow CRD.** Independent decision. This proposal
incidentally removes every server-side spec+status cross-write, which is what
currently makes that split impossible, but the split is not part of this
proposal.
- **Resubmit.** Stays a server-side create; it has none of the two-writer disease.
- **Bulk actions.** One action object targets one workflow. Clients loop, as the
CLI does today.

## Design

### CRD

```yaml
apiVersion: argoproj.io/v1alpha1
kind: WorkflowAction
metadata:
# -- truncated to fit; created with generateName
generateName: my-wf-resume-
namespace: argo
labels:
workflows.argoproj.io/workflow: my-wf # for efficient listing per workflow
spec:
workflowRef:
name: my-wf
uid: "" # optional; pins the target across name reuse.
# Required for archived targets (Retry follow-up).
action: Resume # Suspend | Resume | Stop | Terminate (open enum; Retry follows)
# Exactly one parameter block, matching `action`, for actions that take
# parameters (discriminated union, enforced by CEL validation rules):
resume:
nodeFieldSelector: "" # optional; selector form marks matching
outputParameters: {} # suspended nodes Succeeded with outputs
stop:
message: ""
nodeFieldSelector: "" # selector form marks matching nodes Failed
status:
phase: Pending | Succeeded | Failed
reason: "" # machine-readable, e.g. WorkflowNotFound, WorkflowCompleted,
# NothingToResume, InvalidNodeFieldSelector
message: ""
completionTime: null
# resultingWorkflow: (reserved for the Retry follow-up: retry of an archived
# workflow recreates the object; status reports what was created)
```

- Namespaced, same namespace as the target workflow.
- `action` is an open enum: Retry (follow-up commit) and future actions (e.g.
`skip`, #8031) add a value plus an optional parameter block, not a new CRD.
- CEL validation: at most one parameter block, and it must match `action`.
- Printer columns: `TARGET`, `ACTION`, `PHASE`, `AGE`.
- Actor: the server stamps the authenticated user via the existing `creator` labels
on the action at creation. This is a best-effort breadcrumb, since a direct
`kubectl` creator can write anything; the trustworthy record is the Kubernetes
audit log.

### Controller processing

- The controller runs a `WorkflowAction` informer. Add/update of a pending action
enqueues the *target workflow* (by `workflowRef` / the workflow label).
- At the start of `operate()`, before node assessment, the controller drains all
pending actions for the workflow in `creationTimestamp` order (tiebreak: name),
applying each sequentially to the in-memory workflow. Effects are persisted
through the normal single `persistUpdates` path, so actions are serialized with
reconciliation. That serialization is the point of the whole design.
- No dedup: a second Terminate simply succeeds as a no-op.
- Semantics preserved from today's server-side implementations:
- **Suspend** → `spec.suspend=true` equivalent behavior.
- **Resume** (no selector) → clear suspension, mark active suspend nodes
Succeeded with output defaults. With selector → mark matching suspended nodes
Succeeded with provided `outputParameters`.
- **Stop** (no selector) → shutdown=Stop. With selector → mark matching suspended
nodes Failed with `message`. **Terminate** → shutdown=Terminate.
- Shutdown becomes status: the controller records accepted Stop/Terminate intent
in a new `status.shutdown` field. Shutdown is a sticky mode, not an event, so
the controller honors it in every subsequent reconcile. `spec.shutdown` is
deprecated (still honored for one release, see *Migration*). Side effect: the
Retry follow-up clears shutdown in *status*, so controller-side retry never has
to touch spec.
- `spec.suspend` becomes startup-only: honored at creation ("start suspended",
preserving `argo submit`-paused workflows); runtime suspend/resume is
exclusively via actions. This is not two mechanisms for the same thing: the
field and the action now mean different things.
- Crash safety: the workflow effect and the action status live on different objects
and cannot be written atomically. The controller records applied action UIDs in
the workflow status (bounded list, e.g. `status.appliedActions`) in the same
persist as the effect, as a write-ahead record. Action `status` is then
reconciled from it: after a crash between the two writes, the replayed action is
recognized as already-applied and marked Succeeded rather than misreported as
`NothingToResume`. Entries are pruned once the action status is written (or the
action is gone).
- Terminal workflows: an action targeting a completed workflow is marked
`Failed/WorkflowCompleted` (fixes the inconsistency in #8173). An action that
cannot bind to a workflow is marked `Failed/WorkflowNotFound` immediately, the
first time it is processed, with no reattempt; "pending" is transient by
construction, and there are no waiting-for-workflow semantics. The one exception
is a transient infrastructure failure while resolving the target (e.g. the
archive is unreachable, commit 2): that is not a binding failure, so the action
stays Pending and is requeued with backoff.
- Kubernetes Events are emitted on both the Workflow and the WorkflowAction;
a counter metric records actions by type and outcome.

### Sync contract (server & CLI UX unchanged)

The existing gRPC/REST methods (`Stop`, `Terminate`, `Suspend`, `Resume`) keep
their signatures. Implementation: create the `WorkflowAction` (using the user's own
token under impersonation modes, so RBAC maps to the requester), then watch it to a
terminal phase:

- Get-then-watch from the creation `resourceVersion` (avoids the missed-event race
when the controller wins).
- Default timeout ~30s (aligned with existing server request timeouts).
- `Failed` maps to today's error codes: `NothingToResume` and
`InvalidNodeFieldSelector` become `InvalidArgument`, `WorkflowCompleted` becomes
`FailedPrecondition`, and `WorkflowNotFound` becomes `NotFound`.
- A timeout returns `DeadlineExceeded`, with the action name in the message so the
caller can inspect it later.
- On success the method re-fetches and returns the Workflow, as today.

Acceptance test: `argo resume` on a workflow with nothing suspended still produces
an immediate error in the user's terminal.

### Lifecycle / GC

- No ownerReferences. They cannot express the flagship follow-up case: an action
targeting an archived workflow that no longer exists in the cluster (there is no
UID to reference at creation), or a workflow deleted/archived between action
creation and processing.
- TTL from terminal phase: a controller GC loop (same pattern as workflow TTL)
deletes actions where `now - completionTime > TTL`. TTL is controller-wide
configuration (default 24h). Pending actions never expire by clock: unresolvable
targets fail fast, which starts their TTL. Per-object TTL can be added later
without breaking anything.
- Durable audit is *not* the action objects (they get GC'd): it is the actor/action
labels stamped on the Workflow plus the Kubernetes audit log.

### RBAC

- Requesters: `create` (and optionally `get`/`watch`) on `workflowactions`.
Strictly narrower than today's `patch workflows`.
- Controller: `get/list/watch/delete` on `workflowactions`, `update/patch` on
`workflowactions/status`.
- Upgrade note: Roles using resource wildcards in the `argoproj.io` group silently
gain the ability to create actions.
- Per-verb policy without any new machinery, for those who want it:

```yaml
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicy
metadata:
name: deny-terminate-for-devs
spec:
matchConstraints:
resourceRules:
- apiGroups: ["argoproj.io"]
apiVersions: ["v1alpha1"]
operations: ["CREATE"]
resources: ["workflowactions"]
validations:
- expression: >-
object.spec.action != 'Terminate' ||
!('developers' in request.userInfo.groups)
message: "developers may not terminate workflows"
```

## Migration

Target release: 4.2 for both commits.

- 4.2: the CRD ships; server endpoints create actions (sync contract above); the
controller performs actions and honors both the new mechanism and the old spec
fields (`spec.shutdown`, runtime `spec.suspend`), so old CLIs in kubectl-direct
mode keep working unmodified (they already warn on version mismatch). Direct
spec-field mutation is documented as deprecated.
- 4.3: the old mechanism is removed. `spec.shutdown` is no longer honored
(shutdown state lives in `status.shutdown`) and `spec.suspend` is honored at
admission only. The fields remain in the schema, because removing served fields
is a pointless compat break, but they are inert at runtime.

## Commit plan

1. Commit 1 (this proposal's scope): CRD, controller informer and drain logic,
`status.shutdown`, sync contract in server/CLI, TTL GC, docs, e2e. Actions:
Stop, Terminate, Suspend, Resume.
2. Commit 2 (stacked PR, same release): Retry. Moves `FormulateRetryWorkflow`'s
node surgery and pod deletion into the controller (#12538), adds the `Retry`
enum value and parameter block (nodeFieldSelector, restartSuccessful, parameter
overrides), UID-pinned archived targets and `status.resultingWorkflow`, and
drops `delete pods` from the server role.

## Alternatives considered

- Intent fields on `Workflow.spec` (the stalled #12734 approach, `spec.retry`):
the controller must consume-and-clear the field, making the controller a *spec*
writer (generation bumps, ownership inversion); a field holds one action, so
last-write-wins loses a second Resume with different parameters; there is no
per-action status, so #12863-class silent failures remain unfixable; and there
is no object to put the field on for the archived-retry case.
- Annotation/label triggers: same drawbacks minus the generation bump, plus no
schema, no validation, and no status.
- `ownerReference` from the action to its target Workflow, for binding and/or GC:
- Binding: rejected. The drain path needs an informer index of pending actions
per workflow regardless; an ownerRef only changes the index key, it gives no
reverse lookup. Against that it cannot address the archived-retry case (no
UID for a workflow that is not in the cluster), forces a GET-for-UID round
trip and a name-reuse race at creation, and cannot be set for an action
created before its target exists. A name ref (with optional `uid` pinning) is
what a `kubectl` user knows and is strictly more capable.
- GC: rejected as the mechanism, viable only as a future backstop. OwnerRef
cascade fires on *owner deletion*, but a request/response object needs
retention *after it completes* so a client can still read the outcome:
ownerRef would keep a two-second Suspend action alive for the workflow's
whole lifetime, then let a short workflow `ttlStrategy` delete a
just-completed action before anyone reads it. Archived-retry actions have no
owner and so need TTL GC anyway, meaning ownerRef could only sit *on top of*
the TTL loop, not replace it, and would split the lifecycle into owned
(live-target) and unowned (archived-target) actions. While the controller is
running there is no leak to solve: the binder fail-fasts a missing target to
`Failed/WorkflowNotFound`, which sets `completionTime` and starts the TTL. If
orphaned `Pending` actions on force-deleted workflows ever become a real
problem, an ownerRef set controller-side during binding (where the workflow
and its UID are already in hand, so no client round trip) is a clean additive
backstop then — never a replacement for TTL-from-completion.
- Per-action kinds (`StopRequest`, `SuspendRequest`, … as suggested in #6490):
buys per-verb Kubernetes RBAC, at the cost of five CRDs, five informers, five
entries in every Role and install manifest, and a new CRD per future action.
We judge per-verb RBAC secondary and adequately served by
ValidatingAdmissionPolicy; documented above as a considered trade-off.
- An internal authz engine (Casbin, from the #6490 discussion): solves a different
problem (server-side authz flexibility) and adds exactly the new security
surface this proposal avoids; unchanged Kubernetes RBAC remains the authz model.
- "Imperative actions as CRDs is an anti-pattern": the objection raised (and
answered) in #6490. A `WorkflowAction` is not a verb; it is a request whose
fulfillment is tracked, the same shape as `CertificateSigningRequest`. The
status contract (terminal phases, reasons, GC) is what makes that answer honest,
and it is a first-class part of this design.

## Issues this addresses

Fixes or unblocks: #2942, #12538 (with commit 2), #12863, #8173, #14102.
Materially helps: #16037 (VAP example), #10476 (actions become watchable by
plugins), #11849 / #7618 (explicit record of *why* a workflow stopped), and the
retry-corruption family (#16450, #10918, #12156, #16055, #16638) once retry is
controller-side.

## Open questions

- Default TTL value (proposed: 24h post-completion).
- `workflowRef.uid`: always-optional vs required-when-archived (proposed: optional,
required for archived targets in commit 2).
- Whether the UI should additionally surface pending/failed WorkflowActions beyond
the sync contract (proposed: out of scope for 4.2).

Begin with the controller's operate() path and the existing server Stop, Terminate, Suspend, and Resume methods. Trace how actions are currently applied, then compare the proposed WorkflowAction informer and status flow; done means the CRD transport, controller processing, server/CLI sync behavior, migration, and the argo resume acceptance test are covered.

Written by the indexing model from the issue text.