Skip to content

Conversation

@DamianEdwards
Copy link
Member

@DamianEdwards DamianEdwards commented Jul 16, 2021

Updates all the project templates to use a port range for the Kestrel profile in launchSettings.json so 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

@DamianEdwards DamianEdwards added this to the 6.0-rc1 milestone Jul 16, 2021
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jul 16, 2021
Copy link
Member

@phenning phenning left a 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",
Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

@DamianEdwards DamianEdwards force-pushed the damianedwards/templates-kestrel-ports branch from c1db0b5 to 5f899dd Compare July 20, 2021 19:57
@DamianEdwards DamianEdwards changed the title [WIP] Update web templates to use port range for Kestrel Update web templates to use port range for Kestrel Jul 20, 2021
@DamianEdwards DamianEdwards marked this pull request as ready for review July 20, 2021 22:47
@DamianEdwards DamianEdwards requested review from a team and Pilchie as code owners July 20, 2021 22:47
@DamianEdwards DamianEdwards requested a review from phenning July 20, 2021 22:47
@javiercn
Copy link
Member

Updates all the project templates to use a port range for the Kestrel profile in launchSettings.json so 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.

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.

@sayedihashimi
Copy link
Member

@javiercn auth code looks at launchSettings.json to get the ports. If I understand when the project is created, the full URL with ports is listed in that file. I believe that this should work without any issues. Adding @vijayrkn and @deepchoudhery to double check.

@DamianEdwards
Copy link
Member Author

@javiercn none of that should be hard-coded though right? We already do this today for IIS Express and auth works there.

@davidfowl
Copy link
Member

Part of me wants to block this until we get sequential ports 🙃

@halter73
Copy link
Member

😭 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.

@sayedihashimi
Copy link
Member

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.

@DamianEdwards
Copy link
Member Author

DamianEdwards commented Jul 21, 2021

@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.

@DamianEdwards
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-templates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants