dsccommunity / ActiveDirectoryDsc

This module contains DSC resources for deployment and configuration of Active Directory Domain Services.
MIT License
344 stars 142 forks source link

ADDomainController: RODC requires site name #632

Closed jojiklmts closed 4 years ago

jojiklmts commented 4 years ago

Details of the scenario you tried and the problem that is occurring

When you create configuration for read only DC it requires SiteName. Error message: This does not make any sense. If you are using DSC, chances are you are doing so because you have hundreds of DCs to manage. Hard-coding site name into MOF file is not feasible as each DC will have different site name. To mitigate this I could create new MOF file for each new DC but that is also quite bad because that would result in having hundreds of MOF files and every time you have some change of code you would have to update hundreds of MOF files. As a side note if you build read write DC it does not require SiteName and PowerShell will figure out proper SiteName automatically during config push.

Verbose logs showing the problem

PowerShell DSC resource MSFT_ADDomainController failed to execute Test-TargetResource functionality with error message: System.InvalidOperationException: You have specified 'ReadOnlyReplica', but did not provide a site name. (ADDC0022)

Suggested solution to the issue

ADDomainController resource should use the same logic for site selection for both RODC and RWDC. Meaning it should automatically pick AD site based on DC IP of no site name is specified.

The DSC configuration that is used to reproduce the issue (as detailed as possible)

ADDomainController DCpromotion {
            DomainName = $domain
            Credential = $DaCreds
            SafemodeAdministratorPassword = $DsrmCreds
            DatabasePath = $ADDBpath
            SysvolPath = $SYSVOLpath
            LogPath = $ADLOGpath
            IsGlobalCatalog = $true
            ReadOnlyReplica = $true
            AllowPasswordReplicationAccountName = @('AllowGroups')
            DenyPasswordReplicationAccountName = @('DenyGroups')
            InstallDns = $true
            DependsOn = @("[WindowsFeature]AD-Domain-Services","[WindowsFeature]DNS")

        }

The operating system the target node is running

OsName : Microsoft Windows Server 2012 R2 Standard OsOperatingSystemSKU : StandardServerEdition OsArchitecture : 64-bit WindowsBuildLabEx : 9600.19846.amd64fre.winblue_ltsb_escrow.200923-1735 OsLanguage : en-US OsMuiLanguages : {en-US}

Version and build of PowerShell the target node is running

PSVersion 5.1.14409.1018 PSEdition Desktop PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...} BuildVersion 10.0.14409.1018 CLRVersion 4.0.30319.42000 WSManStackVersion 3.0 PSRemotingProtocolVersion 2.3 SerializationVersion 1.1.0.1

Version of the DSC module that was used

ActiveDirectoryDsc_6.0.1

johlju commented 4 years ago

Then this logic that throws on missing site should be removed:

https://github.com/dsccommunity/ActiveDirectoryDsc/blob/87b1308891fb28d092aa12eb886661cd15d5d018/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1#L296-L304

and

https://github.com/dsccommunity/ActiveDirectoryDsc/blob/87b1308891fb28d092aa12eb886661cd15d5d018/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1#L683-L689

johlju commented 4 years ago

Looking here it looks like SiteName is required for the cmdlet Install-AddsDomainController when not using staging on 2012 R2. Is that the reason it is required? https://docs.microsoft.com/en-us/windows-server/identity/ad-ds/deploy/rodc/install-a-windows-server-2012-active-directory-read-only-domain-controller--rodc---level-200-#rodc-without-staging-windows-powershell

jojiklmts commented 4 years ago

Good find @johlju In my case I am doing the RODC deployment without staging. So according to the article you provided this should apply: "The Domain Controller Options page also enables you to choose the appropriate Active Directory logical site name from the forest configuration. By default, it selects the site with the most correct subnet. If there is only one site, it selects that site automatically." So in other words if no site was specified it should be able to auto select based on subnet. Also according to MS documentation this is the SiteName argument description: "Specifies the name of an existing site where you can place the new domain controller. The default value depends on the type of installation. For a new forest, the default is Default-First-Site-Name. For all other installations, the default is the site that is associated with the subnet that includes the IP address of this server. If no such site exists, the default is the site of the replication source domain controller."

johlju commented 4 years ago

Agree. Also looking at the PR https://github.com/dsccommunity/ActiveDirectoryDsc/pull/406 (and referenced issues and PR) where RODC was introduced (and the evaluation of site name) there is no mention of the site name need to be required.

Conclusion is that the evaluation in the two places mentioned in a previous comment could be safely removed (and related unit tests).

Happy to review a PR for this.

X-Guardian commented 4 years ago

Guys, SiteName is a required parameter for Install-ADDSDomainController when ReadOnlyReplica is specified. You can see it in the cmdlet syntax:

SYNTAX
    Install-ADDSDomainController -DomainName <string> [-SkipPreChecks] [-SafeModeAdministratorPassword <securestring>]
    [-SiteName <string>] [-ADPrepCredential <pscredential>] [-AllowDomainControllerReinstall]
    [-ApplicationPartitionsToReplicate <string[]>] [-CreateDnsDelegation] [-Credential <pscredential>]
    [-CriticalReplicationOnly] [-DatabasePath <string>] [-DnsDelegationCredential <pscredential>] [-NoDnsOnNetwork]
    [-NoGlobalCatalog] [-InstallationMediaPath <string>] [-InstallDns] [-LogPath <string>]
    [-MoveInfrastructureOperationMasterRoleIfNecessary] [-NoRebootOnCompletion] [-ReplicationSourceDC <string>]
    [-SkipAutoConfigureDns] [-SystemKey <securestring>] [-SysvolPath <string>] [-Force] [-WhatIf] [-Confirm]
    [<CommonParameters>]

    Install-ADDSDomainController -DomainName <string> -SiteName <string> [-SkipPreChecks]
    [-SafeModeAdministratorPassword <securestring>] [-ADPrepCredential <pscredential>]
    [-AllowDomainControllerReinstall] [-AllowPasswordReplicationAccountName <string[]>]
    [-ApplicationPartitionsToReplicate <string[]>] [-Credential <pscredential>] [-CriticalReplicationOnly]
    [-DatabasePath <string>] [-DelegatedAdministratorAccountName <string>] [-DenyPasswordReplicationAccountName
    <string[]>] [-NoDnsOnNetwork] [-NoGlobalCatalog] [-InstallationMediaPath <string>] [-InstallDns] [-LogPath
    <string>] [-MoveInfrastructureOperationMasterRoleIfNecessary] [-ReadOnlyReplica] [-NoRebootOnCompletion]
    [-ReplicationSourceDC <string>] [-SkipAutoConfigureDns] [-SystemKey <securestring>] [-SysvolPath <string>]
    [-Force] [-WhatIf] [-Confirm]  [<CommonParameters>]

    Install-ADDSDomainController -DomainName <string> [-SkipPreChecks] [-SafeModeAdministratorPassword <securestring>]
    [-ADPrepCredential <pscredential>] [-ApplicationPartitionsToReplicate <string[]>] [-Credential <pscredential>]
    [-CriticalReplicationOnly] [-DatabasePath <string>] [-NoDnsOnNetwork] [-InstallationMediaPath <string>] [-LogPath
    <string>] [-NoRebootOnCompletion] [-ReplicationSourceDC <string>] [-SkipAutoConfigureDns] [-SystemKey
    <securestring>] [-SysvolPath <string>] [-UseExistingAccount] [-Force] [-WhatIf] [-Confirm]  [<CommonParameters>]

And if you try running the cmdlet without SiteName, you are prompted for it:

PS C:\Users\Administrator> Install-ADDSDomainController -DomainName contoso.com -Credential $cred -ReadOnlyReplica

cmdlet Install-ADDSDomainController at command pipeline position 1
Supply values for the following parameters:
SiteName:

That is why it is a required for the ADDomainController resource when ReadOnlyReplica is specified, and needs to remain so.

johlju commented 4 years ago

Thank you @X-Guardian! I saw that in the documentation but then @jojiklmts findings in the documentation contradicted it so I thought there was a bug with the documentation. Thanks for clearing this up.

If cmdlet is changed in the future then what @jojiklmts suggest might work, but until then I think we can close this issue.

@jojiklmts feel free to reopen the issue if you want to discuss it more.

jojiklmts commented 4 years ago

So this means if you have hundreds of DCs to manage you are going to need hundreds of MOF files. And will have to update all of those MOF files every time there is a change. That is unfortunate.