Skip to content
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

ADDomainControllerProperties: New resource proposal #301

Closed
JakeDean3631 opened this issue May 29, 2019 · 12 comments · Fixed by #474
Closed

ADDomainControllerProperties: New resource proposal #301

JakeDean3631 opened this issue May 29, 2019 · 12 comments · Fixed by #474
Labels
resource proposal The issue is proposing a new resource in the resource module.

Comments

@JakeDean3631
Copy link

JakeDean3631 commented May 29, 2019

Description

Resource to set and manipulate properties specific to Domain Controllers

Proposed properties

ContentFreshness - This would get, set, and test the MaxOfflineTimeInDays setting on Domain Controllers against the specified value.

Special considerations or limitations

I am a PFE for a DoD customer with special requirements for DCs to be offline for over the 30-day default threshold. This resource will allow MaxOfflineTimeInDays to be manipulated and reported via DSC.

Below is the current script resource being used for this:

Script ADContentFreshness
            {
                GetScript  = {
                    $localContentFreshness = (Get-CimInstance -Namespace ROOT/microsoftdfs -Query 'Select MaxOfflineTimeInDays from DfsrMachineConfig').MaxOfflineTimeInDays
                    return @{ Result = $localContentFreshness }
                }
                SetScript  = {
                    $contentFreshness = $using:ContentFreshness
                    $null = Set-CimInstance -Namespace ROOT/microsoftdfs -Query 'Select MaxOfflineTimeInDays from DfsrMachineConfig' -Property @{MaxOfflineTimeInDays = $contentFreshness }
                }
                TestScript = {
                    $contentFreshness = $using:ContentFreshness
                    $localContentFreshness = (Get-CimInstance -Namespace ROOT/microsoftdfs -Query 'Select MaxOfflineTimeInDays from DfsrMachineConfig').MaxOfflineTimeInDays

                    if ($localContentFreshness = $contentFreshness)
                    {
                        return $true
                    }
                    else
                    {
                        return $false
                    }
                }
@devopsjesus devopsjesus added discussion The issue is a discussion. resource proposal The issue is proposing a new resource in the resource module. labels May 29, 2019
@devopsjesus
Copy link
Contributor

@johlju @nyanhp @rchristman89 I couldn't find a great place for Domain Controller properties set after initial install. Where do you think settings for installed DCs should go? I was thinking a new resource that will configure settings applicable to the local DC.

@devopsjesus
Copy link
Contributor

Related issue: #302

@JakeDean3631
Copy link
Author

To add to @devopsjesus point - ContentFreshness is just one setting that we need the capability to configure for our customer currently, but we can add additional post-promotion settings into this resource as needed as well.

@johlju
Copy link
Member

johlju commented May 30, 2019

I suggest adding these kind of properties to the xADDomainController resource. Once the domain controller is installed by the Set-TargetResource it will initiate a reboot. One the next run it will run Test-TargetResource, see that the node is a domain controller but other properties are not in desired state, and run Set-TargetResource to configure those properties.

So I don't see the benefit of adding another resource for this. Is there any benefits that I don't see? 🤔

@johlju johlju added this to To do in All issues and PR's May 31, 2019
@devopsjesus
Copy link
Contributor

I think we were trying to keep the xADDomainController resource like it is to keep it from becoming overly complex. If we agree that it won't add too much complexity, I don't mind having these properties added to the existing xADDomainController resource.

@johlju
Copy link
Member

johlju commented Jun 1, 2019

It would be complex, but not sure it would become too much more complex. 🤔

But I would be okay to have xADDomainControllerProperties, but then I think we should move out the functionality that enforces the properties currently being enforced in the xADDomainController like SiteName and IsGLobalCatalog. The properties that can be set by the cmdlet Install-ADDSDomainController should then only be allowed in xADomainController, but never enforced? Essentially we should make xADDomainController to just promote and demote a domain controller?
The new resource xADDomainControllerProperties will enforce the properties.

@devopsjesus
Copy link
Contributor

Essentially we should make xADDomainController to just promote and demote a domain controller?

Yes, this is what I was thinking, with the xADDomainControllerProperties resource containing all the post-deployment configuration properties and settings, similar to xActiveDirectoryProperties.

We'll go ahead and start planning to get that resource started unless there are any further hesitancies about splitting out a separate resource.

@johlju
Copy link
Member

johlju commented Jun 4, 2019

At least I'm good with this. 🙂 Looking forward to the PR. I label this issue as a breaking change since we are removing properties from another resource?

@johlju johlju added breaking change When used on an issue, the issue has been determined to be a breaking change. in progress The issue is being actively worked on by someone. and removed discussion The issue is a discussion. labels Jun 4, 2019
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Jul 28, 2019
@johlju
Copy link
Member

johlju commented Jul 28, 2019

@devopsjesus Did you work on this issue?

@johlju johlju changed the title xADDomainControllerProperties: New resource proposal ADDomainControllerProperties: New resource proposal Jul 28, 2019
@johlju
Copy link
Member

johlju commented Aug 7, 2019

I see another benefit of having a separate resource for properties not used by Install-ADDSDomainController, it is possible to add configuration to an existing resource and not provide the mandatory parameters required for ADDomainController, like SafeMode password.

Above I suggested moving out some properties, but those are actually used by Install-ADDSDomainController. So this is not a breaking change, we just need to add this new property to a new resource.

I can work on this.

@johlju johlju removed the breaking change When used on an issue, the issue has been determined to be a breaking change. label Aug 7, 2019
@johlju
Copy link
Member

johlju commented Aug 7, 2019

@JakeDean3631 question, why do you suggest the property should be called ContentFreshness and not just MaxOfflineTimeInDays as the property it would set? 🤔 Looking for a description of the property that would make the user know what it is used for. Is content freshness a well know term for "MaxOfflineTimeInDays"?

@johlju
Copy link
Member

johlju commented Aug 7, 2019

@johlju johlju added the in progress The issue is being actively worked on by someone. label Aug 7, 2019
johlju added a commit to johlju/ActiveDirectoryDsc that referenced this issue Aug 7, 2019
- New resource ADDomainControllerProperties (issue dsccommunity#301).
johlju added a commit to johlju/ActiveDirectoryDsc that referenced this issue Aug 8, 2019
- New resource ADDomainControllerProperties (issue dsccommunity#301).
All issues and PR's automation moved this from To do to Done Aug 8, 2019
johlju added a commit that referenced this issue Aug 8, 2019
- Changes to ActiveDirectoryDsc
  - New resource ADDomainControllerProperties (issue #301).
@johlju johlju removed the in progress The issue is being actively worked on by someone. label Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resource proposal The issue is proposing a new resource in the resource module.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants