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

host and app config, appsettings Urls takes precedence over ASPNETCORE /2 #25686

Merged
merged 16 commits into from
May 10, 2022

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Apr 20, 2022

Fixes #25626
Internal review URL

@Hoffs @chertby please review.

@Rick-Anderson Rick-Anderson changed the title host and app config, appsettings Urls takes precedence over ASPNETCOR… host and app config, appsettings Urls takes precedence over ASPNETCORE Apr 20, 2022
@Rick-Anderson
Copy link
Contributor Author

@serpent5 > Application config falls back to host config,
see this comment

What does falls back mean? Host config first, then app config? Just getting started. Please review the issue, I'll have question for you Thursday morning.

@serpent5
Copy link
Contributor

I think it ultimately means a value will be read from host config if it doesn't exist in app config. App config overrides host config.

@Rick-Anderson Rick-Anderson marked this pull request as ready for review April 20, 2022 22:18
@Rick-Anderson
Copy link
Contributor Author

@halter73 please review


<a name="default"></a>

## Default configuration
## Default application configuration
Copy link
Member

@halter73 halter73 Apr 27, 2022

Choose a reason for hiding this comment

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

I think the structure should be as follows.

## Application and Host Configuration
// summary describing config fallback behavior between config providers
// and the fallback behavior between application config and host config
### Default application configuration sources
### Default host configuration sources
### Host bootstrap settings

I also think that the section discussing default application config should be reordered to go from highest to lowest priority along with the same list for default host configuration sources. As ordered right now, the highest priority source for application config (Command-line arguments) is in the middle of the combined list of application and host sources which makes no sense. Application config falls back to host config, so I want to be able to skim down the list of sources without the order resetting in the middle.

The following should be deleted:

  1. ChainedConfigurationProvider : Adds an existing IConfiguration as a source. In the default configuration case, adds the host configuration and setting it as the first source for the app configuration.

It's super confusing. Hardly anyone should care about the mechanism that application config falls back to hosting config (which isn't entirely accurate anymore anyway for WebApplicationBuilder). I think it similar to the user secrets where the actual configuration provider used to make this fallback work (if any) is unimportant.

The last (lowest-priority) element of the list of default application sources should just read:

  1. A fallback to the host configuration described in the next section

There should be nothing in between the list of default application sources and default host sources other than the section heading. This should make the full list of default config sources in priority order more easily scannable.

Can we also update [App secrets](xref:security/app-secrets) to [user-secrets](xref:security/app-secrets) and Environment variables to Non-prefixed environment variables similar to in my comment?

Rick-Anderson and others added 2 commits April 27, 2022 15:40
Co-authored-by: Stephen Halter <halter73@gmail.com>
Co-authored-by: Ignas Maslinskas <5737899+Hoffs@users.noreply.github.com>
@Rick-Anderson Rick-Anderson changed the title host and app config, appsettings Urls takes precedence over ASPNETCORE host and app config, appsettings Urls takes precedence over ASPNETCORE /2 May 4, 2022
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

This looks really good. @Tratcher said he might also take a look.

aspnetcore/fundamentals/configuration/index.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/configuration/index.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/configuration/index.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/configuration/index.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/configuration/index.md Outdated Show resolved Hide resolved
Co-authored-by: Stephen Halter <halter73@gmail.com>
@Tratcher
Copy link
Member

Tratcher commented May 9, 2022

:shipit:

Co-authored-by: Stephen Halter <halter73@gmail.com>
@Rick-Anderson Rick-Anderson merged commit e1370e6 into main May 10, 2022
@Rick-Anderson Rick-Anderson deleted the config-Urls/ra/2 branch May 10, 2022 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

appsettings Urls takes precedence over ASPNETCORE_URLS in 6.0.4
5 participants