Ensure WithHostHttpsPort works when chained inline#13623
Ensure WithHostHttpsPort works when chained inline#13623danegsta merged 2 commits intodotnet:mainfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13623Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13623" |
There was a problem hiding this comment.
Pull request overview
This PR fixes a timing issue where calling .WithHostHttpsPort() inline didn't work properly because the HTTPS endpoint was being created later in the BeforeStartEvent. The solution stores the desired host HTTPS port in the YarpResource and applies it when the HTTPS endpoint is created.
Key Changes:
- Introduced a new
HostHttpsPortproperty onYarpResourceto defer port assignment until endpoint creation - Modified
WithHostHttpsPortto store the port value instead of attempting to modify a potentially non-existent endpoint - Updated the
BeforeStartEventhandler to use the storedHostHttpsPortvalue when creating the HTTPS endpoint - Added comprehensive test coverage for both successful and unsuccessful scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Aspire.Hosting.Yarp/YarpResource.cs |
Added HostHttpsPort property to store the desired HTTPS host port until the endpoint is created |
src/Aspire.Hosting.Yarp/YarpResourceExtensions.cs |
Simplified WithHostHttpsPort to store port value on resource; updated BeforeStartEvent handler to apply stored port when creating HTTPS endpoint |
tests/Aspire.Hosting.Yarp.Tests/AddYarpTests.cs |
Added two test cases validating that WithHostHttpsPort works correctly when HTTPS is configured and is ignored when HTTPS is not configured |
|
Thanks for the quick work on the issue! |
|
I think I have the same issue #13674. |
|
Do you know if there is a workaround available? Would be helpful to mitigate the issue until this is approved and merged. |
|
I built my own workaround inlining the resource registration: var gateway = builder
//.AddYarp("Gateway")
//.WithHttpsEndpoint(8443)
.AddYarpWithHttpsHostPort("Gateway", 8443)
.WithHostPort(8080) |
The other workaround is to use a BeforeStartEvent handler (for timing reasons it has to be registered after the YARP resource is created): var gateway = builder.AddYarp("gateway");
builder.Eventing.Subscribe<BeforeStartEvent>((_, _) =>
{
gateway.WithHostHttpsPort(8443);
return Task.CompletedTask;
}); |
|
/backport to release/13.1 |
|
Started backporting to release/13.1: https://github.com/dotnet/aspire/actions/runs/20417545148 |
Description
Fixes an issue where
.WithHostHttpsPortwouldn't work due to the HTTPS endpoint being created inBeforeStartEvent.