Skip to content

Commit 3ee7223

Browse files
committed
fix(rollback): keep the record for a refused Snapshot delete in the plan preview
#1366 taught the plan LABEL to say "the rollback will REFUSE it" for a Snapshot shape cdkd cannot snapshot, but `applyPlanToPreview` / `applyFailedPlanToPreview` still deleted that record from the preview state. The preview state is what the NEXT (older) journal segment's plan is classified against, so an older segment's real work was downgraded to "skip - already reverted" in the one preview the user reads before typing y. Second-order: the per-item route stamping (#1366) reads the same preview state, so a dropped record also made a later item fall back to the journaled route - the #1366 defect one layer up. Both appliers now take `skipFinalSnapshot` (as the label functions already did) and consult the SAME `refusesFinalSnapshot` predicate with the SAME route, so the label and the previewed state cannot disagree by construction. Under `--skip-final-snapshot` nothing is refused and the unwind is unconditional, matching the executor. Also removes `supportsFinalSnapshot`: zero `src/` callers and never re-exported from `src/index.ts`, so not a public-API break. Its test block is re-homed onto the type SETS as a stronger fence - the union pinned against the literal CloudFormation-documented Snapshot-capable list, plus an explicit DISJOINTNESS assertion nothing had before. Disjointness is load-bearing since #1366: `finalSnapshotMechanism` tests the atomic set first, so a type in both would silently take the atomic arm and never reach the pre-delete snapshot. 4 multi-segment CLI cases + 3 re-homed set fences. Binding-proofed: neutering the carve-out fails 2, and adding AWS::EC2::Volume to the atomic set fails the union + disjointness fences. Closes #1368
1 parent 03692cd commit 3ee7223

6 files changed

Lines changed: 206 additions & 25 deletions

File tree

.claude/rules/code-layout.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ paths:
7676
- **src/types/rollback-journal.ts** - Rollback-journal types + parser (issue [#1183](https://github.com/go-to-k/cdkd/issues/1183)). Defines `RollbackJournal` / `RollbackJournalSegment` / `RollbackSegmentReason`, the `ROLLBACK_JOURNAL_VERSION` constant, `parseRollbackJournal` (JSON parse + validation), and `UnknownRollbackJournalVersionError`. The journal is a **sibling** of `state.json` (`{prefix}/{stackName}/{region}/rollback-journal.json`), deliberately NOT part of the state schema — its own `journalVersion` (starting at 1), no `StackState.version` bump. Read/written via `S3StateBackend.{load,appendSegment,popSegment,delete}RollbackJournal`; `deleteState` sweeps the key so `cdkd destroy` cleans it up.
7777
- **src/provisioning/register-providers.ts** - Shared provider registration (called from deploy.ts and destroy.ts)
7878
- **src/provisioning/data-delete-intent.ts** - Shared destroy data-guard intent helpers (issue #1340): `hasCdkAutoDeleteTag(properties, tagKey)` / `isTruthyCfnBoolean(value)` plus the CDK tag-key constants `S3_AUTO_DELETE_OBJECTS_TAG` (`aws-cdk:auto-delete-objects`, stamped by `autoDeleteObjects: true`) and `ECR_AUTO_DELETE_IMAGES_TAG` (`aws-cdk:auto-delete-images`). Consumed by `S3BucketProvider.delete` (auto-empty of a non-empty bucket only with the tag / `DeleteContext.forceDataDelete`), `S3DirectoryBucketProvider.delete` (issue #1344 — same gate; no CDK opt-in sugar exists for directory buckets, so plain destroy of a non-empty one fails with a manual-empty remediation), and `ECRProvider.delete` (`force: true` only with `EmptyOnDelete: true`, the tag, or `forceDataDelete`) — without an opt-in the AWS not-empty error surfaces like CloudFormation DELETE_FAILED. `DeleteContext.forceDataDelete` (src/provisioning/region-check.ts) is set ONLY by the deploy engine's replacement/recreate delete sites under `--force-stateful-recreation`. See the "Destroy data guards" section in docs/cli-reference.md and the DeleteContext contract note in .claude/rules/providers.md.
79-
- **src/provisioning/final-snapshot.ts** - `DeletionPolicy` / `UpdateReplacePolicy: Snapshot` support (issues #1352 / #1353 / #1354): `ATOMIC_FINAL_SNAPSHOT_TYPES` (RDS DBInstance / DBCluster, Neptune / DocDB clusters, ElastiCache CacheCluster — the delete call sites generate `buildFinalSnapshotIdentifier(physicalId, resourceType)` and thread it via `DeleteContext.finalSnapshotIdentifier`; each provider flips its delete from `SkipFinalSnapshot: true` to the API's atomic final-snapshot form; ONLY on the SDK route — a cc-api-routed atomic type is refused and `CloudControlProvider.delete` fail-closes on the field), `PRE_DELETE_SNAPSHOT_TYPES` + `createPreDeleteFinalSnapshot` dispatcher (all CC-routed: `AWS::EC2::Volume` via EC2 `CreateSnapshot` tagged `cdkd:final-snapshot-of`; `AWS::Redshift::Cluster` via `CreateClusterSnapshot`; `AWS::ElastiCache::ReplicationGroup` via ElastiCache `CreateSnapshot` — each waited to ready, idempotent reuse via the tag / the `finalSnapshotNamePrefix` name prefix across delete re-runs), `unsupportedFinalSnapshotError` / `ccRoutedFinalSnapshotError` refusals, and (issue #1366) `finalSnapshotMechanism(type, route)` / `refusesFinalSnapshot(type, route)` — the mechanism matrix as a PURE function, so the executor that ACTS on it and the `cdkd rollback` plan preview that DESCRIBES it read one source. Consumed by the deploy engine (`prepareFinalSnapshotForDelete` — the shared gate for the DELETE branch AND the four replacement / recreate delete sites), `destroy-runner.ts`, and `rollback-executor.ts` — the latter twice: `rollbackFinalSnapshotId` for the delete-of-the-NEW-resource under `UpdateReplacePolicy` (honors only the atomic SDK-routed shape, plain-deletes otherwise — scope decision on #1354), and `prepareCreateRollbackFinalSnapshot` for a rolled-back CREATE under `DeletionPolicy` (the FULL matrix, refusing what it cannot snapshot — issue #1358). The engine's clients come from `DeployEngineOptions.finalSnapshotClients` (stack-region-pinned `AwsClients`, structurally a `PreDeleteSnapshotClients`), threaded on to `RollbackExecutorContext.finalSnapshotClients`; `--skip-final-snapshot` (deploy / destroy / state destroy / rollback, `skipFinalSnapshotOption` in `src/cli/options.ts` — deliberately NOT in the shared `destroyOptions` array `cdkd orphan` consumes) is the explicit data-loss opt-out.
79+
- **src/provisioning/final-snapshot.ts** - `DeletionPolicy` / `UpdateReplacePolicy: Snapshot` support (issues #1352 / #1353 / #1354): `ATOMIC_FINAL_SNAPSHOT_TYPES` (RDS DBInstance / DBCluster, Neptune / DocDB clusters, ElastiCache CacheCluster — the delete call sites generate `buildFinalSnapshotIdentifier(physicalId, resourceType)` and thread it via `DeleteContext.finalSnapshotIdentifier`; each provider flips its delete from `SkipFinalSnapshot: true` to the API's atomic final-snapshot form; ONLY on the SDK route — a cc-api-routed atomic type is refused and `CloudControlProvider.delete` fail-closes on the field), `PRE_DELETE_SNAPSHOT_TYPES` + `createPreDeleteFinalSnapshot` dispatcher (all CC-routed: `AWS::EC2::Volume` via EC2 `CreateSnapshot` tagged `cdkd:final-snapshot-of`; `AWS::Redshift::Cluster` via `CreateClusterSnapshot`; `AWS::ElastiCache::ReplicationGroup` via ElastiCache `CreateSnapshot` — each waited to ready, idempotent reuse via the tag / the `finalSnapshotNamePrefix` name prefix across delete re-runs), `unsupportedFinalSnapshotError` / `ccRoutedFinalSnapshotError` refusals, and (issue #1366) `finalSnapshotMechanism(type, route)` / `refusesFinalSnapshot(type, route)` — the mechanism matrix as a PURE function, so the executor that ACTS on it and the `cdkd rollback` plan preview that DESCRIBES it read one source (issue #1368 extends that to the preview's STATE effect: a refused Snapshot delete no longer unwinds the record, since the next-older segment is classified against it). The two type sets are DISJOINT by construction — `finalSnapshotMechanism` tests the atomic set first, so a type in both would silently take the atomic arm and never reach the pre-delete snapshot; pinned in `final-snapshot.test.ts` alongside the union-equals-the-CFn-documented-list fence (re-homed there from the deleted `supportsFinalSnapshot` predicate, #1368). Consumed by the deploy engine (`prepareFinalSnapshotForDelete` — the shared gate for the DELETE branch AND the four replacement / recreate delete sites), `destroy-runner.ts`, and `rollback-executor.ts` — the latter twice: `rollbackFinalSnapshotId` for the delete-of-the-NEW-resource under `UpdateReplacePolicy` (honors only the atomic SDK-routed shape, plain-deletes otherwise — scope decision on #1354), and `prepareCreateRollbackFinalSnapshot` for a rolled-back CREATE under `DeletionPolicy` (the FULL matrix, refusing what it cannot snapshot — issue #1358). The engine's clients come from `DeployEngineOptions.finalSnapshotClients` (stack-region-pinned `AwsClients`, structurally a `PreDeleteSnapshotClients`), threaded on to `RollbackExecutorContext.finalSnapshotClients`; `--skip-final-snapshot` (deploy / destroy / state destroy / rollback, `skipFinalSnapshotOption` in `src/cli/options.ts` — deliberately NOT in the shared `destroyOptions` array `cdkd orphan` consumes) is the explicit data-loss opt-out.
8080
- **src/provisioning/ec2-termination-protection.ts** - Shared `--remove-protection` helper for `AWS::EC2::Instance`: `disableInstanceApiTermination()` (flip `DisableApiTermination` off, idempotent, errors swallowed at debug), `isTerminationProtectionPropagationError()` (matches the "may not be terminated. Modify its disableApiTermination" 400 from both `TerminateInstances` and the Cloud Control `DeleteResource` wrapper), and `TERMINATION_PROTECTION_MAX_ATTEMPTS`. Used by `EC2Provider.deleteInstance` (SDK path) and `CloudControlProvider.delete` (CC-API path — an instance routes through Cloud Control whenever its template trips the #614 silent-drop routing) so `--remove-protection` works regardless of which delete path the instance takes; the modify WRITE lags the delete READ, so both callers flip-off + retry the delete to close the propagation window. ALSO used by `ASGProvider.delete` (issue #796): an `AWS::AutoScaling::AutoScalingGroup` whose launch template sets `DisableApiTermination: true` launches instances that survive the group's `DeleteAutoScalingGroup(ForceDelete: true)` (ASG-level DeletionProtection + ForceDelete governs only the group + scale-in protection, not EC2-level termination protection), so under `--remove-protection` the provider enumerates the group's current instances and flips each one's `DisableApiTermination` off before the force delete — the ASG's own async terminate loop then absorbs the modify-WRITE propagation lag, so no per-instance delete retry is needed there. An ASG can ALSO route via Cloud Control when its template sets a silent-drop property such as `AvailabilityZoneIds` (#614 routing) — Cloud Control's `DeleteResource` cannot `ForceDelete` a protected ASG or clear its protection, so `CloudControlProvider.delete` detects `removeProtection === true && resourceType === 'AWS::AutoScaling::AutoScalingGroup'` and delegates to `new ASGProvider().delete(...)` (the single source of truth for protected-ASG deletion), keeping the SDK and CC routing paths behaviourally identical (issue #798; CDK's L2 emits `availabilityZones` names not `AvailabilityZoneIds`, so this CC path only fires for hand-written L1 / imported templates).
8181
- **src/provisioning/unsupported-types.ts** + **unsupported-types.generated.ts** - Pre-flight unsupported-type rejection. The `.generated.ts` ships the provider-coverage Tier 3 set (`ProvisioningType: NON_PROVISIONABLE`) into the runtime, codegen'd from `docs/_generated/provider-coverage.json` by `scripts/gen-unsupported-types.ts` (`vp run gen:unsupported-types`; CI fails on drift). The hand-written `.ts` adds `isNonProvisionable()` + `unsupportedTypeIssueUrl()`; both are consulted by `CloudControlProvider.isSupportedResourceType` (rejects Tier 3) and `ProviderRegistry.validateResourceTypes` (per-type error + issue link). The `--allow-unsupported-types` escape hatch routes named types through Cloud Control via `ProviderRegistry.allowUnsupportedTypes()`.
8282
- **src/provisioning/property-coverage.ts** + **property-coverage.generated.ts** - Pre-flight property-level rejection (parallel to unsupported-types but at top-level CFn property granularity). The `.generated.ts` ships per-Tier-1-type `{ handled, silentDrop }` records, codegen'd from `tests/fixtures/cfn-schemas/*.json` + each SDK provider's `handledProperties` / `unhandledByDesign` declarations by `scripts/gen-property-coverage.ts` (`vp run gen:property-coverage`; CI fails on drift; the codegen parses provider sources via the TypeScript Compiler API so no `dist/` bootstrap is needed). The hand-written `.ts` adds `getPropertyCoverage()` + `findSilentDropProperties()` + `unsupportedPropertyIssueUrl()`; all are consulted by `ProviderRegistry.validateResourceProperties` (per-resource per-property error + 1-click GitHub issue link + dedup'd re-run command). The `--allow-unsupported-properties` escape hatch (deploy only) routes named `<Type>:<Prop>` entries past the reject via `ProviderRegistry.allowUnsupportedProperties()`. Tier 2 (Cloud Control) types are intentionally NOT in the generated map — CC forwards the full property map to AWS, so no write-side silent drop is possible.

0 commit comments

Comments
 (0)