-
Notifications
You must be signed in to change notification settings - Fork 60
feat(assets): add AssetManager and CDK scaffold templates #1809
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: refactor
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| import { afterEach, describe, expect, test } from "bun:test"; | ||
| import { mkdtemp, mkdir, readdir, rm } from "node:fs/promises"; | ||
| import { join, relative, resolve } from "node:path"; | ||
| import { tmpdir } from "node:os"; | ||
| import { AssetManager, resolveSourceRoot } from "./AssetManager"; | ||
|
|
||
| const tempDirectories: string[] = []; | ||
|
|
||
| async function makeTempDirectory(): Promise<string> { | ||
| const directory = await mkdtemp(join(tmpdir(), "agentcore-assets-")); | ||
| tempDirectories.push(directory); | ||
| return directory; | ||
| } | ||
|
|
||
| afterEach(async () => { | ||
| await Promise.all( | ||
| tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true })), | ||
| ); | ||
| }); | ||
|
|
||
| describe("AssetManager", () => { | ||
| test("renders a filesystem asset tree", async () => { | ||
| const root = await makeTempDirectory(); | ||
| const source = join(root, "assets", "cdk"); | ||
| const destination = join(root, "output"); | ||
|
|
||
| await mkdir(join(source, "lib"), { recursive: true }); | ||
| await Bun.write(join(source, "README.md"), "Hello {{projectName}}"); | ||
| await Bun.write(join(source, ".prettierrc"), "{}"); | ||
| await Bun.write(join(source, "gitignore.template"), "dist/"); | ||
| await Bun.write(join(source, "lib", "stack.ts"), "export const name = '{{projectName}}';"); | ||
|
|
||
| await new AssetManager([], join(root, "assets")).render("cdk", destination, { | ||
| projectName: "example", | ||
| }); | ||
|
|
||
| expect(await Bun.file(join(destination, "README.md")).text()).toBe("Hello example"); | ||
| expect(await Bun.file(join(destination, ".prettierrc")).text()).toBe("{}"); | ||
| expect(await Bun.file(join(destination, ".gitignore")).text()).toBe("dist/"); | ||
| expect(await Bun.file(join(destination, "lib", "stack.ts")).text()).toBe( | ||
| "export const name = 'example';", | ||
| ); | ||
| }); | ||
|
|
||
| test("renders embedded asset files", async () => { | ||
| const root = await makeTempDirectory(); | ||
| const destination = join(root, "output"); | ||
| const embedded = new File( | ||
| ["Hello {{projectName}}"], | ||
| "agentcore-assets/src/assets/cdk/README.md", | ||
| ); | ||
|
|
||
| await new AssetManager([embedded]).render("cdk", destination, { | ||
| projectName: "embedded", | ||
| }); | ||
|
|
||
| expect(await Bun.file(join(destination, "README.md")).text()).toBe("Hello embedded"); | ||
| }); | ||
|
|
||
| test("does not HTML-escape code template values", async () => { | ||
| const root = await makeTempDirectory(); | ||
| const source = join(root, "assets", "cdk"); | ||
| const destination = join(root, "output"); | ||
|
|
||
| await mkdir(source, { recursive: true }); | ||
| await Bun.write(join(source, "config.ts"), "export const expr = {{expr}};"); | ||
|
|
||
| await new AssetManager([], join(root, "assets")).render("cdk", destination, { | ||
| expr: "a && b < c", | ||
| }); | ||
|
|
||
| expect(await Bun.file(join(destination, "config.ts")).text()).toBe( | ||
| "export const expr = a && b < c;", | ||
| ); | ||
| }); | ||
|
|
||
| test("rejects an embedded path that escapes the destination", async () => { | ||
| const root = await makeTempDirectory(); | ||
| const destination = join(root, "output"); | ||
| const embedded = new File( | ||
| ["pwned"], | ||
| "agentcore-assets/src/assets/cdk/../../../../../../etc/evil", | ||
| ); | ||
|
|
||
| await expect(new AssetManager([embedded]).render("cdk", destination)).rejects.toThrow( | ||
| "Unsafe asset path", | ||
| ); | ||
| }); | ||
|
|
||
| describe("resolveSourceRoot", () => { | ||
| test("returns the bundled root when assets/ sits beside the module", async () => { | ||
| const moduleDir = await makeTempDirectory(); | ||
| await mkdir(join(moduleDir, "assets"), { recursive: true }); | ||
|
|
||
| expect(resolveSourceRoot(moduleDir)).toBe(resolve(moduleDir, "assets")); | ||
| }); | ||
|
|
||
| test("falls back to ../assets when no sibling assets/ exists", async () => { | ||
| const moduleDir = await makeTempDirectory(); | ||
|
|
||
| expect(resolveSourceRoot(moduleDir)).toBe(resolve(moduleDir, "../assets")); | ||
| }); | ||
| }); | ||
|
|
||
| test("cdk renders the expected output tree", async () => { | ||
| // Real cdk asset, source-tree mode (no injected root, no embedded files). | ||
| // Snapshots the rendered manifest so adding/removing/renaming an asset file | ||
| // is a reviewable diff. Content is covered by the byte-for-byte tests above. | ||
| const destination = await makeTempDirectory(); | ||
| await new AssetManager().render("cdk", destination); | ||
|
|
||
| const rendered = (await readdir(destination, { recursive: true, withFileTypes: true })) | ||
| .filter((entry) => entry.isFile()) | ||
| .map((entry) => | ||
| relative(destination, join(entry.parentPath, entry.name)).replaceAll("\\", "/"), | ||
| ) | ||
| .sort(); | ||
|
|
||
| expect(rendered).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| test("rejects a missing asset", async () => { | ||
| const root = await makeTempDirectory(); | ||
|
|
||
| await expect( | ||
| new AssetManager([], join(root, "assets")).render("cdk", join(root, "output")), | ||
| ).rejects.toThrow("Asset 'cdk' does not exist"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import Handlebars from "handlebars"; | ||
| import { existsSync } from "node:fs"; | ||
| import { mkdir, readFile, readdir } from "node:fs/promises"; | ||
| import { basename, dirname, join, relative, resolve, sep } from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { atomicWrite } from "../fs"; | ||
| import type { AssetFile, AssetName, AssetVariables, EmbeddedFile } from "./types"; | ||
|
|
||
| // Bundled builds place `assets/` beside the module; source layout has it one up. | ||
| export function resolveSourceRoot( | ||
| moduleDirectory = dirname(fileURLToPath(import.meta.url)), | ||
| ): string { | ||
| const bundledRoot = resolve(moduleDirectory, "assets"); | ||
| if (existsSync(bundledRoot)) { | ||
| return bundledRoot; | ||
| } | ||
|
|
||
| return resolve(moduleDirectory, "../assets"); | ||
| } | ||
|
|
||
| export class AssetManager { | ||
|
Contributor
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. does it make sense to decouple template rendering from asset management? I'm wondering if we'll ever want to work with static assets that aren't part of our project templates. |
||
| private handlebars = Handlebars.create(); | ||
| constructor( | ||
| private readonly embeddedFiles: readonly EmbeddedFile[] = [], | ||
| private readonly sourceRoot?: string, | ||
| ) {} | ||
|
|
||
| async render( | ||
| asset: AssetName, | ||
| destination: string, | ||
| variables: AssetVariables = {}, | ||
| ): Promise<void> { | ||
| const assetFiles = | ||
| this.embeddedFiles.length > 0 | ||
| ? this.listEmbeddedFiles(asset) | ||
| : await this.listFileSystemFiles(asset); | ||
|
|
||
| if (assetFiles.length === 0) { | ||
| throw new Error(`Asset '${asset}' does not exist`); | ||
| } | ||
|
|
||
| const resolvedDestination = resolve(destination); | ||
| for (const file of assetFiles) { | ||
|
Contributor
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. should we treat the rendering of files as a transaction or is this up to the caller? |
||
| const outputPath = this.resolveOutputPath(resolvedDestination, file.relativePath); | ||
| const rendered = this.handlebars.compile(await file.text(), { noEscape: true })(variables); | ||
|
|
||
| await mkdir(dirname(outputPath), { recursive: true }); | ||
| await atomicWrite(outputPath, rendered); | ||
| } | ||
| } | ||
|
|
||
| private async listFileSystemFiles(asset: AssetName): Promise<AssetFile[]> { | ||
| const assetRoot = join(this.sourceRoot ?? resolveSourceRoot(), asset); | ||
|
|
||
| let entries; | ||
| try { | ||
| entries = await readdir(assetRoot, { recursive: true, withFileTypes: true }); | ||
| } catch (error) { | ||
| if ((error as NodeJS.ErrnoException).code === "ENOENT") { | ||
| return []; | ||
| } | ||
| throw error; | ||
| } | ||
|
|
||
| return entries | ||
| .filter((entry) => entry.isFile()) | ||
| .map((entry) => relative(assetRoot, join(entry.parentPath, entry.name))) | ||
| .sort() | ||
| .map((relativePath) => ({ | ||
| relativePath, | ||
| text: () => readFile(join(assetRoot, relativePath), "utf8"), | ||
| })); | ||
| } | ||
|
|
||
| private listEmbeddedFiles(asset: AssetName): AssetFile[] { | ||
| const prefix = `agentcore-assets/src/assets/${asset}/`; | ||
|
|
||
| return this.embeddedFiles | ||
| .filter((file) => file.name.startsWith(prefix)) | ||
| .map((file) => ({ | ||
| relativePath: file.name.slice(prefix.length), | ||
| text: () => file.text(), | ||
| })) | ||
| .sort((a, b) => a.relativePath.localeCompare(b.relativePath)); | ||
| } | ||
|
|
||
| private resolveOutputPath(resolvedDestination: string, relativePath: string): string { | ||
| const mapped = this.resolveTemplateName(relativePath).replaceAll("\\", "/"); | ||
| const segments = mapped.split("/"); | ||
|
|
||
| if (mapped.startsWith("/") || segments.some((s) => s === "" || s === "." || s === "..")) { | ||
| throw new Error(`Unsafe asset path '${relativePath}'`); | ||
| } | ||
|
|
||
| const outputPath = resolve(resolvedDestination, mapped); | ||
| if (outputPath !== resolvedDestination && !outputPath.startsWith(resolvedDestination + sep)) { | ||
| throw new Error(`Asset path '${relativePath}' escapes destination`); | ||
| } | ||
|
|
||
| return outputPath; | ||
| } | ||
|
|
||
| private resolveTemplateName(relativePath: string): string { | ||
| const filename = basename(relativePath); | ||
| const ignore = filename.match(/^(docker|git|npm)ignore\.template$/); | ||
| return join(dirname(relativePath), ignore ? `.${ignore[1]}ignore` : filename); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Bun Snapshot v1, https://bun.sh/docs/test/snapshots | ||
|
|
||
| exports[`AssetManager cdk renders the expected output tree 1`] = ` | ||
| [ | ||
| ".gitignore", | ||
| ".npmignore", | ||
| ".prettierrc", | ||
| "README.md", | ||
| "bin/cdk.ts", | ||
| "cdk.json", | ||
| "jest.config.js", | ||
| "lib/cdk-stack.ts", | ||
| "package.json", | ||
| "test/cdk.test.ts", | ||
| "tsconfig.json", | ||
| ] | ||
| `; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export { AssetManager } from "./AssetManager"; | ||
| export { ASSET_NAMES, type AssetName, type AssetVariables } from "./types"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| export const ASSET_NAMES = ["cdk"] as const; | ||
|
|
||
| export type AssetName = (typeof ASSET_NAMES)[number]; | ||
|
|
||
| export type AssetVariables = Record<string, unknown>; | ||
| export interface AssetFile { | ||
| relativePath: string; | ||
| text(): Promise<string>; | ||
| } | ||
| export interface EmbeddedFile { | ||
|
Contributor
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. is it worth adding some docstrings here to clarify the difference between an |
||
| readonly name: string; | ||
| text(): Promise<string>; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "trailingComma": "es5", | ||
| "printWidth": 120, | ||
| "tabWidth": 2, | ||
| "semi": true, | ||
| "singleQuote": true, | ||
| "arrowParens": "avoid" | ||
| } |
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.
should the file name start with lowercase? My understanding was that only react components have upper case file names.