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
99 changes: 99 additions & 0 deletions src/cli/aws/__tests__/target-region.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { applyTargetRegionToEnv, withTargetRegion } from '../target-region.js';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';

describe('target-region', () => {
let savedRegion: string | undefined;
let savedDefaultRegion: string | undefined;

beforeEach(() => {
savedRegion = process.env.AWS_REGION;
savedDefaultRegion = process.env.AWS_DEFAULT_REGION;
delete process.env.AWS_REGION;
delete process.env.AWS_DEFAULT_REGION;
});

afterEach(() => {
if (savedRegion !== undefined) process.env.AWS_REGION = savedRegion;
else delete process.env.AWS_REGION;
if (savedDefaultRegion !== undefined) process.env.AWS_DEFAULT_REGION = savedDefaultRegion;
else delete process.env.AWS_DEFAULT_REGION;
});

describe('applyTargetRegionToEnv', () => {
it('sets AWS_REGION and AWS_DEFAULT_REGION to the provided region', () => {
applyTargetRegionToEnv('ap-southeast-2');
expect(process.env.AWS_REGION).toBe('ap-southeast-2');
expect(process.env.AWS_DEFAULT_REGION).toBe('ap-southeast-2');
});

it('returns a restore function that clears env vars when they were previously unset', () => {
const restore = applyTargetRegionToEnv('eu-west-1');
restore();
expect(process.env.AWS_REGION).toBeUndefined();
expect(process.env.AWS_DEFAULT_REGION).toBeUndefined();
});

it('returns a restore function that restores previous env var values', () => {
process.env.AWS_REGION = 'us-east-1';
process.env.AWS_DEFAULT_REGION = 'us-east-1';

const restore = applyTargetRegionToEnv('ap-south-1');
expect(process.env.AWS_REGION).toBe('ap-south-1');
expect(process.env.AWS_DEFAULT_REGION).toBe('ap-south-1');

restore();
expect(process.env.AWS_REGION).toBe('us-east-1');
expect(process.env.AWS_DEFAULT_REGION).toBe('us-east-1');
});

it('restores each env var independently (only one was previously set)', () => {
process.env.AWS_REGION = 'us-west-2';
// AWS_DEFAULT_REGION intentionally left unset

const restore = applyTargetRegionToEnv('eu-central-1');
expect(process.env.AWS_REGION).toBe('eu-central-1');
expect(process.env.AWS_DEFAULT_REGION).toBe('eu-central-1');

restore();
expect(process.env.AWS_REGION).toBe('us-west-2');
expect(process.env.AWS_DEFAULT_REGION).toBeUndefined();
});
});

describe('withTargetRegion', () => {
it('applies region inside the callback and restores afterwards', async () => {
let seenRegion: string | undefined;
let seenDefaultRegion: string | undefined;

await withTargetRegion('ap-northeast-1', () => {
seenRegion = process.env.AWS_REGION;
seenDefaultRegion = process.env.AWS_DEFAULT_REGION;
return Promise.resolve();
});

expect(seenRegion).toBe('ap-northeast-1');
expect(seenDefaultRegion).toBe('ap-northeast-1');
expect(process.env.AWS_REGION).toBeUndefined();
expect(process.env.AWS_DEFAULT_REGION).toBeUndefined();
});

it('restores env vars even when the callback throws', async () => {
process.env.AWS_REGION = 'us-east-1';

await expect(
withTargetRegion('sa-east-1', () => {
expect(process.env.AWS_REGION).toBe('sa-east-1');
return Promise.reject(new Error('boom'));
})
).rejects.toThrow('boom');

expect(process.env.AWS_REGION).toBe('us-east-1');
expect(process.env.AWS_DEFAULT_REGION).toBeUndefined();
});

it('returns the callback result', async () => {
const result = await withTargetRegion('eu-west-2', () => Promise.resolve(42));
expect(result).toBe(42);
});
});
});
1 change: 1 addition & 0 deletions src/cli/aws/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export { detectAwsContext, type AwsContext } from './aws-context';
export { detectAccount, getCredentialProvider } from './account';
export { detectRegion, type RegionDetectionResult } from './region';
export { applyTargetRegionToEnv, withTargetRegion } from './target-region';
export {
invokeBedrockSync,
invokeClaude,
Expand Down
55 changes: 55 additions & 0 deletions src/cli/aws/target-region.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Make a deployment target's region authoritative for downstream AWS SDK calls.
*
* The AWS SDK (and CDK toolkit-lib's internal clients) resolve region from
* AWS_REGION / AWS_DEFAULT_REGION when constructed without an explicit `region`
* option. aws-targets.json is the user's source of truth for where resources
* should be created, so we promote the target's region onto the environment for
* the operation and restore any prior values afterwards.
*
* Without this override, a user with a non-default region in aws-targets.json
* but no AWS_DEFAULT_REGION set would see resources created in the SDK's default
* region — see https://github.com/aws/agentcore-cli/issues/924.
*/

type RestoreEnv = () => void;

/**
* Set AWS_REGION / AWS_DEFAULT_REGION to `region` and return a restore function.
* Callers that cannot wrap their work in a callback (e.g. CLI entrypoints that
* span many helpers) should use this, and invoke the returned function in a
* `finally` block.
*/
export function applyTargetRegionToEnv(region: string): RestoreEnv {
const prevRegion = process.env.AWS_REGION;
const prevDefaultRegion = process.env.AWS_DEFAULT_REGION;

process.env.AWS_REGION = region;
process.env.AWS_DEFAULT_REGION = region;

return () => {
if (prevRegion === undefined) {
delete process.env.AWS_REGION;
} else {
process.env.AWS_REGION = prevRegion;
}
if (prevDefaultRegion === undefined) {
delete process.env.AWS_DEFAULT_REGION;
} else {
process.env.AWS_DEFAULT_REGION = prevDefaultRegion;
}
};
}

/**
* Run `fn` with `region` applied to AWS_REGION / AWS_DEFAULT_REGION, restoring
* the prior values on return (including when `fn` throws).
*/
export async function withTargetRegion<T>(region: string, fn: () => Promise<T>): Promise<T> {
const restore = applyTargetRegionToEnv(region);
try {
return await fn();
} finally {
restore();
}
}
7 changes: 7 additions & 0 deletions src/cli/commands/deploy/actions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ConfigIO, SecureCredentials } from '../../../lib';
import type { AgentCoreMcpSpec, DeployedState } from '../../../schema';
import { applyTargetRegionToEnv } from '../../aws';
import { validateAwsCredentials } from '../../aws/account';
import { createSwitchableIoHost } from '../../cdk/toolkit-lib';
import {
Expand Down Expand Up @@ -48,6 +49,7 @@ const MEMORY_ONLY_NEXT_STEPS = ['agentcore add agent', 'agentcore status'];

export async function handleDeploy(options: ValidatedDeployOptions): Promise<DeployResult> {
let toolkitWrapper = null;
let restoreEnv: (() => void) | null = null;
const logger = new ExecLogger({ command: 'deploy' });
const { onProgress } = options;
let currentStepName = '';
Expand Down Expand Up @@ -79,6 +81,10 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
logPath: logger.getRelativeLogPath(),
};
}
// Make the resolved target region authoritative for downstream SDK / CDK
// calls that don't receive an explicit region option.
// See https://github.com/aws/agentcore-cli/issues/924.
restoreEnv = applyTargetRegionToEnv(target.region);
endStep('success');

// Read project spec for gateway information (used later for deploy step name and outputs)
Expand Down Expand Up @@ -485,5 +491,6 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep
if (toolkitWrapper) {
await toolkitWrapper.dispose();
}
restoreEnv?.();
}
}
19 changes: 13 additions & 6 deletions src/cli/operations/deploy/teardown.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { CONFIG_DIR, ConfigIO } from '../../../lib';
import type { AwsDeploymentTarget } from '../../../schema';
import { withTargetRegion } from '../../aws';
import { CdkToolkitWrapper, silentIoHost } from '../../cdk/toolkit-lib';
import { type DiscoveredStack, findStack } from '../../cloudformation/stack-discovery';
import { StackSelectionStrategy } from '@aws-cdk/toolkit-lib';
Expand Down Expand Up @@ -60,12 +61,18 @@ export async function destroyTarget(options: DestroyTargetOptions): Promise<void
ioHost: silentIoHost,
});

await toolkit.initialize();
await toolkit.destroy({
stacks: {
strategy: StackSelectionStrategy.PATTERN_MUST_MATCH,
patterns: [target.stack.stackName],
},
// aws-targets.json is authoritative for the destroy region; promote it onto
// the env so CDK toolkit-lib's internal SDK clients hit the right region even
// when AWS_REGION / AWS_DEFAULT_REGION are unset.
// See https://github.com/aws/agentcore-cli/issues/924.
await withTargetRegion(target.target.region, async () => {
await toolkit.initialize();
await toolkit.destroy({
stacks: {
strategy: StackSelectionStrategy.PATTERN_MUST_MATCH,
patterns: [target.stack.stackName],
},
});
});

// Clean up deployed-state.json after successful destroy
Expand Down
44 changes: 40 additions & 4 deletions src/cli/tui/hooks/useCdkPreflight.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ConfigIO, SecureCredentials } from '../../../lib';
import type { DeployedState } from '../../../schema';
import { applyTargetRegionToEnv } from '../../aws';
import { AwsCredentialsError, validateAwsCredentials } from '../../aws/account';
import { type CdkToolkitWrapper, type SwitchableIoHost, createSwitchableIoHost } from '../../cdk/toolkit-lib';
import { getErrorMessage, isExpiredTokenError, isNoCredentialsError } from '../../errors';
Expand Down Expand Up @@ -137,6 +138,11 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
const isRunningRef = useRef(false);
// Keep a ref to the wrapper so we can dispose it when starting a new run
const wrapperRef = useRef<CdkToolkitWrapper | null>(null);
// Restore function for AWS_REGION / AWS_DEFAULT_REGION overrides, applied
// after target resolution so downstream SDK / CDK toolkit-lib clients use the
// aws-targets.json region rather than whatever the SDK default chain resolves.
// See https://github.com/aws/agentcore-cli/issues/924.
const restoreRegionEnvRef = useRef<(() => void) | null>(null);

const updateStep = (index: number, update: Partial<Step>) => {
setSteps(prev => prev.map((s, i) => (i === index ? { ...s, ...update } : s)));
Expand All @@ -158,18 +164,26 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
}
}, []);

// Restore AWS_REGION / AWS_DEFAULT_REGION (no-op when nothing was applied)
const restoreRegionEnv = useCallback(() => {
restoreRegionEnvRef.current?.();
restoreRegionEnvRef.current = null;
}, []);

const startPreflight = useCallback(async () => {
if (isRunningRef.current) return;
// Dispose any existing wrapper before starting a new run
await disposeWrapper();
// Restore any previously-applied region env override before re-running
restoreRegionEnv();
resetSteps();
setCdkToolkitWrapper(null);
setStackNames([]);
setBootstrapContext(null);
setHasTokenExpiredError(false); // Reset token expired state when retrying
setHasCredentialsError(false); // Reset credentials error state when retrying
setPhase('running');
}, [disposeWrapper]);
}, [disposeWrapper, restoreRegionEnv]);

const clearTokenExpiredError = useCallback(() => {
setHasTokenExpiredError(false);
Expand All @@ -183,6 +197,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
useEffect(() => {
const handleInterrupt = () => {
void disposeWrapper();
restoreRegionEnv();
};

process.on('SIGINT', handleInterrupt);
Expand All @@ -193,8 +208,19 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
process.off('SIGTERM', handleInterrupt);
// Dispose on unmount (user navigated away)
void disposeWrapper();
restoreRegionEnv();
};
}, [disposeWrapper]);
}, [disposeWrapper, restoreRegionEnv]);

// Restore region env override when any preflight stage lands in 'error'.
// Individual error branches inside the stage effects only call setPhase('error')
// without cleanup, so this hook is the single place that guarantees restore
// happens on every error path without threading the call into every branch.
useEffect(() => {
if (phase === 'error') {
restoreRegionEnv();
}
}, [phase, restoreRegionEnv]);

const confirmTeardown = useCallback(() => {
// Mark teardown as confirmed and restart the preflight flow
Expand All @@ -206,7 +232,8 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
const cancelTeardown = useCallback(() => {
setPhase('error');
isRunningRef.current = false;
}, []);
restoreRegionEnv();
}, [restoreRegionEnv]);

const confirmBootstrap = useCallback(() => {
setPhase('bootstrapping');
Expand Down Expand Up @@ -274,6 +301,15 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
try {
preflightContext = await validateProject();
setContext(preflightContext);
// Make aws-targets.json region authoritative for downstream SDK / CDK
// toolkit-lib clients that bypass explicit region options. Restored on
// unmount, teardown rejection, or subsequent preflight start.
// See https://github.com/aws/agentcore-cli/issues/924.
const firstTarget = preflightContext.awsTargets[0];
if (firstTarget) {
restoreRegionEnv();
restoreRegionEnvRef.current = applyTargetRegionToEnv(firstTarget.region);
}
logger.endStep('success');
updateStep(STEP_VALIDATE, { status: 'success' });
} catch (err) {
Expand Down Expand Up @@ -493,7 +529,7 @@ export function useCdkPreflight(options: PreflightOptions): PreflightResult {
return () => {
process.off('unhandledRejection', handleUnhandledRejection);
};
}, [phase, logger, switchableIoHost, isInteractive, skipIdentityCheck, teardownConfirmed]);
}, [phase, logger, switchableIoHost, isInteractive, skipIdentityCheck, teardownConfirmed, restoreRegionEnv]);

// Handle identity-setup phase (after user provides credentials)
useEffect(() => {
Expand Down
Loading