briantist / idempotion Goto Github PK

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.

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

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)

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*'

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.

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.

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.

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)

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.

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.

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.

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).

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!

#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.

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