Compare-M365DSCConfigurations and functions that call it have a serious problem

[ricmestre]

Description of the issue

@NikCharlebois Looks like the DSCParser V2 is causing several issues.

The one I just found, and the reason for opening this, is that calling Assert-M365DSCBlueprint will generate a temporary blueprint from the tenant being monitored with a randomly assigned GUID as the configuration name (e.g. Configuration 757e980d-07a0-4393-9ed0-8efd06b772e0), then it calls New-M365DSCDeltaReport and then Compare-M365DSCConfigurations, from here Initialize-M365DSCReporting is called and invokes ConvertTo-DSCObject from DSCParser which can't cope with Configurations that start with a digit instead of a letter, which was working before with V1, and in this case just report that everything inside the blueprint is missing in the tenant (everything will be a drift).

The problem lies in https://github.com/microsoft/DSCParser/blob/7faba1a0e46c88857d438028797a4fc99c50369f/Modules/DSCParser/Modules/DSCParser.psm1#L345 since $Config will always return $null due to not being able to find the Configuration node in the AST, I don't know how to solve this.

Furthermore I also tripped over the same problem as reported in #4676 , somehow it started working again without making any changes in the parser, but surely something is going on here but there's also other problems such as running my pipelines in DevOps since you upgraded Graph to v2.19.0, this is something that happens over and over due to conflicts with the Az module, see microsoftgraph/msgraph-sdk-powershell#2745.

I was able to cheat and install 2.17.0 along with the latest version of M365DSC, but when calling Assert-M365DSCBlueprint there I don't even reach the problem with the Configuration name starting with a digit, in this case it's something inside my Teams workload that it doesn't like around the chunk below in the parser, in the logs it appears as if $associatedCIMProperty.CIMType is $null so I see the error message $typeStaticMethods = [] | gm - ... Missing type name after '['. and the same for TryParse. Funny enough this only seems to happen running through DevOps, running it locally it doesn't happen but again I have the problem that everything is reported as drift.

                else
                {
                    if ($associatedCIMProperty.CIMType -ne 'string' -and `
                        $associatedCIMProperty.CIMType -ne 'stringArray' -and `
                        $associatedCIMProperty.CIMType -ne 'string[]')
                    {
                        # Try to parse the value based on the retrieved type.
                        $scriptBlock = @"
                                        `$typeStaticMethods = [$($associatedCIMProperty.CIMType)] | gm -static
                                        if (`$typeStaticMethods.Name.Contains('TryParse'))
                                        {
                                            [$($associatedCIMProperty.CIMType)]::TryParse(`$subExpression, [ref]`$subExpression) | Out-Null
                                        }
"@
                        Invoke-Expression -Command $scriptBlock | Out-Null
                    }

Microsoft 365 DSC Version

1.24.515.2

Which workloads are affected

other

The DSC configuration

Configuration 757e980d-07a0-4393-9ed0-8efd06b772e0
{
    param (
    )

    $OrganizationName = $ConfigurationData.NonNodeData.OrganizationName

    Import-DscResource -ModuleName 'Microsoft365DSC' -ModuleVersion '1.24.515.2'

    Node localhost
    {
        TeamsAppPermissionPolicy "TeamsAppPermissionPolicy-Global"
        {
            ApplicationId          = $ConfigurationData.NonNodeData.ApplicationId;
            CertificateThumbprint  = $ConfigurationData.NonNodeData.CertificateThumbprint;
            DefaultCatalogApps     = @();
            DefaultCatalogAppsType = "BlockedAppList";
            Ensure                 = "Present";
            GlobalCatalogApps      = @("203a1e2c-26cc-47ca-83ae-be98f960b6b2","c18f658e-c34a-4ee8-ad19-a2cd51e59b71","0d794393-0f87-4c37-ac82-1d2822794372","19a29a41-cbca-4152-a2af-dd9728ea35f1","6eacb5f0-68b0-46f0-9507-9e906c6861fc","8216e453-3db5-48ee-a3d6-5122f505c8a3","88546d4f-9973-4716-98e4-cd181c04bc2d","aa183fd9-7104-46c4-af9f-9ee9b81d717e","d400ec73-d695-4065-b8c2-102d89c18077","30bb610c-6321-40fe-a047-056e7d0dac96");
            GlobalCatalogAppsType  = "AllowedAppList";
            Identity               = "Global";
            PrivateCatalogApps     = @();
            PrivateCatalogAppsType = "BlockedAppList";
            TenantId               = $OrganizationName;
        }
    }
}

Verbose logs showing the problem

N/A

Environment Information + PowerShell Version

N/A

[FabienTschanz]

@ricmestre I don't have a setup for GitHub pipelines, but the issue with the configuration being reported as a complete drift because the naming is a simple GUID should be adressable by prefixing the name with e.g. Export-<GUID>. Visual Studio Code gives me an error when the configuration name does not start with a letter: The configuration name '757e980d-07a0-4393-9ed0-8efd06b772e0' is not valid. Standard names may only contain letters (a-z, A-Z), numbers (0-9), period (.), hyphen (-) and underscore (_). The name may not be null or empty, and should start with a letter.

The second issue you mentioned with the missing types could be related to missing administrative privileges on the target machine. I debugged the ConvertTo-DscObject which calls ConvertFrom-CIMInstanceToHashtable, and that function requires administrative privileges to call Invoke-DscResource to register the CIM type if it does not exist. No idea why it didn't work but suddenly started working again...

And the third one with the Az and Graph modules should be resolved with Az 2.12.0 if I'm not mistaken. Nevertheless, I will have a look at the creation of the Blueprint and check if we can change that to something better, matching the criteria for the parser to have a configuration starting with a letter instead of a number.

[ricmestre]

I'm using Azure DevOps pipelines and they always run with administrative rights and it always worked before, in fact the only way to keep it working with latest M365DSC version is to have a manifest with M365DSC dependencies with a downgraded DSCParser to v1 with which I have no problems with.

The configurations are generated automatically by M365DSC via Assert-M365DSCBlueprint so basically you don't have control over what name to use for the configuration, but even if using GUIDs for the exports it was working with DSCParser v1 without issues, with v2 I get that problem.

I'm aware that the issue between Az and Graph should be resolved with Az 12.0.0, that will be released this week if I'm not mistaken, but their teams don't seem to collaborate when to release and so this problem keeps happening over and over because the modules then use different dlls. Usually I'm able to go around the problem by either instructing the pipeline to either run on a lower or higher Az version, in this case the only workaround I found was to, along with downgrading DSCParser in the manifest, to also downgrade Graph to v2.17.0.

[ricmestre]

The problem selecting a GUID for the configuration name starts here, you might get lucky and the filename of the export is a GUID that starts with a letter and so it's fine, but might start with a number and you're in trouble. Maybe we need to add a check if the filename starts with a number to append a prefix like "M365TenantConfig" which happens a few lines afterwards on L359.

Microsoft365DSC/Modules/Microsoft365DSC/Modules/M365DSCReverse.psm1

Line 347 in 473cd2a

# Create Configuration section

[FabienTschanz]

@ricmestre Don't think we have to go that far. Simply calling Export-M365DSCConfiguration -FileName XYZ inside Assert-M365DSCBlueprint with a name that does not start with a number should be enough, as it should be known that a configuration must start with a letter.

Microsoft365DSC/Modules/Microsoft365DSC/Modules/M365DSCUtil.psm1

Line 2797 in 473cd2a

Export-M365DSCConfiguration -Components $ResourcesInBluePrint `

Updating the references for $TempExportName and $TempBluePrintName should be enough. If you want, I can provide a pull request in a couple of minutes that addresses that, as I've already verified that it does what it's supposed to do.

[ricmestre]

Yes, that will also work, please go ahead with the PR since I'm currently busy adding other workarounds to my DevOps pipelines, which by the way is taking around 1h to install the modules and dependencies, last year I had to change Install-Module to Save-Module and it took about 5min, now it's also taking 1h for Save-Module...