Add support for copying existing files via WithContainerFiles API #8908

danegsta · 2025-04-21T23:24:59Z

Description

Adds the ability to specify an existing file via WithContainerFiles instead of having to specify the contents of a file. Uses the same permission mechanics and slots into the existing ability to create arbitrary files and folders. Specifically, a ContainerFile type passed to WithContainerFiles can take either a Contents property or SourcePath property (an exception will be thrown if both are set).

Fixes #5498

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?
- Yes
  - Link to aspire-docs issue (consider using one of the following templates):
- No

Copilot

Pull Request Overview

This PR adds support for copying existing files via the WithContainerFiles API, replacing the previous bind mount mechanism for realm imports in Keycloak.

Updated tests to validate the new ContainerFileSystemCallbackAnnotation functionality via asynchronous execution.
Modified the Container model to enforce that file entries use either SourcePath or Contents but not both.
Updated the KeycloakResourceBuilderExtensions to utilize the WithContainerFiles API using new C# 12 collection patterns.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
tests/Aspire.Hosting.Keycloak.Tests/KeycloakPublicApiTests.cs	Updated test methods to async, altered assertions to verify ContainerFileSystemCallbackAnnotation usage.
src/Aspire.Hosting/Dcp/Model/Container.cs	Added Source assignment from SourcePath and mutual exclusion check between Contents and SourcePath.
src/Aspire.Hosting/ApplicationModel/ContainerFileSystemCallbackAnnotation.cs	Updated documentation for ContainerFile to clarify mutual exclusion semantics between Contents and SourcePath.
src/Aspire.Hosting/Keycloak/KeycloakResourceBuilderExtensions.cs	Switched from WithBindMount to WithContainerFiles using new collection initialization syntax.

Comments suppressed due to low confidence (1)

tests/Aspire.Hosting.Keycloak.Tests/KeycloakPublicApiTests.cs:224

The variable 'file' is used in the assertion but is not defined anywhere in the test. Consider defining it (e.g., var file = Path.GetFileName(filePath);) before using it in assertions.

Assert.Equal(file, realmFile.Name);

karolz-ms

No bind mount 😍

eerhardt · 2025-04-22T14:36:02Z

src/Aspire.Hosting/ApplicationModel/ContainerFileSystemCallbackAnnotation.cs

    /// </summary>
    public string? Contents { get; set; }
+
+    /// <summary>
+    /// The path to a file on the host system to copy into the container. This path must be absolute and point to a file on the host system.


Why does the path need to be absolute? We can use relative paths elsewhere.

We don’t explicitly check for an absolute path; it’s more of a SHOULD requirement. We can’t necessarily guarantee the orchestrator and container runtime will have the correct working directory in all scenarios, so absolute paths guarantees consistent behavior. We probably should call out the same for the bind mount annotation.

eerhardt · 2025-04-22T14:38:49Z

Are there any direct tests that can be added for this functionality? If we ever deleted Keycloak (unlikely) we wouldn't have any coverage.

src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs

…Files

src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs

danegsta · 2025-05-08T23:44:20Z

This would mitigate some of the bind mount issues in #7078, but there's implications for the generated manifest in switching from bind mount to WithContainerFiles.

danegsta · 2025-05-09T01:40:35Z

This could be supported in the compose publish targets via the configs feature. One undocumented part of the compose configs spec is that you can specify the content inline and they'll be copied in the same way we do in WithContainerFiles.

…Files

…sh output

…Files

davidfowl · 2025-05-10T03:19:25Z

I say we move this to 9.4

davidfowl · 2025-05-16T14:13:28Z

...shots/DockerComposePublisherTests.PublishAsync_GeneratesValidDockerComposeFile.verified.yaml

+    configs:
+      - source: "cache__usr_local_share_redis.conf"
+        target: "/usr/local/share/redis.conf"
+        mode: 0644
+      - source: "cache__usr_local_share_folder_file.sh"
+        target: "/usr/local/share/folder/file.sh"
+        uid: 1000
+        gid: 1000
+        mode: 0700


This is VERY cool 😄

I was definitely happy to find out compose configs can support basically the full set of configuration from WithContainerFiles. Makes having a consistent runtime and publish experience a lot easier.

Spoke too soon; there is one headache in that compose will ignore uid, gid, and mode when using source file mode (vs specifying inline content). They use a volume when a specific file is referenced, but cp when contents are provided.

One option would be to simply read text and convert every referenced file to the inline content form, but that wouldn't work if a user was expecting to use WithContainerFiles for a binary file.

davidfowl · 2025-05-16T14:17:28Z

src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs

@@ -123,6 +140,41 @@ private async Task WriteDockerComposeOutputAsync(DistributedApplicationModel mod
        envFile.Save(envFilePath);
    }

+    private static void HandleComposeFileConfig(ComposeFile composeFile, Service composeService, ContainerFileSystemItem? item, int? uid, int? gid, UnixFileMode umask, string path)


Do we need to copy files to the output folder?

I've updated to copy source files into a subdirectory of the OutputPath named after the service they belong to (so <OutputPath>/<ServiceName>/<FileName>). I'm using a relative path to when generating the compose file to make it portable.

davidfowl · 2025-05-16T20:21:44Z

All good?

danegsta · 2025-05-16T20:52:07Z

All good?

@davidfowl yeah, there’s one annoyance with compose ignoring permissions and ownership settings when giving a source file (vs inline contents), but otherwise everything is working as expected. We can’t fix that permissions behavior (it’s a compose bug that others have by, but we may be able to workaround it. I’m inclined to tackle that in a separate PR though.

danegsta added 2 commits April 21, 2025 12:21

New DCP create files schema

fd41c81

Update keycloak realm init to use WithContainerFiles

9471516

danegsta requested review from davidfowl, sebastienros, eerhardt, karolz-ms and Copilot April 21, 2025 23:24

github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Apr 21, 2025

Copilot AI reviewed Apr 21, 2025

View reviewed changes

karolz-ms approved these changes Apr 22, 2025

View reviewed changes

eerhardt reviewed Apr 22, 2025

View reviewed changes

src/Aspire.Hosting.Keycloak/KeycloakResourceBuilderExtensions.cs Outdated Show resolved Hide resolved

danegsta added 2 commits April 24, 2025 10:13

Update tests, add helpers, cover additional scenarios

f27d19e

Merge remote-tracking branch 'upstream/main' into danegsta/cpExisting…

051e4ef

…Files

danegsta requested a review from mitchdenny as a code owner April 24, 2025 22:43

danegsta commented Apr 24, 2025

View reviewed changes

src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs Show resolved Hide resolved

danegsta added 7 commits April 25, 2025 10:24

Update more usage of BindMounts for config, make old methods Obsolete

8a165da

Don't update generated files

53b6813

Don't change isReadOnly

a2f9177

Update another init bind mount usage to WithContainerFiles

e94fed1

Add updated test file

02fd484

Merge branch 'main' into danegsta/cpExistingFiles

6b63f0e

Remove unused usings

9d6444d

danegsta added 4 commits May 9, 2025 09:20

Merge remote-tracking branch 'upstream/main' into danegsta/cpExisting…

ab8b7ac

…Files

Add support for WithContainerFiles attributes in docker compose publi…

4830606

…sh output

Merge remote-tracking branch 'upstream/main' into danegsta/cpExisting…

56659ba

…Files

Avoid using a path that Verifier will scrub

235e7c2

davidfowl added this to the 9.4 milestone May 10, 2025

davidfowl added blocked NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed blocked labels May 10, 2025

davidfowl reviewed May 16, 2025

View reviewed changes

danegsta added 3 commits May 16, 2025 10:47

Copy WithContainerFiles source files as part of publish

4ae4402

uid and gid are strings

cf90b60

Ensure configs soure paths are written unix style for portability

29d0e0b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for copying existing files via WithContainerFiles API #8908

Add support for copying existing files via WithContainerFiles API #8908

danegsta commented Apr 21, 2025 •

edited

Loading

Copilot AI left a comment

karolz-ms left a comment

eerhardt Apr 22, 2025

danegsta Apr 22, 2025

eerhardt commented Apr 22, 2025

danegsta commented May 8, 2025

danegsta commented May 9, 2025

davidfowl commented May 10, 2025 •

edited

Loading

davidfowl May 16, 2025

danegsta May 16, 2025

danegsta May 16, 2025

davidfowl May 16, 2025 •

edited

Loading

danegsta May 16, 2025

davidfowl commented May 16, 2025

danegsta commented May 16, 2025

Add support for copying existing files via WithContainerFiles API #8908

Are you sure you want to change the base?

Add support for copying existing files via WithContainerFiles API #8908

Conversation

danegsta commented Apr 21, 2025 • edited Loading

Description

Checklist

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

karolz-ms left a comment

Choose a reason for hiding this comment

eerhardt Apr 22, 2025

Choose a reason for hiding this comment

danegsta Apr 22, 2025

Choose a reason for hiding this comment

eerhardt commented Apr 22, 2025

danegsta commented May 8, 2025

danegsta commented May 9, 2025

davidfowl commented May 10, 2025 • edited Loading

davidfowl May 16, 2025

Choose a reason for hiding this comment

danegsta May 16, 2025

Choose a reason for hiding this comment

danegsta May 16, 2025

Choose a reason for hiding this comment

davidfowl May 16, 2025 • edited Loading

Choose a reason for hiding this comment

danegsta May 16, 2025

Choose a reason for hiding this comment

davidfowl commented May 16, 2025

danegsta commented May 16, 2025

danegsta commented Apr 21, 2025 •

edited

Loading

davidfowl commented May 10, 2025 •

edited

Loading

davidfowl May 16, 2025 •

edited

Loading