search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.15k stars 9.92k forks source link

Framework code should not depend on IsDevelopment #28937

Open Xerillio opened 3 years ago

Xerillio commented 3 years ago

Is your feature request related to a problem? Please describe.

I found that I had to manually check for the appropriate environment name(s) to enable user secrets in my application. ASP.NET Core only enables user secrets if the currently running environment is named "Development" and therefore require custom implementation to enable if your team does not use the same naming conventions as has been predefined by ASP.NET Core.

The code snippet in question: https://github.com/dotnet/aspnetcore/blob/d7187b01e87761c778a5c406802e8bfd9451f3cf/src/DefaultBuilder/src/WebHost.cs#L176-L183

IsDevelopment and its sibling extension methods are used internally in ASP.NET Core to enable/disable certain features when running the application in different environments. I suspect some 3rd party libraries do so likewise (although I have no experience with such). This helps a developer setting up their local environment for better debugging capabilities etc., while avoiding enabling those same features in production environments as they may be unsafe/unfit for a production application.

Personally, I would consider a development environment to not only be the developers local machine but could just as well be a server-hosted application shared by multiple developers where the application is deployed prior to a "test" environment. As such two environments may (and very likely will) use a different configuration setup while still enabling certain debugging features, having only one name for what constitutes a "development" environment limits or completely disables the help provided by ASP.NET Core out of the box.

Describe the solution you'd like

To combat the issue described above I would suggest allowing the developer to define when IsDevelopment etc. should return true. This could be done e.g. through a configuration variable similar to ASPNETCORE_ENVIRONMENT (say ASPNETCORE_DEVELOPMENT_ENVIRONMENTS) that takes a comma-separated list of names considered to be a "development" environment. Alternatively, instead of implementing IsDevelopment etc. as extension methods they could be implemented such that they're overridable.

Additional context

I noticed #26539 has already brought this up but was closed. However, I believe the feature request deserves a bit more consideration with some additional context. Hopefully I provided that.

Tratcher commented 3 years ago

We've avoided using IsDevelopment in most framework code exactly because of issues like this. IsDevelopment is primarily used in app/template code so you can customize the environment logic.

Rather than trying to fix IsDevelopment, we should fix these sites where it's used to be based on more targeted external input and have the IsDevelopment check moved back to app code like Program.CreateHostBuilder.

BrennanConroy commented 3 years ago

We need to change this in Runtime too for generic host.

And look at other spots that we use this pattern.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

shirhatti commented 3 years ago

There are currently a few places where we use IsDevelopment() in framework code, where isn't an alternative configuration (via options) to customize the behavior.

We need to address all these cases:

BrennanConroy commented 3 years ago

Triage: Reason we'd want options on the host for these instead of telling users to add the config themselves is because of config ordering. Today we add config in a specific order as last one wins, if users added it, it would be last and have unexpected behavior.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

davidfowl commented 3 years ago

~We're not going to be doing this. Also this needs to be done at the hosting layer (moved to dotnet/runtime), if we do choose to reconsider in the future.~

Seems like we need to design an API for the pieces we want to do.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

javiercn commented 3 years ago

I think this depends on a case by case basis. There are things in WebHostBuilder.CreateDefault that will have an impact on ordering, and maybe those things can be tweaked via environment variables. Something like ASPNECORE_DEV=true or similar to trigger Dev/Local setup/behaviors on the host setup as opposed to remote/publish/staging setups/behaviors and that allows folks to use whatever ASPNETCORE_ENVIRONMENT they want.

In general that normally is the biggest distinction. There are features that we build like UserSecrets or StaticWebAssets that we don't want used outside of a local/dev context, and the best way we've had to do that has been for IsDevelopment.

I think this works for the majority of our customers and we should keep it as it is. At most, we should make sure customers using different environment conventions can achieve a similar experience, however I don't think that necessarily must come without work on their part to replicate some of our opinions.

Tratcher commented 3 years ago

Review: Docker https dev certs: https://github.com/dotnet/aspnetcore/issues/33220

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.