search

aspnet / Templates

This repo is OBSOLETE - please see the README file for information
Other
150 stars 57 forks source link

Remove appsettings.{env.EnvironmentName}.json files #678

Closed cwe1ss closed 7 years ago

cwe1ss commented 8 years ago

ASP.NET Core applications default to Production when there's no ASPNETCORE_ENVIRONMENT environment variable or launchSettings.json file. This has been done for security reasons to make sure, production environments don't show stack traces etc.

However, this also means people have to be careful that they don't get unintended side effects when they accidentally end up targeting the production environment. This could happen for various reasons, but the most obvious one might be that dotnet run does not honor the launchSettings.json file. If people don't set the environment variable on their PC, they could easily target Production when they switch between Visual Studio and dotnet cli.

Of course, people should not store sensitive data (e.g. connection strings) in these files, but even regular "app settings" could lead to unintended side effects (e.g. webhook URLs, ...)

There's also the scenario where people use certificates to get secrets from e.g. Azure Key Vault. In this case, the thumbprint isn't a secret and could be committed to source control. If the production certificate happens to end up on a developer's machine (this is a no-go but it could happen due to troubleshooting sessions etc.), "bad things" :tm: will happen.

Another aspect of this is that people who use IIS/IIS express don't see which environment they are targeting.

The best solution for this would be to make sure, production configs aren't in the same project. Instead, people should use environment variables, azure app settings, CI systems that merge the files on deployment, etc.

This PR would remove the active promotion of this feature. It would require people to make an active decision (and hopefully understand the advantages/disadvantages) if they wanted to keep using this workflow.

Concerns about defaulting to production have been raised in aspnet/hosting ( https://github.com/aspnet/Hosting/issues/863, https://github.com/aspnet/Hosting/pull/720 ), so I'm creating this PR as a discussion point based on @blowdart's and @muratg's feedback.

dnfclas commented 8 years ago

Hi @cwe1ss, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

peterblazejewicz commented 8 years ago

Is there any high-level overview for this change or discussion - other then linked one? I could be wrong, but isn't that only VS tooling creates environment files by default? Other tools do not create multiple environment settings by default, eg.: https://github.com/dotnet/cli/tree/rel/1.0.0/src/dotnet/commands/dotnet-new/CSharp_Web

cwe1ss commented 8 years ago

I don't know of any other discussion about this. I thought that this issue could be used for it. I just created it as a PR because it was easy to implement and better shows the consequences.

I guess @blowdart and @muratg will add their opinion because they said I could create an issue for this topic.

The dotnet new templates definitely need to have the same logic, should this be changed.

phenning commented 7 years ago

We will be keeping these files now that the templates use them by default