search

puppetlabs / Puppet.Dsc

Convert DSC resources into Puppet Resource API types and providers
https://www.powershellgallery.com/packages/Puppet.Dsc
Apache License 2.0
9 stars 12 forks source link

ActiveDirectoryDsc: dscADGroup option non-idempotent #112

Closed eebuta closed 3 years ago

eebuta commented 3 years ago

Describe the Bug

Hi All, I am using the MSFT_ADGroup resource in activedirectorydsc (https://forge.puppet.com/modules/dsc/activedirectorydsc) module to create a new group in AD. The module runs successfully on the first try, then throws errors on the sequential runs. I think this happens because the AD group is configured as required, however the activedirectorydsc module might not be in a position to handle errors regarding the existing objects. What’s the best way to resolve the following issue?

Output

1st run

PS C:\> puppet agent -t
Notice: Local environment: 'production' doesn't match server specified node environment 'development', switching agent to 'development'.
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Info: Caching catalog for 122uks0806.uk.deloitte.com
Info: Applying configuration version '058UKP02057-development-7bf907e2c4f'
Notice: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]/dsc_description: dsc_description changed  to 'Create AD Group for Host/Machine'
Notice: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]/dsc_displayname: dsc_displayname changed  to 'ServerAdmin-test04'
Notice: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]/dsc_ensure: dsc_ensure changed 'Absent' to 'Present'
Notice: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]/dsc_members: dsc_members changed [] to ['Mgmt access']
Notice: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]/dsc_category: dsc_category changed  to 'Security'
Notice: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]/dsc_path: dsc_path changed  to 'OU=AWS,OU=Cloud services,OU=Local Server Access,OU=Groups,OU=UK,DC=uk,DC=testdomain,DC=com'
Notice: dsc_adgroup[{:name=>"ServerAdmin-Group", :dsc_groupname=>"ServerAdmin-test04"}]: Creating: Finished in 1.61 seconds
Notice: Applied catalog in 5.07 seconds

2nd run - Failed

PS C:\> puppet agent -t
Notice: Local environment: 'production' doesn't match server specified node environment 'development', switching agent to 'development'.
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
puppet : Error: dsc_adgroup: The server was unable to process the request due to an internal error.  For more information about the error, either turn on IncludeExceptionDetailInFaults (either from 
ServiceBehaviorAttribute or from the <serviceDebug> configuration behavior) on the server in order to send the exception information back to the client, or turn on tracing as per the Microsoft .NET Framework SDK 
documentation and inspect the server trace logs.
At line:1 char:1
+ puppet agent -t
+ ~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (Error: d...trace logs.:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError

Info: Caching catalog for 122uks0806.uk.deloitte.com
Info: Applying configuration version '058UKP02057-development-7bf907e2c4f'
Error: dsc_adgroup: The server was unable to process the request due to an internal error.  For more information about the error, either turn on IncludeExceptionDetailInFaults (either from ServiceBehaviorAttribute or 
from the <serviceDebug> configuration behavior) on the server in order to send the exception information back to the client, or turn on tracing as per the Microsoft .NET Framework SDK documentation and inspect the server 
trace logs.
Error: /Stage[main]/Profile::Baseline::Common::Domainjoin/Dsc_adgroup[ServerAdmin-Group]: Could not evaluate: undefined method `[]' for nil:NilClass
Notice: Applied catalog in 3.61 seconds

Expected Behavior

Expected Puppet.Dsc to run without failure on other attempts

Steps to Reproduce

Steps to reproduce the behavior:

Please see manifest used to reproduce the issue 

# A description of what this class does
#
# @example
#   include profile::baseline::common::domainjoin
class profile::baseline::common::domainjoin (
  String $machine_ou,
  String $joinuser,
  String $joinpassword,
  String $joindomain  = 'uk.testdomain.com',
  Array $arrcertname  = split($trusted['certname'], '[.]'),
  Array $groups = ['Mgmt access']
) {
  case $facts['osfamily'] {
    default: {}
    'windows': {
      dsc_adgroup { 'ServerAdmin-Group':
        dsc_description => 'Create AD Group for Host/Machine',
        dsc_groupname   => 'ServerAdmin-test04',
        dsc_category    => 'Security',
        dsc_path        => 'OU=AWS,OU=Cloud services,OU=Local Server Access,OU=Groups,OU=UK,DC=uk,DC=testdomain,DC=com',
        dsc_ensure      => 'Present',
        dsc_displayname => 'ServerAdmin-test04',
        dsc_members     => $groups,
        dsc_credential  => {
            'user'     => $joinuser,
            'password' => Sensitive($joinpassword),
        }
      }
    }
  }
}

Environment

OsName : Microsoft Windows Server 2016 Datacenter OsOperatingSystemSKU : DatacenterServerEdition OsArchitecture : 64-bit WindowsBuildLabEx : 14393.3471.amd64fre.rs1_release_1.191218-1729 OsLanguage : en-US OsMuiLanguages : {en-US

Additional Context

Add any other context about the problem here.

Version of the forge module that was used https://forge.puppet.com/modules/dsc/activedirectorydsc - 6.0.1-0-1

michaeltlombardi commented 3 years ago

@eebuta I see you filed an issue with the upstream repository dsccommunity/ActiveDirectoryDsc#639 - that's probably the right place for this. You can get and validate the native DSC output by running puppet agent -t --debug (relevant link in our docs) to get the PowerShell that is being run for Invoke-DscResource - I would get that output, then test it outside of Puppet - if you encounter the same error, it's definitely an issue with the upstream module and you'll have the repro to show it.

If not, we can investigate further here.

eebuta commented 3 years ago

@michaeltlombardi thanks for the response, I ran the ActiveDirectoryDSC resource which was successful and idempotent on multiple runs as shown below

 PS C:\> Start-DscConfiguration -Wait -Verbose -Path "C:\DscConfiguration" -Force
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = SendConfigurationApply,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguratio
n'.
VERBOSE: An LCM method call arrived from computer 122UKS0806 with user sid S-1-5-21-83642069-1626958306-390482200-739856.
VERBOSE: [122UKS0806]: LCM:  [ Start  Set      ]
VERBOSE: [122UKS0806]: LCM:  [ Start  Resource ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]: LCM:  [ Start  Test     ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]:                            [[ADGroup]ServerAdmin-122UKS0806] Retrieving group membership based on 'SamAccountName' property. (ADG0001)
VERBOSE: [122UKS0806]: LCM:  [ End    Test     ]  [[ADGroup]ServerAdmin-122UKS0806]  in 2.5680 seconds.
VERBOSE: [122UKS0806]: LCM:  [ Skip   Set      ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]: LCM:  [ End    Resource ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]: LCM:  [ End    Set      ]
VERBOSE: [122UKS0806]: LCM:  [ End    Set      ]    in  3.0210 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.
VERBOSE: Time taken for configuration job to complete is 3.044 seconds

PS C:\> Start-DscConfiguration -Wait -Verbose -Path "C:\DscConfiguration" -Force
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = SendConfigurationApply,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguratio
n'.
VERBOSE: An LCM method call arrived from computer 122UKS0806 with user sid S-1-5-21-83642069-1626958306-390482200-739856.
VERBOSE: [122UKS0806]: LCM:  [ Start  Set      ]
VERBOSE: [122UKS0806]: LCM:  [ Start  Resource ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]: LCM:  [ Start  Test     ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]:                            [[ADGroup]ServerAdmin-122UKS0806] Retrieving group membership based on 'SamAccountName' property. (ADG0001)
VERBOSE: [122UKS0806]: LCM:  [ End    Test     ]  [[ADGroup]ServerAdmin-122UKS0806]  in 0.3600 seconds.
VERBOSE: [122UKS0806]: LCM:  [ Skip   Set      ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]: LCM:  [ End    Resource ]  [[ADGroup]ServerAdmin-122UKS0806]
VERBOSE: [122UKS0806]: LCM:  [ End    Set      ]
VERBOSE: [22UKS0806]: LCM:  [ End    Set      ]    in  0.4220 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.
VERBOSE: Time taken for configuration job to complete is 0.44 seconds

PS C:\> Test-DscConfiguration -Detailed

PSComputerName  ResourcesInDesiredState        ResourcesNotInDesiredState     InDesiredState 
--------------  -----------------------        --------------------------     -------------- 
localhost       {[ADGroup]UKR-ALL-ITS-Serve...                                True
michaeltlombardi commented 3 years ago

@eebuta sorry, we want to run it via the Invoke-DscResource invocation as used under the covers in Puppet, not an adhoc Start-DscConfiguration run - if you run puppet agent -t --debug you should find lines in your log like this:

Debug: dsc_psrepository: Script:
... (snipped for brevity)
$InvokeParams = @{Name = 'PSRepository'; Method = 'get'; Property = @{name = 'psgAllery'}; ModuleName = @{ModuleName = 'C:/code/puppetlabs/Puppet.Dsc/import/powershellget/spec/fixtures/modules/powershellget/lib/puppet_x/dsc_resources/PowerShellGet/PowerShellGet.psd1'; RequiredVersion = '2.2.4.1'}}
Try {
  $Result = Invoke-DscResource @InvokeParams
} catch {
  $Response.errormessage   = $_.Exception.Message
  return ($Response | ConvertTo-Json -Compress)
}

You'll want to capture those lines and any variables instantiated immediately before $InvokeParams - when you're passing credentials and other CIM Instances, we need to create the appropriate class objects or DSC yells at us - in order to test what Puppet is actually running.

eebuta commented 3 years ago

Hi @michaeltlombardi , thanks for the response, much appreciated.

I have run the Invoke-DscResource cmdlet, please see the following output(which proves to be idempotent). I hope this is the information you are expecting.

Script

$password = "xxxxxxxx" | ConvertTo-SecureString -asPlainText -Force
$username = "xxxxxxx"
[PSCredential] $credential = New-Object System.Management.Automation.PSCredential($username,$password)
[string[]]$members = 'xxxxx'

$Invokeparams = @{
  Name = 'ADGroup'
  Method = 'Set'
  Verbose = $true
  ModuleName = 'ActiveDirectoryDsc'
  Property = @{ GroupName = 'ServerAdmin-122UKS0806-test08'
    GroupScope  = 'Global'
    Category    = 'Security'
    DisplayName = 'ServerAdmin-122UKS0806-test08'
    Path        = "OU=AWS,OU=Cloud services,OU=Local Server Access,OU=Groups,OU=UK,DC=uk,DC=test,DC=com"
    Members     = $members
    Ensure      = 'Present'
    Credential   = $credential
  }
}

Invoke-DscResource @Invokeparams

Output

PS C:\> C:\DscConfiguration\InvokeParam.ps1
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = ResourceSet,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguration'.
VERBOSE: An LCM method call arrived from computer AWSG122UKS0806 with user sid S-1-5-21-83642069-1626958306-390482200-739856.
VERBOSE: [122UKS0806]: LCM:  [ Start  Set      ]  [[ADGroup]DirectResourceAccess]
VERBOSE: [122UKS0806]:                            [[ADGroup]DirectResourceAccess] AD Group 'ServerAdmin-122UKS0806-test08' was not found. (ADG00010)
VERBOSE: [122UKS0806]:                            [[ADGroup]DirectResourceAccess] Creating AD Group 'ServerAdmin-122UKS0806-test08'. (ADG0005)
VERBOSE: [AWSG122UKS0806]: LCM:  [ End    Set      ]  [[ADGroup]DirectResourceAccess]  in 3.5120 seconds.
VERBOSE: [AWSG122UKS0806]: LCM:  [ End    Set      ]    in  4.3120 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.

RebootRequired 
-------------- 
False          
VERBOSE: Time taken for configuration job to complete is 4.456 seconds

PS C:\> C:\DscConfiguration\InvokeParam.ps1
VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = ResourceSet,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/DesiredStateConfiguration'.
VERBOSE: An LCM method call arrived from computer 122UKS0806 with user sid S-1-5-21-83642069-1626958306-390482200-739856.
VERBOSE: [122UKS0806]: LCM:  [ Start  Set      ]  [[ADGroup]DirectResourceAccess]
VERBOSE: [122UKS0806]:                            [[ADGroup]DirectResourceAccess] Retrieving group membership based on 'SamAccountName' property. (ADG0001)
VERBOSE: [122UKS0806]:                            [[ADGroup]DirectResourceAccess] Updating AD Group 'ServerAdmin-122UKS0806-test08'. (ADG0006)
VERBOSE: [122UKS0806]:                            [[ADGroup]DirectResourceAccess] Retrieving group membership based on 'SamAccountName' property. (ADG0001)
VERBOSE: [122UKS0806]:                            [[ADGroup]DirectResourceAccess] Adding '1' member(s) to AD group 'ServerAdmin-AWSG122UKS0806-test08'. (ADG0003)
VERBOSE: [122UKS0806]: LCM:  [ End    Set      ]  [[ADGroup]DirectResourceAccess]  in 1.7490 seconds.
VERBOSE: [122UKS0806]: LCM:  [ End    Set      ]    in  1.8140 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.

RebootRequired 
-------------- 
False          
VERBOSE: Time taken for configuration job to complete is 1.934 seconds

Thanks for the support.

michaeltlombardi commented 3 years ago

Ahh @eebuta, sorry, that looks like you built your own hash for Invoke-DscResource instead of reusing the one puppet generated. When you run in debug mode, you should get a script output like this (it's long, but includes some helper functions at the top):

function new-pscredential {
  [CmdletBinding()]
  param (
    [parameter(Mandatory = $true,
      ValueFromPipelineByPropertyName = $true)]
    [string]
    $user,

    [parameter(Mandatory = $true,
      ValueFromPipelineByPropertyName = $true)]
    [string]
    $password
  )

  $secpasswd = ConvertTo-SecureString $password -AsPlainText -Force
  $credentials = New-Object System.Management.Automation.PSCredential ($user, $secpasswd)
  return $credentials
}

Function ConvertTo-CanonicalResult {
  [CmdletBinding()]
  param(
    [Parameter(Mandatory, Position = 1)]
    [psobject]
    $Result,

    [Parameter(DontShow)]
    [string]
    $PropertyPath,

    [Parameter(DontShow)]
    [int]
    $RecursionLevel = 0
  )

  $MaxDepth = 5
  $CimInstancePropertyFilter = { $_.Definition -match 'CimInstance' -and $_.Name -ne 'PSDscRunAsCredential' }

  # Get the properties which are/aren't Cim instances
  $ResultObject = @{ }
  $ResultPropertyList = $Result | Get-Member -MemberType Property | Where-Object { $_.Name -ne 'PSComputerName' }
  $CimInstanceProperties = $ResultPropertyList | Where-Object -FilterScript $CimInstancePropertyFilter

  foreach ($Property in $ResultPropertyList) {
    $PropertyName = $Property.Name
    if ($Property -notin $CimInstanceProperties) {
      $Value = $Result.$PropertyName
      if ($PropertyName -eq 'Ensure' -and [string]::IsNullOrEmpty($Result.$PropertyName)) {
        # Just set 'Present' since it was found /shrug
        # If the value IS listed as absent, don't update it unless you want flapping
        $Value = 'Present'
      }
      else {
        if ($Value -is [string] -or $value -is [string[]]) {
          $Value = $Value
        }

        if ($Value.Count -eq 1 -and $Property.Definition -match '\\[\\]') {
          $Value = @($Value)
        }
      }
    }
    elseif ($null -eq $Result.$PropertyName) {
      if ($Property -match 'InstanceArray') {
        $Value = @()
      }
      else {
        $Value = $null
      }
    }
    elseif ($Result.$PropertyName.GetType().Name -match 'DateTime') {
      # Handle DateTimes especially since they're an edge case
      $Value = Get-Date $Result.$PropertyName -UFormat "%Y-%m-%dT%H:%M:%S%Z"
    }
    else {
      # Looks like a nested CIM instance, recurse if we're not too deep in already.
      $RecursionLevel++

      if ($PropertyPath -eq [string]::Empty) {
        $PropertyPath = $PropertyName
      }
      else {
        $PropertyPath = "$PropertyPath.$PropertyName"
      }

      if ($RecursionLevel -gt $MaxDepth) {
        # Give up recursing more than this
        return $Result.ToString()
      }

      $Value = foreach ($item in $Result.$PropertyName) {
        ConvertTo-CanonicalResult -Result $item -PropertyPath $PropertyPath -RecursionLevel ($RecursionLevel + 1) -WarningAction Continue
      }

      # The cim instance type is the last component of the type Name
      # We need to return this for ruby to compare the result hashes
      # We do NOT need it for the top-level properties as those are defined in the type
      If ($RecursionLevel -gt 1 -and ![string]::IsNullOrEmpty($Value) ) {
        # If there's multiple instances, you need to add the type to each one, but you
        # need to specify only *one* name, otherwise things end up *very* broken.
        if ($Value.GetType().Name -match '\[\]') {
          $Value | ForEach-Object -Process {
            $_.cim_instance_type = $Result.$PropertyName.CimClass.CimClassName[0]
          }
        }
        else {
          $Value.cim_instance_type = $Result.$PropertyName.CimClass.CimClassName
          # Ensure that, if it should be an array, it is
          if ($Result.$PropertyName.GetType().Name -match '\[\]') {
            $Value = @($Value)
          }
        }
      }
    }

    if ($Property.Definition -match 'InstanceArray') {
      if ($Value.Count -lt 2) { $Value = @($Value) }
    }

    $ResultObject.$PropertyName = $Value
  }

  # Output the final result
  $ResultObject
}
$script:ErrorActionPreference = 'Stop'
$script:WarningPreference = 'SilentlyContinue'

$response = @{
  indesiredstate = $false
  rebootrequired = $false
  errormessage   = ''
}

$4b7530d1_7732_4d94_92f6_06d6c6530b65 = New-PSCredential -User USERNAME -Password '#<Sensitive [value redacted]>'
$3f9261d5_3e4c_4305_b37a_47ee4e447a4b = New-CimInstance -ClientOnly -ClassName 'PSCredential' -Property $3f9261d5_3e4c_4305_b37a_47ee4e447a4b
$InvokeParams = @{Name = 'PSRepository'; Method = 'get'; Property = @{name = 'Foo'; psdscrunascredential = $4b7530d1_7732_4d94_92f6_06d6c6530b65 }; ModuleName = @{ModuleName = 'C:/code/puppetlabs/pwsh/puppet.dsc/import/powershellget/spec/fixtures/modules/powershellget/lib/puppet_x/dsc_resources/PowerShellGet/PowerShellGet.psd1'; RequiredVersion = '2.2.5' } }
Try {
  $Result = Invoke-DscResource @InvokeParams
}
catch {
  $Response.errormessage = $_.Exception.Message
  return ($Response | ConvertTo-Json -Compress)
}

# keep the switch for when Test passes back changed properties
Switch ($invokeParams.Method) {
  'Test' {
    $Response.indesiredstate = $Result.InDesiredState
    return ($Response | ConvertTo-Json -Compress)
  }
  'Set' {
    $Response.indesiredstate = $true
    $Response.rebootrequired = $Result.RebootRequired
    return ($Response | ConvertTo-Json -Compress)
  }
  'Get' {
    $CanonicalizedResult = ConvertTo-CanonicalResult -Result $Result
    return ($CanonicalizedResult | ConvertTo-Json -Compress -Depth 10)
  }
}

The critical part for you is these lines (which will have different property values, but should look roughly familiar:

$4b7530d1_7732_4d94_92f6_06d6c6530b65 = New-PSCredential -User USERNAME -Password '#<Sensitive [value redacted]>'
$3f9261d5_3e4c_4305_b37a_47ee4e447a4b = New-CimInstance -ClientOnly -ClassName 'PSCredential' -Property $3f9261d5_3e4c_4305_b37a_47ee4e447a4b
$InvokeParams = @{Name = 'PSRepository'; Method = 'get'; Property = @{name = 'Foo'; psdscrunascredential = $4b7530d1_7732_4d94_92f6_06d6c6530b65 }; ModuleName = @{ModuleName = 'C:/code/puppetlabs/pwsh/puppet.dsc/import/powershellget/spec/fixtures/modules/powershellget/lib/puppet_x/dsc_resources/PowerShellGet/PowerShellGet.psd1'; RequiredVersion = '2.2.5' } }

The first two lines are Puppet creating the PSCredential to pass, the last is the full hash being passed to Invoke-DSCResource.

If you copy the full debug output for this script, you should be able to run that and see what is happening / whether it is idempotent - posting it here will also help us narrow things down a little.

eebuta commented 3 years ago

Hi @michaeltlombardi apologies for the misunderstanding, I am new to Puppet and still trying to get round to understanding the flow.

I've captured and run the following instructions, hope I've got it right this time round, happy to catch up on the Slack Puppet Community group, if that helps.

    [CmdletBinding()]
    param (
        [parameter(Mandatory = $true,
            ValueFromPipelineByPropertyName = $true)]
        [string]
        $user,

        [parameter(Mandatory = $true,
            ValueFromPipelineByPropertyName = $true)]
        [string]
        $password
    )

    $secpasswd = ConvertTo-SecureString $password -AsPlainText -Force
    $credentials = New-Object System.Management.Automation.PSCredential ($user, $secpasswd)
    return $credentials
}

Function ConvertTo-CanonicalResult {
  [CmdletBinding()]
  param(
      [Parameter(Mandatory, Position = 1)]
      [psobject]
      $Result,

      [Parameter(DontShow)]
      [string]
      $PropertyPath,

      [Parameter(DontShow)]
      [int]
      $RecursionLevel = 0
  )

  $MaxDepth = 5
  $CimInstancePropertyFilter = { $_.Definition -match 'CimInstance' -and $_.Name -ne 'PSDscRunAsCredential' }

  # Get the properties which are/aren't Cim instances
  $ResultObject = @{ }
  $ResultPropertyList = $Result | Get-Member -MemberType Property | Where-Object { $_.Name -ne 'PSComputerName' }
  $CimInstanceProperties = $ResultPropertyList | Where-Object -FilterScript $CimInstancePropertyFilter

  foreach ($Property in $ResultPropertyList) {
      $PropertyName = $Property.Name
      if ($Property -notin $CimInstanceProperties) {
          $Value = $Result.$PropertyName
          if ($PropertyName -eq 'Ensure' -and [string]::IsNullOrEmpty($Result.$PropertyName)) {
              # Just set 'Present' since it was found /shrug
              # If the value IS listed as absent, don't update it unless you want flapping
              $Value = 'Present'
          }
          else {
              if ($Value -is [string] -or $value -is [string[]]) {
                  $Value = $Value
              }

              if ($Value.Count -eq 1 -and $Property.Definition -match '\\[\\]') {
                  $Value = @($Value)
              }
          }
      }
      elseif ($null -eq $Result.$PropertyName) {
          if ($Property -match 'InstanceArray') {
              $Value = @()
          }
          else {
              $Value = $null
          }
      }
      elseif ($Result.$PropertyName.GetType().Name -match 'DateTime') {
          # Handle DateTimes especially since they're an edge case
          $Value = Get-Date $Result.$PropertyName -UFormat "%Y-%m-%dT%H:%M:%S%Z"
      }
      else {
          # Looks like a nested CIM instance, recurse if we're not too deep in already.
          $RecursionLevel++

          if ($PropertyPath -eq [string]::Empty) {
              $PropertyPath = $PropertyName
          }
          else {
              $PropertyPath = "$PropertyPath.$PropertyName"
          }

          if ($RecursionLevel -gt $MaxDepth) {
              # Give up recursing more than this
              return $Result.ToString()
          }

          $Value = foreach ($item in $Result.$PropertyName) {
              ConvertTo-CanonicalResult -Result $item -PropertyPath $PropertyPath -RecursionLevel ($RecursionLevel + 1) -WarningAction Continue
          }

          # The cim instance type is the last component of the type Name
          # We need to return this for ruby to compare the result hashes
          # We do NOT need it for the top-level properties as those are defined in the type
          If ($RecursionLevel -gt 1 -and ![string]::IsNullOrEmpty($Value) ) {
              # If there's multiple instances, you need to add the type to each one, but you
              # need to specify only *one* name, otherwise things end up *very* broken.
              if ($Value.GetType().Name -match '\[\]') {
                  $Value | ForEach-Object -Process {
                      $_.cim_instance_type = $Result.$PropertyName.CimClass.CimClassName[0]
                  }
              } else {
                  $Value.cim_instance_type = $Result.$PropertyName.CimClass.CimClassName
                  # Ensure that, if it should be an array, it is
                  if ($Result.$PropertyName.GetType().Name -match '\[\]') {
                      $Value = @($Value)
                  }
              }
          }
      }

      if ($Property.Definition -match 'InstanceArray') {
          if ($Value.Count -lt 2) { $Value = @($Value) }
      }

      $ResultObject.$PropertyName = $Value
  }

  # Output the final result
  $ResultObject
}
$script:ErrorActionPreference = 'Stop'
$script:WarningPreference = 'SilentlyContinue'

$response = @{
    indesiredstate = $false
    rebootrequired = $false
    errormessage   = ''
}

$b5020517_2949_4754_ac43_c48468014242 = New-PSCredential -User Username -Password 'Password'
$48c942c5_b429_4304_b873_e48ad0b0388b = New-CimInstance -ClientOnly -ClassName 'PSCredential' -Property $48c942c5_b429_4304_b873_e48ad0b0388b
$InvokeParams = @{Name = 'ADGroup'; Method = 'set'; Property = @{groupname = 'UKR-ALL-ITS-ServerAdmin-AWSG122UKS0806-test09'; description = 'Create AD Group for Host/Machine'; displayname = 'UKR-ALL-ITS-ServerAdmin-AWSG122UKS0806-test09'; ensure = 'Present'; members = @('AWS-Mgmt access'); category = 'Security'; path = 'OU=AWS,OU=Cloud services,OU=Local Server Access,OU=Groups,OU=UK,DC=uk,DC=test,DC=com'; credential = $b5020517_2949_4754_ac43_c48468014242}; ModuleName = @{ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/dsc_resources/ActiveDirectoryDsc/ActiveDirectoryDsc.psd1'; RequiredVersion = '6.0.1'}}
Try {
  $Result = Invoke-DscResource @InvokeParams
} catch {
  $Response.errormessage   = $_.Exception.Message
  return ($Response | ConvertTo-Json -Compress)
}

# keep the switch for when Test passes back changed properties
Switch ($invokeParams.Method) {
  'Test' {
    $Response.indesiredstate = $Result.InDesiredState
    return ($Response | ConvertTo-Json -Compress)
  }
  'Set' {
    $Response.indesiredstate = $true
    $Response.rebootrequired = $Result.RebootRequired
    return ($Response | ConvertTo-Json -Compress)
  }
  'Get' {
    $CanonicalizedResult = ConvertTo-CanonicalResult -Result $Result
    return ($CanonicalizedResult | ConvertTo-Json -Compress -Depth 10)
  }
}

Output

 PS C:\> .\ciminstance.ps1
New-CimInstance : Cannot bind parameter 'Property'. Cannot convert the "PSCredential" value of type "Microsoft.Management.Infrastructure.CimInstance#root\cimv2/PSCredential" to type "System.Collections.IDictionary".
At C:\ciminstance.ps1:136 char:105
+ ... ssName 'PSCredential' -Property $48c942c5_b429_4304_b873_e48ad0b0388b
+                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [New-CimInstance], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : CannotConvertArgumentNoMessage,Microsoft.Management.Infrastructure.CimCmdlets.NewCimInstanceCommand

Do you see a different behaviour when you run this in your lab? Thanks for the support.

michaeltlombardi commented 3 years ago

Aha, I think I see the problem, note the variable names:

$b5020517_2949_4754_ac43_c48468014242 = New-PSCredential -User Username -Password 'Password'
$48c942c5_b429_4304_b873_e48ad0b0388b = New-CimInstance -ClientOnly -ClassName 'PSCredential' -Property $48c942c5_b429_4304_b873_e48ad0b0388b
$InvokeParams = @{
  Name = 'ADGroup'
  Method = 'set'
  Property = @{
    groupname = 'UKR-ALL-ITS-ServerAdmin-AWSG122UKS0806-test09'
    description = 'Create AD Group for Host/Machine'
    displayname = 'UKR-ALL-ITS-ServerAdmin-AWSG122UKS0806-test09'
    ensure = 'Present'
    members = @('AWS-Mgmt access')
    category = 'Security'
    path = 'OU=AWS,OU=Cloud services,OU=Local Server Access,OU=Groups,OU=UK,DC=uk,DC=test,DC=com'
    credential = $b5020517_2949_4754_ac43_c48468014242
  }
  ModuleName = @{
    ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/dsc_resources/ActiveDirectoryDsc/ActiveDirectoryDsc.psd1'
    RequiredVersion = '6.0.1'
  }
}

It looks like for some reason the conversion for a credential isn't working correctly here and the wrong variable is being passed (it's trying to pass the variable it's also trying to set in the same line).

I'm going to try to confirm on my end if I see the same behavior, but what version of puppetlabs/pwshlib are you using in your environment?

eebuta commented 3 years ago

@michaeltlombardi , I am using 'puppetlabs-pwshlib', '0.6.2'

eebuta commented 3 years ago

Hi @michaeltlombardi, is there an update on the following issue.

Thanks in advance for the support.

Kind Regards, Tam

michaeltlombardi commented 3 years ago

@eebuta we fixed a possibly related error in puppetlabs-pwshlib and have updated that to 0.7.0, can you:

michaeltlombardi commented 3 years ago

Also @eebuta doing some quick research, does this issue on the upstream repository resemble your implementation at all?

https://github.com/dsccommunity/ActiveDirectoryDsc/issues/616

The scenario is that we have 2 domains "Domain A" and "Domain B" where a one-way trust is established so that it is outgoing from Domain A and incoming for Domain B. This setup ensures that users from Domain B are trusted by Domain A. If I am trying to manage AD groups in Domain A where any resources from Domain B have been set as members, the usage of the Get-ADGroupMember cmdlet in the ADGroup resource fails spectacularly in the strangest way.

It's possible that the (merged but unreleased) changes from the PR for that issue may help you here too if the problem is actually upstream (which I suspect it is, but we need to prove that via the Invoke-DscResource log output).

michaeltlombardi commented 3 years ago

Closing due to lack of response; if this issue is still reproducible, please reopen and @ me! Thank you!