Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Proxyless endpoint does not default the TargetPort when using the callback ctor #4225

Closed
davidebbo opened this issue May 18, 2024 · 0 comments · Fixed by #4226
Closed

Proxyless endpoint does not default the TargetPort when using the callback ctor #4225

davidebbo opened this issue May 18, 2024 · 0 comments · Fixed by #4226

Comments

@davidebbo
Copy link
Contributor

davidebbo commented May 18, 2024 •
edited

Suppose you have something like:

                    builder.WithEndpoint(schemeAsEndpointName ?? endpoint.EndpointName, e =>
                    {
                        e.Port = 5000;
                        e.UriScheme = "http";
                        e.IsProxied = false;
                    });

The intent is to have the TargetPort defaults to the Port, per

aspire/src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs

Lines 51 to 68 in 5f2af96

if (!isProxied)
{
// For proxy-less Endpoints the client port and target port should be the same.
// Note that this is just a "sensible default"--the consumer of the EndpointAnnotation is free
// to change Port and TargetPort after the annotation is created, but if the final values are inconsistent,
// the associated resource may fail to run.
// It also depends on what the EndpointAnnotation is applied to.
// In the Container case the TargetPort is the port that the process listens on inside the container,
// and the Port is the host interface port, so it is fine for them to be different.
if (port is null && targetPort is not null)
{
Port = targetPort;
}
if (port is not null && targetPort is null)
{
TargetPort = port;
}
}

But this breaks because that logic happens in the ctor, before the callback is called. So we end up without a DefaultPort, which blows up at runtime.
davidebbo added a commit to davidebbo/aspire that referenced this issue May 18, 2024
@davidebbo
Fix port/targetport fallback logic in proxyless path
498ae4e 
Fixes dotnet#4225
davidebbo added a commit to davidebbo/aspire that referenced this issue May 18, 2024
@davidebbo
Fix port/targetport fallback logic in proxyless path
c178586 
Fixes dotnet#4225
@davidebbo davidebbo mentioned this issue May 18, 2024
Merged
davidfowl pushed a commit that referenced this issue May 22, 2024
@davidebbo
Fix port/targetport fallback logic in proxyless path (#4226)
b88cc07 
* Fix port/targetport fallback logic in proxyless path

Fixes #4225

* Add tests for port/targetport logic

* Allow caller to set Port/TargetPort to null

* Make the tests cover more scenarios
radical pushed a commit to radical/aspire that referenced this issue May 22, 2024
@davidebbo @radical
Fix port/targetport fallback logic in proxyless path (dotnet#4226)
edf7d5f 
* Fix port/targetport fallback logic in proxyless path

Fixes dotnet#4225

* Add tests for port/targetport logic

* Allow caller to set Port/TargetPort to null

* Make the tests cover more scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix port/targetport fallback logic in proxyless path davidebbo/aspire
1 participant
@davidebbo