Enable universal support for container-to-host communication#12361
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12361Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12361" |
14dcd58 to
ab6ad2f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enables universal container-to-host communication by leveraging a new container tunnel capability in DCP. The changes introduce a network-aware model where EndpointReference objects are resolved in the context of specific networks (represented by NetworkIdentifier). This allows endpoints to be resolved differently depending on whether they're accessed from localhost, container networks, or public internet contexts.
Key changes:
- Introduced
NetworkIdentifiertype and network-aware value resolution viaINetworkAwareValueProvider - Modified
EndpointReferenceandEndpointAnnotationto support multiple allocated endpoints per network context - Added container tunnel proxy infrastructure to enable container-to-host connectivity
- Refactored expression resolution to be network-context-aware instead of using a single "container host name"
Reviewed Changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ApplicationModel/Network.cs | Introduces NetworkIdentifier and KnownNetworkIdentifiers/KnownHostNames for network context modeling |
| src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs | Adds support for multiple allocated endpoints per network and tracks endpoint references |
| src/Aspire.Hosting/ApplicationModel/EndpointReference.cs | Modified to resolve endpoints in specific network contexts |
| src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs | Refactored to use NetworkIdentifier instead of containerHostName parameter |
| src/Aspire.Hosting/ApplicationModel/INetworkAwareValueProvider.cs | New interface for network-aware value resolution |
| src/Aspire.Hosting/Dcp/DcpExecutor.cs | Major refactoring to create container tunnels and handle network-specific endpoint allocation |
| src/Aspire.Hosting/Dcp/Model/ContainerTunnel.cs | New DCP model types for container tunnel proxy functionality |
| tests/**/*.cs | Updated tests to use new network-aware APIs and fix broken assumptions about container host names |
Comments suppressed due to low confidence (1)
src/Aspire.Hosting/ApplicationModel/EndpointReference.cs:1
- Add space between XML tag and 'object' in the returns documentation
// Licensed to the .NET Foundation under one or more agreements.
992e539 to
2ad7c72
Compare
8b77a29 to
157de6d
Compare
| /// <summary> | ||
| /// The identifier of the network that serves as the context for value resolution. | ||
| /// </summary> | ||
| public NetworkIdentifier? Network { get; init; } |
There was a problem hiding this comment.
I assume we expect this value to not be set in certain scenarios. For example, I'd expect this to be null during deployment mode. If so, how do we think about modeling the ValueProviderContext under different execution contexts?
There was a problem hiding this comment.
It is a property bag-like, so I can imagine stuffing some deployment-mode data into it in future. Maybe, as you suggested offline, have a proper property bag (dictionary) with data on it too.
There was a problem hiding this comment.
I think we could model this like:
class ValueProviderContext
{
public IServiceProvider? Services { get; init; }
public IResource? Caller { get; init; }
}
class NetworkValueProviderContext : ValueProviderContext
{
public NetworkIdentifier? Network { get; init; }
} | .WithHttpEndpoint(name: "primary") | ||
| .WithEndpoint("primary", ep => | ||
| { | ||
| ep.AllocatedEndpoint = new AllocatedEndpoint(ep, "localhost", 90, targetPortExpression: """{{- portForServing "container1_primary" -}}"""); |
There was a problem hiding this comment.
Is this needed? What happens if we leave this as is?
There was a problem hiding this comment.
Yes. If this change is reverted, the test will hang forever, waiting for endpoint address to be allocated.
The reason is we are simulating endpoint allocation and then testing container-to-container communication. So we need an AllocatedEndpoint in container network space.
Endpoint.AllocatedEndpoint = something allocates endpoint in localhost network space.
Description
Leverages the container tunnel capability that has been recently added to DCP to provide container-to-host connectivity independent from what is supported by container orchestrators.
To make this work, I had to make some changes to how
EndpointReferenceswork. Specifically,EndpointReferencesare not resolved in context of a network (represented by abstractNetworkIdentifier). They are also tracked by theirEndpointAnnotationso we can answer questions like "who is referencing this particularEndpoint".Marking as draft for now because this is a big change that affects the core of Aspire application model. It needs more testing, automated tests, and most likely, fixes to existing tests.
Fixes #6547
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate