Skip to content

Commit d1ce1ec

Browse files
isadeksisadeks
committed
refactor(agent): make the #58 log-delivery pins an OPT-IN, stack-gated migration shim (no more hardcoded-into-every-stack)
User flagged the hardcode: the #58 fix pinned backgroundagent-dev's live CloudFormation logical IDs + account-unique Names directly into agent.ts, applied UNCONDITIONALLY. The old comment falsely claimed a fresh stack "just deploys clean" via tryFindChild no-op, but the children DO exist on every stack, so it would force ANY stack (a fresh deploy, another account, CI, this code on main) to adopt the dev stack's identities. Wrong. Why the pins exist at all: only a stack deployed BEFORE the agentcore-alpha bump has pre-existing churned resources to collide with (AlreadyExists on create-before-delete). A fresh stack has nothing to collide with and MUST synth the current alpha's natural ids. Fix: PINNED_LOG_DELIVERY_BY_STACK keyed by stackName (only "backgroundagent-dev" listed) plus maybePinChurnedLogResources that applies the overrides ONLY when the running stackName matches (or a "-c pinnedLogDeliveryStack=name" context selects it). Every other stack gets no overrides and a pristine synth. Verified: real backgroundagent-dev synth still pins (deploy stays unblocked); a different stack name synths natural alpha ids. Delete the table entry + helper once the dev stack is migrated. agent.test.ts (44) green.
1 parent 0d5c6b0 commit d1ce1ec

1 file changed

Lines changed: 111 additions & 59 deletions

File tree

‎cdk/src/stacks/agent.ts‎

Lines changed: 111 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -394,65 +394,30 @@ export class AgentStack extends Stack {
394394

395395
runtimeArnHolder = runtime.agentRuntimeArn;
396396

397-
// --- AgentCore log-delivery: pin to the already-deployed identity ---
398-
// The agentcore-alpha Runtime auto-creates AWS::Logs::DeliverySource +
399-
// Delivery + DeliveryDestination for each loggingConfig. A construct-path
400-
// rename across alpha builds CHURNED both the CFN logical IDs AND the
401-
// account-scoped DeliverySource ``Name`` (live stack:
402-
// ``RuntimeCDKSource…``/``cdk-{type}-source-…``; current synth:
403-
// ``RuntimeApplicationLogsDeliverySource…``/``backgroundagentdevRuntime…``).
404-
// Because a DeliverySource Name is account-unique, CFN's create-before-
405-
// delete on the churned id hits ``AlreadyExists`` and rolls the whole stack
406-
// back. Pin the two DeliverySources' logical IDs AND Names back to the
407-
// deployed values so CFN sees them as the SAME resources (no-op update),
408-
// not new ones. Log delivery is stateless routing config — no data risk.
409-
// Best-effort lookup by child id; if a future alpha renames the children
410-
// this silently no-ops (and a fresh stack just deploys clean).
411-
// Pin ALL THREE auto-created resource kinds to their deployed logical IDs:
412-
// - DeliverySource + DeliveryDestination: also pin ``Name`` (both are
413-
// account-unique, so a churned logical id collides on create).
414-
// - Delivery: logical-id pin ONLY. A Delivery has no Name, but it IS
415-
// unique per (source, destination) pair — and since the source+dest
416-
// are now pinned, a churned Delivery logical id would create-before-
417-
// delete a SECOND link over the same pair and hit ``AlreadyExists``
418-
// ('identifier null already exists'). Pinning the logical id makes CFN
419-
// update it in place. (Pass liveName undefined for these.)
420-
const pinLogResource = (childId: string, liveLogicalId: string, liveName?: string): void => {
421-
const res = runtime.node.tryFindChild(childId) as CfnResource | undefined;
422-
if (!res) return;
423-
res.overrideLogicalId(liveLogicalId);
424-
if (liveName !== undefined) res.addPropertyOverride('Name', liveName);
425-
};
426-
pinLogResource(
427-
'ApplicationLogsDeliverySource',
428-
'RuntimeCDKSourceAPPLICATIONLOGSbackgroundagentdevRuntimeBC0AE9ED96A02E02',
429-
'cdk-applicationlogs-source-backgroundagentdevRuntimeBC0AE9ED',
430-
);
431-
pinLogResource(
432-
'UsageLogsDeliverySource',
433-
'RuntimeCDKSourceUSAGELOGSbackgroundagentdevRuntimeBC0AE9ED544FBB22',
434-
'cdk-usagelogs-source-backgroundagentdevRuntimeBC0AE9ED',
435-
);
436-
pinLogResource(
437-
'ApplicationLogsDest',
438-
'RuntimeCdkLogGroupApplicationLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeApplicationLogGroup454A95E8DestapplicationlogsE09F77DC',
439-
'cdk-cwl-Destapplication-logs-dest-backgrounp454A95E829BF8A27',
440-
);
441-
pinLogResource(
442-
'UsageLogsDest',
443-
'RuntimeCdkLogGroupUsageLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeUsageLogGroup7FA1FA67Destusagelogs9AB608D0',
444-
'cdk-cwl-Destusage-logs-dest-backgroundagroup7FA1FA67A8A16CEE',
445-
);
446-
// The two Delivery links: logical-id pin only (no Name — unique per
447-
// source/destination pair, which are now pinned).
448-
pinLogResource(
449-
'ApplicationLogsDelivery',
450-
'RuntimeCdkLogGroupApplicationLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeApplicationLogGroup454A95E8Delivery92FE492C',
451-
);
452-
pinLogResource(
453-
'UsageLogsDelivery',
454-
'RuntimeCdkLogGroupUsageLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeUsageLogGroup7FA1FA67Delivery40F023D7',
455-
);
397+
// --- AgentCore log-delivery: OPT-IN migration shim for ONE pre-existing
398+
// stack whose logical IDs churned under an agentcore-alpha bump ---
399+
//
400+
// Background: the agentcore-alpha Runtime auto-creates AWS::Logs::
401+
// DeliverySource + Delivery + DeliveryDestination per loggingConfig. An
402+
// alpha construct-path rename CHURNED both the CFN logical IDs and the
403+
// account-scoped DeliverySource/DeliveryDestination ``Name`` of an
404+
// ALREADY-DEPLOYED stack. Because those Names are account-unique, CFN's
405+
// create-before-delete on the new ids collides with the live ones →
406+
// ``AlreadyExists`` → whole-stack rollback. The fix is to re-pin the
407+
// churned resources to the values CFN already has so it updates them in
408+
// place instead of recreating.
409+
//
410+
// CRITICAL: this is needed ONLY by a stack that was deployed BEFORE the
411+
// alpha bump. A fresh stack (a new env, CI, this PR on a clean account)
412+
// has NO pre-existing resources to collide with and MUST synth the
413+
// current alpha's natural ids — so the shim is OFF by default and is
414+
// enabled per-stack via context:
415+
// cdk deploy -c pinnedLogDeliveryStack=<stackName>
416+
// (or the `pinnedLogDelivery` map in cdk.json). When the running stack
417+
// doesn't match, NONE of the overrides apply and synth is pristine.
418+
// Once the affected stack has been migrated + a clean redeploy confirmed,
419+
// this shim and its context entry can be deleted outright.
420+
maybePinChurnedLogResources(this, runtime);
456421

457422
// --- Session storage (preview) ---
458423
// The L2 construct does not yet expose filesystemConfigurations; use the
@@ -1303,3 +1268,90 @@ export class AgentStack extends Stack {
13031268
});
13041269
}
13051270
}
1271+
1272+
/**
1273+
* A churned log-delivery resource to re-pin: the construct child id under the
1274+
* Runtime, the logical id CFN already has deployed, and (for the account-unique
1275+
* Source/Destination kinds) the deployed ``Name``. ``liveName`` is omitted for
1276+
* Delivery links, which have no Name.
1277+
*/
1278+
interface PinnedLogResource {
1279+
readonly childId: string;
1280+
readonly liveLogicalId: string;
1281+
readonly liveName?: string;
1282+
}
1283+
1284+
/**
1285+
* Per-stack pin tables for the agentcore-alpha log-delivery churn (#247 #58).
1286+
* Keyed by ``stackName``. ONLY the listed stack is migrated; every other stack
1287+
* (fresh deploys, CI, new envs) is absent here → synth is pristine. A stack can
1288+
* also be supplied at deploy time via context (see {@link maybePinChurnedLogResources}).
1289+
*
1290+
* ``backgroundagent-dev`` was deployed before an alpha bump churned its
1291+
* DeliverySource/Destination/Delivery logical ids + account-unique Names; these
1292+
* values come from `aws cloudformation list-stack-resources` on that live stack.
1293+
* Delete this entry once that stack is migrated + a clean redeploy is confirmed.
1294+
*/
1295+
const PINNED_LOG_DELIVERY_BY_STACK: Record<string, readonly PinnedLogResource[]> = {
1296+
'backgroundagent-dev': [
1297+
{
1298+
childId: 'ApplicationLogsDeliverySource',
1299+
liveLogicalId: 'RuntimeCDKSourceAPPLICATIONLOGSbackgroundagentdevRuntimeBC0AE9ED96A02E02',
1300+
liveName: 'cdk-applicationlogs-source-backgroundagentdevRuntimeBC0AE9ED',
1301+
},
1302+
{
1303+
childId: 'UsageLogsDeliverySource',
1304+
liveLogicalId: 'RuntimeCDKSourceUSAGELOGSbackgroundagentdevRuntimeBC0AE9ED544FBB22',
1305+
liveName: 'cdk-usagelogs-source-backgroundagentdevRuntimeBC0AE9ED',
1306+
},
1307+
{
1308+
childId: 'ApplicationLogsDest',
1309+
liveLogicalId: 'RuntimeCdkLogGroupApplicationLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeApplicationLogGroup454A95E8DestapplicationlogsE09F77DC',
1310+
liveName: 'cdk-cwl-Destapplication-logs-dest-backgrounp454A95E829BF8A27',
1311+
},
1312+
{
1313+
childId: 'UsageLogsDest',
1314+
liveLogicalId: 'RuntimeCdkLogGroupUsageLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeUsageLogGroup7FA1FA67Destusagelogs9AB608D0',
1315+
liveName: 'cdk-cwl-Destusage-logs-dest-backgroundagroup7FA1FA67A8A16CEE',
1316+
},
1317+
// Delivery links: logical-id pin only (no Name — unique per source/dest pair).
1318+
{
1319+
childId: 'ApplicationLogsDelivery',
1320+
liveLogicalId: 'RuntimeCdkLogGroupApplicationLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeApplicationLogGroup454A95E8Delivery92FE492C',
1321+
},
1322+
{
1323+
childId: 'UsageLogsDelivery',
1324+
liveLogicalId: 'RuntimeCdkLogGroupUsageLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeUsageLogGroup7FA1FA67Delivery40F023D7',
1325+
},
1326+
],
1327+
};
1328+
1329+
/**
1330+
* OPT-IN migration shim (#247 #58): re-pin the agentcore-alpha-churned
1331+
* log-delivery resources of ONE already-deployed stack to the logical ids +
1332+
* Names CFN already has, so a stack deployed before an alpha bump updates them
1333+
* in place instead of hitting ``AWS::Logs::DeliverySource AlreadyExists`` on
1334+
* create-before-delete. NO-OP unless the running ``stackName`` is listed in
1335+
* {@link PINNED_LOG_DELIVERY_BY_STACK} OR named via context
1336+
* (`-c pinnedLogDeliveryStack=<name>`, which selects which table entry applies)
1337+
* — so fresh stacks, CI, and other accounts synth the current alpha's natural
1338+
* ids untouched. Once the affected stack is migrated, delete this helper + its
1339+
* table entry.
1340+
*/
1341+
function maybePinChurnedLogResources(stack: Stack, runtime: agentcore.Runtime): void {
1342+
// A deploy can override WHICH stack name to treat as the pinned one (e.g. a
1343+
// renamed env that inherited the churned resources); defaults to the running
1344+
// stack's own name, so the table is matched by stackName out of the box.
1345+
const targetStackName = (stack.node.tryGetContext('pinnedLogDeliveryStack') as string | undefined)
1346+
?? stack.stackName;
1347+
if (targetStackName !== stack.stackName) return; // context names a different stack → don't touch this one
1348+
const pins = PINNED_LOG_DELIVERY_BY_STACK[stack.stackName];
1349+
if (!pins) return; // not a pre-existing churned stack → pristine synth
1350+
1351+
for (const pin of pins) {
1352+
const res = runtime.node.tryFindChild(pin.childId) as CfnResource | undefined;
1353+
if (!res) continue; // a future alpha rename → silently skip (re-derive then)
1354+
res.overrideLogicalId(pin.liveLogicalId);
1355+
if (pin.liveName !== undefined) res.addPropertyOverride('Name', pin.liveName);
1356+
}
1357+
}

0 commit comments

Comments
 (0)