DSC Resources missing mandatory properties

[ShawnHardwick]

Describe the Bug

Execute both resources in the same Puppet catalog:

  dsc_systemlocale { 'ja-JP':
    dsc_issingleinstance => 'yes',
    dsc_systemlocale     => 'ja-JP',
  }

  dsc_timezone { 'Eastern Standard Time':
    dsc_timezone         => 'Eastern Standard Time',
    dsc_issingleinstance => 'yes',
  }

Catalog apply fails with below error:

dsc_timezone: raw data received: {"rebootrequired"=>false, "indesiredstate"=>false, "errormessage"=>"Could not find mandatory property TimeZone. Add this property and try again."}
dsc_timezone: Could not find mandatory property TimeZone. Add this property and try again.

dsc_systemlocale applies successfully

Expected Behavior

Both resource succeed without failure

Environment

• Puppet Agent 7.20.0
• Windows Server 2016 and 2019
• dsc-computermanagementdsc 8.5.0-0-1
• pwshlib 0.10.2

Additional Context

I added some debugging statements to the get() function within the dsc_base_provider provider.

dsc_timezone: Collecting data from the DSC Resource
dsc_timezone: No cached results
dsc_timezone: cached_canonicalized_resource: {:dsc_issingleinstance=>"yes", :dsc_systemlocale=>"ja-JP", :name=>"ja-JP", :validation_mode=>"resource"}
dsc_timezone: cached_canonicalized_resource: {:dsc_issingleinstance=>"yes", :dsc_timezone=>"Eastern Standard Time", :name=>"Eastern Standard Time", :validation_mode=>"resource"}
dsc_timezone: mandatory_get_attributes: [:dsc_issingleinstance, :dsc_timezone]
dsc_timezone: namevar_attributes: [:name, :dsc_issingleinstance]
dsc_timezone: Include attribute: dsc_issingleinstance
dsc_timezone: Include attribute: dsc_systemlocale
dsc_timezone: Include attribute: name
dsc_timezone: Include attribute: validation_mode
dsc_timezone: mandatory_properties: {:dsc_issingleinstance=>"yes", :dsc_systemlocale=>"ja-JP", :name=>"ja-JP", :validation_mode=>"resource"}
dsc_timezone: retrieving {:name=>"ja-JP", :dsc_issingleinstance=>"yes", :dsc_systemlocale=>"ja-JP", :validation_mode=>"resource"}
dsc_timezone: invocable_resource: {:parameters=>{:dsc_issingleinstance=>{:value=>"yes", :mof_type=>"String", :mof_is_embedded=>false}}, :name=>"dsc_timezone", :dscmeta_resource_friendly_name=>"TimeZone", :dscmeta_resource_name=>"DSC_TimeZone", :dscmeta_resource_implementation=>"MOF", :dscmeta_module_name=>"ComputerManagementDsc", :dscmeta_module_version=>"8.5.0", :dsc_invoke_method=>"get", :vendored_modules_path=>"C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/computermanagementdsc/dsc_resources", :attributes=>nil}

It appears that the @@cached_canonicalized_resource contains other DSC resources from the catalog when it shouldn't. Due to this line, it grabs the first one. I'm not sure yet if @@cached_canonicalized_resource contains all defined resources from the catalog or all resources from the catalog for that module (dsc-computermanagementdsc). https://github.com/puppetlabs/ruby-pwsh/blob/main/lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb#L132

In the above case, it is comparing the mandatory and namevar attributes against the dsc_systemlocale DSC resource which is the wrong one.

It incorrectly generates the below DSC script. Notice that the Invoke-DSCResource is missing the TimeZone parameter.

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 ([string]::IsNullOrEmpty($value)) {
                  # While PowerShell can happily treat empty strings as valid for returning
                  # an undefined enum, Puppet expects undefined values to be nil.
                  $Value = $null
              }

              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 ($null -eq $Value -or $Value.GetType().Name -notmatch '\[\]') { $Value = @($Value) }
      }

      $ResultObject.$PropertyName = $Value
  }

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

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

$InvokeParams = @{Name = 'TimeZone'; Method = 'get'; Property = @{issingleinstance = 'yes'}; ModuleName = @{ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/computermanagementdsc/dsc_resources/ComputerManagementDsc/ComputerManagementDsc.psd1'; RequiredVersion = '8.5.0'}}
Try {
  $Result = Invoke-DscResource @InvokeParams
} catch {
  $Response.errormessage   = $_.Exception.Message
  return ($Response | ConvertTo-Json -Compress)
} Finally {
  If (![string]::IsNullOrEmpty($UnmungedPSModulePath)) {
    # Reset the PSModulePath
    [System.Environment]::SetEnvironmentVariable('PSModulePath', $UnmungedPSModulePath, [System.EnvironmentVariableTarget]::Machine)
    $env:PSModulePath = [System.Environment]::GetEnvironmentVariable('PSModulePath', 'machine')
  }
}

# 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)
  }
}

[chelnak]

Another interesting one. Thanks @ShawnHardwick

[Hvid]

I'm experiencing the same issue. Is there a workaround available?

[ShawnHardwick]

@Hvid The current workaround I have in place to continue my testing involved converting the class variables to instance variables in the dsc provider in pwshlib. You can find the exact changes here:

https://github.com/puppetlabs/ruby-pwsh/compare/main...ShawnHardwick:ruby-pwsh:bugfix_missing_mandatory_props

I am not running this in production so use at your own risk. I'll let Puppet develop the correct solution.

[chelnak]

@ShawnHardwick nice. Do you want to submit this as a PR on pwshlib?

[GSPatton]

hi @ShawnHardwick. This is a really good spot, we appreciate the work. I'm going to ticket this up and have a look at it soon. Thanks again for the contributions.

[ShawnHardwick]

Sorry for the long delay in responding. I am no longer in a role to follow up on this unfortunately so I won't be able to contribute much outside of what I've provided.

[jordanbreen28]

Hey @ShawnHardwick - Understand this may be of no help to you now.. But just wanted to shout out that we shipped your fix, so thanks for the help with this one!

[ShawnHardwick]

Even though I'm not actively working on it, I'm glad this was made available for others. Thanks for following up on it @jordanbreen28