-
Notifications
You must be signed in to change notification settings - Fork 596
Context-Based Endpoint Resolution does not seem to work correctly when using "WithEnvironment" #8574
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
Comments
Could be a regression. cc @adamint |
Looks like var redis = builder.AddRedis("redis");
var endpoint = redis.GetEndpoint("tcp");
builder.AddRabbitMQ("rabbitmq")
.WithEnvironment("REDIS_HOST", $"{endpoint.Property(EndpointProperty.Host)}")
.WithEnvironment("REDIS_IPV4HOST", $"{endpoint.Property(EndpointProperty.IPV4Host)}")
.WithEnvironment("REDIS_URL", $"{endpoint.Property(EndpointProperty.Url)}")
; Reproed on both <Sdk Name="Aspire.AppHost.Sdk" Version="9.1.0" />
<ItemGroup>
<PackageReference Include="Aspire.Hosting.AppHost" Version="9.1.0" />
<PackageReference Include="Aspire.Hosting.Azure.Storage" Version="9.1.0" />
<PackageReference Include="Aspire.Hosting.Redis" Version="9.1.0" />
<PackageReference Include="Aspire.Hosting.RabbitMq" Version="9.1.0" />
</ItemGroup> |
Excellent that it's not a regression, just a bug. |
Logic will be here:
|
Oh I think I recall why we did this. We don't blindly replace the host and port individually. We need to see both the host and the port to replace them. |
Do you recall why that is? With This is also getting even more confusing in 9.2 with the addition of builder.AddRabbitMQ("rabbitmq")
.WithEnvironment("REDIS_HOST", $"{endpoint.Property(EndpointProperty.Host)}")
.WithEnvironment("REDIS_PORT", $"{endpoint.Property(EndpointProperty.Port)}")
.WithEnvironment("REDIS_HOSTANDPORT", $"{endpoint.Property(EndpointProperty.HostAndPort)}")
; |
I guess it's related not fully supporting #7153 . When resolving an endpoint to yourself, you usually (but not always) want the Port to reference your internal port, not your external one. That however is likely not the case if referencing the host/port of an endpoint for another resource (which is happening in this case). |
We don’t want to blindly replace the port as you might actually want the host port and not the target port. Generally this is an areas of ambiguity and we may just need to design the concept of “which network” you are resolving for with a way to override. |
Is there an existing issue for this?
Describe the bug
According to article Configuring, Referencing, and Resolving Endpoints in .NET, endpoints should be resolved based on their context. When spinning up 2 containers and using "WithEnvironment", as decribed in mentioned document, the host name is always resolved as "localhost", instead of the target container hostname.This issue was encountered when following up on advice given in this issue: issue #8561
Expected Behavior
I would expect the referenced containers hostname to be resolved instead of the value "localhost".
Steps To Reproduce
The code below is all that's needed to reproduce the issue. It makes no sense for RabbitMQ to refer to Redis, but in the end these are "just" two containers refering to each other.
Exceptions (if any)
No response
.NET Version info
9.0.201
Anything else?
No response
The text was updated successfully, but these errors were encountered: