search

dsccommunity / HyperVDsc

This module contains DSC resources for deployment and configuration of Microsoft Hyper-V.
MIT License
114 stars 65 forks source link

VMNetworkAdapter: throws `You cannot call a method on a null-valued expression` - introduced in version 3.17 #191

Open BarnumD opened 3 years ago

BarnumD commented 3 years ago

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

I'm using puppet's dsc xhyper_v module (a copy of this one) to manage xvmnetworkadapter. On version 3.16.0-0-3, the following code works. On version 3.17.0-0-0, it breaks.

Puppet code

dsc_xvmnetworkadapter{'VMNetAdapter_V_Management':
    dsc_ensure     => 'Present',
    dsc_name       => 'V-Management',
    dsc_switchname => 'guest_switch',
    dsc_vmname     => 'ManagementOS',
    dsc_id         => fqdn_uuid("v.management.${$facts['fqdn']}"),
}

In version 3.16.0-0-3, the output is:

[0;36mDebug: dsc_xvmnetworkadapter: invocable_resource: {:parameters=>{:dsc_name=>{:value=>"V-Management", :mof_type=>"String", :mof_is_embedded=>false}, :dsc_switchname=>{:value=>"guest_switch", :mof_type=>"String", :mof_is_embedded=>false}, :dsc_vmname=>{:value=>"ManagementOS", :mof_type=>"String", :mof_is_embedded=>false}, :dsc_id=>{:value=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", :mof_type=>"String", :mof_is_embedded=>false}}, :name=>"dsc_xvmnetworkadapter", :dscmeta_resource_friendly_name=>"xVMNetworkAdapter", :dscmeta_resource_name=>"MSFT_xVMNetworkAdapter", :dscmeta_resource_implementation=>nil, :dscmeta_module_name=>"xHyper-V", :dscmeta_module_version=>"3.16.0.0", :dsc_invoke_method=>"get", :vendored_modules_path=>"C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/dsc_resources", :attributes=>nil}
Debug: dsc_xvmnetworkadapter: Script:
 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 = 'xVMNetworkAdapter'; Method = 'get'; Property = @{name = 'V-Management'; switchname = 'guest_switch'; vmname = 'ManagementOS'; id = 'e49f84e4-a553-595d-9096-ada90a8ec9e7'}; ModuleName = @{ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/dsc_resources/xHyper-V/xHyper-V.psd1'; RequiredVersion = '3.16.0.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)
  }
}

Debug: dsc_xvmnetworkadapter: raw data received: {"Id"=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", "Name"=>"V-Management", "ResourceId"=>nil, "MacAddress"=>"00155DD15209", "DynamicMacAddress"=>false, "PsDscRunAsCredential"=>nil, "ConfigurationName"=>nil, "Ensure"=>"Present", "DependsOn"=>nil, "SourceInfo"=>nil, "SwitchName"=>"guest_switch", "ModuleVersion"=>"3.16.0.0", "ModuleName"=>"C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/dsc_resources/xHyper-V/xHyper-V.psd1", "VMName"=>"ManagementOS"}
Debug: dsc_xvmnetworkadapter: Returned to Puppet as {:dsc_dynamicmacaddress=>false, :dsc_ensure=>"Present", :dsc_id=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", :dsc_macaddress=>"00155DD15209", :dsc_name=>"V-Management", :dsc_switchname=>"guest_switch", :dsc_vmname=>"ManagementOS", :name=>"VMNetAdapter_V_Management"}
Debug: dsc_xvmnetworkadapter: Canonicalized Resources: [{:dsc_ensure=>"Present", :dsc_id=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", :dsc_name=>"V-Management", :dsc_switchname=>"guest_switch", :dsc_vmname=>"ManagementOS", :name=>"VMNetAdapter_V_Management"}]

In version 3.17.0-0-0, the I get the message You cannot call a method on a null-valued expression, as seen in this output: The same is true for version 3.17.0-0-3.

[0;36mDebug: dsc_xvmnetworkadapter: retrieving {:name=>"VMNetAdapter_V_Management", :dsc_ensure=>"Present", :dsc_name=>"V-Management", :dsc_switchname=>"guest_switch", :dsc_vmname=>"ManagementOS", :dsc_id=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", :require=>["Dsc_xvmswitch[VMSwitch_guest_switch]"]}
[0;36mDebug: dsc_xvmnetworkadapter: invocable_resource: {:parameters=>{:dsc_name=>{:value=>"V-Management", :mof_type=>"String", :mof_is_embedded=>false}, :dsc_switchname=>{:value=>"guest_switch", :mof_type=>"String", :mof_is_embedded=>false}, :dsc_vmname=>{:value=>"ManagementOS", :mof_type=>"String", :mof_is_embedded=>false}, :dsc_id=>{:value=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", :mof_type=>"String", :mof_is_embedded=>false}}, :name=>"dsc_xvmnetworkadapter", :dscmeta_resource_friendly_name=>"xVMNetworkAdapter", :dscmeta_resource_name=>"MSFT_xVMNetworkAdapter", :dscmeta_resource_implementation=>"MOF", :dscmeta_module_name=>"xHyper-V", :dscmeta_module_version=>"3.17.0.0", :dsc_invoke_method=>"get", :vendored_modules_path=>"C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/xhyper_v/dsc_resources", :attributes=>nil}
Debug: dsc_xvmnetworkadapter: Script:
 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 = 'xVMNetworkAdapter'; Method = 'get'; Property = @{name = 'V-Management'; switchname = 'guest_switch'; vmname = 'ManagementOS'; id = 'e49f84e4-a553-595d-9096-ada90a8ec9e7'}; ModuleName = @{ModuleName = 'C:/ProgramData/PuppetLabs/puppet/cache/lib/puppet_x/xhyper_v/dsc_resources/xHyper-V/xHyper-V.psd1'; RequiredVersion = '3.17.0.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)
  }
}
[0m
[0;36mDebug: dsc_xvmnetworkadapter: raw data received: {"rebootrequired"=>false, "indesiredstate"=>false, "errormessage"=>"You cannot call a method on a null-valued expression."}[0m
[1;31mError: dsc_xvmnetworkadapter: You cannot call a method on a null-valued expression.[0m

[0;36mDebug: dsc_xvmnetworkadapter: Canonicalized Resources: [{:name=>"VMNetAdapter_V_Management", :dsc_ensure=>"Present", :dsc_name=>"V-Management", :dsc_switchname=>"guest_switch", :dsc_vmname=>"ManagementOS", :dsc_id=>"e49f84e4-a553-595d-9096-ada90a8ec9e7", :require=>["Dsc_xvmswitch[VMSwitch_guest_switch]", "Notify[foobar]"]}][0m

Suggested solution to the issue

Did some code in the version 3.17 pull request add a reference that is not properly checked if it exists? Is it perhaps the inclusion of vlanid or networksetting, that I do not include but seems to be new in that version of the code?

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

N/A

The operating system the target node is running

OsName               : Microsoft Windows Server 2016 Datacenter
OsOperatingSystemSKU : DatacenterServerEdition
OsArchitecture       : 64-bit
WindowsBuildLabEx    : 14393.4583.amd64fre.rs1_release.210730-1850
OsLanguage           : en-US
OsMuiLanguages       : {en-US}

Version and build of PowerShell the target node is running

Name                           Value
----                           -----
PSVersion                      5.1.14393.4583
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14393.4583
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Version of the DSC module that was used

3.16.0-0-3 - Known good 3.17.0-0-0--3 - Known bad.

johlju commented 2 years ago

I think PR #189 will resolve this when it is refactored, but we should add a unit test to verify the parameters in this issue.