Skip to content
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

Move PublishAsDockerFile to use container resources. #7072

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

davidfowl
Copy link
Member

@davidfowl davidfowl commented Jan 11, 2025

Description

This allows full customization of the docker building process and removes a dependency on the limited docker.v0 resource type.

  • Updated DockerfileBuildAnnotation to allow nullable build arguments.
  • Modified WithBuildArg to accept nullable values.
  • Revised tests in ManifestGenerationTests.cs and PublishAsDockerfileTests.cs to align with new behavior.
  • Adjusted SchemaTests.cs to use temporary context paths for Dockerfile testing.

Fixes #4067

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

This allows full customization of the docker builing process and removes a dependency on the limited docker.v0 resource type.

- Updated `DockerfileBuildAnnotation` to allow nullable build arguments.
- Modified `WithBuildArg` to accept nullable values.
- Updated API documentation in `PublicAPI.Shipped.txt` and `PublicAPI.Unshipped.txt`.
- Revised tests in `ManifestGenerationTests.cs` and `PublishAsDockerfileTests.cs` to align with new behavior.
- Adjusted `SchemaTests.cs` to use temporary context paths for Dockerfile testing.

Fixes #4067
@davidfowl davidfowl requested a review from mitchdenny as a code owner January 11, 2025 20:42
@davidfowl davidfowl requested review from JamesNK, eerhardt and sebastienros and removed request for mitchdenny January 11, 2025 20:42
@davidfowl davidfowl added this to the 9.1 milestone Jan 11, 2025
@davidfowl davidfowl added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jan 11, 2025
- Updated the ExecutableReference's manifest writer even though the resource gets removed from the list.
- Updated tests (including the python tests)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to override name or path of Dockerfile in .PublishAsDockerFile()
1 participant