briantist / idempotion Goto Github PK

For some resources, the language of the command is more natural if the result of the test method is inverted.

For example with xPendingReboot, converted with Idempotion:

if (Test-xPendingReboot) { }

It feels like that should be $true if there is a pending reboot, but it's the opposite (because in the case of the DSC resource we want to run set if there is a pending reboot)'.

I envision an -InvertTest switch parameter on Convert-DscResourceToFunction which would translate to a new definition variable to be used (or ignored) by the template definition.

The nature of this snippet seemingly makes it difficult to put in its own function because it relies on 2 variables that are auto-populated based on the current function ($PSBoundParameters and $MyInvocation), however this is possible by taking these as parameters; the invoking function then passes these in.

I'm thinking of this approach from the point of view of another meta function I created to treat parameters as required without prompting.

The advantages of this are not having a hard-coded variable name in the result, not adding additional worker variables, and clearer separation / less repeated code.

I'm somewhat thinking of uses outside of this module (the usage within this module would still be repeating lots of code but it's auto-generated at runtime so I don't care).

This came about from working on issue #22

#PowerShell protip: If your function accepts a [ScriptBlock] param, please prepend [Security.SecurityCritical()] to the param block.

— Matt Graeber (@mattifestation) April 30, 2017

Although Idempotion isn't directly taking a scriptblock and executing it, it does accept a hashtable, which contains strings, which will become functions, which will become a module, which can be imported, and then can be executed.

Strictly speaking, this module doesn't really execute any user supplied code.

But I wonder if it makes sense to offer a -SecurityCritical parameter that would add the attribute to the generated functions.

Also unclear: later in the twitter thread, it seems like this attribute may or may not be needed/useful, and it may also be for internal use. So it requires more investigation and though.

For now I don't think Idempotion is doing anything that would aid security bypass, intentionally or accidentally. Discussion welcome.

The -ExcludeProperty and -ExcludeMandatory parameters are defined but not implemented.

-ExcludeProperty takes a resource:property value in the style of $PSDefaultParameterValues, to determine which properties can or should be excluded (mandatory parameters will not be excluded).

-ExcludeMandatory is a switch which overrides the behavior of -ExcludeProperty so that it can exclude mandatory parameters as well. This would be used in a case where a template provides the value for the mandatory parameters, so they shouldn't be available in the function definitions.

By default, -ExcludeProperty will have a value of *:DependsOn for example. This will ensure that the generated functions do not accept a -DependsOn parameter, which would be useless for the purpose of this module.

This will require new parameters and changes to the internal functions New-ParameterBlockFromResourceDefinition and New-ParameterFromResourcePropertyInfo.

Need to write help for this module.

Expected Topics

• Convert-DscResourceToCommand
• about_Idempotion_Definitions (cover templating)

Optional?

• Internal Commands -- not yet sure if they need help.

This is the biggest method and the only one that's public; it should really have tests!

Calling Convert-DscResourceToCommand with -Force twice in a row will result in 2 modules being loaded. 3 times in a row, 3 modules, etc. It's not replacing the loaded module.

Additionally, not calling it with -Force does the exact same thing, so it's not properly following the semantics of Import-Module as it was designed to do.

What if: Performing the operation "File DSC Resource" on target "Set".
This comes from the following line in the template functions:

if (`$PSCmdlet.ShouldProcess('${Verb}', '${Resource} DSC Resource')) {

(the parameters to $PSCmdlet.ShouldProcess are target and action respectively, so the values should be reversed)

Since DSC resources can't really use [Switch] parameters, they use [bool], but this can be a bit awkward or less idiomatic when converted to a function. For exmaple:

Test-xPendingReboot -SkipPendingFileRename $true
# would be better as
Test-xPendingReboot -SkipPendingFileRename

I imagine this implemented similarly to -ExcludeProperty in that it would take a [ResourcePropertyPattern] in the style of $PSDefaultParameterValues.

That would allow converting only certain properties since [Switch] may not be appropriate for all.

Then we could do something like -BoolToSwitch 'xPendingReboot:Skip*'

This snippet to discover the parameters:

foreach($h in $MyInvocation.MyCommand.Parameters.GetEnumerator()) {
    try {
        $key = $h.Key
        $val = Get-Variable -Name $key -ValueOnly -ErrorAction Stop
        if (([String]::IsNullOrEmpty($val) -and (!$PSBoundParameters.ContainsKey($key)))) {
            throw "A blank value that wasn't supplied by the user."
        }
        $params[$key] = $val
    } catch {}
}

is using exceptions in a bad way (tsk tsk). This has some nasty side effects.

First, if pollutes the $Error variable, which is a bad thing for anyone who wants to use it.

I think I could work around this by something like $Error.RemoveAt(0) in the snippet's catch block.

The other problem I only just noticed in using automatic transcription. All of those errors show up as lines in the transcript, like so:

PS>TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'Verbose'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'Debug'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'ErrorAction'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'WarningAction'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'InformationAction'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'ErrorVariable'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'WarningVariable'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'InformationVariable'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'OutVariable'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'OutBuffer'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'PipelineVariable'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'WhatIf'."
>> TerminatingError(Get-Variable): "The running command stopped because the preference variable "ErrorActionPreference" or common parameter is set to Stop: Cannot find a variable with the name 'Confirm'."

This really sucks.

I'd like to rewrite the snippet in a way that doesn't (ab)use exceptions, to solve both problems.

Examples would be really helpful in showing how this can/should be used.

Ideal examples would also show how the same would be accomplished without DSC resources (but this more labor intensive; the whole point of this module is being able to avoid all that).

I recently discovered that Invoke-DscResource fails when there is more than version of a module installed. You can explicitly give it a version of the module, but it won't discover it for you.

So the proposal here is two-fold:

1. Ensure Idempotion can handle a module specification [hashtable] (not just a name).
2. Default behavior when given only a name should be to discover local versions and use the latest automatically.

This was really difficult to track down. I noticed it when using the xADGroup resource, which works perfectly fine until you use any of the Members* attributes, which are all [string[]]. Then it fails with:

Failed to serialize properties into CimInstance

Which is an error you usually see with composite resources (a known limitation of Invoke-DscResource).

It turns out that the snippet I use (from my own blog 😓 ) to put the parameters into a hashtable results in array parameters reverting to [object[]] even if they were originally more strongly typed.

I traced it back to this line:

$val = Get-Variable -Name $key -ErrorAction Stop | Select-Object -ExpandProperty Value -ErrorAction Stop

Using Select-Object -ExpandProperty Value turns it into [object[]] even though it was originally strongly typed as [string[]].

Replacing this with:

$val = Get-Variable -Name $key -ValueOnly -ErrorAction Stop

Should fix the issue (and it's clearer anyway).

For most code, this is indistinguishable and wouldn't cause a problem.

But of course Invoke-DscResource bombs out on it spectacularly, which an unbelievably unhelpful error message.

If literal single quotes were included in a [ValidateSet()] item from a resource, that would be a problem when generating the [ValidateSet()] attribute for the function, as it uses single-quoted strings.

As a result, my code currently does a simple replace in New-ParameterFromResourcePropertyInfo:

.Replace("'" , "''")

This only works for one possible valid single quote character ': U+0027 (ANSI 39/0x27).

The problem is that other characters are valid for use as a single quote in PowerShell to denote a single quoted string. Consider for instance ’: U+2019 (8217/0x2019).

In the (admittedly unlikely) chance that a DSC resource includes this character (or any of several others) in a default value the code will fail in a big way!

Solution

Rather than testing of all of the possible characters, we can use PowerShell v5's code generation methods to help:

[System.Management.Automation.Language.CodeGeneration]::EscapeSingleQuotedStringContent($_)

This is by far the best solution.

The tests must be updated for this condition.

The calls throw an exception early on:

"The attribute cannot be added because variable Attributes with value would no longer be valid."

This appears to be related to this PowerShell bug regarding closures.

I'm testing on a method that does in fact use an attribute ([ValidateSet()]).

Possible Fix

I think the only viable resolution here is to offer an option that doesn't include [ValidateSet()] when generating the function bodies. This will require:

• Additional parameters on Convert-DscResourceToCommand, and the internal functions New-ParameterBlockFromResourceDefinition and New-ParameterFromResourcePropertyInfo.
• Possible changes to the template (but I think not)

If you call Invoke-DscResource directly with -Verbose, you get output similar to what you would see if you ran the resource in the LCM:

VERBOSE: Perform operation 'Invoke CimMethod' with following parameters, ''methodName' = ResourceTest,'className' = MSFT_DSCLocalConfigurationManager,'namespaceName' = root/Microsoft/Windows/Desi
redStateConfiguration'.
VERBOSE: An LCM method call arrived from computer STANCHION with user sid S-1-5-21-90865308-1833148694-2130556682-1001.
VERBOSE: [COMP]: LCM:  [ Start  Test     ]  [[File]DirectResourceAccess]
VERBOSE: [COMP]:                            [[File]DirectResourceAccess] The destination object was found and no action is required.
VERBOSE: [COMP]: LCM:  [ End    Test     ]  [[File]DirectResourceAccess] True in 0.0120 seconds.
VERBOSE: [COMP]: LCM:  [ End    Set      ]    in  0.0260 seconds.
VERBOSE: Operation 'Invoke CimMethod' complete.

InDesiredState 
-------------- 
True           
VERBOSE: Time taken for configuration job to complete is 0.199 seconds

However, when running an Idempotion-generated function, the -Verbose output adds these additional lines at the top:

VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\PSDesiredStateConfiguration.psm1'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Get-DSCConfiguration.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Get-DSCLocalConfigurationManager.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Restore-DSCConfiguration.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Get-DscConfigurationStatus.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Stop-DscConfiguration.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Remove-DscConfigurationDocument.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Disable-DscDebug.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\Enable-DscDebug.cdxml'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\DSCClassResources\WindowsPackageCab\WindowsPackageCab.psd1'.
VERBOSE: Loading module from path 'C:\WINDOWS\system32\WindowsPowerShell\v1.0\Modules\PSDesiredStateConfiguration\DSCClassResources\WindowsPackageCab\WindowsPackageCab.psm1'.

This really makes what would otherwise be very useful output into kind of a mess, especially on an Update where both Test and Set will be run (independently), so that will end up in the output twice.

Why?

I think this is a difference between -Verbose being specified and it being inherited (via $VerbosePreference). Since I am wrapping Invoke-DscResource inside an advanced function, specifying -Verbose on the outer function sets the preference variable.

I think what happens is that code inside Invoke-DscResource reads the preference from a higher scope and displays that extra output, whereas it wouldn't do that if the preference wasn't set before setting -Verbose on the Invoke-DscResource call.

Confirmation of Above

I have confirmed that the preference inheritance is to blame here. This can be demonstrated with the following code:

$demoFile = 'C:\my\path\file.txt'

$VerbosePreference = 'SilentlyContonue'  # default

# Shorter verbose output
Invoke-DscResource -Name File -Method Test -ModuleName PSDesiredStateConfiguration -Property @{ DestinationPath = $demoFile ; Contents = "Hello" } -Verbose

$VerbosePreference = 'Continue'

# Longer verbose output
Invoke-DscResource -Name File -Method Test -ModuleName PSDesiredStateConfiguration -Property @{ DestinationPath = $demoFile ; Contents = "Hello" } -Verbose

Solution

I think I can fix this in the definitions with an ugly looking hack:

$oldVerbosePreference = $VerbosePreference
$VerbosePreference = [System.Management.Automation.ActionPreference]::SilentlyContinue

Invoke-DscResource -Name 'File' -ModuleName 'PSDesiredStateConfiguration' -Method 'Test' -Property $params -Verbose:$oldVerbosePreference

$VerbosePreference = $oldVerbosePreference