Fix ad domain controller (dsccommunity#686)

Borgquite · May 17, 2022 · 3a48b72 · 3a48b72
1 parent 4a20b4e
commit 3a48b72
Show file tree

Hide file tree

Showing 12 changed files with 486 additions and 166 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,15 @@ For older change log history see the [historic changelog](HISTORIC_CHANGELOG.md)
 
 ### Changed
 
+- ActiveDirectoryDSC.Common
+  - Created Get-DomainObject to wrap Get-ADDomain with common retry logic.
+- ADDomainController
+  - Refactored to use Get-DomainObject ([issue #673](https://github.com/dsccommunity/ActiveDirectoryDsc/issues/673)).
+  - Refactored Unit Tests.
+- ADDomain
+  - Refactored to use Get-DomainObject.
+  - Refactored Unit Tests.
+
 - ActiveDirectoryDsc
   - Updated Pipeline to Ubuntu 18.04 from Ubuntu 16.04
     ([issue #667](https://github.com/dsccommunity/ActiveDirectoryDsc/issues/667))

diff --git a/source/DSCResources/MSFT_ADDomain/MSFT_ADDomain.psm1 b/source/DSCResources/MSFT_ADDomain/MSFT_ADDomain.psm1
@@ -34,11 +34,11 @@ $script:localizedData = Get-LocalizedData -DefaultUICulture 'en-US'
         Used Functions:
             Name                           | Module
             -------------------------------|--------------------------
-            Get-ADDomain                   | ActiveDirectory
             Get-ADForest                   | ActiveDirectory
             Assert-Module                  | DscResource.Common
             New-InvalidOperationException  | DscResource.Common
             Resolve-DomainFQDN             | MSFT_ADDomain
+            Get-DomainObject               | ActiveDirectoryDsc.Common
             ConvertTo-DeploymentForestMode | ActiveDirectoryDsc.Common
             ConvertTo-DeploymentDomainMode | ActiveDirectoryDsc.Common
 #>
@@ -93,48 +93,7 @@ function Get-TargetResource
 
         Write-Verbose ($script:localizedData.QueryDomain -f $domainFQDN)
 
-        $retries = 0
-        $maxRetries = 15
-        $retryIntervalInSeconds = 30
-
-        do
-        {
-            $domainFound = $true
-            try
-            {
-                $domain = Get-ADDomain -Identity $domainFQDN -Server localhost -ErrorAction Stop
-            }
-            catch [Microsoft.ActiveDirectory.Management.ADServerDownException], `
-                [System.Security.Authentication.AuthenticationException], `
-                [System.InvalidOperationException], `
-                [System.ArgumentException]
-            {
-                Write-Verbose ($script:localizedData.ADServerNotReady -f $domainFQDN)
-                $domainFound = $false
-                # will fall into the retry mechanism.
-            }
-            catch
-            {
-                $errorMessage = $script:localizedData.GetAdDomainUnexpectedError -f $domainFQDN
-                New-InvalidOperationException -Message $errorMessage -ErrorRecord $_
-            }
-
-            if (-not $domainFound)
-            {
-                $retries++
-
-                Write-Verbose ($script:localizedData.RetryingGetADDomain -f
-                    $retries, $maxRetries, $retryIntervalInSeconds)
-
-                Start-Sleep -Seconds $retryIntervalInSeconds
-            }
-        } while ((-not $domainFound) -and $retries -lt $maxRetries)
-
-        if ($retries -eq $maxRetries)
-        {
-            $errorMessage = $script:localizedData.MaxDomainRetriesReachedError -f $domainFQDN
-            New-InvalidOperationException -Message $errorMessage
-        }
+        $domain = Get-DomainObject -Identity $domainFQDN -Server localhost -ErrorOnUnexpectedExceptions -ErrorOnMaxRetries -Verbose
     }
     else
     {

diff --git a/source/DSCResources/MSFT_ADDomain/en-US/MSFT_ADDomain.strings.psd1 b/source/DSCResources/MSFT_ADDomain/en-US/MSFT_ADDomain.strings.psd1
@@ -1,17 +1,13 @@
 # culture="en-US"
 ConvertFrom-StringData @'
     QueryDomain                        = Querying for domain '{0}'. (ADD0001)
-    ADServerNotReady                   = The AD Server for domain '{0}' is currently not ready. (ADD0002)
     DomainFound                        = Active Directory domain '{0}' found. (ADD0003)
     CreatingChildDomain                = Creating domain '{0}' as a child of domain '{1}'. (ADD0004)
     CreatedChildDomain                 = Child domain '{0}' created. (ADD0005)
     CreatingForest                     = Creating AD forest '{0}'. (ADD0006)
     CreatedForest                      = AD forest '{0}' created. (ADD0007)
     DomainInDesiredState               = The domain '{0}' is in the desired state. (ADD0008)
     DomainNotInDesiredState            = The domain '{0}' is NOT in the desired state. (ADD0009)
-    RetryingGetADDomain                = Attempt {0} of {1} to call Get-ADDomain failed, retrying in {2} seconds. (ADD0010)
     SysVolPathDoesNotExistError        = The expected SysVol Path '{0}' does not exist. (ADD0011)
-    MaxDomainRetriesReachedError       = Maximum Get-ADDomain retries reached and the domain did not respond. (ADD0012)
-    GetAdDomainUnexpectedError         = Error getting AD domain '{0}'. (ADD0013)
     GetAdForestUnexpectedError         = Error getting AD forest '{0}'. (ADD0014)
 '@
diff --git a/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 b/source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1
@@ -57,14 +57,12 @@ function Get-TargetResource
 
     Write-Verbose -Message ($script:localizedData.ResolveDomainName -f $DomainName)
 
-    try
-    {
-        $domain = Get-ADDomain -Identity $DomainName -Credential $Credential
-    }
-    catch
+    $Domain = Get-DomainObject -Identity $DomainName -Credential $Credential -ErrorOnUnexpectedExceptions -Verbose:$VerbosePreference
+
+    if (-not $Domain)
     {
         $errorMessage = $script:localizedData.MissingDomain -f $DomainName
-        New-ObjectNotFoundException -Message $errorMessage -ErrorRecord $_
+        New-ObjectNotFoundException -Message $errorMessage
     }
 
     Write-Verbose -Message ($script:localizedData.DomainPresent -f $DomainName)
@@ -191,14 +189,14 @@ function Get-TargetResource
             Name                                               | Module
             ---------------------------------------------------|--------------------------
             Install-ADDSDomainController                       | ActiveDirectory
-            Get-ADDomain                                       | ActiveDirectory
             Get-ADForest                                       | ActiveDirectory
             Set-ADObject                                       | ActiveDirectory
             Move-ADDirectoryServer                             | ActiveDirectory
             Move-ADDirectoryServerOperationMasterRole          | ActiveDirectory
             Remove-ADDomainControllerPasswordReplicationPolicy | ActiveDirectory
             Add-ADDomainControllerPasswordReplicationPolicy    | ActiveDirectory
             Get-DomainControllerObject                         | ActiveDirectoryDsc.Common
+            Get-DomainObject                                   | ActiveDirectoryDsc.Common
             New-InvalidOperationException                      | DscResource.Common
 #>
 function Set-TargetResource

diff --git a/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psd1 b/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psd1
@@ -38,6 +38,7 @@
         'Restore-ADCommonObject'
         'Get-ADDomainNameFromDistinguishedName'
         'Set-ADCommonGroupMember'
+        'Get-DomainObject'
         'Get-DomainControllerObject'
         'Test-IsDomainController'
         'Convert-PropertyMapToObjectProperties'

diff --git a/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1 b/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1
@@ -1143,6 +1143,146 @@ function Set-ADCommonGroupMember
     }
 }
 
+<#
+    .SYNOPSIS
+        Gets the domain object.
+
+    .DESCRIPTION
+        The Get-DomainObject function is used to get the domain object with retries, otherwise it returns $null.
+
+    .EXAMPLE
+        Get-DomainObject -DomainName contoso.com
+
+    .PARAMETER Identity
+        Specifies an Active Directory domain object, most commonly a DNS domain name.
+
+    .PARAMETER Server
+        Specifies the Active Directory Domain Services instance to connect to, most commonly a Fully qualified domain name.
+
+    .PARAMETER Credential
+        Specifies the credentials to use when accessing the domain, or use the current user if not specified.
+
+    .PARAMETER MaximumRetries
+        Specifies the maximum number of retries to attempt.
+
+    .PARAMETER RetryIntervalInSeconds
+        Specifies the time to wait in seconds between retries attempts.
+
+    .PARAMETER ErrorOnUnexpectedExceptions
+        Switch to indicate if the function should throw an exception on unexpected errors rather than returning null.
+
+    .PARAMETER ErrorOnMaxRetries
+        Switch to indicate if the function should throw an exception when the maximum retries are exceeded.
+
+    .INPUTS
+        None
+
+    .OUTPUTS
+        System.DirectoryServices.ActiveDirectory.Domain
+
+#>
+function Get-DomainObject
+{
+    [CmdletBinding()]
+
+    param
+    (
+        [Parameter(Mandatory = $true)]
+        [System.String]
+        $Identity,
+
+        [Parameter()]
+        [System.String]
+        $Server,
+
+        [Parameter()]
+        [System.Management.Automation.PSCredential]
+        $Credential,
+
+        [Parameter()]
+        [System.Int32]
+        $MaximumRetries = 15,
+
+        [Parameter()]
+        [System.Int32]
+        $RetryIntervalInSeconds = 30,
+
+        [Parameter()]
+        [System.Management.Automation.SwitchParameter]
+        $ErrorOnUnexpectedExceptions,
+
+        [Parameter()]
+        [System.Management.Automation.SwitchParameter]
+        $ErrorOnMaxRetries
+    )
+
+    $getADDomainParameters = @{
+        Identity = $Identity
+        ErrorAction = 'Stop'
+    }
+
+    if ($PSBoundParameters.ContainsKey('Credential'))
+    {
+        $getADDomainParameters['Credential'] = $Credential
+    }
+    if ($PSBoundParameters.ContainsKey('Server'))
+    {
+        $getADDomainParameters['Server'] = $Server
+    }
+
+    $retries = 0
+    $domainObject = $null
+
+    do
+    {
+        $domainFound = $true
+        try
+        {
+            $domainObject = Get-ADDomain @getADDomainParameters
+        }
+        catch [Microsoft.ActiveDirectory.Management.ADServerDownException], `
+            [System.Security.Authentication.AuthenticationException], `
+            [System.InvalidOperationException], `
+            [System.ArgumentException]
+        {
+            Write-Verbose ($script:localizedData.ADServerNotReady -f $Identity)
+            $domainFound = $false
+            # will fall into the retry mechanism.
+        }
+        catch
+        {
+            if ($ErrorOnUnexpectedExceptions)
+            {
+                $errorMessage = $script:localizedData.GetAdDomainUnexpectedError -f $Identity
+                New-InvalidOperationException -Message $errorMessage -ErrorRecord $_
+            }
+            return $null
+        }
+
+        if (-not $domainFound)
+        {
+            $retries++
+
+            Write-Verbose ($script:localizedData.RetryingGetADDomain -f
+                    $retries, $MaximumRetries, $RetryIntervalInSeconds)
+
+            Start-Sleep -Seconds $RetryIntervalInSeconds
+        }
+    } while ((-not $domainFound) -and $retries -lt $MaximumRetries)
+
+    if ($retries -eq $MaximumRetries)
+    {
+        if ($ErrorOnMaxRetries)
+        {
+            $errorMessage = $script:localizedData.MaxDomainRetriesReachedError -f $Identity
+            New-InvalidOperationException -Message $errorMessage
+        }
+        Write-Verbose -Message ($script:localizedData.MaxDomainRetriesReachedError -f $Identity) -Verbose
+    }
+
+    return $domainObject
+}
+
 <#
     .SYNOPSIS
         Gets the domain controller object if the node is a domain controller.

diff --git a/source/Modules/ActiveDirectoryDsc.Common/README.md b/source/Modules/ActiveDirectoryDsc.Common/README.md
@@ -58,6 +58,9 @@ Gets the contents of a file as a byte array.
 ### [Get-CurrentUser](docs/Get-CurrentUser.md)
 Gets the current user identity.
 
+### [Get-DomainObject](docs/Get-DomainObject.md)
+Gets the domain object with retries.
+
 ### [Get-DomainControllerObject](docs/Get-DomainControllerObject.md)
 Gets the domain controller object if the node is a domain controller.