-
Notifications
You must be signed in to change notification settings - Fork 35
feat: multi-environment-deploy #1186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f1d264c
401e381
b1c9b04
d2a56ff
8ce30cf
25eb822
37e11e4
af9dd92
3b8b09c
e718910
6ffe1cf
81d0f93
a1f5472
8bc5c3e
151cc6d
c370204
482d15c
fdaa0fa
9f1dd89
d3becdc
eb71658
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import type { DeployToTargetsOptions } from '../../../operations/deploy/multi-target'; | ||
| import { handleEnvDeploy } from '../actions'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| // Capture every call to deployToTargets so we can assert the orchestrator | ||
| // observes the parallel / continueOnError flags forwarded by handleEnvDeploy. | ||
| const deployToTargetsMock = vi.fn((_targets: unknown, _options: DeployToTargetsOptions, _fn: unknown) => | ||
| Promise.resolve({ successes: [], failures: [] }) | ||
| ); | ||
|
|
||
| vi.mock('../../../operations/deploy/multi-target', () => ({ | ||
| deployToTargets: (targets: unknown, options: DeployToTargetsOptions, fn: unknown) => | ||
| deployToTargetsMock(targets, options, fn), | ||
| })); | ||
|
|
||
| // Stub heavy ConfigIO + project operations so we never touch a real project. | ||
| vi.mock('../../../../lib', () => ({ | ||
| ConfigIO: class { | ||
| readAwsTargetsFull() { | ||
| return Promise.resolve({ | ||
| targets: [{ name: 'dev-a', account: '111111111111', region: 'us-west-2' }], | ||
| environments: { dev: { targets: ['dev-a'] } }, | ||
| }); | ||
| } | ||
| resolveAWSDeploymentTargets() { | ||
| return Promise.resolve([{ name: 'dev-a', account: '111111111111', region: 'us-west-2' }]); | ||
| } | ||
| }, | ||
| SecureCredentials: class {}, | ||
| })); | ||
|
|
||
| vi.mock('../../../operations/deploy', () => ({ | ||
| bootstrapEnvironment: vi.fn(), | ||
| buildCdkProject: vi.fn(), | ||
| checkBootstrapNeeded: vi.fn(), | ||
| checkStackDeployability: vi.fn(), | ||
| getAllCredentials: () => [], | ||
| hasIdentityApiProviders: () => false, | ||
| hasIdentityOAuthProviders: () => false, | ||
| performStackTeardown: vi.fn(), | ||
| setupApiKeyProviders: vi.fn(), | ||
| setupOAuth2Providers: vi.fn(), | ||
| setupTransactionSearch: vi.fn(), | ||
| synthesizeCdk: vi.fn(), | ||
| validateProject: vi.fn(), | ||
| })); | ||
|
|
||
| describe('handleEnvDeploy flag forwarding', () => { | ||
| it('forwards parallel + continueOnError to deployToTargets', async () => { | ||
| deployToTargetsMock.mockClear(); | ||
| await handleEnvDeploy({ | ||
| env: 'dev', | ||
| parallel: true, | ||
| continueOnError: true, | ||
| onLog: () => undefined, | ||
| }); | ||
| expect(deployToTargetsMock).toHaveBeenCalledTimes(1); | ||
| const opts = deployToTargetsMock.mock.calls[0]![1]; | ||
| expect(opts.environmentName).toBe('dev'); | ||
| expect(opts.parallel).toBe(true); | ||
| expect(opts.continueOnError).toBe(true); | ||
| }); | ||
|
|
||
| it('defaults parallel + continueOnError to undefined when not provided', async () => { | ||
| deployToTargetsMock.mockClear(); | ||
| await handleEnvDeploy({ env: 'dev', onLog: () => undefined }); | ||
| expect(deployToTargetsMock).toHaveBeenCalledTimes(1); | ||
| const opts = deployToTargetsMock.mock.calls[0]![1]; | ||
| expect(opts.parallel).toBeUndefined(); | ||
| expect(opts.continueOnError).toBeUndefined(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| import { ConfigIO, SecureCredentials } from '../../../lib'; | ||
| import type { AgentCoreMcpSpec, DeployedState } from '../../../schema'; | ||
| import type { | ||
| AgentCoreMcpSpec, | ||
| AwsDeploymentTarget, | ||
| DeployedState, | ||
| EnvironmentOverrides, | ||
| Environments, | ||
| } from '../../../schema'; | ||
| import { applyTargetRegionToEnv } from '../../aws'; | ||
| import { validateAwsCredentials } from '../../aws/account'; | ||
| import { CdkToolkitWrapper, createSwitchableIoHost } from '../../cdk/toolkit-lib'; | ||
|
|
@@ -33,7 +39,9 @@ import { | |
| synthesizeCdk, | ||
| validateProject, | ||
| } from '../../operations/deploy'; | ||
| import { mergeOverrides, resolveEnvironment } from '../../operations/deploy/environment'; | ||
| import { formatTargetStatus, getGatewayTargetStatuses } from '../../operations/deploy/gateway-status'; | ||
| import { deployToTargets } from '../../operations/deploy/multi-target'; | ||
| import { deleteOrphanedABTests, setupABTests } from '../../operations/deploy/post-deploy-ab-tests'; | ||
| import { | ||
| resolveConfigBundleComponentKeys, | ||
|
|
@@ -51,6 +59,12 @@ export interface ValidatedDeployOptions { | |
| verbose?: boolean; | ||
| plan?: boolean; | ||
| diff?: boolean; | ||
| /** | ||
| * Optional env-var overrides applied in-memory to every runtime in the | ||
| * loaded project spec just before CDK synth. Set by --env deploys; never | ||
| * persisted to disk. | ||
| */ | ||
| envVarOverrides?: EnvironmentOverrides; | ||
| onProgress?: (step: string, status: 'start' | 'success' | 'error') => void; | ||
| onResourceEvent?: (message: string) => void; | ||
| } | ||
|
|
@@ -142,6 +156,13 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep | |
| // Preflight: validate project | ||
| startStep('Validate project'); | ||
| const context = await validateProject(); | ||
| // Apply per-environment envVar overrides (in-memory only) before synth. | ||
| if (options.envVarOverrides && context.projectSpec.runtimes) { | ||
| context.projectSpec = { | ||
| ...context.projectSpec, | ||
| runtimes: context.projectSpec.runtimes.map(rt => mergeOverrides(rt, options.envVarOverrides)), | ||
| }; | ||
| } | ||
| endStep('success'); | ||
|
|
||
| // Teardown confirmation: if this is a teardown deploy, require --yes | ||
|
|
@@ -705,3 +726,106 @@ export async function handleDeploy(options: ValidatedDeployOptions): Promise<Dep | |
| */ | ||
| // resolveConfigBundleComponentKeys and resolveComponentKey moved to | ||
| // src/cli/operations/deploy/post-deploy-config-bundles.ts | ||
|
|
||
| export interface EnvDeployOptions { | ||
| env: string; | ||
| autoConfirm?: boolean; | ||
| verbose?: boolean; | ||
| plan?: boolean; | ||
| diff?: boolean; | ||
| /** Run env targets concurrently via Promise.allSettled. */ | ||
| parallel?: boolean; | ||
| /** Continue past per-target failures (sequential mode only). */ | ||
| continueOnError?: boolean; | ||
| onProgress?: (step: string, status: 'start' | 'success' | 'error') => void; | ||
| onResourceEvent?: (message: string) => void; | ||
| /** Sink for orchestrator progress + summary lines. Defaults to console.log. */ | ||
| onLog?: (line: string) => void; | ||
| } | ||
|
|
||
| export interface EnvDeployResult { | ||
| success: boolean; | ||
| envName: string; | ||
| results: DeployResult[]; | ||
| /** Set when the env couldn't be resolved (no per-target results available). */ | ||
| error?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Read aws-targets.json supporting both the legacy array shape and the new | ||
| * `{ targets, environments }` object shape. Returns environments only when | ||
| * the file uses the object shape; otherwise environments is undefined. | ||
| */ | ||
| /** | ||
| * Read aws-targets.json with environments if present. Delegates to | ||
| * `ConfigIO.readAwsTargetsFull` and then applies region fallback so the same | ||
| * environment/profile precedence used by the rest of the CLI is honored. | ||
| */ | ||
| async function readAwsTargetsWithEnvironments( | ||
| configIO: ConfigIO | ||
| ): Promise<{ targets: AwsDeploymentTarget[]; environments?: Environments }> { | ||
| const full = await configIO.readAwsTargetsFull(); | ||
| // Apply the standard region fallback (env vars / profile config) by going | ||
| // through resolveAWSDeploymentTargets — its result preserves saved regions | ||
| // and fills in only blanks, so we can safely substitute it for the targets | ||
| // we just read while keeping the environments map from the object shape. | ||
| const resolved = await configIO.resolveAWSDeploymentTargets(); | ||
| return full.environments ? { targets: resolved, environments: full.environments } : { targets: resolved }; | ||
| } | ||
|
|
||
| /** | ||
| * Deploy a named environment: resolve the env to its targets, apply any | ||
| * env-level overrides per target, and run them through the multi-target | ||
| * orchestrator. Sequential / fail-fast for v1 (T9 adds parallel + continue-on-error). | ||
| */ | ||
| export async function handleEnvDeploy(options: EnvDeployOptions): Promise<EnvDeployResult> { | ||
| const configIO = new ConfigIO(); | ||
|
|
||
| let resolved; | ||
| try { | ||
| const awsTargets = await readAwsTargetsWithEnvironments(configIO); | ||
| resolved = resolveEnvironment(options.env, awsTargets); | ||
| } catch (err: unknown) { | ||
| return { | ||
| success: false, | ||
| envName: options.env, | ||
| results: [], | ||
| error: getErrorMessage(err), | ||
| }; | ||
| } | ||
|
|
||
| const results: DeployResult[] = []; | ||
| const aggregate = await deployToTargets( | ||
| resolved.targets, | ||
| { | ||
| environmentName: options.env, | ||
| parallel: options.parallel, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The orchestrator in
Options to fix:
Any of these is fine — option 1 is the long-term correct answer, option 3 is the lowest-risk for this PR. Whatever is chosen, please also add a test that fans out |
||
| continueOnError: options.continueOnError, | ||
| log: options.onLog, | ||
| }, | ||
| async target => { | ||
| const result = await handleDeploy({ | ||
| target: target.name, | ||
| autoConfirm: options.autoConfirm, | ||
| verbose: options.verbose, | ||
| plan: options.plan, | ||
| diff: options.diff, | ||
| envVarOverrides: resolved.overrides, | ||
| onProgress: options.onProgress, | ||
| onResourceEvent: options.onResourceEvent, | ||
| }); | ||
| results.push(result); | ||
| if (!result.success) { | ||
| // Throw so the orchestrator records this as a failure (fail-fast). | ||
| throw new Error(result.error ?? `Deploy failed for target ${target.name}`); | ||
| } | ||
| return result; | ||
| } | ||
| ); | ||
|
|
||
| return { | ||
| success: aggregate.failures.length === 0 && results.every(r => r.success), | ||
| envName: options.env, | ||
| results, | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnvDeployOptionsis missingparallelandcontinueOnError, andhandleEnvDeploybelow never forwards them todeployToTargets. Combined withcommand.tsxnot passing them through either,agentcore deploy --env dev --paralleland--continue-on-errorare accepted by the validator but have zero runtime effect — every env deploy is sequential fail-fast regardless of flags.Please:
parallel?: booleanandcontinueOnError?: booleantoEnvDeployOptions.deployToTargets(..., { environmentName, parallel, continueOnError, log }, ...)call (line ~799).cliOptions.parallel/cliOptions.continueOnErrorfromhandleDeployCLIincommand.tsx(line ~73).A test that exercises
handleEnvDeploywithparallel: trueand asserts the orchestrator observes it (e.g. by spying ondeployToTargetsor by verifying concurrent execution) would prevent this from regressing.