ActiveDirectoryDsc
ActiveDirectoryDsc copied to clipboard
ADDomainController: Ensure Property should be renamed or reimplemented
Details of the scenario you tried and the problem that is occurring
The schema for the ADDomainController
Resource includes the Ensure
property as a read-only
string
:
https://github.com/dsccommunity/ActiveDirectoryDsc/blob/9346a1d153515d5a4852375575bdddc917ea030e/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.schema.mof#L13
This is counter to the expectation and idiomatic implementation of the Ensure
property across DSC Resources, which is taken to be the enforceable state of a Resource.
Furthermore, the actual implementation of Get-TargetResource
for ADDomainController
returns $true
or $false
for Ensure
, which is also non-idiomatic (I would expect Present
or Absent
):
https://github.com/dsccommunity/ActiveDirectoryDsc/blob/9346a1d153515d5a4852375575bdddc917ea030e/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1#L96
https://github.com/dsccommunity/ActiveDirectoryDsc/blob/9346a1d153515d5a4852375575bdddc917ea030e/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1#L118
Suggested solution to the issue
I suggest either renaming the read-only Ensure
property to something like IsPromoted
(with a Boolean
type) or reworking the Resource entirely such that it becomes an ensurable resource and the Ensure
property is idiomatic.
Version of the DSC module that was used
- Latest code on
main
Agreed on all accounts. Given when this resource was originally written, I guess the reason it's still this way is to minimize breaking changes and the lack of time to refactor.
That said, you're right and I'll let the maintainers of @dsccommunity/activedirectorydsc to comment on feasibility and preferred approach.
Yes it should use Ensure
property as any other DSC resource, meaning using the two values Present
and Absent
. If I recall correctly the current implementation was a workaround to fix a bug and not make a breaking change.
This issue is related to issue https://github.com/dsccommunity/ActiveDirectoryDsc/issues/251 which suggest that the ADDomainController
should be able to demote a domain controller.
We are happy to review a PR that implements this breaking change, preferably that also fixes issue #251. 🙂
We are happy to review a PR that implements this breaking change, preferably that also fixes issue #251. 🙂
I probably can't commit to implementing the PR myself (I'm hip-deep in the Puppet-DSC integration work myself, we found this via a customer raising a bug) at this time, unfortunately.
No worries @michaeltlombardi. Hopefully someone in the community can pick this up.