-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Update web templates to use port range for Kestrel #34410
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
Conversation
phenning
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my single comment below, rest looks good so far.
| "ports": [ | ||
| { | ||
| "name": "HttpPort", | ||
| "name": "kestrelHttpPort", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ports section from the host file isn't used by the VS 2022 any longer - this is when VS generated the ports - since the generator is built in you don't need to modify this.
In fact, I just noticed my PR for renaming the host files was not completed, you'll need to rebase when it completes. I hadn't fully cleaned up the ports in that PR, since I wanted to be 100% sure it's not needed. I'll likely send a PR next week to do more cleanup - we won't need the "supportedAuthentications" section in the host file any longer either due to recent dev17 changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK so that whole section will/can be removed from that file then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
Selected 5000-5300 for HTTP and 7000-7300 for HTTPS. Note 6000 is reserved (see https://github.com/dotnet/templating/blob/94b1e0839ab49026b3ff654f2b5e5e1a1b4b2c53/src/Microsoft.TemplateEngine.Orchestrator.RunnableProjects/Macros/Config/GeneratePortNumberConfig.cs#L19).
c1db0b5 to
5f899dd
Compare
Have you thought of auth here? This can make AAD/AAD B2C auth harder, since they depend on the url for things like the redirect callback and so on. |
|
@javiercn auth code looks at |
|
@javiercn none of that should be hard-coded though right? We already do this today for IIS Express and auth works there. |
|
Part of me wants to block this until we get sequential ports 🙃 |
|
😭 One of my favorite things about Kestrel vs. IIS Express is being able to rely on a new project always listening on 5000. I think we should wait on sequential ports at a minimum. Or maybe just not do this at all. |
|
If a user creates a web project and a web api project (or any other web project), we can’t have them use the same ports because they can’t launch more than one at a time. This is an important scenario for VS, so we need to enable that in some way. |
|
@davidfowl @halter73 if we care that deeply we could make a simple PR to dotnet/templating to update the port generator to support a parameter that indicates the port number attempt should not be randomized. That way, as long as there's nothing listening on port 5000/7000 at project creation time, those are the ports you'll get. Of course, doing that will likely lead to folks getting conflicts far more often because if they create a project in their solution while the others aren't running, they'll get 5000/7000. Do that just twice and you have the conflict again. I think the real solution to this eventually is CLI preferences where you could simply change your personal preference for a given template. I know the CLI team wants to enable scenarios like that eventually but not sure when. In any case, I made the change to templating in a branch but in doing so came to the realization above. |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Updates all the project templates to use a port range for the Kestrel profile in
launchSettings.jsonso that new projects don't all get created with the same port. This is the existing behavior for the IIS Express profile but in .NET 6 the default profile is changing to the Kestrel profile.Selected 5000-5300 for HTTP and 7000-7300 for HTTPS. Note 6000 is reserved (see https://github.com/dotnet/templating/blob/94b1e0839ab49026b3ff654f2b5e5e1a1b4b2c53/src/Microsoft.TemplateEngine.Orchestrator.RunnableProjects/Macros/Config/GeneratePortNumberConfig.cs#L19).
Addresses #34363