Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 27, 2025

Description

Bind mounts in Docker Compose publishing now use environment variable placeholders instead of hardcoded host paths, improving portability across machines.

Changes:

  • Bind mount sources replaced with placeholders named {RESOURCE_NAME}_BINDMOUNT_{INDEX} (e.g., MY_CONTAINER_BINDMOUNT_0)
  • Host-specific paths written to .env file as default values
  • Docker socket paths (/var/run/docker.sock, \\.\pipe\docker_engine) preserved as-is
  • Warning logged when bind mounts are encountered to inform users of portability concerns
  • Fixed .env file generation to include default values for all captured environment variables
  • Added Windows-specific test for absolute path bind mounts (e.g., C:\Users\TestUser\Data) with snapshot files
  • Created CapturedEnvironmentVariable type to replace tuple in CapturedEnvironmentVariables for better API surface:
    • Name property is readonly to prevent breaking compose file references
    • IResource? property to track which resource the environment variable is associated with
    • Source property to hold the originating object (e.g., ContainerMountAnnotation)
  • Pass ContainerMountAnnotation and associated resource when adding bind mount environment variables
  • Added ConfigureEnvironment extension method that allows programmatic mutation of captured environment variables before they are written to the .env file (similar to ConfigureComposeFile)

Example output:

docker-compose.yaml:

volumes:
  - type: "bind"
    target: "/container/data"
    source: "${MY_CONTAINER_BINDMOUNT_0}"

.env:

# Bind mount source for my-container:/container/data
MY_CONTAINER_BINDMOUNT_0=/host/path/data

Example using ConfigureEnvironment to modify paths:

builder.AddDockerComposeEnvironment("docker-compose")
    .ConfigureEnvironment(envVars =>
    {
        foreach (var envVar in envVars.Values)
        {
            if (envVar.Source is ContainerMountAnnotation mount)
            {
                // Change absolute paths to relative paths
                envVar.DefaultValue = "./" + Path.GetFileName(mount.Source);
            }
        }
    });

Example using ConfigureEnvironment to remove generated placeholders:

var certDir = builder.AddParameter("certDir");

yarp.WithBindMount("./certs", "/app/certs", isReadOnly: true);

builder.AddDockerComposeEnvironment("docker-compose")
    .ConfigureEnvironment(envVars =>
    {
        // Remove auto-generated bind mount placeholders for yarp
        var keysToRemove = envVars.Where(kv => 
            kv.Value.Resource?.Name == "yarp" && 
            kv.Value.Source is ContainerMountAnnotation).Select(kv => kv.Key).ToList();

        foreach (var key in keysToRemove)
        {
            envVars.Remove(key);
        }
    });

yarp.PublishAsDockerComposeService((svc, infra) =>
{
    // Replace with parameter placeholder
    infra.Volumes[0].Source = certDir.AsEnvironmentPlaceholder(svc);
});

Fixes #9055

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?
Original prompt

This section details on the original issue you should resolve

<issue_title>Docker compose Bind mounts should be replace the source with a relative OS neutral environment place holder</issue_title>
<issue_description>We want to make sure that is is possible to make bind mounts work with manual intervention but make it so that there are very little machine specific values in the compose file, we can leave it in the .env.</issue_description>

<agent_instructions>Bind mounts are usually resolved to a full path but we dont want to put that in the docker file. Instead, make environment place holders for the source of each bind mount. Name it after the resource and give it an index to make it unique. The env file can have the specifc path on the host, using the most portable syntax possible.

Skip over the docker socket being mounted and leave it as is.

Also warn when you encounter a bind mount and let the user know that they have a host specific path as the bind mount source and that it can potentially make their application less portable.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Update bind mounts to use relative OS neutral placeholders Replace bind mount sources with environment placeholders in Docker Compose publishing Nov 27, 2025
Copilot AI requested a review from davidfowl November 27, 2025 10:54
Copilot finished work on behalf of davidfowl November 27, 2025 10:54
@github-actions
Copy link
Contributor

github-actions bot commented Nov 27, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13223

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13223"

@davidfowl davidfowl marked this pull request as ready for review November 27, 2025 19:03
Copilot AI review requested due to automatic review settings November 27, 2025 19:03
Copilot finished reviewing on behalf of davidfowl November 27, 2025 19:05
Copy link
Contributor

Copilot AI left a 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 enhances Docker Compose publishing to improve portability by replacing bind mount source paths with environment variable placeholders. Instead of hardcoding host-specific paths directly in the docker-compose.yaml file, the implementation now uses environment variable placeholders (e.g., ${MY_CONTAINER_BINDMOUNT_0}) with the actual paths stored in the .env file. This allows developers to customize paths for their specific machines while keeping the compose file portable.

Key changes:

  • Bind mount sources replaced with environment placeholders named {RESOURCE_NAME}_BINDMOUNT_{INDEX}
  • Docker socket paths (/var/run/docker.sock, \\.\pipe\docker_engine) preserved as-is for portability
  • Warning message logged when bind mounts are encountered to inform users of portability concerns

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Aspire.Hosting.Docker/DockerComposeEnvironmentContext.cs Core implementation that creates environment placeholders for bind mount sources and skips Docker socket paths
src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs Fixed .env file generation to include default values for captured environment variables
src/Aspire.Hosting.Docker/DockerComposePublisherLoggerExtensions.cs Added warning log message for bind mounts with host-specific paths
tests/Aspire.Hosting.Docker.Tests/DockerComposePublisherTests.cs Added three new tests covering bind mount placeholder replacement, Docker socket preservation, and mixed scenarios
tests/Aspire.Hosting.Docker.Tests/Snapshots/* Updated snapshot files verifying correct placeholder generation in yaml and default values in .env files

Assert.DoesNotContain("OLD_STAGING_KEY", envFileContent);
}

[Fact]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot add a test with a windows specific absoulte path specified for the bind mount source (that only runs on windows).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test PublishAsync_WindowsAbsoluteBindMountPath_ReplacedWithEnvironmentPlaceholders that uses Windows-specific paths like C:\Users\TestUser\Data and skips on non-Windows platforms using Assert.Skip(). Fixed in 2580b5f.

Copilot AI requested a review from davidfowl November 27, 2025 20:17
Copilot finished work on behalf of davidfowl November 27, 2025 20:17
@davidfowl
Copy link
Member

@copilot generate snapshots for the new test.

Copy link
Contributor Author

Copilot AI commented Nov 27, 2025

@copilot generate snapshots for the new test.

Generated snapshot files for the Windows test. Added both .verified.yaml and .verified.env files with the expected output (Windows paths like C:\Users\TestUser\Data). Fixed in 1e2c8ad.

Copilot finished work on behalf of davidfowl November 27, 2025 20:43
@davidfowl
Copy link
Member

@copilot We need to make 2 changes:

  • We need to pass the ContainerMountAnnotation to AddEnvironmentVariable on
    src/Aspire.Hosting.Docker/DockerComposeEnvironmentContext.cs:100
  • We should expose CapturedEnvironmentVariables but we need a type to represent the environment variable instead of a tuple.
  • Then we need to expose a ConfigureEnvironment that looks like ConfigureComposeFile, but allows mutating the CapturedEnvironmentVariables before they get written to the compose file on disk.

This should allow for mutating the full set of generated content programmatically

…k for programmatic mutation of env vars

Co-authored-by: davidfowl <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 28, 2025

@copilot We need to make 2 changes:

  • We need to pass the ContainerMountAnnotation to AddEnvironmentVariable on
    src/Aspire.Hosting.Docker/DockerComposeEnvironmentContext.cs:100...

Implemented all three changes in 227dbfd:

  1. Pass ContainerMountAnnotation to AddEnvironmentVariable as the source
  2. Created CapturedEnvironmentVariable type to replace the tuple in CapturedEnvironmentVariables
  3. Added ConfigureEnvironment extension method that allows mutating the captured environment variables before they are written to the .env file

Added a test demonstrating how to use ConfigureEnvironment to modify bind mount paths from absolute paths to relative paths.

Copilot finished work on behalf of davidfowl November 28, 2025 07:21
@davidfowl
Copy link
Member

@copilot The name on CapturedEnvironmentVariable needs to be readonly. Changing it can mess up the references in the docker compose file.

We also need IResource? on CapturedEnvironmentVariable because sometimes the source might be an object that has no resouce reference but might be an annotation on a resource. We want to be able to tell which resource if any that this environment variable is associated with.

Add a test that looks something like:

// Mount certs directory
var certDir = builder.AddParameter("certDir");

yarp.WithBindMount("./certs", "/app/certs", isReadOnly: true);

yarp.PublishAsDockerComposeService((svc, infra) =>
{
    // Work around before https://github.com/dotnet/aspire/pull/13223 is merged
    infra.Volumes[0].Source = certDir.AsEnvironmentPlaceholder(svc);
});

This sample needs to be able to remove the generated placeholder the default logic added

…moving generated placeholders

Co-authored-by: davidfowl <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 28, 2025

@copilot The name on CapturedEnvironmentVariable needs to be readonly. Changing it can mess up the references in the docker compose file.

We also need IResource? on CapturedEnvironmentVariable becau...

Implemented in 15a57ab:

  1. Name property on CapturedEnvironmentVariable is already readonly (getter only)
  2. Added IResource? property to CapturedEnvironmentVariable to track which resource the env var is associated with
  3. Updated AddEnvironmentVariable to accept and pass through the resource parameter
  4. Updated ProcessVolumes to pass serviceResource.TargetResource when creating bind mount env vars
  5. Added test PublishAsync_ConfigureEnvironment_CanRemoveGeneratedPlaceholder that demonstrates the scenario - creating a parameter, using ConfigureEnvironment to remove the auto-generated bind mount placeholder, and using PublishAsDockerComposeService to override the volume source with the parameter placeholder

Copilot finished work on behalf of davidfowl November 28, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker compose Bind mounts should be replace the source with a relative OS neutral environment place holder

3 participants