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

Conversation

jambar42
Copy link

Pull Request (PR) description

Ensure empty array output is not enumerated prematurely.

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry under the Unreleased section in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

@ghost
Copy link

ghost commented Jul 28, 2020

CLA assistant check
All CLA requirements met.

@PlagueHO PlagueHO added the needs review The pull request needs a code review. label Jul 28, 2020
@@ -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.

$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

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

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 
 }

Tests/Unit/MSFT_WindowsOptionalFeature.Tests.ps1 Outdated Show resolved Hide resolved
@jambar42 jambar42 requested a review from PlagueHO March 19, 2021 16:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs review The pull request needs a code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WindowsOptionalFeature: Get Method fails if OptionalFeature has no custom properties when called by the LCM.
2 participants