How TargetResource functions should be used discussion

[kungfoome]

I feel like there should be a helper function for test or something may need to change. I see a lot of resources checking parameters that are invalid and only setting those, instead of just setting all parameters.

I think this is ok, but this can cause a lot of duplicate code I noticed. If you were to go this direction, there should be a best practice or guidelines to suggest a certain way to structure your code.

Something I was thinking is:

Get-TargetResource - Nothing changes Test-TargetResource - Wrapper function to a helper function that returns a boolean Test-TargetResourceHelper - helper function that returns a hashtable maybe? This should also call Get-TargetResource for comparison. Something like:

@{
   'Ensure' = @{ 'expected' = 'Absent'; 'actual' = 'Present' }
   'invalidParameter' = @{ 'expected' = 'parameter passed in'; 'actual' = 'actual value set' }
}

or

@(
     @{'parameter'='Ensure'; 'expected' = 'Absent'; 'actual' = 'Present' },
     @{'parameter'='InvalidParam'; 'expected' = 'parameter passed in'; 'actual' = 'actual value set' }
 )

This is why we should do something like this. Set-TargetResource can now call Test-TargetResourceHelper and get back an array or hashtable. You can now simply iterate over this and set only these values, but now you only need one code source for testing and you're not duplicating code.

This would not be required, but at least it can defined and thought of in case someone does want to use it. Seems like it would make maintenance a bit easier.

[gaelcolas]

I feel like a compare-* function is more natural fit for this. It returns the keys that have non matching values.

[gaelcolas]

Changing the Test-DscParameterState function into something like a Compare-* should be easy and do the job well. Maybe just split that function in 2:

• Test-DscParameterState
• Compare-DscParameterState, and the Test-DscParameterState just call the compare

[kungfoome]

Adding a Compare would be really nice. I was thinking about this a bit more. I was thinking at a lower level (private), but I think something like the second option would be better, except return an array of objects instead.

@(
    [pscustomobject] @{
        DSCModuleName   = 'NetworkingDsc'
        DSCResourceName = 'MSFT_IPAddress'
        Configuration   = 'SetIPAddressForAdapter1'
        Parameter       = 'IPAddress'
        Expected        = '192.168.1.1'
        Actual          = '192.168.1.2'
        Pass            = $false
    },
    [pscustomobject] @{
        DSCModuleName   = 'xActiveDirectory'
        DSCResourceName = 'MSFT_xADUser'
        Configuration   = 'AddServiceAccountForSQL'
        Parameter       = 'Ensure'
        Expected        = $true
        Actual          = $true
        Pass            = $true
    }
)

DSCModuleName    DSCResourceName Configuration           Parameter Expected    Actual       Pass
-------------    --------------- -------------           --------- --------    ------       ----
NetworkingDsc    MSFT_IPAddress  SetIPAddressForAdapter1 IPAddress 192.168.1.1 192.168.1.2 False
xActiveDirectory MSFT_xADUser    AddServiceAccountForSQL Ensure    True        True         True

Only downside is, if you are trying to get values that are not compliant, it makes it a little more difficult and little more code, but benefits may out weigh that.

This is all assuming this would be a public interface. Now I can get a list of ALL the configuration settings, what it should be, what it's set to. I can call this from test and set and filter what I need and don't need.

Here is a proof of concept https://github.com/kungfu71186/xActiveDirectory/blob/883d07b6c0fc6c39acf8df0bf24e90816446fc99/DSCResources/MSFT_xADManagedServiceAccount/MSFT_xADManagedServiceAccount.psm1#L613