-
Notifications
You must be signed in to change notification settings - Fork 770
Support a docker build arg to specify the source image in PublishWithContainerFiles #12716
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
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12716Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12716" |
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.
Pull Request Overview
This PR refactors Dockerfile generation to use parameterized base images through ARG statements and multi-stage FROM statements when using PublishWithContainerFiles. Instead of directly referencing source container images (e.g., sourceimage:latest), the generated Dockerfiles now define ARG variables and named stages for source containers, improving build-time flexibility and following Docker multi-stage build best practices.
Key Changes:
- Added global ARG statement support to
DockerfileBuilderto parameterize base images before FROM statements - Created new
ContainerFilesExtensionshelper class to centralize Dockerfile container files logic - Refactored Python, JavaScript, and YARP extensions to use the new centralized container files handling
- Updated test snapshots to reflect the new Dockerfile format with ARG and stage-based references
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ApplicationModel/Docker/DockerfileBuilder.cs | Added support for global ARG statements and logic to write them before stages |
| src/Aspire.Hosting/ApplicationModel/Docker/ContainerFilesExtensions.cs | New file centralizing container files handling with ARG/stage-based approach |
| src/Aspire.Hosting/ApplicationModel/ProjectResource.cs | Updated to call new centralized container files methods |
| src/Aspire.Hosting.Yarp/YarpResourceExtensions.cs | Refactored to use centralized container files extensions |
| src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs | Removed local AddContainerFiles method in favor of centralized version |
| src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs | Updated to use centralized container files extensions |
| tests/Aspire.Hosting.Tests/ProjectResourceTests.cs | Enhanced test to verify Dockerfile content through callback |
| tests/Aspire.Hosting.Tests/Publishing/FakeContainerRuntime.cs | Added callback support for BuildImageAsync |
| tests/Aspire.Hosting.JavaScript.Tests/AddNodeAppTests.cs | Added new test for Node.js app with container files |
| Multiple snapshot files | Updated to reflect new ARG-based Dockerfile format |
src/Aspire.Hosting/ApplicationModel/Docker/ContainerFilesExtensions.cs
Outdated
Show resolved
Hide resolved
...rojectResourceTests.ProjectResourceWithContainerFilesDestinationAnnotationWorks.verified.txt
Outdated
Show resolved
Hide resolved
…ContainerFiles This allows an external system to provide the image name for the source image to copy the files from instead of always hard coding the image tag inline.
c2eff0a to
c4ca227
Compare
|
/backport to release/13.0 |
|
Started backporting to release/13.0: https://github.com/dotnet/aspire/actions/runs/19123477978 |
Description
This allows an external system, like azd, to provide the image name for the source image to copy the files from instead of always hard coding the image tag inline.
Checklist
<remarks />and<code />elements on your triple slash comments?