Test-TargetResource Streamlining #5787
Conversation
There was a problem hiding this comment.
Absolute crazy work @NikCharlebois. I added some comments and here's another list of resources that either do something with properties or exclude them from the comparison. This is for information purposes so you can easily disregard it, but I personally think it's important that the comparison is as equal as it gets.
- AADIdentityAPIConnector --> Password
- AADRoleAssignmentScheduleRequest --> ScheduleInfo with custom date range logic
- AADRoleDefinition --> TemplateId
- AADRoleEligibilityScheduleRequest --> ScheduleInfo, Action, IsValidationOnly, Justification
- EXODistributionGroup --> OrganizationalUnit
- EXOSafeLinksPolicy --> UseTranslatedNotificationText
- EXOTenantAllowBlockListItems --> Entries, ExpirationDate
- IntuneDeviceConfigurationAdministrativeTemplatePolicyWindows10 --> PolicyConfigurationIngestionType
- IntuneDeviceConfigurationSCEPCertificatePolicyWindows10 --> RootCertificateDisplayName
- IntuneDeviceConfigurationWiredNetworkPolicyWindows10 --> 5 different properties
- IntuneDeviceManagementAndroidDeviceOwnerEnrollmentProfile --> 6 different properties
- IntuneEndpointDetectionAndResponsePolicy --> ConfigurationBlob
- IntuneMobileAppsMacOSLobApp --> LargeIcon
- IntuneMobileAppsWindowsOfficeSuiteApp --> OfficePlatformArchitecture
- IntuneMobileThreatDefenseConnector --> LastHeartbeatDateTime
- PPPowerAppsEnvironment --> ProvisionDatabase, LanguageName, CurrencyName
- SCRoleGroupMember --> Description
- TeamsClientConfiguration --> RestrictedSenderList
- TeamsMeetingPolicy --> AllowAnonymousUsersToDialOut, AllowIPVideo, AllowUserToJoinExternalMeeting, ForceStreamingAttendeeMode (deprecated), AllowRecordingStorageOutsideRegion
- TeamsMessagingPolicy --> Tenant
- TeamsShiftsPolicy --> EnableShiftPresence
- TeamsTeam --> GroupID
- TeamsUpgradePolicy --> Users
- TeamsUser --> Role
| #Ensure the proper dependencies are installed in the current environment. | ||
| Confirm-M365DSCDependencies | ||
|
|
||
| Write-Verbose -Message "Testing configuration of the Azure AD Access Review Definition with Id {$Id} and DisplayName {$DisplayName}" |
There was a problem hiding this comment.
This probably should be something similar to Write-Verbose -Message "Testing configuration of the resource $ResourceName". Maybe add a Key and AlternateKey parameter for resources that have e.g. a DisplayName as well as an Id property?
| .Description | ||
| Centralized method to evaluate the result of the various Test-TargetResource functions | ||
| #> | ||
| function Test-M365DSCTargetResource |
There was a problem hiding this comment.
I guess that for whatever reason, there are resources for which specific properties are excluded from comparison. Would you like to just go "with the flow" and see if this is really necessary or build an array with properties to exclude from comparison?
Example with AADEntitlementManagementAccessPackageCatalogResource, it got the following three properties excluded before comparison.
- AddedBy
- AddedOn
- IsPendingOnboarding
| $CurrentValues = Get-TargetResource @PSBoundParameters | ||
| $ValuesToCheck = ([Hashtable]$PSBoundParameters).clone() | ||
| $ValuesToCheck.Remove('Action') | Out-Null | ||
| if ($null -ne $CurrentValues.ScheduleInfo -and $null -ne $ValuesToCheck.ScheduleInfo) |
There was a problem hiding this comment.
How do we deal with custom comparison logic like here?
| $CurrentValues = Get-TargetResource @PSBoundParameters | ||
| $ValuesToCheck = ([Hashtable]$PSBoundParameters).Clone() | ||
|
|
||
| if ($null -ne $CurrentValues.ScheduleInfo -and $null -ne $ValuesToCheck.ScheduleInfo) |
There was a problem hiding this comment.
Another custom logic comparison.
| # Need to remove Identity as Get-ArcConfig doesn't return Identity | ||
| $ValuesToCheck.Remove('Identity') | Out-Null | ||
|
|
||
| $PSBoundParameters.ArcTrustedSealers = $PSBoundParameters.ArcTrustedSealers -Join ',' |
There was a problem hiding this comment.
Some other custom logic here.
|
|
||
| if ($TestResult) | ||
| { | ||
| $TestResult = Compare-SPOTheme -existingThemePalette $currentValues.Palette -configThemePalette $Palette |
There was a problem hiding this comment.
Custom comparison of Palette
| $DesiredValues = $PSBoundParameters | ||
| if ($null -ne $DesiredValues.EmergencyNumbers -and $DesiredValues.EmergencyNumbers.Count -gt 0) | ||
| { | ||
| $numbers = Convert-CIMToTeamsEmergencyNumbers -Numbers $DesiredValues.EmergencyNumbers |
There was a problem hiding this comment.
Custom conversion in place.
| if ($CurrentValues.SdnApiToken -eq '**********') | ||
| { | ||
| $CurrentValues.Remove('SdnApiToken') | Out-Null | ||
| } |
There was a problem hiding this comment.
Removes the SdnApiToken if it's anonymized.
| if ($null -ne $NormalizationRules) | ||
| { | ||
| $desiredRules = @() | ||
| foreach ($rule in $NormalizationRules) | ||
| { | ||
| $desiredRule = @{ |
There was a problem hiding this comment.
Custom conversion in place.
| if ($PSBoundParameters.ContainsKey('UpdateTimeOfDay')) | ||
| { | ||
| Write-Verbose -Message "Converting UpdateTimeOfDay ($UpdateTimeOfDay) to the current culture format" | ||
| $dtUpdateTimeOfDay = [datetime]::Parse($PSBoundParameters.UpdateTimeOfDay) | ||
| $PSBoundParameters.UpdateTimeOfDay = $dtUpdateTimeOfDay.ToShortTimeString() | ||
| Write-Verbose -Message " Converted value $($PSBoundParameters.UpdateTimeOfDay))" | ||
| } |
There was a problem hiding this comment.
Custom conversion in place.
Pull Request (PR) description
This Pull Request (PR) fixes the following issues