Microsoft365DSC icon indicating copy to clipboard operation
Microsoft365DSC copied to clipboard

Resources using Test-M365DSCParameterState do not support default Ensure value when using -DesiredValues $PSBoundParameters

Open Borgquite opened this issue 1 year ago • 2 comments

Description of the issue

When trying to use EXODistributionGroup to create a resource without specifying 'Ensure', Test-TargetResource always returns True even if the resource doesn't exist

It looks like this is because Test-M365DSCParameterState is called with -DesiredValues $PSBoundParameters, which of course does not include parameters which have the 'default value' - therefore the value is not tested:

https://github.com/microsoft/Microsoft365DSC/blob/ca59c6cec43e920714a85d37a9e7e1fdd18a1c5f/Modules/Microsoft365DSC/DSCResources/MSFT_EXODistributionGroup/MSFT_EXODistributionGroup.psm1#L983

This seems to affect many resources using this template style (e.g. ADGroup)

Not sure the best way to fix. Perhaps $PSBoundParameters could be modified to always add Ensure = $Ensure?

Microsoft 365 DSC Version

dev

Which workloads are affected

Exchange Online

The DSC configuration

Configuration Example
{
    param(
        [Parameter()]
        [System.String]
        $ApplicationId,

        [Parameter()]
        [System.String]
        $TenantId,

        [Parameter()]
        [System.String]
        $CertificateThumbprint
    )
    Import-DscResource -ModuleName Microsoft365DSC

    node localhost
    {
        EXODistributionGroup 'Distribution Group Name'
        {
            Alias                              = "DistributionGroupName";
            Identity                           = "Distribution Group Name";
            Name                               = "Distribution Group Name";
            PrimarySmtpAddress                 = "[email protected]";
            Type                  = 'Security'
            ApplicationId         = $ApplicationId
            TenantId              = $TenantId
            CertificateThumbprint = $CertificateThumbprint
        }
    }
}

Verbose logs showing the problem

VERBOSE: [HOSTNAME]: LCM:  [ Start  Resource ]  [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME]
VERBOSE: [HOSTNAME]: LCM:  [ Start  Test     ]  [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME]
VERBOSE: [HOSTNAME]:                            [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME] Testing Distribution Group configuration for {Distribution Group Name}
VERBOSE: [HOSTNAME]:                            [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME] Getting configuration of Distribution Group for Distribution Group Name
VERBOSE: [HOSTNAME]:                            [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME] Current Values: Alias=DistributionGroupName
ApplicationId=***
CertificateThumbprint=***
Ensure=Absent
Identity=Distribution Group Name
Name=Distribution Group Name
[email protected]
TenantId=***
Type=Security
Verbose=True
VERBOSE: [HOSTNAME]:                            [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME] Target Values: Alias=DistributionGroupName
ApplicationId=***
CertificateThumbprint=***
Identity=Distribution Group Name
Name=Distribution Group Name
[email protected]
TenantId=***
Type=Security
Verbose=True
VERBOSE: [HOSTNAME]:                            [[EXODistributionGroup]Distribution Group Name::[DomainController]HOSTNAME] Test-TargetResource returned True

Environment Information + PowerShell Version

OsName               : Microsoft Windows Server 2019 Standard
OsOperatingSystemSKU : StandardServerEdition
OsArchitecture       : 64-bit
WindowsVersion       : 1809
WindowsBuildLabEx    : 17763.1.amd64fre.rs5_release.180914-1434
OsLanguage           : en-US
OsMuiLanguages       : {en-US}

Key   : PSVersion
Value : 5.1.17763.6189
Name  : PSVersion

Key   : PSEdition
Value : Desktop
Name  : PSEdition

Key   : PSCompatibleVersions
Value : {1.0, 2.0, 3.0, 4.0...}
Name  : PSCompatibleVersions

Key   : BuildVersion
Value : 10.0.17763.6189
Name  : BuildVersion

Key   : CLRVersion
Value : 4.0.30319.42000
Name  : CLRVersion

Key   : WSManStackVersion
Value : 3.0
Name  : WSManStackVersion

Key   : PSRemotingProtocolVersion
Value : 2.3
Name  : PSRemotingProtocolVersion

Key   : SerializationVersion
Value : 1.1.0.1
Name  : SerializationVersion

Borgquite avatar Sep 23 '24 17:09 Borgquite

Updating the $PSBoundParameters value in all resources would be quite the amount of work to put in. In other resources, we have the following check that exits early if the Ensure property is different:

if ($CurrentValues.Ensure -ne $Ensure)
{
    Write-Verbose -Message "Test-TargetResource returned $false"
    return $false
}

Either we add the above check to the resources that don't have it, or we add the Ensure check with its default value Present to the Test-M365DSCParameterState function.

@NikCharlebois What do you think is the better approach? Personally, I would go for the second one, far simpler, much easier to achieve and affects all resources at once.

FabienTschanz avatar Sep 25 '24 15:09 FabienTschanz

@NikCharlebois Any thoughts on this?

Borgquite avatar Oct 17 '24 14:10 Borgquite

@NikCharlebois Checking in, what do you think of this?

FabienTschanz avatar Nov 07 '24 13:11 FabienTschanz

@FabienTschanz I'd put in a PR for option 2 and see what happens :)

Borgquite avatar Nov 21 '24 12:11 Borgquite

@Borgquite Agree. Should I add the implementation and open the PR or do you want to do it?

FabienTschanz avatar Nov 21 '24 13:11 FabienTschanz

@FabienTschanz If you don't mind doing it, that would be really helpful :)

Borgquite avatar Nov 21 '24 13:11 Borgquite

Alright, I'll take a shot at it. Will report back once I have the PR ready.

FabienTschanz avatar Nov 21 '24 14:11 FabienTschanz

@Borgquite Could you please test the solution suggested by @FabienTschanz in PR #5449? If it works, I will merge the PR.

ykuijs avatar Nov 22 '24 10:11 ykuijs

@FabienTschanz @ykuijs Sorry it's taken me soo long and it's already merged but thanks - it worked :)

(For the record!)

Borgquite avatar Aug 15 '25 17:08 Borgquite