Add resource intune wifi enterprise configuration policy windows10 #6563
Conversation
MrR0bert
requested review from
M365DSCApprover,
NikCharlebois,
m365dscapproval and
ykuijs
as code owners
|
I messed up something locally and found that the set function is not yet working in this commit. I'll update when it's all fixed |
…ows10' of https://github.com/MrR0bert/Microsoft365DSC into Add-ResourceIntuneWifiEnterpriseConfigurationPolicyWindows10
|
@MrR0bert Is the PR ready for a review? |
Yes please! |
FabienTschanz
left a comment
FabienTschanz
left a comment
There was a problem hiding this comment.
Please add a Microsoft365DSC.IntuneWifiEnterpriseConfigurationPolicyWindows10.Tests.ps1 file with the test cases as well.
Creating resources is a very challenging task, and the one you chose for that is particularly difficult. I refrained from creating it because on my first attempts, it gave me too much of a headache. Still, appreciate the work you're doing for the project.
| #region resource generator code | ||
| [Parameter()] | ||
| [System.String] | ||
| $Id,# |
There was a problem hiding this comment.
| $Id,# | |
| $Id, |
There is one # at the end of the line.
|
|
||
| [Parameter()] | ||
| [System.String[]] | ||
| $TrustedServerCertificateNames,# |
There was a problem hiding this comment.
| $TrustedServerCertificateNames,# | |
| $TrustedServerCertificateNames, |
There is one # at the end of the line.
| [Parameter()] | ||
| [ValidateSet('open', 'wpaPersonal', 'wpaEnterprise', 'wep', 'wpa2Personal', 'wpa2Enterprise')] | ||
| [System.String] | ||
| $WifiSecurityType,# |
There was a problem hiding this comment.
| $WifiSecurityType,# | |
| $WifiSecurityType, |
There is one # at the end of the line.
Please check in the other functions as well.
| { | ||
| if (-not $Script:exportedInstance -or $Script:exportedInstance.DisplayName -ne $DisplayName) | ||
| { | ||
| $ConnectionMode = New-M365DSCConnection -Workload 'MicrosoftGraph' ` |
There was a problem hiding this comment.
| $ConnectionMode = New-M365DSCConnection -Workload 'MicrosoftGraph' ` | |
| $null = New-M365DSCConnection -Workload 'MicrosoftGraph' ` |
We now use $null as the assignment to reduce the number of unassigned variables.
| $getValue = Get-MgBetaDeviceManagementDeviceConfiguration -All -Filter "DisplayName eq '$($DisplayName -replace "'", "''")'" -ErrorAction SilentlyContinue | Where-Object ` | ||
| -FilterScript { | ||
| $_.AdditionalProperties.'@odata.type' -eq '#microsoft.graph.windowsWifiEnterpriseEAPConfiguration' | ||
| } |
There was a problem hiding this comment.
It's more efficient to filter on the server side using the isof Odata query.
| [array]$getValue = Get-MgBetaDeviceManagementDeviceConfiguration -Filter $Filter -All ` | ||
| -ErrorAction Stop | Where-Object ` | ||
| -FilterScript { ` | ||
| $_.AdditionalProperties.'@odata.type' -eq '#microsoft.graph.windowsWifiEnterpriseEAPConfiguration' ` |
There was a problem hiding this comment.
You could use the $baseFilter logic from other resources here. See here for example:
| [ClassVersion("1.0.0.1")] | ||
| class MSFT_DeviceManagementConfigurationPolicyAssignments | ||
| { | ||
| [Write, Description("The type of the target assignment."), ValueMap{"#microsoft.graph.groupAssignmentTarget","#microsoft.graph.allLicensedUsersAssignmentTarget","#microsoft.graph.allDevicesAssignmentTarget","#microsoft.graph.exclusionGroupAssignmentTarget","#microsoft.graph.configurationManagerCollectionAssignmentTarget"}, Values{"#microsoft.graph.groupAssignmentTarget","#microsoft.graph.allLicensedUsersAssignmentTarget","#microsoft.graph.allDevicesAssignmentTarget","#microsoft.graph.exclusionGroupAssignmentTarget","#microsoft.graph.configurationManagerCollectionAssignmentTarget"}] String dataType; | ||
| [Write, Description("The type of filter of the target assignment i.e. Exclude or Include. Possible values are:none, include, exclude."), ValueMap{"none","include","exclude"}, Values{"none","include","exclude"}] String deviceAndAppManagementAssignmentFilterType; | ||
| [Write, Description("The Id of the filter for the target assignment.")] String deviceAndAppManagementAssignmentFilterId; | ||
| [Write, Description("The display name of the filter for the target assignment.")] String deviceAndAppManagementAssignmentFilterDisplayName; | ||
| [Write, Description("The group Id that is the target of the assignment.")] String groupId; | ||
| [Write, Description("The group Display Name that is the target of the assignment.")] String groupDisplayName; | ||
| [Write, Description("The collection Id that is the target of the assignment.(ConfigMgr)")] String collectionId; | ||
| }; |
There was a problem hiding this comment.
The current version 1.0.0.2. Please make sure to align this class with the one in other resources, otherwise it won't work.
| "Global", | ||
| "USGov" | ||
| ] | ||
| } |
There was a problem hiding this comment.
Check the other resources and add the last keys as well:
"mode": "Configuration",
"commands": [...]| MSFT_DeviceManagementConfigurationPolicyAssignments{ | ||
| deviceAndAppManagementAssignmentFilterType = "none" | ||
| dataType = "#microsoft.graph.groupAssignmentTarget" | ||
| } |
There was a problem hiding this comment.
Please make sure that the assignments are a valid object. Even if the groupId or groupDisplayName are not enforced, it makes sense to add them, just like in a real scenario.
| Instead, use the `IntuneWifiConfigurationPolicyAndroidEnterpriseWorkProfile` resource. | ||
| * IntuneWifiEnterpriseConfigurationPolicyWindows10 | ||
| * Added new resource for enterprise wifi profiles | ||
| * Fixes #5839 |
There was a problem hiding this comment.
Please make sure to update your branch with the latest changes, especially in the changelog. You can merge the upstream Dev branch in your feature branch or rebase your changes onto the ones from Dev. I recommend Merge for now unless you know what to do with Rebase.
|
Thanks for the review. I eventually stopped trying to get the test to work. That's more of a Pester and me thing than the resource not working though (I've been testing this live in my own tenant). I just can't get the test to behave, but will give it another go. |
|
@MrR0bert Can you please update the PR with the requested changes? Thanks. If you need some help with the tests, let me know. |
2 participants
Pull Request (PR) description
This PR adds a new resource: IntuneWifiEnterpriseConfigurationPolicyWindows10. This resource is used to configure enterprise level wifi profiles based on 801.x.
This Pull Request (PR) fixes the following issues
Task list
Entry should say what was changed and how that affects users (if applicable), and
reference the issue being resolved (if applicable).