Skip to content

Commit bacfa1b

Browse files
committed
fix(analyzer): delete AWS::CloudWatch::CompositeAlarm before the metric Alarms its AlarmRule references
A CompositeAlarm references its child alarms by NAME inside its AlarmRule string (e.g. ALARM("cdkd-getatt-chain-alarm")), which is a plain string with no Ref / Fn::GetAtt, so cdkd's DAG saw no dependency edge and could schedule the referenced metric alarm for deletion while the composite still existed. CloudWatch rejects that with "Cannot delete <alarm> as there are composite alarm(s) depending on it." and the destroy failed. Add a per-resource implicit delete-ordering edge: parse each CompositeAlarm's AlarmRule for referenced alarm names (ALARM / OK / INSUFFICIENT_DATA tokens, bare or quoted, plus the arn:...:alarm:<name> form) and emit an edge making the composite alarm delete BEFORE every metric/composite Alarm it references in the same stack (matched by AlarmName property or physical id). The per-AlarmRule edge handles composite-of-composite chains. Both delete consumers (the deploy engine DELETE phase and the standalone destroy command) add these edges alongside the existing type-pair rules. Adds computeImplicitDeleteEdges / extractReferencedAlarmNames to src/analyzer/implicit-delete-deps.ts plus unit tests for the parser and the edge computation (bare/quoted/ARN names, multi-reference rules, composite-of-composite, self-cycle guard, references outside the delete set).
1 parent 19f6947 commit bacfa1b

5 files changed

Lines changed: 327 additions & 4 deletions

File tree

.claude/rules/analyzer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,5 @@ paths:
3232
- Determines execution order with topological sort
3333
- **Implicit edge for Custom Resources**: any `AWS::IAM::Policy` / `AWS::IAM::RolePolicy` / `AWS::IAM::ManagedPolicy` attached to a Custom Resource's ServiceToken Lambda execution role automatically gets an edge to the Custom Resource, preventing the handler from being invoked before inline policy attachment returns (avoids mid-deploy AccessDenied race)
3434
- **Implicit edge for Lambda VpcConfig**: every `AWS::EC2::Subnet` / `AWS::EC2::SecurityGroup` referenced by a Lambda's `Properties.VpcConfig.SubnetIds` / `SecurityGroupIds` gets an explicit edge to the Lambda (`src/analyzer/lambda-vpc-deps.ts`). Defense-in-depth on top of `extractDependencies`; for the reversed deletion traversal this guarantees Lambda is removed before its Subnet/SG so the asynchronous ENI detach has time to complete before EC2 rejects the subnet/SG delete with `DependencyViolation`.
35-
- **Type-based deletion ordering rules**: `src/analyzer/implicit-delete-deps.ts` centralizes type-pair rules (e.g. VPC after Subnet, Subnet after Lambda, IGW + VPCGatewayAttachment after NatGateway) shared by the deploy DELETE phase and the standalone destroy command. The IGW / VPCGatewayAttachment after NatGateway edge (issue [#817](https://github.com/go-to-k/cdkd/issues/817)) mirrors the NAT-before-IGW ordering CloudFormation enforces: a NAT Gateway holds an Elastic IP mapped to the VPC's public address space, so detaching the IGW before the NAT is gone fails with `Network vpc-xxx has some mapped public address(es)` and the IGW delete then hangs (~19 min observed). No type-based rule is needed for the EIP itself — the NAT Ref's its EIP via `AllocationId`, so the reversed delete traversal already deletes the NAT before the EIP is released.
35+
- **Type-based deletion ordering rules**: `src/analyzer/implicit-delete-deps.ts` centralizes type-pair rules (e.g. VPC after Subnet, Subnet after Lambda, IGW + VPCGatewayAttachment after NatGateway) shared by the deploy DELETE phase and the standalone destroy command. The IGW / VPCGatewayAttachment after NatGateway edge (issue [#817](https://github.com/go-to-k/cdkd/issues/817)) mirrors the NAT-before-IGW ordering CloudFormation enforces: a NAT Gateway holds an Elastic IP mapped to the VPC's public address space, so detaching the IGW before the NAT is gone fails with `Network vpc-xxx has some mapped public address(es)` and the IGW delete then hangs (~19 min observed). No type-based rule is needed for the EIP itself — the NAT Ref's its EIP via `AllocationId`, so the reversed delete traversal already deletes the NAT before the EIP is released. The same module also exposes `computeImplicitDeleteEdges(resources)` for per-RESOURCE delete-ordering edges no type-pair rule can express: an `AWS::CloudWatch::CompositeAlarm` references its child alarms (metric `AWS::CloudWatch::Alarm` or other composite alarms) by NAME inside its `AlarmRule` string (`ALARM("name")` / `OK(name)` / `INSUFFICIENT_DATA(name)`, plus the `arn:...:alarm:<name>` form) — a plain string, so cdkd's DAG sees no `Ref` / `Fn::GetAtt` edge. `extractReferencedAlarmNames` parses those names and the helper emits an edge making the composite alarm delete BEFORE each referenced alarm (matched by `AlarmName` property or physical id), since CloudWatch rejects deleting a metric alarm while a composite alarm still references it (`Cannot delete <alarm> as there are composite alarm(s) depending on it.`). The per-AlarmRule edge handles composite-of-composite chains; both delete consumers add these edges alongside the type-pair rules.
3636
- **CDK-defensive DependsOn relaxation (default-on)**: `src/analyzer/cdk-defensive-deps.ts` lists the (depender, dependee) type pairs CDK adds defensively for VPC-Lambda runtime egress (IAM Role / Policy / Lambda::Function / Lambda::Url / Lambda::EventSourceMapping → EC2 Route / SubnetRouteTableAssociation). The deploy code path constructs `DagBuilder({ relaxCdkVpcDefensiveDeps: true })` by default; the matching DependsOn edges are dropped at graph-build time so CloudFront Distribution + Lambda::Url + VPC Lambda dispatch in parallel with NAT Gateway stabilization (~55% faster on `bench-cdk-sample`). Pass `cdkd deploy --no-aggressive-vpc-parallel` to opt out (escape hatch for stacks where the user wants the strict CDK-defensive ordering — e.g. a Custom Resource that synchronously invokes a VPC Lambda outside cdkd's Lambda-ServiceToken Active wait). Only DependsOn entries in the allowlist are dropped — Ref / GetAtt and other DependsOn pairs are untouched.

src/analyzer/implicit-delete-deps.ts

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,137 @@ export const IMPLICIT_DELETE_DEPENDENCIES: Record<string, readonly string[]> = {
7474
'AWS::Lambda::Function',
7575
],
7676
};
77+
78+
/**
79+
* A single implicit delete-ordering edge: the resource at `before` (logical id)
80+
* must finish deleting BEFORE the resource at `after` (logical id).
81+
*
82+
* Unlike {@link IMPLICIT_DELETE_DEPENDENCIES} (which expresses ordering between
83+
* TYPES, so every instance of type X orders against every instance of type Y),
84+
* these edges are computed per-RESOURCE — they are derived from the actual
85+
* references one resource carries, so they can express "this specific composite
86+
* alarm before this specific metric alarm" without forcing an all-pairs rule.
87+
*/
88+
export interface ImplicitDeleteEdge {
89+
/** Logical id of the resource that must be deleted first. */
90+
before: string;
91+
/** Logical id of the resource that must be deleted after `before`. */
92+
after: string;
93+
}
94+
95+
/**
96+
* A resource as seen by the delete-ordering computation. Both the deploy DELETE
97+
* phase (`StackState.resources[id]`) and the standalone destroy command shape
98+
* carry these fields, so this is the common subset both call sites can pass.
99+
*/
100+
export interface DeleteOrderingResource {
101+
resourceType: string;
102+
physicalId?: string;
103+
properties?: Record<string, unknown>;
104+
}
105+
106+
/**
107+
* Matches one alarm-state function token in a CompositeAlarm `AlarmRule`:
108+
* ALARM("name") | OK('name') | INSUFFICIENT_DATA(name)
109+
* The argument is captured raw (with any surrounding quotes) and trimmed /
110+
* unquoted by {@link extractReferencedAlarmNames}. CloudWatch also accepts the
111+
* boolean literals TRUE / FALSE which carry no argument, so they never match.
112+
*/
113+
const ALARM_RULE_FUNCTION_REGEX =
114+
/\b(?:ALARM|OK|INSUFFICIENT_DATA)\s*\(\s*([^)]*?)\s*\)/gi;
115+
116+
/**
117+
* Extract every alarm NAME (or ARN) referenced by a CompositeAlarm `AlarmRule`
118+
* string. The rule references its child alarms by NAME (or ARN) as a plain
119+
* string — there is no `Ref` / `Fn::GetAtt`, so cdkd's DAG sees no dependency
120+
* edge from these references. We parse them out so a delete-ordering edge can be
121+
* synthesized (CloudWatch refuses to delete a metric alarm while a composite
122+
* alarm still references it).
123+
*
124+
* Handles the three alarm-state functions (`ALARM` / `OK` / `INSUFFICIENT_DATA`)
125+
* and both the bare-name and quoted-name forms. An ARN argument
126+
* (`arn:aws:cloudwatch:...:alarm:<name>`) is reduced to its trailing `<name>`
127+
* so it can be matched against a referenced alarm's `AlarmName` / physical id
128+
* the same way a bare name is.
129+
*/
130+
export function extractReferencedAlarmNames(alarmRule: string): string[] {
131+
const names = new Set<string>();
132+
for (const match of alarmRule.matchAll(ALARM_RULE_FUNCTION_REGEX)) {
133+
let arg = (match[1] ?? '').trim();
134+
if (arg.length === 0) continue;
135+
// Strip a single pair of surrounding quotes (single or double).
136+
if (
137+
(arg.startsWith('"') && arg.endsWith('"')) ||
138+
(arg.startsWith("'") && arg.endsWith("'"))
139+
) {
140+
arg = arg.slice(1, -1);
141+
}
142+
if (arg.length === 0) continue;
143+
// ARN form: arn:aws:cloudwatch:<region>:<acct>:alarm:<name> — reduce to the
144+
// trailing name so it matches an AlarmName / physical id.
145+
const arnAlarmMatch = /:alarm:(.+)$/.exec(arg);
146+
if (arnAlarmMatch?.[1]) {
147+
names.add(arnAlarmMatch[1]);
148+
} else {
149+
names.add(arg);
150+
}
151+
}
152+
return [...names];
153+
}
154+
155+
/**
156+
* Compute per-resource delete-ordering edges that cannot be inferred from
157+
* Ref / Fn::GetAtt edges or from the type-pair {@link IMPLICIT_DELETE_DEPENDENCIES}
158+
* table.
159+
*
160+
* Currently this synthesizes edges for `AWS::CloudWatch::CompositeAlarm`: a
161+
* composite alarm references its child alarms (metric `AWS::CloudWatch::Alarm`
162+
* or other composite alarms) by NAME inside its `AlarmRule` string. Because the
163+
* reference is a plain string (no `Ref` / `Fn::GetAtt`), cdkd's DAG sees no
164+
* dependency edge, so without this the metric alarm can be scheduled for
165+
* deletion while the composite still exists — and CloudWatch rejects that with
166+
* `Cannot delete <alarm> as there are composite alarm(s) depending on it.`
167+
* We therefore emit an edge making the composite alarm delete BEFORE every
168+
* alarm its `AlarmRule` references (handling composite-of-composite too).
169+
*
170+
* @param resources logical id -> resource (the subset of resources participating
171+
* in the delete). Only entries whose logical id is a key in this record are
172+
* considered as edge endpoints.
173+
*/
174+
export function computeImplicitDeleteEdges(
175+
resources: Record<string, DeleteOrderingResource>
176+
): ImplicitDeleteEdge[] {
177+
const edges: ImplicitDeleteEdge[] = [];
178+
179+
// Index alarm resources (metric + composite) by the name a CompositeAlarm's
180+
// AlarmRule would reference them by: their AlarmName property if set, else
181+
// their physical id (CloudWatch alarm physical id IS the alarm name).
182+
const alarmTypes = new Set(['AWS::CloudWatch::Alarm', 'AWS::CloudWatch::CompositeAlarm']);
183+
const nameToLogicalId = new Map<string, string>();
184+
for (const [logicalId, resource] of Object.entries(resources)) {
185+
if (!alarmTypes.has(resource.resourceType)) continue;
186+
const alarmName =
187+
typeof resource.properties?.['AlarmName'] === 'string'
188+
? (resource.properties['AlarmName'] as string)
189+
: resource.physicalId;
190+
if (alarmName) nameToLogicalId.set(alarmName, logicalId);
191+
}
192+
193+
for (const [logicalId, resource] of Object.entries(resources)) {
194+
if (resource.resourceType !== 'AWS::CloudWatch::CompositeAlarm') continue;
195+
const alarmRule = resource.properties?.['AlarmRule'];
196+
if (typeof alarmRule !== 'string') continue;
197+
198+
for (const referencedName of extractReferencedAlarmNames(alarmRule)) {
199+
const referencedLogicalId = nameToLogicalId.get(referencedName);
200+
// Skip names we can't resolve to a same-stack resource and skip a
201+
// self-reference (a composite alarm cannot reference itself, but guard
202+
// against it so we never emit a self-cycle).
203+
if (!referencedLogicalId || referencedLogicalId === logicalId) continue;
204+
// The composite (logicalId) must be deleted BEFORE the referenced alarm.
205+
edges.push({ before: logicalId, after: referencedLogicalId });
206+
}
207+
}
208+
209+
return edges;
210+
}

src/cli/commands/destroy-runner.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import { setAwsClients, AwsClients } from '../../utils/aws-clients.js';
77
import type { S3StateBackend } from '../../state/s3-state-backend.js';
88
import type { LockManager } from '../../state/lock-manager.js';
99
import { DagBuilder } from '../../analyzer/dag-builder.js';
10-
import { IMPLICIT_DELETE_DEPENDENCIES } from '../../analyzer/implicit-delete-deps.js';
10+
import {
11+
IMPLICIT_DELETE_DEPENDENCIES,
12+
computeImplicitDeleteEdges,
13+
} from '../../analyzer/implicit-delete-deps.js';
1114
import { ProviderRegistry } from '../../provisioning/provider-registry.js';
1215
import { registerAllProviders } from '../../provisioning/register-providers.js';
1316
import { shouldRetainResource, type ResourceState, type StackState } from '../../types/state.js';
@@ -645,6 +648,25 @@ export async function runDestroyForStack(
645648
}
646649
}
647650

651+
// Per-resource implicit delete edges that cannot be inferred from a
652+
// type-pair rule (e.g. CompositeAlarm -> the metric alarms its AlarmRule
653+
// references by name, which carry no Ref / Fn::GetAtt edge). `before` must
654+
// be deleted before `after`, so `before` DependsOn `after` (creation order
655+
// is reversed for deletion, so `before` is torn down first).
656+
for (const { before, after } of computeImplicitDeleteEdges(state.resources)) {
657+
const existing = template.Resources[before]?.DependsOn ?? [];
658+
const depsArray = Array.isArray(existing) ? existing : [existing];
659+
if (!depsArray.includes(after)) {
660+
template.Resources[before] = {
661+
...template.Resources[before]!,
662+
DependsOn: [...depsArray, after],
663+
};
664+
logger.debug(
665+
`Implicit delete dependency: ${before} (${state.resources[before]?.resourceType}) must be deleted before ${after} (${state.resources[after]?.resourceType})`
666+
);
667+
}
668+
}
669+
648670
const dagBuilder = new DagBuilder();
649671
const graph = dagBuilder.buildGraph(template);
650672
const executionLevels = dagBuilder.getExecutionLevels(graph);

src/deployment/deploy-engine.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ import type { DagBuilder } from '../analyzer/dag-builder.js';
2828
import type { DiffCalculator } from '../analyzer/diff-calculator.js';
2929
import { ProviderRegistry } from '../provisioning/provider-registry.js';
3030
import { TemplateParser } from '../analyzer/template-parser.js';
31-
import { IMPLICIT_DELETE_DEPENDENCIES } from '../analyzer/implicit-delete-deps.js';
31+
import {
32+
IMPLICIT_DELETE_DEPENDENCIES,
33+
computeImplicitDeleteEdges,
34+
} from '../analyzer/implicit-delete-deps.js';
3235
import { withRetry } from './retry.js';
3336
import { withResourceDeadline } from './resource-deadline.js';
3437

@@ -2563,6 +2566,26 @@ export class DeployEngine {
25632566
}
25642567
}
25652568
}
2569+
2570+
// Per-resource implicit delete edges that cannot be inferred from a
2571+
// type-pair rule (e.g. CompositeAlarm -> the metric alarms its AlarmRule
2572+
// references by name, which carry no Ref / Fn::GetAtt edge).
2573+
const scoped: Record<string, ResourceState> = {};
2574+
for (const id of deleteIds) {
2575+
const resource = state.resources[id];
2576+
if (resource) scoped[id] = resource;
2577+
}
2578+
for (const { before, after } of computeImplicitDeleteEdges(scoped)) {
2579+
// `before` must be deleted before `after`, so `before` is in `after`'s
2580+
// deletion deps (picked / deleted first).
2581+
if (!dependedBy.has(after)) dependedBy.set(after, new Set());
2582+
if (!dependedBy.get(after)!.has(before)) {
2583+
dependedBy.get(after)!.add(before);
2584+
this.logger.debug(
2585+
`Implicit delete dependency: ${before} (${scoped[before]?.resourceType}) must be deleted before ${after} (${scoped[after]?.resourceType})`
2586+
);
2587+
}
2588+
}
25662589
}
25672590

25682591
/**

0 commit comments

Comments
 (0)