Conversation
|
@FabienTschanz Wow I bet this one is leaps and bounds better than the current parser but we really got to give some time to test it, in fact even if it works wonders I'd kinda consider it as "breaking change" as far as m365dsc goes, the same way that the powershell graph sdk team is making changes around WAM but also the module loading priority issues and we cannot update the graph version in m365dsc because of that. can this be put on hold at least for a couple of weeks? My solution depends 100% on DSCParser (and ReverseDSC as well) and whenever it deviates even if just so slightly everything breaks on my side. Not saying it should not go ahead, just asking for some time to test it, I'm waiting for a new M365DSC version so that I can cross a major version and can get rid of the several patches I have on my side and after that I'll check this one out. |
|
No problem with waiting from my side. I did test it with full exports of all workloads with the most complex and nested configurations you can achieve. This version also fixes quite some issues with indentation and deeply nested objects when using I have similar optimizations running and under development for M365DSC. All of the changes here are also in a private ADO repo which I'm developing to use for my company, and since I started developing this version around Christmas, it was getting better and better. Right now, I have no issues left to solve from my side. If you discover any issue once you start testing it, let me know how I can reproduce them and I will gladly try to fix them. Using the AST with its strongly typed code, I can debug the parsed configuration way easier than before and it also performs way faster. My 20k lines configuration is parsed in 4-6 seconds, depending on Windows PowerShell vs PowerShell 7. PowerShell 7 is a little bit slower because |
|
And with my automated export and report pipeline, I discovered so many issues with M365DSC (and the other modules) that I almost can't keep up with fixing them. I guess you saw the number of PRs I put up in the M365DSC repo, and I'm still not done. Quite some more things to come, including new development things. My priority is directed to making the modules faster and much more stable since we want to use them for our customers very soon. |
|
For sure, I actually also have an exporter script as part of my pipelines and after that another script which uses ConvertTo-DSCObject and whenever there's a misbehaving resource which breaks the pipeline my customers complain about it, just had one that reported a problem that actually is already fixed in M365DSC latest version but I still have my Frankenstein version so I had to apply yet another workaround and hate doing that so I just hope Nik releases a new version without this patch so I have time to look into it. Then I have yet another script with my own version of ConvertFrom-DSCObject (because I first need to convert Markdown policies to PowerShell objects) which gets all indentations right so they all look "right" comparing to what M365DSC exports :) |
|
I guess we're the masters of "patching" our stuff together so it just works 😆 |
|
I now rigorously tested this version and added an improved In my pipeline, I can convert all resources and compare them with the report cmdlets from Microsoft365DSC much faster than before. I know that the conversion works because it's able to detect changes in e.g. double nested CIM instances. In my example, that is from an Intune Remediation assignment where the time was updated. I also went ahead and did a conversion to and from DSC objects and outputted it back to a file. This implementation is better in every aspect and more precise due to being typed. If there is an issue with the parser, we know exactly where it fails and have the necessary debug information available to trace the issue down. If somebody wants to test it out, I can help you get there with building the module. |
|
@ricmestre Do you want to test this version? Nik is afaik looking internally with a couple of teams, feedback is very much appreciated to make it failproof. |
|
Quick question, I Add-Type'd the dlls and imported the module, but I get the below error message so I guess it depends on the changes you have done on the M365DSC PR to work properly? |
|
Nah doesn't seem so, I just found a way to break your parser :) M365DSC has code to replace the hardcoded value of OrganizationName in almost everywhere but I have code which also ensures that it is replaced in ResourceInstanceName but your parser expects a constant string here and so it blows up with my config. By the way this is not the only replacement I have on ResourceInstanceName, I also replace TenantGuid and TenantId so it should have a generic fix to allow variables. |
|
Thank you, that's very helpful. Will work on a fix this evening and push it. Should be easy to do 👀 I know that I don't have 100% coverage with the code, but I'm pretty close. Appreciate your testing, I did a whole bunch with variations, but not an |
|
Instead of using this config I'll now try next the ones I usually use for testing, they're full of weird stuff on purpose to actually test DSCParser/ReverseDSC and my own parsers, I'll comment these entries with variables in the ResourceInstanceName and check if there's anything else it complains about. No rush. |
|
With the diff below by casting the object to dynamic instead of hardcoding it to StringConstantExpressionAst the configs now load without issues on ConvertTo-DscObject. Does it fix it? Yes! Is this the proper fix? Not sure :) diff --git a/src/DSCParser.CSharp/DscParser.cs b/src/DSCParser.CSharp/DscParser.cs
index 434bd07..8f20a92 100644
--- a/src/DSCParser.CSharp/DscParser.cs
+++ b/src/DSCParser.CSharp/DscParser.cs
@@ -273,7 +273,7 @@ namespace DSCParser.CSharp
// 1 - Resource Instance Name
// 2 - Key/Pair Value list of parameters.
string resourceType = resource.CommandElements[0].ToString();
- string resourceInstanceName = ((StringConstantExpressionAst)resource.CommandElements[1]).Value;
+ string resourceInstanceName = ((dynamic)resource.CommandElements[1]).Value;
currentResourceInfo.ResourceName = resourceType;
currentResourceInfo.ResourceInstanceName = resourceInstanceName;
|
|
Take a look at the Update blueprint I gave you, this one fails elsewhere with ConvertTo-DSCObject : Error parsing DSC configuration: Exception calling "ConvertToDscObject" with "4" argument(s): "Unable to cast object of type The resource which causes the issue is this one, at least EXOOwaMailboxPolicy "EXOOwaMailboxPolicy-EXOOwaMailboxPolicy_1"
{
ActionForUnknownFileAndMIMETypes = "ForceSave";
ActiveSyncIntegrationEnabled = $True;
AdditionalStorageProvidersAvailable = $True;
AllAddressListsEnabled = $True;
AllowCopyContactsToDeviceAddressBook = $True;
AllowedFileTypes = @(".rpmsg",".xlsx",".xlsm",".xlsb",".tiff",".pptx",".pptm",".ppsx",".ppsm",".docx",".docm",".zip",".xls",".wmv",".wma",".wav",".vsd",".txt",".tif",".rtf",".pub",".ppt",".png",".pdf",".one",".mp3",".jpg",".gif",".doc",".bmp",".avi");
AllowedMimeTypes = @("image/jpeg","image/png","image/gif","image/bmp");
ApplicationId = $EXOApplicationId;
BlockedFileTypes = @(".settingcontent-ms",".printerexport",".appcontent-ms",".appref-ms",".vsmacros",".website",".msh2xml",".msh1xml",".diagcab",".webpnp",".ps2xml",".ps1xml",".mshxml",".gadget",".theme",".psdm1",".mhtml",".cdxml",".xbap",".vhdx",".pyzw",".pssc",".psd1",".psc2",".psc1",".msh2",".msh1",".jnlp",".aspx",".appx",".xnk",".xml",".xll",".wsh",".wsf",".wsc",".wsb",".vsw",".vst",".vss",".vhd",".vbs",".vbp",".vbe",".url",".udl",".tmp",".shs",".shb",".sct",".scr",".scf",".reg",".pyz",".pyw",".pyo",".pyc",".pst",".ps2",".ps1",".prg",".prf",".plg",".pif",".pcd",".ops",".msu",".mst",".msp",".msi",".msh",".msc",".mht",".mdz",".mdw",".mdt",".mde",".mdb",".mda",".mcf",".maw",".mav",".mau",".mat",".mas",".mar",".maq",".mam",".mag",".maf",".mad",".lnk",".ksh",".jse",".jar",".its",".isp",".ins",".inf",".htc",".hta",".hpj",".hlp",".grp",".fxp",".exe",".der",".csh",".crt",".cpl",".com",".cnt",".cmd",".chm",".cer",".bat",".bas",".asx",".asp",".app",".apk",".adp",".ade",".ws",".vb",".py",".pl",".js");
BlockedMimeTypes = @("application/x-javascript","application/javascript","application/msaccess","x-internet-signup","text/javascript","application/xml","application/prg","application/hta","text/scriplet","text/xml");
CertificateThumbprint = $EXOCertThumbprint;
ClassicAttachmentsEnabled = $True;
ConditionalAccessPolicy = "Off";
DefaultTheme = "";
DirectFileAccessOnPrivateComputersEnabled = $True;
DirectFileAccessOnPublicComputersEnabled = $False;
DisplayPhotosEnabled = $True;
Ensure = "Present";
ExplicitLogonEnabled = $True;
ExternalImageProxyEnabled = $True;
ForceSaveAttachmentFilteringEnabled = $False;
ForceSaveFileTypes = @(".vsmacros",".ps2xml",".ps1xml",".mshxml",".gadget",".psc2",".psc1",".aspx",".wsh",".wsf",".wsc",".vsw",".vst",".vss",".vbs",".vbe",".url",".tmp",".swf",".spl",".shs",".shb",".sct",".scr",".scf",".reg",".pst",".ps2",".ps1",".prg",".prf",".plg",".pif",".pcd",".ops",".mst",".msp",".msi",".msh",".msc",".mdz",".mdw",".mdt",".mde",".mdb",".mda",".maw",".mav",".mau",".mat",".mas",".mar",".maq",".mam",".mag",".maf",".mad",".lnk",".ksh",".jse",".its",".isp",".ins",".inf",".hta",".hlp",".fxp",".exe",".dir",".dcr",".csh",".crt",".cpl",".com",".cmd",".chm",".cer",".bat",".bas",".asx",".asp",".app",".adp",".ade",".ws",".vb",".js");
ForceSaveMimeTypes = @("Application/x-shockwave-flash","Application/octet-stream","Application/futuresplash","Application/x-director ","$($OrganizationName)","$($DomainShortName)","Application/x-comma1,","$($TenantGuid)","Application/x-comma2, ,","`$Test1","`$Test2");
ForceWacViewingFirstOnPrivateComputers = $False;
ForceWacViewingFirstOnPublicComputers = $False;
FreCardsEnabled = $True;
GlobalAddressListEnabled = $True;
GroupCreationEnabled = $True;
InstantMessagingEnabled = $True;
InstantMessagingType = "Ocs";
InterestingCalendarsEnabled = $True;
IRMEnabled = $True;
IsDefault = $False;
JournalEnabled = $True;
LocalEventsEnabled = $False;
LogonAndErrorLanguage = 0;
Name = "EXOOwaMailboxPolicy_1";
NotesEnabled = $True;
NpsSurveysEnabled = $True;
OnSendAddinsEnabled = $False;
OrganizationEnabled = $True;
OutboundCharset = "AutoDetect";
OutlookBetaToggleEnabled = $True;
OWALightEnabled = $True;
PersonalAccountCalendarsEnabled = $True;
PhoneticSupportEnabled = $False;
PlacesEnabled = $True;
PremiumClientEnabled = $True;
PrintWithoutDownloadEnabled = $True;
PublicFoldersEnabled = $True;
RecoverDeletedItemsEnabled = $True;
ReferenceAttachmentsEnabled = $True;
RemindersAndNotificationsEnabled = $True;
ReportJunkEmailEnabled = $True;
RulesEnabled = $True;
SatisfactionEnabled = $True;
SaveAttachmentsToCloudEnabled = $True;
SearchFoldersEnabled = $True;
SetPhotoEnabled = $True;
SetPhotoURL = "";
SignaturesEnabled = $True;
SkipCreateUnifiedGroupCustomSharepointClassification = $True;
TeamSnapCalendarsEnabled = $True;
TenantId = $OrganizationName;
TextMessagingEnabled = $True;
ThemeSelectionEnabled = $True;
UMIntegrationEnabled = $True;
UseGB18030 = $False;
UseISO885915 = $False;
UserVoiceEnabled = $True;
WacEditingEnabled = $True;
WacExternalServicesEnabled = $True;
WacOMEXEnabled = $False;
WacViewingOnPrivateComputersEnabled = $True;
WacViewingOnPublicComputersEnabled = $True;
WeatherEnabled = $True;
WebPartsFrameOptionsType = "SameOrigin";
}Bet it's this line: |
|
As discussed separately, seems all good so far. I'm currently setting up unit tests so that we have at least some tests in the project before we release it instead of nothing. |
|
Unit tests ready and retested. Ready for review @NikCharlebois. |
|
@NikCharlebois I've also tested this thoroughly through my unit/integration tests and after Fabien's latest fixes they all pass as well |
2 participants
This PR completely rewrites the parser logic with C# for improved type handling, memory handling and performance. It also gets rid of the CIM instance instantiation, which was previously used for checking the underlying resource state. This is now no longer a part of the module.
Some figures: The new logic is massively quicker to convert from and to dsc objects. Even without CIM instantiations, the parsing is between 2 and 5 times faster, and the moment CIM instantiations (with elevated privileges in PowerShell) are required, it cannot be compared anymore. The bigger the configuration to parse, the greater the performance benefits.