Skip to content

Conversation

@mitchdenny
Copy link
Member

Fixes: #8332

This PR adds a hack in to filter out emission of https service discovery variables in Docker Compose. This is because we don't yet do anything around configuring TLS with Docker Compose either in terms of having a certificate file injected into the container or setting up some kind of proxy.

As a result, the webfrontend in the default template can't talk to the API because it preferences HTTPS because the variable is there.

Copilot AI review requested due to automatic review settings March 28, 2025 04: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 pull request addresses the issue of inadvertently exposing HTTPS service discovery variables in Docker Compose, which prevents the webfrontend from connecting to the API due to its preference for HTTPS.

  • Added a helper method to filter out HTTPS service discovery variables from the environment variables.
  • Updated Docker Compose configuration to remove the problematic HTTPS variable entry.

Reviewed Changes

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

File Description
src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs Introduces the RemoveHttpsServiceDiscoveryVariables method to filter out HTTPS variables.
playground/publishers/Publishers.AppHost/docker-compose.yaml Removes the HTTPS service discovery variable to align with the updated filtering logic.
Comments suppressed due to low confidence (2)

src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs:341

  • [nitpick] Consider extracting the 'services__' prefix into a named constant to improve clarity and maintainability.
kvp.Key.StartsWith("services__")

src/Aspire.Hosting.Docker/DockerComposePublishingContext.cs:335

  • Add unit tests for RemoveHttpsServiceDiscoveryVariables to verify that HTTPS service discovery variables are correctly removed.
private static void RemoveHttpsServiceDiscoveryVariables(Dictionary<string, object> environmentVariables)

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 28, 2025
@mitchdenny mitchdenny self-assigned this Mar 28, 2025
@mitchdenny mitchdenny added area-deployment and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 28, 2025
@mitchdenny mitchdenny added this to the 9.2 milestone Mar 28, 2025
@mitchdenny mitchdenny merged commit bc37e0c into main Mar 28, 2025
171 checks passed
@mitchdenny mitchdenny deleted the mitchdenny/dont-emit-https-endpoints branch March 28, 2025 05:06
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker Compose publisher is emitting unusable HTTPS service discovery environment variables.

3 participants