Skip to content

Allow disabling shared configuration check from hosting bundle. #6498

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

Merged
merged 6 commits into from
Jan 16, 2019

Conversation

jkotalik
Copy link
Contributor

@jkotalik jkotalik commented Jan 8, 2019

Issue: #4451

Description

When installing the 2.2 Windows Hosting Bundle, today, we block the use of shared applicationHost.config redirection. This was a change made in the time period between 2.2-preview3 and 2.2. However, some customers relied on this behavior when installing the hosting bundle to update a shared configuration file on the same machine. More importantly, there was no way to disable the check in the new hosting bundle.

The fix is to allow users to specify the parameter "OPT_NO_SHARED_CONFIG_CHECK" to disable the shared configuration check.

Usage:

dotnet-hosting-2.2.x.exe OPT_NO_SHARED_CONFIG_CHECK=1

Regression?

Regression from 2.1 hosting bundle.

Risk

Very low. Default scenario are still the same and this only skips a check for shared configuration.

@jkotalik jkotalik requested review from pakrym and joeloff January 8, 2019 21:35
@jkotalik jkotalik added the Servicing-consider Shiproom approval is required for the issue label Jan 8, 2019
@jkotalik
Copy link
Contributor Author

jkotalik commented Jan 8, 2019

cc @muratg @DamianEdwards

@muratg muratg added this to the 2.2.x milestone Jan 8, 2019
@muratg
Copy link
Contributor

muratg commented Jan 8, 2019

We'll review this on Thursday's shiproom.

@jamshedd jamshedd added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 10, 2019
@jamshedd jamshedd modified the milestones: 2.2.x, 2.2.2 Jan 10, 2019
@jamshedd
Copy link
Member

Approved for 2.2.2.

@muratg
Copy link
Contributor

muratg commented Jan 10, 2019

@jkotalik @pakrym Before this is checked in -- what was the reason the block was introduced in the first place? Who made that?

@jkotalik
Copy link
Contributor Author

This was introduced because a majority of the time, when IIS is redirecting configuration, it is pointing to a location where the current user doesn't have write permissions to. When the hosting bundle runs, it will fail to install with no warning/ error (just says it fails to install). There was a few customers complaining about this, and I found that many other IIS installers (UrlRewrite, ARR, etc) had a check for if there was a shared configuration used, and if so fail to install. I added this check to the ANCM msis, but then we realized there was no way to revert this behavior.

@dougbu
Copy link
Member

dougbu commented Jan 14, 2019

Not sure if this change affects the ANCM packages. If it does, please update the 2.2.2 section of eng/PatchConfig.props: https://github.com/aspnet/Extensions/blob/d8407116d183debaadb801c0cbc41d65927999d6/eng/PatchConfig.props#L7-L10

@jkotalik
Copy link
Contributor Author

This change will not affect ANCM packages, just the hosting bundle.

@jkotalik
Copy link
Contributor Author

@pakrym @joeloff updated based on feedback. The shared config check will now only run on install.

@muratg
Copy link
Contributor

muratg commented Jan 16, 2019

Where are we with this one? Is this ready to check-in?

cc @mikaelm12 @dougbu FYI.

@pakrym
Copy link
Contributor

pakrym commented Jan 16, 2019

@muratg
Copy link
Contributor

muratg commented Jan 16, 2019

FYI I believe @joeloff is OOF today.

@mikaelm12
Copy link
Contributor

Any updates here? This is the last thing we have for 2.2.2

@joeloff
Copy link
Member

joeloff commented Jan 16, 2019

I posted a comment this morning about the uninstall check. Update them to be REMOVE="ALL"

@jkotalik
Copy link
Contributor Author

Verified the installers work with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants