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.

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

Read HTTP_PORTS and HTTPS_PORTS into IServerAddresses #44194

Merged
merged 3 commits into from
Sep 29, 2022

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Sep 26, 2022

Fixes #43135

Container images want to be able to specify ports rather than urls. We want to avoid overloading the definition for ASPNETCORE_URLS, so this adds support for ASPNETCORE_HTTP_PORTS and ASPNETCORE_HTTPS_PORTS. URLS still takes priority, these new variables will be ignored if URLS is set.

Each is a list of semi-colon delimited values that will be mapped to http(s)://*:{port} respectively.

This will cause Kestrel to bind to all public IPs with IPv4 and IPv6 for the given ports.

This would also work with Http.Sys though we have a warning there not to use wildcards.
https://learn.microsoft.com/en-us/aspnet/core/fundamentals/servers/httpsys?view=aspnetcore-6.0#configure-windows-server

This is only implemented for generic web host and newer, not legacy webhost.

@Tratcher Tratcher added this to the 8.0-preview1 milestone Sep 26, 2022
@Tratcher Tratcher requested a review from davidfowl September 26, 2022 23:07
@Tratcher Tratcher self-assigned this Sep 26, 2022
@Tratcher Tratcher requested a review from halter73 as a code owner September 26, 2022 23:07
@@ -73,6 +73,21 @@ public async Task StartAsync(CancellationToken cancellationToken)
urls = Options.WebHostOptions.ServerUrls;
}

if (string.IsNullOrEmpty(urls))
{
// HTTP_PORTS and HTTPS_PORTS, these are lower priority than Urls.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we log a warning if we ignore HTTP_PORTS and/or HTTPS_PORTS becuase URLS is set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warn: Microsoft.AspNetCore.Hosting.Diagnostics[15]
      Overriding HTTP_PORTS '5002' and HTTPS_PORTS ''. Binding to values defined by URLS instead 'http://*:5003'.
warn: Microsoft.AspNetCore.Server.Kestrel[0]
      Overriding address(es) 'http://*:5003'. Binding to endpoints defined via IConfiguration and/or UseKestrel() instead.
info: Microsoft.Hosting.Lifetime[14]
      Now listening on: http://[::]:5004

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of warnings.......... 😄

@Tratcher Tratcher enabled auto-merge (squash) September 29, 2022 16:42
@Tratcher Tratcher merged commit 0117723 into dotnet:main Sep 29, 2022
@Tratcher Tratcher deleted the tratcher/httpport branch September 29, 2022 17:09
@Tratcher Tratcher added the blog-candidate Consider mentioning this in the release blog post label Sep 30, 2022
@ghost
Copy link

ghost commented Sep 30, 2022

@Tratcher, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions blog-candidate Consider mentioning this in the release blog post
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow specifying host and port or port in ASPNETCORE_URLS
6 participants