Skip to content

Commit 916c028

Browse files
authored
fix(rollback): report the delete's real route and stop previewing a refused snapshot (#1367)
1 parent cf67c4c commit 916c028

9 files changed

Lines changed: 636 additions & 44 deletions

File tree

.claude/rules/code-layout.md

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

docs/changelog-cdkd.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary
1717
---
1818

1919
**Recently Implemented** (2026-08-03):
20+
- ✅ **Rollback events report the route the delete took, and the plan preview stops promising a snapshot it will refuse (issue [#1366](https://github.com/go-to-k/cdkd/issues/1366))** — `src/provisioning/final-snapshot.ts` (new pure `finalSnapshotMechanism(type, route)` -> `atomic-delete-parameter` / `pre-delete-snapshot` / `refuse-cc-routed` / `refuse-unsupported-type`, plus the `refusesFinalSnapshot` predicate), `src/deployment/rollback-executor.ts` (`prepareCreateRollbackFinalSnapshot` switches on that matrix instead of re-deriving it; the four CREATE-rollback arms emit the EFFECTIVE route; plan items carry `effectiveProvisionedBy`), `src/cli/commands/rollback.ts` (shared `snapshotNote` helper for both label functions). **Before:** (1) both delete sites resolved the routing layer via `effectiveProvisionedBy` (state record first) and routed the provider lookup with it, but the emitted `ROLLBACK_RESOURCE_*` event still carried `op.provisionedBy` — so when a legacy journal entry disagreed with the state record, `cdkd events` named a layer the delete did not use; (2) `actionLabel` / `failedActionLabel` rendered `[DeletionPolicy Snapshot — final snapshot, then delete]` for EVERY Snapshot-policy resource, including the cc-api-routed atomic types and no-mechanism types the executor was about to REFUSE, and the preview dropped their records as if the delete would happen. **Now:** the event reports the route actually used (resolved BEFORE the record is dropped on the orphan arms, where the authoritative side is about to vanish), and the preview reads the same matrix the replay runs — a shape that will be refused is labelled `cdkd cannot snapshot this resource; the rollback will REFUSE it (re-run with --skip-final-snapshot to delete without one)`. `--skip-final-snapshot` still wins over both (nothing is refused under the opt-out). **Design note:** the refusal verdict lives in the LABEL layer, not as a new `refuse-*` action kind from the classifier — whether a Snapshot shape is refused also depends on `--skip-final-snapshot`, a CLI flag the pure classifier deliberately cannot see, and the label functions already take it. **Tests:** 4 matrix units in `final-snapshot.test.ts` (per-route atomic polarity, route-agnostic pre-delete types, unsupported arm, the predicate pinned in both directions), 7 in `rollback-executor.test.ts` (event route on completed delete / orphan / failed delete / failed orphan — each with the journal and the record DELIBERATELY disagreeing, which is the only shape that can observe the bug — plus the two plan-stamping cases and the legacy fallback), 5 CLI plan-label units. Binding-proofed three ways: neutering `refusesFinalSnapshot` fails 4, reverting the event route fails 4, reverting the plan stamping fails 4. **Scoped out:** `deploy-engine.ts`'s `prepareFinalSnapshotForDelete` still carries a third copy of the matrix — routing it through the new helper is the obvious consolidation but that file is in the `integ-destroy` + `integ-broad` gate scope, so a pure refactor there would force a broad real-AWS integ; left for whenever that file is next touched for a behavior reason.
2021
- ✅ **`--revert-failed`'s delete of a FAILED in-flight CREATE now honors `DeletionPolicy` (issue [#1362](https://github.com/go-to-k/cdkd/issues/1362))** — `src/deployment/rollback-executor.ts` (two new `FailedOpActionKind`s — `orphan-failed-create-retain` / `delete-failed-create-with-final-snapshot`; `classifyFailedOp` reads the CURRENT state record's `deletionPolicy`; the delete branch resolves the routing layer ONCE via `effectiveProvisionedBy` and shares it between the snapshot gate and the provider lookup; `prepareCreateRollbackFinalSnapshot` widened to a structural op type so both sibling paths run the SAME mechanism matrix), `src/cli/commands/rollback.ts` (plan labels for both new actions, `skipFinalSnapshot`-aware like the completed-op labels; preview state drops the record for both). **Before:** the `delete-failed-create` branch read NO `DeletionPolicy` at all — a `Snapshot`-policy resource whose CREATE failed AFTER AWS provisioned it (a physical id is recorded, which is exactly what the action requires) was deleted with no final snapshot and no refusal, and a `Retain`-policy one was deleted outright while the COMPLETED-CREATE path orphaned it. **Now:** one matrix for both paths — `Retain` leaves the resource in AWS and drops the record, `Snapshot` snapshots then deletes (atomic delete parameter on the SDK route, `createPreDeleteFinalSnapshot` for the pre-delete types, REFUSAL for a cc-api-routed atomic type or any other Snapshot shape), `RetainExceptOnCreate` / `Delete` / absent delete plainly. The `Retain` arm is a deliberate scope extension beyond the issue text (recorded on the issue before implementation): fixing only `Snapshot` would leave the classifier reading `deletionPolicy` for one value and ignoring it for another, and CloudFormation applies the policy to a failed create's rollback delete too — which is why `RetainExceptOnCreate` exists as a separate value to opt OUT of exactly that. **Why strict (refuse) rather than tolerant (warn-and-plain-delete)** for a shape that cannot be snapshotted: a refusal is a per-op failure, so the journal is KEPT and the op stays in `remainingFailedOps` — a half-created resource that is not snapshot-capable yet (an RDS instance rejects a final-snapshot delete while `creating`) becomes snapshot-able once it settles, so a re-run completes the job, and `--skip-final-snapshot` remains the explicit opt-out. The tolerant option's failure mode is unrecoverable by comparison. **Tests:** 15 new units in `tests/unit/deployment/rollback-executor.test.ts` (classifier per policy incl. the mismatch guard running FIRST, Retain-orphan, atomic+SDK identifier threading with `attemptedProperties` still flowing, pre-delete snapshot-before-delete ordering, snapshot failure aborting the delete AND keeping the op for a re-run, cc-api refusal, unsupported-shape refusal, `--skip-final-snapshot` covering the refused shapes but NOT overriding `Retain`, routing-source alignment) plus 4 CLI units in `tests/unit/cli/commands/rollback.test.ts` (identifier reaching the delete through the real command, both flag polarities, `Retain` issuing no delete at all, and the plan labels). Binding-proofed: reverting the two classifier lines fails 13 of them. **Docs:** cli-reference (`DeletionPolicy: Snapshot` section, `cdkd rollback` flag table + limitations), `.claude/rules/code-layout.md`.
2122
- ✅ **A rolled-back CREATE under `DeletionPolicy: Snapshot` is snapshotted then deleted, not orphaned (issue [#1358](https://github.com/go-to-k/cdkd/issues/1358))** — `src/deployment/rollback-executor.ts` (new `delete-with-final-snapshot` `RollbackActionKind`; `classifyRollbackOp` splits `Retain` from `Snapshot`; new `prepareCreateRollbackFinalSnapshot` mirroring the deploy engine's `prepareFinalSnapshotForDelete` mechanism matrix; `RollbackExecutorContext` gains `finalSnapshotClients` + `skipFinalSnapshot`), `src/deployment/deploy-engine.ts` (threads both from `DeployEngineOptions` into the executor context), `src/cli/commands/rollback.ts` (`--skip-final-snapshot` + stack-region-pinned `AwsClients`), `src/cli/options.ts` (doc comment). **Before:** `classifyRollbackOp` mapped BOTH `Retain` and `Snapshot` to `orphan-retain`, so a deploy that failed after creating a `Snapshot`-policy resource LEFT IT IN AWS and dropped it from state — an untracked, billing resource, reported as a clean rollback (`Leaving Cache (AWS::ElastiCache::ReplicationGroup) in AWS (DeletionPolicy: Retain) — removed from state`). Deliberate when written (cdkd could not create a final snapshot at all), obsolete once `src/provisioning/final-snapshot.ts` shipped (#1352 / #1353). **Now:** CloudFormation semantics — rolling a CREATE back IS a delete, so `Snapshot` takes the final snapshot and then deletes: atomic delete parameter for the SDK-routed `ATOMIC_FINAL_SNAPSHOT_TYPES`, `createPreDeleteFinalSnapshot` + wait for `PRE_DELETE_SNAPSHOT_TYPES`, and a REFUSAL (counted as a per-op failure, so the segment is not popped and the journal is kept for a re-run — never a silent fall-back to orphaning) for a cc-api-routed atomic type or any other Snapshot-tagged shape. `cdkd rollback --skip-final-snapshot` is the data-loss opt-out; `Retain` / `RetainExceptOnCreate` / `Delete` are unchanged. The cc-api test uses the CURRENT state record's `provisionedBy` with the journaled op's as the legacy fallback (shared `effectiveProvisionedBy` helper, also used by #1354's `rollbackFinalSnapshotId`). The delete-of-the-NEW-resource direction (`UpdateReplacePolicy`, #1354) keeps its bounded atomic-only scope — that delete is load-bearing for same-name re-creation. **Tests:** 13 new units in `tests/unit/deployment/rollback-executor.test.ts` plus 4 CLI-wiring units in `tests/unit/cli/commands/rollback.test.ts` (the flag actually reaching the executor context, the plan label under both polarities, and the option being declared on the `rollback` subcommand itself — the #1097 failure class) (classifier split; atomic+SDK threads the identifier; atomic+cc-api refuses via the state record AND via the op fallback; pre-delete type snapshots before deleting with the context clients; pre-delete snapshot failure aborts the delete; unsupported type refuses; `--skip-final-snapshot` plain-deletes incl. the otherwise-refused shapes; `Retain` still orphans; `RetainExceptOnCreate` still deletes). Binding-proofed: reverting the classifier line fails 9 of them. The command installs a STACK-REGION-PINNED `AwsClients` as the process-global for the replay (mirroring `destroy-runner.ts`'s cross-region destroy) and restores the original in its `finally`: pinning only the snapshot would be worse than not pinning it at all, since a `--stack-region` run would then take a real, billable snapshot and fail the delete that follows. **Docs:** cli-reference (`DeletionPolicy: Snapshot` section + `cdkd rollback` flag table / limitations, incl. the re-run snapshot-cost note for the name-keyed types), `.claude/rules/{code-layout,providers}.md`. **Scoped out at the time, shipped since:** `--revert-failed`'s delete of a resource whose CREATE FAILED mid-flight was still policy-unaware — closed by issue [#1362](https://github.com/go-to-k/cdkd/issues/1362) (entry above).
2223
- ✅ **`AWS::EC2::Volume` immutable-property changes now drive a REPLACEMENT (issues #1356 / #1357)** — `src/analyzer/replacement-rules.ts`, `src/provisioning/stateful-types.ts`, `tests/integration/deletion-policy-snapshot/**`. **Before:** changing an immutable EBS volume property (`AvailabilityZone` / `AvailabilityZoneId` / `Encrypted` / `KmsKeyId` / `OutpostArn` / `SnapshotId`) was classified as an in-place UPDATE and AWS rejected the deploy (`Volume properties other than AutoEnableIO, type, size, and IOPS cannot be updated`), where CloudFormation replaces the volume. Root cause was NOT a cdkd logic bug: the `create-only-properties.ts` CFn-schema fallback found nothing because the AWS registry schema for this type declares **no `createOnlyProperties` at all** (live-verified 2026-08-03), and the hand-authored `ReplacementRulesRegistry` had no entry. **Now:** a hand-written rule classifies the six immutable properties as replacement and the `ModifyVolume`-mutable set (`AutoEnableIO` / `Iops` / `Size` / `Throughput` / `VolumeType` / `Tags`) as explicitly in-place; the three remaining schema properties (`MultiAttachEnabled` / `SourceVolumeId` / `VolumeInitializationRate`) are deliberately left unclassified rather than guessed at, so the schema fallback still applies to them and no mutable property is ever mistaken for a replacement. `AWS::EC2::Volume` also joins `STATEFUL_TYPES`, so the deploy engine's property-driven replacement path demands `--force-stateful-recreation` before destroying a volume's data — the change is therefore "deploy fails with an AWS error" → "deploy refuses until you confirm the data loss", never a silent delete. **Tests:** new `replacement-rules-ec2-volume.test.ts` (per-property replacement / in-place polarity, `isClassified` pinning both directions, stateful guard). **Integ:** the `deletion-policy-snapshot` fixture regains the replacement phase #1354 had to drop — `CDKD_TEST_REPLACE=true` flips `VolumeKeep`'s AZ, and the run asserts the OLD volume is gone, a completed final snapshot of it exists (`UpdateReplacePolicy: Snapshot` honored at the engine's create-first cleanup delete site — the live coverage #1357 asked for), and the replacement landed in the expected AZ. `covers: AWS::EC2::Volume`.

docs/cli-reference.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2180,7 +2180,12 @@ credentials, etc.).
21802180
from state (the plan labels it `orphan`); `Snapshot` takes the final
21812181
snapshot and THEN deletes, refusing (as a per-op failure, journal kept) any
21822182
shape cdkd cannot snapshot unless `--skip-final-snapshot` is passed (issue
2183-
[#1358](https://github.com/go-to-k/cdkd/issues/1358)); `RetainExceptOnCreate`
2183+
[#1358](https://github.com/go-to-k/cdkd/issues/1358)). The plan preview says
2184+
which of the two will happen BEFORE you confirm — a shape cdkd cannot
2185+
snapshot on the route the delete will take is labelled
2186+
`cdkd cannot snapshot this resource; the rollback will REFUSE it` rather
2187+
than promising a final snapshot (issue
2188+
[#1366](https://github.com/go-to-k/cdkd/issues/1366)); `RetainExceptOnCreate`
21842189
and the default `Delete` delete plainly. `--revert-failed`'s delete of a
21852190
resource whose CREATE FAILED mid-flight applies the SAME matrix since issue
21862191
[#1362](https://github.com/go-to-k/cdkd/issues/1362) — it acts only on a

src/cli/commands/rollback.ts

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { forwardSigtermToSigint } from '../../utils/interrupt-signals.js';
1313
import { PartialFailureError, withErrorHandling } from '../../utils/error-handler.js';
1414
import { ProviderRegistry } from '../../provisioning/provider-registry.js';
1515
import { registerAllProviders } from '../../provisioning/register-providers.js';
16+
import { refusesFinalSnapshot } from '../../provisioning/final-snapshot.js';
1617
import { withNestedStackContext } from '../../provisioning/nested-stack-context.js';
1718
import { withStackName } from '../../provisioning/resource-name.js';
1819
import { setupStateBackend, resolveSingleRegion } from './state.js';
@@ -89,6 +90,32 @@ async function findJournalCandidates(
8990
return refs;
9091
}
9192

93+
/**
94+
* The `DeletionPolicy Snapshot — ...` note a planned Snapshot delete carries
95+
* (issue #1366). Consults the SAME mechanism matrix the replay will run, so
96+
* the preview cannot promise a final snapshot for a shape the executor is
97+
* about to REFUSE (a cc-api-routed atomic type, or a type with no snapshot
98+
* mechanism at all). `skipFinalSnapshot` is threaded in because the
99+
* classifier is pure and cannot see CLI flags — under the opt-out every shape
100+
* plain-deletes, refusals included, so the flag is checked first.
101+
*/
102+
function snapshotNote(
103+
resourceType: string,
104+
effectiveProvisionedBy: 'sdk' | 'cc-api' | undefined,
105+
skipFinalSnapshot: boolean
106+
): string {
107+
if (skipFinalSnapshot) {
108+
return 'DeletionPolicy Snapshot — NO final snapshot (--skip-final-snapshot)';
109+
}
110+
if (refusesFinalSnapshot(resourceType, effectiveProvisionedBy)) {
111+
return (
112+
'DeletionPolicy Snapshot — cdkd cannot snapshot this resource; the rollback will ' +
113+
'REFUSE it (re-run with --skip-final-snapshot to delete without one)'
114+
);
115+
}
116+
return 'DeletionPolicy Snapshot — final snapshot, then delete';
117+
}
118+
92119
/**
93120
* Human label for a planned rollback action (plan preview). `skipFinalSnapshot`
94121
* is threaded in because the classifier is pure (it cannot see CLI flags) and
@@ -102,9 +129,10 @@ function actionLabel(item: RollbackPlanItem, skipFinalSnapshot: boolean): string
102129
case 'delete':
103130
return ` - delete ${op.logicalId} (${op.resourceType})${rep}`;
104131
case 'delete-with-final-snapshot':
105-
return skipFinalSnapshot
106-
? ` - delete ${op.logicalId} (${op.resourceType}) [DeletionPolicy Snapshot — NO final snapshot (--skip-final-snapshot)]`
107-
: ` - delete ${op.logicalId} (${op.resourceType}) [DeletionPolicy Snapshot — final snapshot, then delete]`;
132+
return (
133+
` - delete ${op.logicalId} (${op.resourceType}) ` +
134+
`[${snapshotNote(op.resourceType, item.effectiveProvisionedBy, skipFinalSnapshot)}]`
135+
);
108136
case 'orphan-retain':
109137
return ` - orphan ${op.logicalId} (${op.resourceType}) [DeletionPolicy Retain — left in AWS]`;
110138
case 'orphan-flag':
@@ -140,9 +168,10 @@ function failedActionLabel(item: FailedOpPlanItem, skipFinalSnapshot: boolean):
140168
case 'delete-failed-create':
141169
return ` - delete ${op.logicalId} (${op.resourceType}) [FAILED create]`;
142170
case 'delete-failed-create-with-final-snapshot':
143-
return skipFinalSnapshot
144-
? ` - delete ${op.logicalId} (${op.resourceType}) [FAILED create, DeletionPolicy Snapshot — NO final snapshot (--skip-final-snapshot)]`
145-
: ` - delete ${op.logicalId} (${op.resourceType}) [FAILED create, DeletionPolicy Snapshot — final snapshot, then delete]`;
171+
return (
172+
` - delete ${op.logicalId} (${op.resourceType}) [FAILED create, ` +
173+
`${snapshotNote(op.resourceType, item.effectiveProvisionedBy, skipFinalSnapshot)}]`
174+
);
146175
case 'orphan-failed-create-retain':
147176
return ` - orphan ${op.logicalId} (${op.resourceType}) [FAILED create, DeletionPolicy Retain — left in AWS]`;
148177
case 'skip-failed-unknown':

0 commit comments

Comments
 (0)