CommonTasks icon indicating copy to clipboard operation
CommonTasks copied to clipboard

Composite best practices: InstanceName

Open ChristophHannappel opened this issue 3 years ago • 3 comments

After I lost 2 or 3 days on a stupid oversight and wasted @raandree time I'd like to propose something like a best practices page (if there isn't something already) on composite creation and save someone else a lot of time.

I made a composite where i mixed Get-DscSplattedResource and the conventional Resource definition syntax.

configuration SharePointFarm
{
    param
    (
        # Shortend
    )
    Import-DscResource -ModuleName SharePointDSC
    Import-DscResource -ModuleName xPSDesiredStateConfiguration

    $PSBoundParameters.Remove('InstanceName')
    # Make the Setup Account a local Administrator
    xGroup 'LocalAdministrators' {
        Ensure           = 'Present'
        GroupName        = 'Administrators'
        MembersToInclude =  $SetupAccount.UserName
    }

    $ExecutionProperties = $PSBoundParameters
    $ExecutionProperties.Add('Ensure', 'Present')
    $ExecutionProperties.Add('IsSingleInstance', 'Yes')
    $ExecutionProperties.Add('PsDscRunAsCredential', $SetupAccount)
    if ($ExecutionProperties.ContainsKey('DependsOn'))
    {
        $ExecutionProperties.DependsOn = '[xGroup]LocalAdministrators'
    }
    else
    {
        $ExecutionProperties.Add('DependsOn', '[xGroup]LocalAdministrators')
    }

    $ExecutionProperties.Remove('SetupAccount')
    (Get-DscSplattedResource -ResourceName SPFarm -ExecutionName 'SPFarm' -Properties $ExecutionProperties -NoInvoke).Invoke($ExecutionProperties)


    # Setup Wizzard
    SPConfigWizard RunConfigWizard
    {
        IsSingleInstance     = "Yes"
        PsDscRunAsCredential = $SetupAccount
        DependsOn            = '[SPFarm]SPFarm'
    }
}

This code ends up make Composites with the Resouce ID [SharePointFarm] instead of [SharePointFarm]SharePointFarm What i didn't know is that while Get-DscSplattedResource needs to have the InstanceNameremoved the conventional Resource definition needs it to create the ResourceID correctly.

After moving the .Remove('InstanceName') to the Get-DscSplattedResource Hashtable it works as expected.

configuration SharePointFarm
{
    param
    (
        # Shortend
    )
    Import-DscResource -ModuleName SharePointDSC
    Import-DscResource -ModuleName xPSDesiredStateConfiguration
    #
    # Make the Setup Account a local Administrator
    xGroup 'LocalAdministrators' {
        Ensure           = 'Present'
        GroupName        = 'Administrators'
        MembersToInclude =  $SetupAccount.UserName
    }

    # Create or Join Farm with SPFarm
    $spFarmValidParameters = @('AdminContentDatabaseName', 'ApplicationCredentialKey', 'CentralAdministrationAuth', 'CentralAdministrationPort', 'CentralAdministrationUrl', 'DatabaseCredentials', 'DatabaseServer', 'DeveloperDashboard', 'Ensure', 'FarmAccount', 'FarmConfigDatabaseName', 'Passphrase', 'RunCentralAdmin', 'ServerRole', 'SkipRegisterAsDistributedCacheHost',
        'UseSQLAuthentication')

    $spFarm = @{
        IsSingleInstance     = 'Yes'
        PsDscRunAsCredential = $SetupAccount
        DependsOn            = '[xGroup]LocalAdministrators'
    }
    foreach ($parameter in $spFarmValidParameters)
    {
        if ($PSBoundParameters.ContainsKey($parameter))
        {
            $spFarm.Add($parameter, $PSBoundParameters.Item($parameter))
        }
    }
    (Get-DscSplattedResource -ResourceName SPFarm -ExecutionName 'SPFarm' -Properties $spFarm -NoInvoke).Invoke($spFarm)


    # Setup Wizzard
    SPConfigWizard RunConfigWizard
    {
        IsSingleInstance     = "Yes"
        PsDscRunAsCredential = $SetupAccount
        DependsOn            = '[SPFarm]SPFarm'
    }
}

For discussion: Could it be a good habit to never remove a Key from the $PSBoundParameters and always call Get-DscSplattedResource with a dedicated Hashtable so you won't stab yourself in the back later if you add a resource. Alternatively don't mix Get-DscSplattedResource with the conventional definition.

ChristophHannappel avatar Mar 18 '22 08:03 ChristophHannappel

Thanks for solving this particular issue as well as coming up with the idea of best practice guide, @ChristophHannappel.

@gaelcolas, @stehlih, @nyanhp, @ykuijs, do you have by accident or purpose written down some guidance already?

raandree avatar Mar 18 '22 09:03 raandree

I don't have anything specifically in writing, but I never copy PSBoundParameters and then edit that copy. I have noticed that copying PSBoundParameters copies the pointer to the object, meaning that if you edit the copy you also edit the original object.

That is why I always generate a new hashtable using the method you have also used in the 2nd example.

ykuijs avatar Mar 22 '22 09:03 ykuijs

As soon as you deal with reference types (classes) and not value types (structs) you always deal with references. Also the method described above may duplicate references.

The line $b.k2.Remove('kk2') will remove the key kk2 from both hashtables, as hashtables are also reference types.

$a = @{
    k1 = 'v1'
    k2 = @{
        kk1 = 'vv1'
        kk2 = 'vv2'
    }
}

$b = @{}
$validKeys = 'k1', 'k2'
foreach ($validKey in $validKeys)
{
    if ($a.ContainsKey($validKey))
    {
        $b.Add($validKey, $a.Item($validKey))
    }
}

Write-Host '-------- Before removal of kk2 -------------'
Write-Host "Key count of a.k2: $($a.k2.Keys.Count)"
Write-Host "Key count in b.k2: $($b.k2.Keys.Count)"

$b.k2.Remove('kk2')
Write-Host '-------- After removal of kk2 --------------'
Write-Host "Key count of a.k2: $($a.k2.Keys.Count)"
Write-Host "Key count in b.k2: $($b.k2.Keys.Count)"

raandree avatar Mar 22 '22 11:03 raandree