xADDomainController: Fix Issue #155 - Get-TargetResource fails

[devopsjesus]

Pull Request (PR) description

Added a read property (Ensure) to the xADDomainController resource
This required making a variety of edits to the Tests file to have them run outside a deployed Domain Controller

This Pull Request (PR) fixes the following issues

• Fixes xADDomainController: Get-DscConfiguration fails #155

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 

[codecov-io]

Codecov Report

Merging #249 into dev will increase coverage by <1%.
The diff coverage is n/a.

@@         Coverage Diff          @@
##            dev   #249    +/-   ##
====================================
+ Coverage    85%    85%   +<1%     
====================================
  Files        18     18            
  Lines      1554   1554            
  Branches     10     11     +1     
====================================
+ Hits       1325   1326     +1     
+ Misses      219    217     -2     
- Partials     10     11     +1

[johlju]

I just merged a PR that fixes the problem with the tests failing. Please rebase this PR.

[johlju]

Thank you for adding the stubs, and cleaning up the unit test! 🙇‍♂️ Just a couple of review comments.

Reviewed 3 of 5 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @devopsjesus)

---

README.md, line 126 at r2 (raw file):

_(Read)_

I think this should be Write, see other comment.

---

DSCResources/MSFT_xADDomainController/MSFT_xADDomainController.schema.mof, line 12 at r2 (raw file):

[Read

I actually think this should be write, because there is a Ensure parameter in the *-TargetResource functions with a ValidateSet. The property must just have been missed in the schema.mof.

https://github.com/PowerShell/xActiveDirectory/blob/f69f64fe65850cbc042501d82b292d61b386eb8f/DSCResources/MSFT_xADComputer/MSFT_xADComputer.psm1#L50-L51

And actually be this (you may change the description to something more suitable):

[Write, Description("Specifies the desired state of the Domain Controller. When set to 'Present' the node will be promoted to a domain controller. When set to 'Absent' the node will be demoted. Default value is 'Present'."), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure;

I'm not sure the 'Absent' part will work, but that is a problem for a different issue if it doesn't?

[devopsjesus]

Here were my thoughts on the Read attribute for Ensure:

I tagged the Ensure property with the Read attribute, since it seems that the xADDomainController resource doesn't really support the demotion of a DC using an Absent value for Ensure, and I didn't want to confuse users by having that option available to set in a configuration. I figured if we want to add the ability to demote a DC, we could then change the property attribute with that feature request.

[johlju]

I’m good with that if there are not a problem using the resource if the parameter, and that we also create an issue to track that it should be made a Write property once we have the absent logic added. So it’s no confusing why it’s a read property in the future.

Although, we could also make it a Write property today and create an issue tracking that Absent logic should be added. 🤔

[devopsjesus]

Added Issue #251 to track the Ensure property is updated with a Write attribute when implementing the feature to demote a DC using Ensure = 'Absent'

[johlju]

@devopsjesus This PR seems to want to added changes already changed in the dev-branch, for example the file MSFT_xADObjectPermissionEntry.psm1. Can you rebase (git rebase) this PR again (so your commits are replayed over the dev-branch)?.

[devopsjesus]

did the rebase - I also merged upstream/dev into my fork, which might be why those extra file changes are shown?

[johlju]

Reviewed 1 of 2 files at r4, 1 of 1 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved

[johlju]

@devopsjesus merged! Awesome work on this one! Thank you! 😄