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
2 changes: 1 addition & 1 deletion .claude/rules/cli-internals.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ paths:

- `--app` (`-a`) is optional: falls back to `CDKD_APP` env var, then `cdk.json` `"app"` field. Accepts either a shell command (`"node app.ts"`) or a path to a pre-synthesized cloud assembly directory (`cdk.out`); when a directory is given, synthesis is skipped and the manifest is read directly.
- `--state-bucket` is optional: falls back to `CDKD_STATE_BUCKET` env var, then `cdk.json` `context.cdkd.stateBucket`
- `--region` is **bootstrap-only** as of PR #63 (v0.12.0). `cdkd bootstrap` uses it to pick the region of the new state bucket; every other command (`deploy`, `destroy`, `diff`, `synth`, `list`, `state`, `force-unlock`, `publish-assets`) accepts `--region` for backward compatibility but emits a deprecation warning and ignores the value — provisioning clients pick up the region from `AWS_REGION` / the AWS profile, and the state-bucket client auto-detects the bucket's region via `GetBucketLocation` (PR #60, v0.10.0).
- `--region` on `cdkd bootstrap` picks the region of the new state bucket (a real, non-deprecated, `--help`-visible option there). On every other command (`deploy`, `destroy`, `diff`, `synth`, `list`, `state`, `force-unlock`, `publish-assets`, …) `--region` is **deprecated but still honored** (PR #63, v0.12.0): it is registered as a hidden `deprecatedRegionOption`, emits a one-shot `warnIfDeprecatedRegion` deprecation warning, and IS consumed as the highest-precedence region source — every command resolves `const region = options.region || process.env['AWS_REGION'] || 'us-east-1'`, passes it to `applyRoleArnIfSet` / the `AwsClients` constructor, and `deploy` / `destroy` / `import` / `export` / `orphan` additionally inject it into `process.env.AWS_REGION` so the CDK synth subprocess inherits it. The recommended way to pick the region is `AWS_REGION` / the AWS profile, but passing `--region` is NOT a no-op (issue #818 corrected the earlier "has no effect" warning, which contradicted the actual consumption in `deploy.ts` etc.). The state-bucket S3 client still auto-detects the bucket's region via `GetBucketLocation` independent of `--region` (PR #60, v0.10.0). `warnIfDeprecatedRegion` + `deprecatedRegionOption` live in `src/cli/options.ts`.
- `--context` / `-c` is optional: accepts `key=value` pairs (repeatable), merged with cdk.json context (CLI takes precedence)
- Stack names are positional arguments: `cdkd deploy MyStack` (not `--stack-name`)
- `--all` flag targets all stacks for deploy/diff/destroy (`destroy --all` only targets stacks from the current CDK app via synthesis)
Expand Down
1 change: 1 addition & 0 deletions docs/changelog-cdkd.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ The CLAUDE.md `## Known Limitations` section retains the load-bearing summary

**Recently Implemented** (2026-06-13):

- ✅ **`--region` deprecation warning no longer contradicts the actual behavior (issue [#818](https://github.com/go-to-k/cdkd/issues/818))** — `src/cli/options.ts`. `warnIfDeprecatedRegion` and the hidden `deprecatedRegionOption` help text both claimed `--region` "has no effect" on non-bootstrap commands, but every non-bootstrap command (`deploy`, `destroy`, `diff`, `synth`, `list`, `state`, `force-unlock`, `publish-assets`, `import`, `export`, `orphan`, `drift`, `events`, `local *`, …) actually consumes `options.region` as the highest-precedence region source: `const region = options.region || process.env['AWS_REGION'] || 'us-east-1'` feeds the provisioning / state-bucket SDK clients and the `applyRoleArnIfSet` STS hop, and `deploy` / `destroy` / `import` / `export` / `orphan` additionally inject it into `process.env.AWS_REGION` so the CDK synth subprocess inherits it (e.g. `deploy.ts` ~L167/L175/L341). The warning and the code therefore disagreed — a user passing `--region` was told it did nothing while it silently took effect. **Investigation determined `--region` IS legitimately honored everywhere (option B in the issue), so the fix is purely in the warning + help text — no command implementation (`deploy.ts` etc.) was touched**, keeping the change out of the `integ-broad` merge-gate scope and carrying zero behavior-change risk. The warning now reads "`--region is deprecated and will be removed in a future release. It is still honored for now (it overrides AWS_REGION / your AWS profile), but prefer the AWS_REGION environment variable or your AWS profile…`" and the option description drops the false "No effect" claim. The recommended mechanism is still `AWS_REGION` / the AWS profile; the flag stays hidden + deprecated, just honestly described. Docs corrected: two "deprecated and ignored" lines in [docs/cli-reference.md](cli-reference.md) and the `--region` bullet in [.claude/rules/cli-internals.md](../.claude/rules/cli-internals.md). Tests: `tests/unit/cli/options.test.ts` — the existing message assertion updated, plus new assertions that neither the warning nor the option description contains "no effect" and that both mention the flag is "still honored" (issue #818).
- ✅ **`deploy` retries the ECS CapacityProvider same-stack infrastructure-role IAM-propagation race (issue [#805](https://github.com/go-to-k/cdkd/issues/805))** — `src/deployment/retryable-errors.ts`. cdkd's event-driven DAG dispatches the Cloud Control `CreateResource` for an `AWS::ECS::CapacityProvider` (Managed Instances) as soon as its same-stack infrastructure role finishes creating, and cdkd's fast SDK path creates the IAM role without waiting for propagation — so ECS tried to assume the just-created `InfrastructureRoleArn` before IAM had propagated it and rejected the create with `Caught ServiceAccessDeniedException for ECSInfrastructureRole[arn:...]`. The CC API handler classifies this as a terminal `InvalidRequest` (no internal retry, `SDK Attempt Count: 1`), and none of the existing message patterns matched it, so the deploy failed fast on a transient error. The fix adds `'Caught ServiceAccessDeniedException'` to `RETRYABLE_ERROR_MESSAGE_PATTERNS` — mirroring the `ENHANCED_MONITORING` pattern added for #794 — so the deploy engine's existing `withRetry` (8 attempts, ~47s cumulative) absorbs the propagation window; the phrase is anchored on the CC-API/ECS handler wording so a genuine, permanent role misconfiguration only burns the bounded retries before surfacing. Generic by design: any Cloud-Control-provisioned type that validates a same-stack IAM role at create time and surfaces `ServiceAccessDeniedException` is covered. Tests: the exact wire message from the issue classifies retryable + a plain `AccessDeniedException` (without the handler's "Caught" anchor) stays non-retryable in `retryable-errors.test.ts`. Verified by the issue reporter against the real-world 33-resource stack that surfaced the bug (the capacity provider create retried through the window and completed).
- ✅ **`AWS::ECS::TaskDefinition` `Volumes[].ConfiguredAtLaunch` no longer silently dropped (issue [#806](https://github.com/go-to-k/cdkd/issues/806))** — `src/provisioning/providers/ecs-provider.ts`. `ECSProvider.convertVolumes` mapped only `Name` / `Host` / `EFSVolumeConfiguration` when converting CFn `Volumes` to the `RegisterTaskDefinition` wire shape; `ConfiguredAtLaunch` was dropped, so the registered task definition had no `configuredAtLaunch` volume and a same-stack `AWS::ECS::Service` carrying `VolumeConfigurations` (a managed EBS volume — CDK's `ServiceManagedVolume`) failed to create with "Volume configuration provided but no matching configuredAtLaunch volume found in task definition". The pre-flight property-coverage gate could not catch this class: it works at top-level property granularity and `Volumes` IS in `handledProperties` — the gap was one level down, inside the handled property. `convertVolumes` now forwards `configuredAtLaunch` via a `coerceBool` helper (same pattern as `EC2Provider`'s) that normalizes CFn boolean-ish values (`true` / `"true"` / `false` / `"false"`) at the wire boundary and returns `undefined` for absent props so the field is omitted from the SDK input (AWS keeps its default). No parallel `update()` change is needed — ECS TaskDefinitions are immutable revisioned resources; property changes route through Replace (CREATE then DELETE). Tests: 4 unit tests (present-true forwarded, string `"true"` / `"false"` coerced, absent omitted, explicit `false` preserved as distinct from omit). The `ecs-fargate` integ fixture gains a `ServiceManagedVolume` (1 GiB gp3, XFS) mounted into the container and attached to the Service via `service.addVolume()` — synthesizing exactly the `ConfiguredAtLaunch` + `VolumeConfigurations` pairing the bug broke (with `desiredCount: 0` no task launches, so no EBS volume is actually created); `verify.sh` asserts the registered task definition's `ebs-data` volume has `configuredAtLaunch == true` (probed via jq `has()` — the `//` operator would map an explicit `false` to the fallback) and that `DescribeServices` shows the deployment carrying the `ebs-data` volume configuration. Remaining `convertVolumes` sub-property gaps of the same class (`DockerVolumeConfiguration` / `FSxWindowsFileServerVolumeConfiguration` unmapped; `Host` / `EFSVolumeConfiguration` cast without PascalCase-to-camelCase conversion) are tracked separately per the issue.
- ✅ **Cloud Control UPDATE re-includes write-only properties in every patch document (issue [#809](https://github.com/go-to-k/cdkd/issues/809))** — `src/provisioning/cloud-control-provider.ts` + new `src/provisioning/write-only-properties.ts`. Cloud Control applies UPDATE patches read-modify-write: the type's read handler returns the current model, the patch is applied on top, and the result becomes the desired state — but read handlers cannot return **write-only properties**, so any write-only property absent from cdkd's minimal previous-vs-desired patch silently vanished from the desired state on every CC-routed UPDATE. `AWS::ECS::Service` (writeOnlyProperties: `ServiceConnectConfiguration` / `VolumeConfigurations` / `ForceNewDeployment`) hard-failed: a task-definition-only change on a service with a managed EBS volume produced a patch without `VolumeConfigurations`, and `UpdateService` rejected with "Task definition has configuredAtLaunch volume but no volume configuration provided at runtime", wedging the stack (state still recorded the old properties, so every subsequent deploy retried the same failing patch). Types whose handler accepts the write-only-less state lost the configuration silently instead. The fix mirrors `terraform-provider-awscc`: `CloudControlProvider.update` now resolves the type's `writeOnlyProperties` from the registry schema via `cloudformation:DescribeType` (reduced to the top-level containing property — a nested path like `/properties/Foo/Bar` strips to `Foo`), removes those properties from the PREVIOUS side, and regenerates the patch — the generator then naturally emits `add` ops for every write-only property present in the desired properties, which is exactly what the CC read-modify-write contract requires. Only write-only properties are force-included (blanket-upserting all desired properties would risk false replacement signals on `createOnlyProperties` whose read-back form differs from the stored form). Only SUCCESSFUL DescribeType results are cached per resource type for the deploy lifetime in a module-level map, so repeated updates of the same type pay one throttled-API call; a DescribeType failure (missing IAM permission, transient throttle / 5xx) is NOT cached — it warns and falls back to the pre-#809 minimal patch for that update, and a later update of the same type retries DescribeType. Caching failures would let one transient throttle on the first CC-routed UPDATE silently disable write-only re-inclusion for every CC-routed type for the rest of the deploy, reintroducing the exact hard-fail this fixes. No regression for callers permanently without the new `cloudformation:DescribeType` permission — each update simply re-warns and re-falls-back. A DescribeType response without a `Schema` (e.g. a still-registering type) is treated as "no write-only properties" — a successful, cacheable, warning-free lookup. Removal-only write-only diffs skip the update entirely (CC cannot remove what its read handler never returns — pre-fix a `remove` op against a path absent from the current model would have failed), and the no-change fast path still skips without any DescribeType call. Tests: 11 unit tests in `tests/unit/provisioning/cloud-control-provider.test.ts` (unchanged write-only prop rides along as `add`; changed write-only prop not duplicated; nested-path top-level strip; no-write-only type keeps the minimal patch; DescribeType failure warning + fallback; per-type caching of successful lookups; failures NOT cached so a later update retries; retry-after-failure uses the populated set on success; Schema-less response = no write-only props + no warning; no-change skip; removal-only skip). Integ: the `ecs-fargate` fixture's `ServiceManagedVolume` + `CDKD_TEST_UPDATE` pass (issues #806/#807) exercises this exact path end-to-end once both land.
Expand Down
6 changes: 4 additions & 2 deletions docs/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,8 @@ time — the same auto-fallback the deploy engine applies (#614). DELETE
lines are not annotated; deletes route via the recorded `provisionedBy`
on each resource's state, not via template inspection.

Like every non-bootstrap command, `--region` is deprecated and ignored.
Like every non-bootstrap command, `--region` is deprecated (prefer
`AWS_REGION` / your AWS profile) but still honored if passed.
Stack selection (`<stacks...>` / `--all` / wildcards / display paths)
follows the same rules as `cdkd deploy` / `cdkd destroy`.

Expand Down Expand Up @@ -994,7 +995,8 @@ Flags:
(`--accept`) or pushing changes back to AWS (`--revert`).
- `--state-bucket`, `--state-prefix`, `--profile`, `--verbose`,
`--role-arn`, `--region` — same as on every other state-driven
command. `--region` is deprecated and ignored (PR 5).
command. `--region` is deprecated (prefer `AWS_REGION` / your AWS
profile) but still honored if passed (PR 5).

Exit codes:

Expand Down
12 changes: 7 additions & 5 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,13 @@ avoids the SDK glitch) and rebuild their S3 clients to that region
before any state or lock operation. If you still see either error,
please file a bug with the full stack trace.

You no longer need to set the region to match the bucket region. As of
PR #63 (v0.12.0), `--region` is reserved for `cdkd bootstrap` (where it
picks the new bucket's region); on every
other command it is deprecated and ignored. Use `AWS_REGION` or your
AWS profile to control the SDK's default region for provisioning.
You no longer need to set the region to match the bucket region (the
state-bucket client auto-detects it via `GetBucketLocation`). As of
PR #63 (v0.12.0), `--region` is a first-class option only on
`cdkd bootstrap` (where it picks the new bucket's region); on every
other command it is deprecated (prefer `AWS_REGION` / your AWS profile)
but still honored if passed. Use `AWS_REGION` or your AWS profile to
control the SDK's default region for provisioning.

---

Expand Down
25 changes: 18 additions & 7 deletions src/cli/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,37 @@ export const commonOptions = [
* Deprecated `--region` option attached to non-bootstrap commands.
*
* Kept (rather than fully removed) so that scripts or muscle memory passing
* `--region` do not break. The value is parsed but ignored — see
* `--region` do not break. The value IS still honored — every non-bootstrap
* command consumes `options.region` as the highest-precedence region source
* (`options.region || AWS_REGION || 'us-east-1'`): it sets the region of the
* provisioning / state-bucket SDK clients, the `applyRoleArnIfSet` STS hop,
* and (for `deploy` / `destroy` / `import` / `export` / `orphan`) the
* `AWS_REGION` env var inherited by the CDK synth subprocess. The flag is
* *deprecated* — the recommended way to choose the region is `AWS_REGION` or
* your AWS profile — but passing it is NOT a no-op. See
* `warnIfDeprecatedRegion` for the runtime warning. Final removal is
* tracked in PR 99 (see `docs/plans/05-region-flag-cleanup.md`).
*/
export const deprecatedRegionOption = new Option(
'--region <region>',
'[deprecated] No effect on this command; use AWS_REGION or your AWS profile'
'[deprecated] Prefer AWS_REGION or your AWS profile; --region is still honored but will be removed in a future release'
).hideHelp();

/**
* Emit a one-shot stderr warning when a non-bootstrap command receives
* `--region`. PR 5 consolidates `--region` to bootstrap-only; everywhere
* else the SDK picks up the region from `AWS_REGION` / profile, and
* passing the flag does nothing useful.
* `--region`. The flag is deprecated in favor of `AWS_REGION` / profile, but
* — contrary to an earlier message that claimed "no effect" — it is NOT a
* no-op: every non-bootstrap command consumes `options.region` as the
* highest-precedence region source (see `deprecatedRegionOption`). So the
* warning steers users toward the recommended mechanism WITHOUT falsely
* telling them their flag had no effect (issue #818).
*/
export function warnIfDeprecatedRegion(options: { region?: string }): void {
if (options.region !== undefined) {
process.stderr.write(
'Warning: --region is deprecated for this command and has no effect. ' +
'Use the AWS_REGION environment variable or your AWS profile to override the SDK default region.\n'
'Warning: --region is deprecated and will be removed in a future release. ' +
'It is still honored for now (it overrides AWS_REGION / your AWS profile), ' +
'but prefer the AWS_REGION environment variable or your AWS profile to choose the region.\n'
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/cli/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('cdkd list', () => {
// Command still runs to completion.
expect(stdout).toBe('StackA\n');
// Warning is on stderr, mentions --region and points users at AWS_REGION.
expect(stderr).toMatch(/--region is deprecated for this command and has no effect/);
expect(stderr).toMatch(/--region is deprecated and will be removed in a future release/);
expect(stderr).toMatch(/AWS_REGION/);
});

Expand Down
22 changes: 21 additions & 1 deletion tests/unit/cli/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ describe('cli/options.ts', () => {
// The exact field is internal but observable.
expect((deprecatedRegionOption as unknown as { hidden?: boolean }).hidden).toBe(true);
});

it('help text does NOT claim the flag has no effect (issue #818)', () => {
// The flag IS still honored on every non-bootstrap command (it feeds the
// SDK client region / AWS_REGION injection); the description must not
// falsely advertise it as a no-op.
expect(deprecatedRegionOption.description).not.toMatch(/no effect/i);
expect(deprecatedRegionOption.description).toMatch(/still honored/i);
expect(deprecatedRegionOption.description).toMatch(/AWS_REGION/);
});
});

describe('warnIfDeprecatedRegion', () => {
Expand All @@ -85,10 +94,21 @@ describe('cli/options.ts', () => {
it('writes a deprecation warning to stderr when region is set', () => {
warnIfDeprecatedRegion({ region: 'us-east-1' });
const all = stderrChunks.join('');
expect(all).toMatch(/--region is deprecated for this command and has no effect/);
expect(all).toMatch(/--region is deprecated and will be removed in a future release/);
expect(all).toMatch(/AWS_REGION/);
});

it('does NOT claim the flag has no effect (issue #818)', () => {
// The warning must steer users toward AWS_REGION / profile WITHOUT
// falsely telling them the flag they just passed did nothing: deploy /
// destroy / diff / list / etc. all consume options.region as the
// highest-precedence region source, so "has no effect" was a lie.
warnIfDeprecatedRegion({ region: 'eu-west-1' });
const all = stderrChunks.join('');
expect(all).not.toMatch(/no effect/i);
expect(all).toMatch(/still honored/i);
});

it('is silent when region is undefined', () => {
warnIfDeprecatedRegion({});
expect(stderrChunks).toEqual([]);
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/cli/publish-assets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ describe('cdkd publish-assets', () => {

const { stderr, error } = await runCmd(['--region', 'us-east-1']);
expect(error).toBeUndefined();
expect(stderr).toMatch(/--region is deprecated for this command and has no effect/);
expect(stderr).toMatch(/--region is deprecated and will be removed in a future release/);
});
});
});
Loading