dsccommunity / StorageDsc

DSC resource module is used to manage storage on Windows Servers.
https://dsccommunity.org
MIT License
71 stars 52 forks source link

Prevent ResourceHelper and Common cmdlets from being exported - Fixes #93 #94

Closed PlagueHO closed 7 years ago

PlagueHO commented 7 years ago

This resolves a high priority issue with newer versions of xNetworking, xCertificate, xStorage and xDFS conflicting with each other because the ResourceHelper cmdlets are being exported due to the way each resource imported them (using the PSD1 nested modules array).

This should be resolved in xNetworking, xStorage and xDFS as well to ensure no other conflicts.

This also contains a new integration test that will check for any further conflicts between resource modules. At some point it might be possible to move this test into the DSCResource.Tests common tests to provide more comprehensive conflict prevention. However, because the conflicts will potentially be between three different modules the number of modules compared will have to go in over time.

This fixes #93


This change is Reviewable

codecov-io commented 7 years ago

Codecov Report

Merging #94 into dev will increase coverage by <1%. The diff coverage is 100%.

Impacted file tree graph

@@        Coverage Diff         @@
##           dev   #94    +/-   ##
==================================
+ Coverage   91%   92%   +<1%     
==================================
  Files        5     5            
  Lines      631   647    +16     
==================================
+ Hits       580   596    +16     
  Misses      51    51
PlagueHO commented 7 years ago

Hi @Johlju - if you get a chance, could you review this one? There is a bit of a problem with xStorage conflicting with xNetworking (and xCertificate) due to how the supporting modules are loaded (using NestedModules in the manifest). This PR will fix the issue - and I've raised similar changes to the other conflicting repos as well and also added some new integration/regression tests for the issue.

johlju commented 7 years ago

I'm on it. Awesome work getting this fixed so quickly!! :smile:


Reviewed 8 of 8 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion.


Tests/Integration/ModuleConflict.Tests.ps1, line 37 at r1 (raw file):

Should not throw

Should Not Throw Or Should -Not -Throw


Comments from Reviewable

PlagueHO commented 7 years ago

All done! Thank you so much for reviewing @johlju - I really appreciate that - definitely not an ideal bug to get released.


Review status: 7 of 8 files reviewed at latest revision, 1 unresolved discussion.


Tests/Integration/ModuleConflict.Tests.ps1, line 37 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
> `Should not throw` `Should Not Throw` Or `Should -Not -Throw`

Done.

I actually should do a global search replace on this because I've done this in all tests. Will raise an issue to correct it as I don't want to muddy this one because of the urgency :cry:


Comments from Reviewable

johlju commented 7 years ago
:lgtm:

No problem at all. Bad things happen sometimes... :/ Hopefully we can quickly resolve it. :)


Reviewed 1 of 1 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

johlju commented 7 years ago

@PlagueHO all four PR's are reviewed. Just need these merged and get @kwirkykat attention and see if she have time to release the four problematic modules again.

kwirkykat commented 7 years ago

@johlju @PlagueHO I can release them all again whenever you guys are ready

PlagueHO commented 7 years ago

Thanks @johlju and @kwirkykat - I really appreciate your help here!

PlagueHO commented 7 years ago

@kwirkykat - I've fixed and merged all of these and they should be good to go. Thanks again to you guys for helping me get this sorted!

johlju commented 7 years ago

Again, great work getting this fixed so quickly @PlagueHO! This is good team work! 😄