From eb4ea4ffb090cb0dd8182987eab9ec609a9c0469 Mon Sep 17 00:00:00 2001 From: notgitika Date: Thu, 13 Aug 2026 13:04:47 -0400 Subject: [PATCH] fix(exec): derive region from the ARN so --runtime needs no project `agentcore exec --runtime ` failed with "AWS Targets config file not found" unless --region was also passed. The ARN short-circuit in loadExecContext was gated on `startsWith('arn:') && options.region`, so omitting --region fell through to readAWSDeploymentTargets() / readDeployedState(), which throw before anything else runs. The fall-through branch already existed and was labelled "--runtime with no --region", but it sat after those reads and used config for one thing: `options.region ?? targetConfig.region`. That region is field 3 of the ARN the caller just supplied, so exec was demanding a project, an aws-targets.json and a completed deploy to recover a value it already had. This blocks exec for anyone deploying runtimes outside the CLI (CDK, pipelines, personal stacks), who have no reason to own an agentcore project at all. Parse the region from the ARN instead and drop the --region requirement from both the --runtime and --harness short-circuits. Config is now read only when a *name* needs resolving, or when the ARN's region field is empty or malformed. An explicit --region still wins. regionFromArn moves from operations/jobs/shared/region to a new cli/aws/arn module so exec does not have to depend on operations/jobs; the jobs path re-exports it. It lives apart from cli/aws/region because that module detects the ambient region via env and shared config files, while this is a pure function over an ARN the caller already holds -- and several jobs tests replace cli/aws/region wholesale with a detectRegion-only factory mock. --- src/cli/aws/arn.ts | 17 +++++ .../commands/exec/__tests__/action.test.ts | 68 +++++++++++++++++-- src/cli/commands/exec/action.ts | 27 ++++++-- src/cli/operations/jobs/shared/region.ts | 15 ++-- 4 files changed, 103 insertions(+), 24 deletions(-) create mode 100644 src/cli/aws/arn.ts diff --git a/src/cli/aws/arn.ts b/src/cli/aws/arn.ts new file mode 100644 index 000000000..5f132ed79 --- /dev/null +++ b/src/cli/aws/arn.ts @@ -0,0 +1,17 @@ +/** + * ARN parsing helpers. + * + * Kept separate from region.ts: that module *detects* the ambient region from the environment and + * shared config files, while these are pure string functions over an ARN the caller already holds. + */ + +/** + * Parse the region out of a service ARN. + * ARN format: arn:{partition}:{service}:{region}:{account}:{resource} → field index 3 is the region. + * Splitting on ':' rather than matching a partition keeps this correct for GovCloud and China ARNs. + * Returns undefined for a malformed or region-less ARN so callers can fall back. + */ +export function regionFromArn(arn: string): string | undefined { + const region = arn.split(':')[3]; + return region && region.length > 0 ? region : undefined; +} diff --git a/src/cli/commands/exec/__tests__/action.test.ts b/src/cli/commands/exec/__tests__/action.test.ts index a33bfbf59..e8d4c57d8 100644 --- a/src/cli/commands/exec/__tests__/action.test.ts +++ b/src/cli/commands/exec/__tests__/action.test.ts @@ -767,13 +767,55 @@ describe('loadExecContext --runtime as ARN or name', () => { ).not.toHaveBeenCalled(); }); - it('resolves region from config when --runtime is a full ARN but --region is omitted', async () => { + // A full ARN already carries its region, so exec must not need a project to recover it. Reading + // config here used to fail outright with "AWS Targets config file not found" for anyone deploying + // runtimes outside this CLI (CDK, pipelines, personal stacks) — the ARN was enough all along. + it('takes the region from the ARN when --region is omitted, without reading config', async () => { + // Config reads throw, standing in for "no project / no aws-targets.json in cwd". + const noProject = { + readAWSDeploymentTargets: vi.fn().mockRejectedValue(new Error('AWS Targets config file not found')), + readDeployedState: vi.fn().mockRejectedValue(new Error('State config file not found')), + } as unknown as ConfigIO; + + const ctx = await loadExecContext({ runtimeArn: 'arn:aws:bedrock-agentcore:eu-west-2:123:runtime/X' }, noProject); + + expect(ctx.region).toBe('eu-west-2'); // from the ARN, not config + expect(ctx.runtimeArn).toBe('arn:aws:bedrock-agentcore:eu-west-2:123:runtime/X'); + expect( + (noProject as unknown as { readAWSDeploymentTargets: ReturnType }).readAWSDeploymentTargets + ).not.toHaveBeenCalled(); + expect( + (noProject as unknown as { readDeployedState: ReturnType }).readDeployedState + ).not.toHaveBeenCalled(); + }); + + it('lets an explicit --region override the region in the ARN', async () => { const ctx = await loadExecContext( - { runtimeArn: 'arn:aws:bedrock-agentcore:us-east-1:123:runtime/X' }, + { runtimeArn: 'arn:aws:bedrock-agentcore:us-east-1:123:runtime/X', region: 'us-west-2' }, TWO_AGENT_CONFIG ); + expect(ctx.region).toBe('us-west-2'); + }); + + // Region is field 3 regardless of partition, so GovCloud and China ARNs resolve the same way. + it.each([ + ['arn:aws-us-gov:bedrock-agentcore:us-gov-west-1:123:runtime/X', 'us-gov-west-1'], + ['arn:aws-cn:bedrock-agentcore:cn-north-1:123:runtime/X', 'cn-north-1'], + ])('parses the region out of a non-commercial partition ARN (%s)', async (arn, expected) => { + const throwing = { + readAWSDeploymentTargets: vi.fn().mockRejectedValue(new Error('should not be read')), + readDeployedState: vi.fn().mockRejectedValue(new Error('should not be read')), + } as unknown as ConfigIO; + + const ctx = await loadExecContext({ runtimeArn: arn }, throwing); + expect(ctx.region).toBe(expected); + }); + + // An ARN with an empty region field carries no region to use, so config remains the fallback. + it('falls back to config when the ARN has no region field', async () => { + const ctx = await loadExecContext({ runtimeArn: 'arn:aws:bedrock-agentcore::123:runtime/X' }, TWO_AGENT_CONFIG); expect(ctx.region).toBe('us-east-1'); // from config - expect(ctx.runtimeArn).toBe('arn:aws:bedrock-agentcore:us-east-1:123:runtime/X'); + expect(ctx.runtimeArn).toBe('arn:aws:bedrock-agentcore::123:runtime/X'); }); it('resolves runtimeArn when --runtime is an agent name', async () => { @@ -858,10 +900,22 @@ describe('loadExecContext with harnesses', () => { ).not.toHaveBeenCalled(); }); - it('resolves region from config for --harness when --region is omitted', async () => { - const ctx = await loadExecContext({ harnessName: HARNESS_ARN }, HARNESS_ONLY_CONFIG); - expect(ctx.runtimeArn).toBe(HARNESS_ARN); - expect(ctx.region).toBe('us-east-1'); // from config target + it('takes the region from a --harness when --region is omitted, without reading config', async () => { + const noProject = { + readAWSDeploymentTargets: vi.fn().mockRejectedValue(new Error('AWS Targets config file not found')), + readDeployedState: vi.fn().mockRejectedValue(new Error('State config file not found')), + } as unknown as ConfigIO; + + const ctx = await loadExecContext( + { harnessName: 'arn:aws:bedrock-agentcore:eu-west-2:123:harness/h1-abc' }, + noProject + ); + + expect(ctx.runtimeArn).toBe('arn:aws:bedrock-agentcore:eu-west-2:123:harness/h1-abc'); + expect(ctx.region).toBe('eu-west-2'); // from the ARN, not config + expect( + (noProject as unknown as { readAWSDeploymentTargets: ReturnType }).readAWSDeploymentTargets + ).not.toHaveBeenCalled(); }); it('rejects a runtime ARN passed to --harness (with --region)', async () => { diff --git a/src/cli/commands/exec/action.ts b/src/cli/commands/exec/action.ts index 8fb5c1fef..4f20d5b32 100644 --- a/src/cli/commands/exec/action.ts +++ b/src/cli/commands/exec/action.ts @@ -1,5 +1,6 @@ import { ConfigIO } from '../../../lib'; import { executeBashCommand } from '../../aws/agentcore'; +import { regionFromArn } from '../../aws/arn'; import { connectShell, startKeepalive } from '../../aws/connect-shell'; import { ShellChannel, ShellFramer, parseStatusFrame } from '../../aws/shell-framer'; import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js'; @@ -38,6 +39,7 @@ function assertInteractiveHarnessUnsupported(options: ExecOptions, ctx: ExecCont /** Resolve region + runtimeArn from options and/or agentcore.json deployed state. * --runtime accepts either a full ARN (arn:...) or an agent name from deployed state. + * A full ARN resolves with no project and no config on disk; only a *name* needs deployed state. */ export async function loadExecContext(options: ExecOptions, configIO: ConfigIO = new ConfigIO()): Promise { // Mutual exclusion: --runtime and --harness cannot both be set. Checked first so it applies to @@ -46,19 +48,29 @@ export async function loadExecContext(options: ExecOptions, configIO: ConfigIO = throw new Error('Cannot specify both --runtime and --harness.'); } - // Short-circuit: explicit ARN + region — no need to read deployed state - if (options.runtimeArn?.startsWith('arn:') && options.region) { - return assertInteractiveHarnessUnsupported(options, { region: options.region, runtimeArn: options.runtimeArn }); + // Short-circuit: an explicit ARN already carries its region in field 3, so --region is optional. + // Reading config here would demand a project, an aws-targets.json and a completed deploy purely to + // recover a value the caller already supplied — which blocks `exec` for anyone who deploys their + // runtimes outside this CLI (CDK, pipelines, personal stacks). + // A region-less/malformed ARN still falls through so config can supply the region. + if (options.runtimeArn?.startsWith('arn:')) { + const region = options.region ?? regionFromArn(options.runtimeArn); + if (region) { + return assertInteractiveHarnessUnsupported(options, { region, runtimeArn: options.runtimeArn }); + } } - // Same short-circuit for --harness + region. Validate it's a harness ARN (not a runtime ARN). - if (options.harnessName?.startsWith('arn:') && options.region) { + // Same short-circuit for --harness . Validate it's a harness ARN (not a runtime ARN). + if (options.harnessName?.startsWith('arn:')) { if (!isHarnessArn(options.harnessName)) { throw new Error( `--harness expects a harness ARN (…:harness/…), got '${options.harnessName}'. Use --runtime for a runtime ARN.` ); } - return assertInteractiveHarnessUnsupported(options, { region: options.region, runtimeArn: options.harnessName }); + const region = options.region ?? regionFromArn(options.harnessName); + if (region) { + return assertInteractiveHarnessUnsupported(options, { region, runtimeArn: options.harnessName }); + } } const awsTargets = await configIO.readAWSDeploymentTargets(); @@ -85,7 +97,8 @@ export async function loadExecContext(options: ExecOptions, configIO: ConfigIO = const runtimeKeys = Object.keys(targetState?.resources?.runtimes ?? {}); const harnessKeys = Object.keys(targetState?.resources?.harnesses ?? {}); - // --runtime with no --region: ARN provided but region must come from config + // --runtime whose region field is empty or malformed: only reachable when the short-circuit + // above could not derive a region, so config is the last resort. if (options.runtimeArn?.startsWith('arn:')) { return assertInteractiveHarnessUnsupported(options, { region: options.region ?? targetConfig.region, diff --git a/src/cli/operations/jobs/shared/region.ts b/src/cli/operations/jobs/shared/region.ts index 739b9d64f..238c40562 100644 --- a/src/cli/operations/jobs/shared/region.ts +++ b/src/cli/operations/jobs/shared/region.ts @@ -3,8 +3,13 @@ * no regression to either legacy path) and baked into the stored ARN; refresh/stop/archive * parse it back out of the ARN rather than storing a separate field. */ +import { regionFromArn } from '../../../aws/arn'; import { detectRegion } from '../../../aws/region'; +// regionFromArn is shared with exec's target resolution, so it lives in cli/aws/arn. +// Re-exported here to keep the jobs-facing import path stable. +export { regionFromArn }; + /** AWS targets carry a per-target region; we only need that field here. */ interface RegionTarget { region: string; @@ -24,13 +29,3 @@ export async function resolveJobRegion(optsRegion: string | undefined, awsTarget const { region } = await detectRegion(); return region; } - -/** - * Parse the region out of a service ARN. - * ARN format: arn:{partition}:{service}:{region}:{account}:{resource} → field index 3 is the region. - * Engine-created ARNs are always well-formed; returns undefined for a malformed/region-less ARN. - */ -export function regionFromArn(arn: string): string | undefined { - const region = arn.split(':')[3]; - return region && region.length > 0 ? region : undefined; -}