-
Notifications
You must be signed in to change notification settings - Fork 74
feat(project): add deploy command contract #2056
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
Open
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
0a716ea
feat(project): add deploy command contract
notgitika dc15b6a
fix(project): sync supported deployment regions
notgitika 550dd61
test(project): route the deploy handler test through the real CLI wiring
notgitika d61a7b5
refactor(project): dispatch deploy through the project backend
notgitika b56cfee
fix(project): reject underscores in deployment target names
notgitika 26d38ca
fix(errors): surface why deserialization failed
notgitika e8aabe3
refactor(project): address review feedback on the deploy handler
notgitika File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,13 @@ | ||
| import type { Project, ProjectEvent } from "../../../handlers/project/types"; | ||
| import type { DeployResult, Project, ProjectEvent } from "../../../handlers/project/types"; | ||
| import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; | ||
|
|
||
| export type DeployBackendInput = { | ||
| /** Fully resolved account and region selected from aws-targets.json. */ | ||
| target: AwsDeploymentTarget; | ||
| }; | ||
|
|
||
| /** Builds the deployable artifacts owned by a project's selected backend. */ | ||
| export interface ProjectBackend { | ||
| build(project: Project): AsyncGenerator<ProjectEvent, void>; | ||
| deploy(project: Project, input: DeployBackendInput): AsyncGenerator<ProjectEvent, DeployResult>; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| export { FsProjectManager } from "./manager"; | ||
| export { CdkBackend, type CdkBackendConfig } from "./backends/cdk"; | ||
| export type { ProjectBackend } from "./backends/types"; | ||
| export type { DeployBackendInput, ProjectBackend } from "./backends/types"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| import { afterEach, describe, expect, test } from "bun:test"; | ||
| import { mkdtemp, rm, writeFile } from "node:fs/promises"; | ||
| import { join } from "node:path"; | ||
| import { tmpdir } from "node:os"; | ||
| import { createRootHandler } from "../../index"; | ||
| import { | ||
| createSilentLogger, | ||
| TestCoreClient, | ||
| TestGlobalConfigAccessor, | ||
| testIO, | ||
| } from "../../../testing"; | ||
| import type { DeployBackendInput, ProjectBackend } from "../../../core/project"; | ||
| import type { AwsDeploymentTarget } from "../../../projectSchemas/aws-targets"; | ||
| import type { DeployResult, Project, ProjectEvent } from "../types"; | ||
|
|
||
| const DEFAULT_TARGET: AwsDeploymentTarget = { | ||
| name: "default", | ||
| account: "111122223333", | ||
| region: "us-east-1", | ||
| }; | ||
| const STAGING_TARGET: AwsDeploymentTarget = { | ||
| name: "staging", | ||
| account: "444455556666", | ||
| region: "eu-west-1", | ||
| }; | ||
| const TARGETS = [DEFAULT_TARGET, STAGING_TARGET]; | ||
|
|
||
| /** | ||
| * A ProjectBackend that deploys successfully, which CdkBackend cannot do until | ||
| * CDK deployment is implemented. Stubbing the backend rather than the whole | ||
| * manager keeps the real FsProjectManager in the path, so target resolution and | ||
| * withProject run for real. | ||
| */ | ||
| function fakeBackend(result: DeployResult, events: ProjectEvent[] = []) { | ||
| const calls: { project: Project; input: DeployBackendInput }[] = []; | ||
| const backend: ProjectBackend = { | ||
| async *build() {}, | ||
| async *deploy(project, input) { | ||
| calls.push({ project, input }); | ||
| yield* events; | ||
| return result; | ||
| }, | ||
| }; | ||
| return { calls, backend }; | ||
| } | ||
|
|
||
| function testDeployCommand(result: DeployResult, events: ProjectEvent[] = []) { | ||
| const io = testIO(); | ||
| const fake = fakeBackend(result, events); | ||
| const core = new TestCoreClient({ backends: { CDK: fake.backend } }); | ||
| const root = createRootHandler(core, { | ||
| io: io.io, | ||
| globalConfigAccessor: new TestGlobalConfigAccessor(), | ||
| logger: createSilentLogger(), | ||
| }); | ||
|
|
||
| return { | ||
| ...fake, | ||
| io, | ||
| run: (args: string[] = []) => root.route(["node", "agentcore", "project", "deploy", ...args]), | ||
| create: (args: string[]) => root.route(["node", "agentcore", "project", ...args]), | ||
| }; | ||
| } | ||
|
|
||
| const originalCwd = process.cwd(); | ||
| const tempDirectories: string[] = []; | ||
|
|
||
| async function inTempDirectory(): Promise<string> { | ||
| const directory = await mkdtemp(join(tmpdir(), "agentcore-deploy-")); | ||
| tempDirectories.push(directory); | ||
| process.chdir(directory); | ||
| // cwd is the realpath (macOS tmpdir lives behind a /var -> /private/var | ||
| // symlink), matching the paths the manager derives from process.cwd(). | ||
| return process.cwd(); | ||
| } | ||
|
|
||
| afterEach(async () => { | ||
| process.chdir(originalCwd); | ||
| await Promise.all( | ||
| tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true })), | ||
| ); | ||
| }); | ||
|
|
||
| /** Scaffolds a project whose aws-targets.json holds exactly `contents`, and cds into it. */ | ||
| async function inProjectWithTargets( | ||
| subject: ReturnType<typeof testDeployCommand>, | ||
| contents: string = JSON.stringify(TARGETS), | ||
| ): Promise<string> { | ||
| const directory = await inTempDirectory(); | ||
| await subject.create(["create", "--name", "orders", "--skip-install", "--skip-git"]); | ||
| const projectRoot = join(directory, "orders"); | ||
| await writeFile(join(projectRoot, "agentcore", "aws-targets.json"), contents); | ||
| process.chdir(projectRoot); | ||
| return projectRoot; | ||
| } | ||
|
|
||
| describe("project deploy handler", () => { | ||
| test("defaults to the default target and keeps progress off stdout", async () => { | ||
| const subject = testDeployCommand( | ||
| { outputs: { ZetaUrl: "https://zeta.example", AlphaArn: "arn:alpha" } }, | ||
| [{ message: "Preparing deployment" }, { message: "Deploying stack" }], | ||
| ); | ||
| await inProjectWithTargets(subject); | ||
|
|
||
| await subject.run(); | ||
|
|
||
| expect(subject.calls.map(({ input }) => input)).toEqual([{ target: DEFAULT_TARGET }]); | ||
| expect(subject.io.stderr()).toContain("Preparing deployment\nDeploying stack"); | ||
| expect(subject.io.stderr()).toContain("Deployed project 'orders' to target 'default'"); | ||
| expect(subject.io.stdout()).toBe("AlphaArn: arn:alpha\nZetaUrl: https://zeta.example"); | ||
| }); | ||
|
|
||
| test("passes an explicit target and renders the result as JSON", async () => { | ||
| const result = { outputs: { ServiceUrl: "https://service.example" } }; | ||
| const subject = testDeployCommand(result); | ||
| await inProjectWithTargets(subject); | ||
|
|
||
| await subject.run(["--target", "staging", "--json"]); | ||
|
|
||
| expect(subject.calls.map(({ input }) => input)).toEqual([{ target: STAGING_TARGET }]); | ||
| expect(JSON.parse(subject.io.stdout())).toEqual(result); | ||
| }); | ||
|
|
||
| test("rejects an unknown target without invoking the backend", async () => { | ||
| const subject = testDeployCommand({ outputs: {} }); | ||
| await inProjectWithTargets(subject); | ||
|
|
||
| await expect(subject.run(["--target", "nope"])).rejects.toThrow( | ||
| /no deployment target named 'nope'/, | ||
| ); | ||
| expect(subject.calls).toEqual([]); | ||
| }); | ||
|
|
||
| test("requires deployment targets to be configured", async () => { | ||
| const subject = testDeployCommand({ outputs: {} }); | ||
| await inProjectWithTargets(subject, JSON.stringify([])); | ||
|
|
||
| await expect(subject.run()).rejects.toThrow(/No deployment targets are configured/); | ||
| expect(subject.calls).toEqual([]); | ||
| }); | ||
| }); | ||
|
|
||
| /** The message the user would see on stderr, since the reporter prints only that. */ | ||
| async function messageFrom(command: Promise<void>): Promise<string> { | ||
| try { | ||
| await command; | ||
| } catch (error) { | ||
| return (error as Error).message; | ||
| } | ||
| throw new Error("expected the command to fail"); | ||
| } | ||
|
|
||
| describe("project deploy reports which field of aws-targets.json is wrong", () => { | ||
| test("names the offending field for an unsupported region", async () => { | ||
| const subject = testDeployCommand({ outputs: {} }); | ||
| await inProjectWithTargets( | ||
| subject, | ||
| JSON.stringify([{ name: "default", account: "111122223333", region: "us-east-11" }]), | ||
| ); | ||
|
|
||
| const message = await messageFrom(subject.run()); | ||
|
|
||
| expect(message).toContain("aws-targets.json"); | ||
| expect(message).toContain("at [0].region"); | ||
| expect(message).toContain('"us-east-1"'); | ||
| expect(subject.calls).toEqual([]); | ||
| }); | ||
|
|
||
| test("surfaces the duplicate target name", async () => { | ||
| const subject = testDeployCommand({ outputs: {} }); | ||
| await inProjectWithTargets(subject, JSON.stringify([DEFAULT_TARGET, DEFAULT_TARGET])); | ||
|
|
||
| await expect(subject.run()).rejects.toThrow(/Duplicate deployment target name: default/); | ||
| expect(subject.calls).toEqual([]); | ||
| }); | ||
|
|
||
| test("surfaces the account id rule", async () => { | ||
| const subject = testDeployCommand({ outputs: {} }); | ||
| await inProjectWithTargets( | ||
| subject, | ||
| JSON.stringify([{ name: "default", account: "123", region: "us-east-1" }]), | ||
| ); | ||
|
|
||
| await expect(subject.run()).rejects.toThrow(/AWS account ID must be exactly 12 digits/); | ||
| expect(subject.calls).toEqual([]); | ||
| }); | ||
|
|
||
| test("surfaces the parse error for malformed json", async () => { | ||
| const subject = testDeployCommand({ outputs: {} }); | ||
| await inProjectWithTargets(subject, '[{ "name": "default", }]'); | ||
|
|
||
| await expect(subject.run()).rejects.toThrow(/JSON Parse error/); | ||
| expect(subject.calls).toEqual([]); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,46 @@ | ||
| import { createHandler } from "../../../router"; | ||
| import { NotImplementedError } from "../../../errors"; | ||
| import z from "zod"; | ||
| import type { AppIO } from "../../../io"; | ||
| import { createHandler, flag, ProjectKey } from "../../../router"; | ||
| import { JsonRendererKey } from "../../../tui"; | ||
| import { JsonKey } from "../../keys"; | ||
| import type { ProjectManager } from "../types"; | ||
|
|
||
| export const createDeployProjectHandler = () => | ||
| type DeployProjectHandlerConfig = { | ||
| projectManager: ProjectManager; | ||
| io: AppIO; | ||
| }; | ||
|
|
||
| export const createDeployProjectHandler = (config: DeployProjectHandlerConfig) => | ||
| createHandler({ | ||
| name: "deploy", | ||
| description: "deploy the project to AWS", | ||
| handle: async () => { | ||
| throw new NotImplementedError("agentcore project deploy is not implemented yet"); | ||
| flags: [ | ||
| flag("target", "name of the aws-targets.json entry to deploy", z.string().default("default")), | ||
| ], | ||
| handle: async (ctx, flags) => { | ||
| // withProject has already resolved the enclosing project. | ||
| const project = ctx.require(ProjectKey); | ||
|
|
||
| // Progress goes to stderr, keeping stdout for machine output. Driven by | ||
| // hand rather than `for await` because the outputs we render below are the | ||
| // generator's return value, which `for await` discards. | ||
| const deployment = config.projectManager.deploy(project, { target: flags.target }); | ||
| let next = await deployment.next(); | ||
| while (!next.done) { | ||
| config.io.stderr.write(`${next.value.message}\n`); | ||
| next = await deployment.next(); | ||
| } | ||
| const result = next.value; | ||
|
|
||
| config.io.stderr.write(`Deployed project '${project.name}' to target '${flags.target}'\n`); | ||
| if (ctx.require(JsonKey)) { | ||
| ctx.require(JsonRendererKey).renderJson(result); | ||
| return; | ||
| } | ||
| for (const [key, value] of Object.entries(result.outputs).sort(([a], [b]) => | ||
| a.localeCompare(b), | ||
| )) { | ||
| config.io.stdout.write(`${key}: ${value}\n`); | ||
| } | ||
| }, | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.