AADGroup: Implements #2301 #2316
Conversation
|
@salbeck-sit can you please take a look at the failing Unit Tests? Thanks |
| @@ -14,12 +14,14 @@ class MSFT_AADGroup : OMI_BaseResource | |||
| [Write, Description("Specifies an ID for the group.")] String Id; | |||
| [Write, Description("User Service Principal values for the group's owners.")] String Owners[]; | |||
| [Write, Description("User Service Principal values for the group's members.")] String Members[]; | |||
| [Write, Description("DisplayName values for the groups that htis group is a member of.")] String MemberOf[]; | |||
| @@ -164,7 +172,7 @@ function Get-TargetResource | |||
| if ($Group.MembershipRuleProcessingState -ne 'On') | |||
| { | |||
| # Members | |||
| [Array]$members = Get-MgGroupMember -GroupId $Group.Id -All:$true | |||
| [Array]$members = Get-MgGroupMember -GroupId $Group.Id | |||
There was a problem hiding this comment.
Why are we removing the -All here? It should be included to make sure we catch all members
There was a problem hiding this comment.
-ALL is apparently no longer supported by Get-MgGroup* cmdlets in v1.12.0 of the Microsoft.Graph.Groups module.
Running the unit-tests with -All parameters resulted in a number of 'caught throws' where the text in the corresponding M365DSC eventlog stated that "A parameter cannot be found that matches parameter name 'All'". The only cmdlets in the Get-TargetResource function that used -All were the Get-MgGroup* cmdlets. Removing -All everywhere resulted in a passed unit-test.
The question of whether this should have been brought to the attention of the folks responsible for Graph-modules is another matter. I guess you could have gotten them to update the relevant module in a day or two as something similar has happened before, but I really just wanted to get the change under wraps.
There was a problem hiding this comment.
There was an intermediate release. Graph modules at up to 1.12.1 now and do allow the -all parameter
Pull Request (PR) description
AADGroup: Add properties MemberOf and AssignedToRole to resource
This Pull Request (PR) fixes the following issues
Implements #2301