BREAKING CHANGE: xWebConfigProperty: Added Location to allow access to locked sections of ApplicationHost.Config

[geoffguynn]

xWebConfigPropertyLocation: Added to allow access to locked sections of ApplicationHost.Config

Pull Request (PR) description

This new resource adds an additional parameter for the Location of the configuration property.

This Pull Request (PR) fixes the following issues

None

Task list

• Added an entry under the Unreleased section of the change log in the README.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Resource documentation added/updated in README.md.
• Resource parameter descriptions added/updated in README.md, schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[msftclas]

All CLA requirements met.

[codecov-io]

Codecov Report

Merging #400 into dev will decrease coverage by 0.02%.
The diff coverage is 95%.

@@            Coverage Diff             @@
##              dev     #400      +/-   ##
==========================================
- Coverage   90.67%   90.64%   -0.03%     
==========================================
  Files          17       17              
  Lines        2412     2405       -7     
==========================================
- Hits         2187     2180       -7     
  Misses        225      225

Impacted Files	Coverage Δ
...FT_xWebConfigProperty/MSFT_xWebConfigProperty.psm1	92% <95%> (-0.99%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85ff094...6e1bf9c. Read the comment docs.

[regedit32]

Hello @geoffguynn , thank you for submitting this PR. Can you please open an issue for this change? At first look, it appears that this is functionality that we can extend in the xWebConfigProperty resource instead of creating a new one. An open issue would be the best place for the community to discuss. Thank you!

[geoffguynn]

As requested
#401

[regedit32]

@geoffguynn There are still some failing tests in the AppVeyor build, so we'll wait to review until those are passing. Also, can you add unit and integration tests to validate the changes as well? We'll want to cover both scenarios of using the resource with and without a value for Location. Thanks!

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[geoffguynn]

@regedit32 Had a few spare cycles to get back to this, let me know if there are any other changes needed.

[regedit32]

@geoffguynn Thanks for resolving the test failures. I only had time to do a quick review with a few comments, but I will try to come back later and fully review. There was another PR to merge, can you also rebase? Thank you!

Reviewable status: 0 of 8 files reviewed, 5 unresolved discussions (waiting on @geoffguynn)

---

README.md, line 321 at r6 (raw file):

<<<<<<< HEAD

Looks like changes weren't saved when resolving merge conflicts. This syntax was left over.

---

README.md, line 322 at r6 (raw file):

* Added new reosurce xWebConfigProperty extening functionality provided by xWebConfigProperty to allow writing of locked sections in ApplicationHost.Config

Based on your latest commits, this should have a different note about adding Location property to the xWebConfigProperty resource

---

README.md, line 330 at r6 (raw file):

>>>>>>> 0b60a44eea60673f883544ecbf5cd294ecc750a8

Same as the previous comment, this syntax was left over when resolving merge conflicts.

---

DSCResources/MSFT_xWebConfigProperty/MSFT_xWebConfigProperty.psm1, line 74 at r6 (raw file):

    if ( -not($existingValue) ) {

Looks like throughout the file the opening brace has been moved (perhaps automatically by local .vscode settings?). Per DSC resource style guidelines, opening braces should be preceded by a newline. I won't comment on each, but if you can take a first attempt at correcting those, I can comment on any that were missed.

---

DSCResources/MSFT_xWebConfigProperty/MSFT_xWebConfigProperty.psm1, line 156 at r6 (raw file):

            { $setValue = $Value }

Same as the opening brace comment, closing braces should be preceded by a newline

[geoffguynn]

Reviewable status: 0 of 8 files reviewed, 5 unresolved discussions (waiting on @regedit32)

---

README.md, line 321 at r6 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

<<<<<<< HEAD

Looks like changes weren't saved when resolving merge conflicts. This syntax was left over.

Removed

---

README.md, line 322 at r6 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

* Added new reosurce xWebConfigProperty extening functionality provided by xWebConfigProperty to allow writing of locked sections in ApplicationHost.Config

Based on your latest commits, this should have a different note about adding Location property to the xWebConfigProperty resource

Modified - * Added new parameter 'Location' to xWebConfigProperty extending functionality to allow writing of locked sections in ApplicationHost.Config

---

README.md, line 330 at r6 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

>>>>>>> 0b60a44eea60673f883544ecbf5cd294ecc750a8

Same as the previous comment, this syntax was left over when resolving merge conflicts.

Removed

---

DSCResources/MSFT_xWebConfigProperty/MSFT_xWebConfigProperty.psm1, line 74 at r6 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

    if ( -not($existingValue) ) {

Looks like throughout the file the opening brace has been moved (perhaps automatically by local .vscode settings?). Per DSC resource style guidelines, opening braces should be preceded by a newline. I won't comment on each, but if you can take a first attempt at correcting those, I can comment on any that were missed.

Updated all opening and closing braces

---

DSCResources/MSFT_xWebConfigProperty/MSFT_xWebConfigProperty.psm1, line 156 at r6 (raw file):

Previously, regedit32 (Reggie Gibson) wrote…

            { $setValue = $Value }

Same as the opening brace comment, closing braces should be preceded by a newline

Updated all opening and closing braces

[regedit32]

Reviewable status: 0 of 8 files reviewed, 4 unresolved discussions (waiting on @geoffguynn)

---

README.md, line 330 at r8 (raw file):

* Added new reosurce xWebConfigProperty extening functionality provided by xWebConfigProperty to allow writing of locked sections in ApplicationHost.Config
* Added new reosurce xWebConfigProperty extening functionality provided by xWebConfigProperty to allow writing of locked sections in ApplicationHost.Config

I don't think you meant to add these two lines here

---

DSCResources/MSFT_xWebConfigProperty/MSFT_xWebConfigProperty.psm1, line 365 at r8 (raw file):

[Parameter(Mandatory = $false)]

Per style guidelines, format for non-mandatory parameters should be [Parameter()]. There are a few older spots in the resource that are incorrect, but we can fix in a seperate PR at some point to bring the module up to HQRM standard.

---

Examples/Sample_xWebConfigProperty_Add.ps1, line 28 at r8 (raw file):

            Location = ''

Let's also add another example for an actual value for Location.

---

Tests/Integration/MSFT_xWebConfigProperty.Integration.Tests.ps1, line 50 at r8 (raw file):

                    Location             = ''

Since this is significant breaking change we're adding, I think it would be better to test both an empty Location plus an actual location to always validate both scenarios are working. Preferably a locked section that would have previously failed before this change.

[psychonic]

@geoffguynn Do you plan to continue working on this?

I would like to see this land and have time to work on the requested changes if needed.

[geoffguynn]

@psychonic I do, but I've been slammed with other work for awhile now. If you have some cycles, by all means :)

[varnav]

I vote for this

[danielmiu]

+1

[randomnote1]

Curious if any work has been done on this in the last two years? If not, I'd be willing to pick up on finishing this.