Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

WindowsOptionalFeature: Ensure empty array output is not enumerated prematurely #192

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

WindowsOptionalFeature: Ensure empty array output is not enumerated prematurely #192

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8dfbdc2
Ensure empty array output is not enumerated prematurely.
jambar42 Jul 28, 2020
a7a903a
Added unit test
jambar42 Jul 28, 2020
c6323a4
Removed unneeded variable assignment in unit test.
jambar42 Jul 28, 2020
bc54ae4
Updated Unit test description.
jambar42 Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Proposed fix for WindowsOptionalFeature DSC Resource GET method when the OptionalFeature has no Custom Properties.
[Issue #191](https://github.com/PowerShell/PSDscResources/issues/191)

## 2.12.0.0

* Ports style fixes that were recently made in xPSDesiredStateConfiguration
Expand Down
2 changes: 1 addition & 1 deletion DscResources/MSFT_WindowsOptionalFeature/MSFT_WindowsOptionalFeature.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ function Convert-CustomPropertyArrayToStringArray
}
}

return $propertiesAsStrings
Write-Output $propertiesAsStrings -NoEnumerate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jambar42 - can you use a parameter name here? E.g. Write-Output -InputObject ... or $propertiesAsStrings | Write-Output -NoEnumerate?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried both of the suggestions listed above and both return the same results. The act of piping the object or feeding it into -InputObject both cause enumeration resulting in a failed LCM call to the Get method.

Invoke-DscResource -Name WindowsOptionalFeature -Method Get -ModuleName PSDSCResources -Property @{Name = 'HyperVisorPlatform'; Ensure = 'Present'}
A general error occurred that is not covered by a more specific error code.
+ CategoryInfo : NotSpecified: (root/Microsoft/...gurationManager:String) [], CimException
+ FullyQualifiedErrorId : MI RESULT 1
+ PSComputerName : localhost

Another option that may work is to make the output of line 58 always be an array.

PSDscResources/DscResources/MSFT_WindowsOptionalFeature/MSFT_WindowsOptionalFeature.psm1

Lines 54 to 65 in c6323a4

$windowsOptionalFeatureResource = @{
LogPath = $windowsOptionalFeatureProperties.LogPath
Ensure = Convert-FeatureStateToEnsure -State $windowsOptionalFeatureProperties.State
CustomProperties = Convert-CustomPropertyArrayToStringArray `
-CustomProperties $windowsOptionalFeatureProperties.CustomProperties
Name = $windowsOptionalFeatureProperties.FeatureName
LogLevel = $windowsOptionalFeatureProperties.LogLevel
Description = $windowsOptionalFeatureProperties.Description
DisplayName = $windowsOptionalFeatureProperties.DisplayName
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is really strange. We have a standard that we must only used named parameters, rather than positional. Specifying a parameter name shouldn't have changed the behavior here. I don't know what could have caused that.

What you could try is casting the value to an array in the return - although not sure this would work either:

return @($propertiesAsStrings)

Copy link
Author

@jambar42 jambar42 Aug 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are my results using the various outputs from Convert-CustomPropertyArrayToStringArray. So far, I've only found the first to return an empty array directly from this function. If you have other methods of ensuring this function always returns an array, even if empty I am curious to see them.

Write-Output $propertiesAsStrings -NoEnumerate

(Convert-CustomPropertyArrayToStringArray -CustomProperties @()).GetType()

IsPublic IsSerial Name BaseType
True True String[] System.Array

Write-Output -InputObject $propertiesAsStrings -NoEnumerate

(Convert-CustomPropertyArrayToStringArray -CustomProperties @()).GetType()

You cannot call a method on a null-valued expression.
At line:1 char:1
(Convert-CustomPropertyArrayToStringArray -CustomProperties @()).GetT ...
CategoryInfo : InvalidOperation: (:) [], RuntimeException
FullyQualifiedErrorId : InvokeMethodOnNull

return @($propertiesAsStrings)

(Convert-CustomPropertyArrayToStringArray -CustomProperties @()).GetType()

You cannot call a method on a null-valued expression.
At line:1 char:1
(Convert-CustomPropertyArrayToStringArray -CustomProperties @()).GetT ...
CategoryInfo : InvalidOperation: (:) [], RuntimeException
FullyQualifiedErrorId : InvokeMethodOnNull

Copy link
Author

@jambar42 jambar42 Aug 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other option would be to adjust the calling of this function, however I don't think this is as clean and doesn't fit well with being unit tested.

[System.String[]]$CustomProperties =  Convert-CustomPropertyArrayToStringArray `
-CustomProperties $windowsOptionalFeatureProperties.CustomProperties

$windowsOptionalFeatureResource = @{ 
     LogPath = $windowsOptionalFeatureProperties.LogPath 
     Ensure = Convert-FeatureStateToEnsure -State $windowsOptionalFeatureProperties.State 
     CustomProperties = $CustomProperties
     Name = $windowsOptionalFeatureProperties.FeatureName 
     LogLevel = $windowsOptionalFeatureProperties.LogLevel 
     Description = $windowsOptionalFeatureProperties.Description 
     DisplayName = $windowsOptionalFeatureProperties.DisplayName 
 }

}

<#
Expand Down
4 changes: 4 additions & 0 deletions Tests/Unit/MSFT_WindowsOptionalFeature.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ try
$propertiesAsStrings.Contains("Name = Object $objectNumber, Value = Value $objectNumber, Path = Path $objectNumber") | Should -BeTrue
}
}

It 'Should return an empty array and not a null object when input is empty' {
jambar42 marked this conversation as resolved.
Show resolved Hide resolved
(Convert-CustomPropertyArrayToStringArray -CustomProperties @()) -is [String[]] | Should -BeTrue
}
}
}
}
Expand Down