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

[release/6.0] Make UseUrls() override default hosting config #39836

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

halter73
Copy link
Member

@halter73 halter73 commented Jan 28, 2022

Description

Prior to this change, default config (typically loaded from DOTNET_/ASPNET_ environment variables and command line arguments) could override the application-level configuration. This would prevent GenericWebHostService from seeing the latest configuration set by UseUrls() of DOTNET_URLS, ASPNET_URLS or --urls was set.

Fixes #38185

Customer Impact

This is a big gotcha to customers using WebApplicationBuilder (which is used in all the ASP.NET Core 6 templates) who expect the following to work:

var builder = WebApplication.CreateBuilder(args);
builder.WebHost.UseUrls("http://*:8080");
var app = builder.Build();
app.Run();

A comment on the issue suggesting we patch this has gotten 5 thumbs ups not counting mine.

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low

This is a small well tested change which only affects the loading of default config sources and those added via a HostFactoryResolver to host configuration.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@halter73 halter73 requested a review from davidfowl January 28, 2022 17:27
@halter73 halter73 requested a review from Tratcher as a code owner January 28, 2022 17:27
@ghost ghost added the area-runtime label Jan 28, 2022
@ghost ghost added this to the 6.0.x milestone Jan 28, 2022
@ghost
Copy link

ghost commented Jan 28, 2022

Hi @halter73. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@halter73 halter73 added Servicing-consider Shiproom approval is required for the issue and removed area-runtime labels Jan 28, 2022
@ghost
Copy link

ghost commented Jan 28, 2022

Hi @halter73. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Feb 1, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.3 Feb 1, 2022
@wtgodbe
Copy link
Member

wtgodbe commented Feb 2, 2022

/azp run

@wtgodbe
Copy link
Member

wtgodbe commented Feb 2, 2022

We can merge this once CI is green & reviewers have signed off

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@halter73 halter73 requested a review from captainsafia February 2, 2022 18:38
@halter73
Copy link
Member Author

halter73 commented Feb 2, 2022

@davidfowl @Tratcher @captainsafia Can one of you give this a review?

@wtgodbe
Copy link
Member

wtgodbe commented Feb 2, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@wtgodbe wtgodbe enabled auto-merge (squash) February 2, 2022 23:00
@wtgodbe wtgodbe merged commit c6154ba into release/6.0 Feb 3, 2022
@wtgodbe wtgodbe deleted the halter73/38185-6.0 branch February 3, 2022 00:28
@halter73 halter73 linked an issue Feb 5, 2022 that may be closed by this pull request
@wtgodbe
Copy link
Member

wtgodbe commented Feb 16, 2022

@halter73 this breaks in conjunction with dotnet/runtime#63816 - we're going to revert it for now to unblock builds. Can try again next month when there's more time to react.

halter73 pushed a commit that referenced this pull request Mar 5, 2022
…t-efcore dnceng/internal/dotnet-runtime

 - Revert "Make UseUrls() override default hosting config (#39836)"
@halter73 halter73 modified the milestones: 6.0.3, 6.0.4 Apr 14, 2022
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 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 Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Builder.WebHost.UseUrls does not seem to override default url
7 participants