feat(issue-1236): enhance docker build with additional build configuration#1284
feat(issue-1236): enhance docker build with additional build configuration#1284EdouardCourty wants to merge 8 commits into
Conversation
…ation - allow buildContextPath argument to change build context path - allow custom docker build arguments
github-actions
Bot
added
the
agentcore-harness-reviewing
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for the contribution! The CLI-side wiring (schema, dev server, local packager) is clean and well-tested.
However, I think there is one blocking correctness issue: the new buildContextPath and customDockerBuildArgs fields are not honored by agentcore deploy — only by agentcore dev and the local agentcore package validation. The deploy path goes through the CDK construct (@aws/agentcore-cdk → AgentCoreApplication → ContainerSourceAsset + ContainerBuildProject), which:
- Uploads only
codeLocationas the source asset (sobuildContextPathoutsidecodeLocationis impossible — files referenced in the Dockerfile won't be in the upload). - Runs a hard-coded buildspec:
docker build -t $IMAGE_URI -f $DOCKERFILE_PATH .— no--build-argflags, no overridable context. - Does not include
buildContextPath/customDockerBuildArgsin its ownAgentEnvSpecschema, so even if the CLI sends them, zod will strip them on the construct side.
The net result is that a user following the new "Shared Dockerfile (monorepo)" docs example will see it work locally with agentcore dev and agentcore package, but agentcore deploy will silently use the wrong context (just codeLocation) and no build args, producing either a broken image or a deploy-time docker build failure with a confusing error.
See inline comments for specifics and options to address this. Other comments are smaller follow-ups.
| const buildResult = spawnSync( | ||
| runtime, | ||
| ['build', '-t', imageName, '-f', dockerfilePath, ...getUvBuildArgs(), codeLocation], | ||
| ['build', '-t', imageName, '-f', dockerfilePath, ...getUvBuildArgs(), ...buildArgFlags, buildContext], |
There was a problem hiding this comment.
Blocking — feature not honored on agentcore deploy.
ContainerPackager is only used by agentcore package (local validation) and agentcore dev. The actual deploy path goes through the CDK construct (@aws/agentcore-cdk), where:
ContainerSourceAsset(inagentcore-l3-cdk-constructs) uploads onlyresolveCodeLocation(spec.codeLocation, configRoot)as the source asset —buildContextPathis never read, so files referenced from outsidecodeLocationwon't be uploaded.ContainerBuildProject's buildspec hard-codesdocker build -t $IMAGE_URI -f $DOCKERFILE_PATH .with no--build-argplumbing.- The L3 construct's own
AgentEnvSpecschema doesn't include these new fields, so zod will strip them on the construct side anyway.
This means the documented "Shared Dockerfile (monorepo)" workflow in docs/container-builds.md will work locally and silently break (or behave wrongly) on deploy. That is a serious foot-gun.
Options to address:
- (Preferred) Land a paired change in
aws/agentcore-l3-cdk-constructsfirst that:- Adds
buildContextPathandcustomDockerBuildArgstoAgentEnvSpecSchema. - Updates
ContainerSourceAssetto uploadbuildContextPath(when set) instead ofcodeLocation, and adjusts the dockerfile path so it remains valid relative to that asset. - Updates
ContainerBuildProject's buildspec /ContainerImageBuilderto forward--build-argflags through CodeBuild env vars (e.g. via the custom resource properties, with the buildspec interpolating$BUILD_ARGS). - Bumps the construct dep here once published.
- Adds
- Hold this PR until that construct change is merged + published, and gate the schema additions in this repo on having the new construct version.
- As an interim, validate in the CLI's schema (or in the deploy command) that these fields are not set and reject with a clear "only supported for local dev/package" error — but that significantly waters down the feature and the docs example would have to be removed.
I'd lean towards option 1. Happy to help review the construct-side PR.
| } | ||
| ``` | ||
|
|
||
| The shared `Dockerfile` can then branch on the build arg: |
There was a problem hiding this comment.
Tied to the deploy issue: this example will not work end-to-end with agentcore deploy until the CDK construct side is updated. If we go with the "land construct first" path that's fine; otherwise this doc section should be reworded to make clear this is local-dev-only, or removed.
| "customDockerBuildArgs": { "AGENT_NAME": "agent-one" } | ||
| }, | ||
| { | ||
| "name": "agent-two", |
There was a problem hiding this comment.
Separate issue with this example regardless of the deploy plumbing: dockerfile is validated as a filename (no path separators — see src/schema/schemas/agent-env.ts and getDockerfilePath in src/lib/constants.ts), and getDockerfilePath joins it onto codeLocation. So the implied "single Dockerfile at the project root" setup isn't achievable today — each agent's codeLocation would still need its own Dockerfile file (even if it's just a one-liner that delegates).
Either (a) extend dockerfile to accept a path relative to the build context (and resolve it accordingly), or (b) update this section to clarify that each codeLocation must still contain a Dockerfile (which can simply be a thin wrapper / symlink) and adjust the example accordingly.
| * Useful for parameterising a shared Dockerfile per agent (e.g. `{ "AGENT_NAME": "myagent" }`). | ||
| * Container builds only. | ||
| */ | ||
| customDockerBuildArgs: z.record(z.string().min(1), z.string()).optional(), |
There was a problem hiding this comment.
Optional, but worth considering: the existing AgentEnvSpecSchema in the sibling construct repo already has a comment near this validator block (If adding more Container-specific fields, consider consolidating into a containerConfig object (see networkConfig pattern)). With this PR we're now up to three flat container-only fields (dockerfile, buildContextPath, customDockerBuildArgs) plus the cross-cutting validators. Grouping these under a single optional containerConfig: { dockerfile?, buildContextPath?, customDockerBuildArgs? } would make the cross-build-type validation simpler (build !== 'Container' && data.containerConfig becomes one check) and is more future-proof. Not strictly required for this PR, but if you agree it's worth doing it now before this ships.
github-actions
Bot
removed
the
agentcore-harness-reviewing
|
Hey @EdouardCourty after reviewing it seems the above comments are true regarding this feature not being honored by I can also go ahead and make the necessary updates if there is no activity on this PR by 06/05/2026 or with your explicit permission. Thank you! |
|
Hey @aidandaly24 if you can make the needed changes to I could not find the repository myself as it does not seem to be public (the NPM package page has a GitHub link which shows a 404...). |
|
@aidandaly24 - I've been successfully running this branch as a fork of the CLI specifically for the |
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
|
@nxf5025 We discussed this, and @aidandaly24 will make the changes to L3 CDK. |
Resolves the agent-env schema conflict (keeps both the new Container-only validators and main's filesystemConfigurations block) and completes the feature so it is honored on `agentcore deploy`, not just dev/package: - Resolve `dockerfile` relative to the build context (`buildContextPath ?? codeLocation`) in the packager, dev server, and deploy preflight; relax `getDockerfilePath` and the schema to allow a safe relative subpath (no absolute paths / no `..` traversal). This makes the shared-Dockerfile monorepo example in docs actually work end to end. - Tighten `customDockerBuildArgs` keys to valid identifiers (they become CodeBuild env vars / Dockerfile ARG names on deploy). - Docs: clarify Dockerfile-relative-to-context resolution and deploy parity. The paired @aws/agentcore-cdk change (honoring both fields in CodeBuild) lands in aws/agentcore-l3-cdk-constructs.
…ld-context resolution Address adversarial-review findings (paired with the @aws/agentcore-cdk change): - Reject customDockerBuildArgs keys reserved by the CodeBuild build environment (ECR_REGISTRY, IMAGE_URI, DOCKERFILE_PATH, BUILD_ARG_FLAGS, or CODEBUILD_*/AWS_* prefixes) so a config that builds locally can't fail only on `agentcore deploy`. Tests: - schema: reserved-key rejection. - packager: assert the Dockerfile (-f) resolves against the build context, not codeLocation. - dev server: assert customDockerBuildArgs are forwarded and buildContextPath is used as both the build context and the Dockerfile root.
Package TarballHow to installgh release download pr-1284-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.22.0.tgz |
|
Hey @nxf5025 @EdouardCourty, going to get this reviewed and it will be released this Thursday. Sorry for the delay! |
…t .dockerignore Address code-review findings (paired with @aws/agentcore-cdk): - Reserved build-arg keys: narrow the denylist to genuinely build-breaking names (PATH, HOME, IFS, LD_PRELOAD, LD_LIBRARY_PATH) plus the docker-client vars (DOCKER_HOST, DOCKER_CONFIG, DOCKER_TLS_VERIFY, DOCKER_CERT_PATH); stop over-rejecting common ARGs (USER, LANG, ...). - Constrain customDockerBuildArgs values (max 4096, no control chars) so a schema-valid config can't be rejected by CodeBuild only on deploy. - Deduplicate the --build-arg flag construction into getCustomBuildArgs() in build-args.ts (used by the package and dev build paths). - deploy preflight: warn when buildContextPath is set but the context root has no .dockerignore (secrets/junk would otherwise be sent to Docker/CodeBuild). Both local and deploy honor the same root .dockerignore. - docs + compacted schema (@regex/@max) updated. npm test (5836), typecheck, lint, build all green.
…textPath is set Closes the secret-exposure gap without reintroducing the local/deploy divergence: when a Container agent sets buildContextPath and the build-context root has no .dockerignore, the CLI generates one (excluding .env, .env.*, .git/, node_modules/, .venv/, __pycache__/, agentcore/). Because it's a real file, both local `docker build` and the CodeBuild deploy path honor the same rules — secrets/junk stay out of the image and the S3 upload, consistently, with no forced-exclude false positives (users edit the file to include anything intentional). This mirrors the CodeZip packager's always-on secret guard. - New shared helper ensureBuildContextDockerignore(); never overwrites an existing file. - Wired into the package, dev, and deploy-preflight build paths (replaces the prior "no .dockerignore" warning with actually creating one + a log line). - docs updated; unit tests for the helper (create/no-op) and the preflight wiring. Validated live end to end: the generated .dockerignore trimmed the CodeBuild context (agentcore/, .git, .venv excluded) and the build succeeded.
…uild-with-additional-arguments
…uild-with-additional-arguments
`agentcore package` built its local image tag as `agentcore-package-${agentName}`
without lowercasing, so `docker build -t` rejected any capitalized agent name
(e.g. the default `AgentOne`) with "repository name must be lowercase" — the
package command was broken for essentially every default container agent. The dev
server already lower-cases its tag; the packager now matches.
Pre-existing bug (unrelated to buildContextPath/customDockerBuildArgs), surfaced
while live-testing `agentcore package` on a container agent. Added a unit test with
a mixed-case name.
5 participants
Description
Add support for two new options in the runtime configurations for Docker-based builds :
buildContextPathcustomDockerBuildArgsBoth are optional and this change is backward-compatible.
Related Issue
Closes #1236
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.