UpdateServicesComputerTargetGroup: New resource - managing computer target groups

[Borgquite]

Pull Request (PR) description

A rebased pull for #60 It looks OK to me, waiting to see if it passes the Pester tests as these may need updating to support 5

This Pull Request (PR) fixes the following issues

• Fixes #44

Task list

• [x] Added an entry under the Unreleased section of the change log in the CHANGELOG.md. Entry should say what was changed, and how that affects users (if applicable).
• [x] Resource documentation added/updated in README.md.
• [x] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
• [x] Comment-based help added/updated.
• [x] Localization strings added/updated in all localization files as appropriate.
• [x] Examples appropriately added/updated.
• [x] Unit tests added/updated. See DSC Resource Testing Guidelines.
• [ ] Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• [x] New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[Borgquite]

Hey, @johlju. Trying to rebase per our discussion in #60. However the tests are failing for resources that I'm not touching, in a way that I haven't seen before - all saying ' Cannot find the Windows PowerShell data file 'MSFT_UpdateServicesApprovalRule.strings.psd1' in directory xxx' but the files they reference seem to appear in the module - feels like this is unrelated to my pull request. Can you advise?

I guess I may also need to update the pester tests for v5 similar to #74 but I think this issue needs resolving first.

[johlju]

Will have a look as soon as I have time.

[Borgquite]

@johlju Quick reminder when you have a few minutes to check out the issue with the tests here :)

[johlju]

@Borgquite if you wouldn't mind reviewing PR #79 so I can merge it, then this PR should be able to run the tests again after it has been rebased. 🙂

[johlju]

I merged PR #79 so this one can be rebased.

[Borgquite]

I merged PR #79 so this one can be rebased.

Thanks - I'll see if I can get the tests to resolve myself now, will keep you posted!

[johlju]

Ping me when the tests passes on this one, I will try to get the time to review. 🙂

[Borgquite]

Ping me when the tests passes on this one, I will try to get the time to review. 🙂

Thanks - doing some fairly major reworking of the rest of UpdateServicesDsc (it's missing tonnes of useful parameters, breaks a lot of DSC 'rules' from my PoV, e.g. as others have noted, certain parameters take effect even when they are 'unspecified') and it generally just needs some love. I'll keep you posted.

[coderabbitai[bot]]

Walkthrough

Introduces a new DSC resource UpdateServicesComputerTargetGroup to manage WSUS computer target groups with full lifecycle support. Includes resource module with get/set/test functions, schema definition, localization strings, configuration examples, comprehensive unit tests, and updates to test helpers to mock computer target group operations.

Changes

Cohort / File(s)	Summary
DSC Resource Implementation source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.psm1	New module implementing Get-TargetResource, Set-TargetResource, Test-TargetResource, and Get-ComputerTargetGroupPath functions for managing WSUS computer target groups with hierarchical path traversal and error handling.
Resource Schema & Localization source/DSCResources/DSC_UpdateServicesComputerTargetGroup/DSC_UpdateServicesComputerTargetGroup.schema.mof, source/DSCResources/DSC_UpdateServicesComputerTargetGroup/en-US/DSC_UpdateServicesComputerTargetGroup.strings.psd1	New MOF schema defining resource properties (Name, Path, Ensure, Id) and localized message strings (14 message keys for success/error/state scenarios).
Configuration Examples source/Examples/Resources/UpdateServicesComputerTargetGroup/1-UpdateServicesComputerTargetGroup_AddComputerTargetGroup_Config.ps1, source/Examples/Resources/UpdateServicesComputerTargetGroup/2-UpdateServicesComputerTargetGroup_DeleteComputerTargetGroup_Config.ps1	New DSC configuration examples demonstrating creation of nested computer target groups with dependency ordering and deletion via Ensure "Absent".
Unit Test Suite tests/Unit/DSC_UpdateServicesComputerTargetGroup.Tests.ps1	New comprehensive Pester test file covering Get-ComputerTargetGroupPath, Get-TargetResource, Test-TargetResource, and Set-TargetResource with error handling, path validation, and WSUS integration scenarios.
Test Helper Updates tests/Unit/TestHelpers/CommonTestHelper.psm1	Extended Get-WsusServerTemplate with ComputerTargetGroups structure, added CreateComputerTargetGroup method on WsusServer, implemented lifecycle methods (Delete, GetParentTargetGroup, GetChildTargetGroups) for mock objects, and added TargetingMode property to configuration.
Unit Test Expectations tests/Unit/DSC_UpdateServicesServer.Tests.ps1	Updated ClientTargetingMode assertion from null/empty to string value 'Client' in Get-TargetResource test cases.
Integration Test Refactoring tests/Integration/WildcardInProduct/updateservicesDsc_WilcardInProduct.Tests.ps1, tests/Integration/allProducts/updateservicesDsc_AllProducts.Tests.ps1, tests/Integration/defaultProducts/updateservicesDsc_defaultProducts.Tests.ps1, tests/Integration/onlyOneProduct/updateservicesDsc_basicInstall.Tests.ps1	Removed explicit -Not -Throw assertions from Get-WsusServer calls across four test files, replacing them with direct assignments (error handling now implicit).
Documentation CHANGELOG.md	Added entry for new UpdateServicesComputerTargetGroup resource addressing issue #44, updated issue reference formatting.

Sequence Diagram

sequenceDiagram
    participant DSC as DSC Resource
    participant Get as Get-TargetResource
    participant Set as Set-TargetResource
    participant Path as Get-ComputerTargetGroupPath
    participant WSUS as WSUS Server

    DSC->>Get: Invoke with Name, Path
    Get->>WSUS: Get-WsusServer()
    alt Server Not Configured
        WSUS-->>Get: Fail
        Get-->>DSC: Return Absent, null Id
    else Server Exists
        Get->>WSUS: Find target group by Name
        alt Group Exists
            WSUS-->>Path: Return ComputerTargetGroup
            Path->>WSUS: Traverse parents via GetParentTargetGroup
            Path-->>Get: Full hierarchical path
            Get->>Get: Validate path matches desired Path
            alt Path Mismatch
                Get-->>DSC: Throw exception
            else Path Match
                Get-->>DSC: Return Present, Name, Path, Id
            end
        else Group Missing
            Get-->>DSC: Return Absent, Name, Path, null Id
        end
    end

    DSC->>Set: Invoke with Ensure, Name, Path
    Set->>WSUS: Resolve parent group via Get-ComputerTargetGroupPath
    alt Ensure = "Present"
        Set->>WSUS: CreateComputerTargetGroup(Name, parentGroup)
        WSUS-->>Set: Success
    else Ensure = "Absent"
        Set->>WSUS: Delete target group
        WSUS-->>Set: Success
    end
    Set-->>DSC: Return

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

• Path traversal logic: Review Get-ComputerTargetGroupPath for correct parent-child hierarchy computation and "All Computers" special handling
• Parent group resolution: Verify Set-TargetResource correctly locates and validates parent groups before creation
• Mock object structure: Examine CommonTestHelper.psm1 modifications—ensure ComputerTargetGroups collection and lifecycle methods (Delete, GetParentTargetGroup, GetChildTargetGroups) correctly simulate WSUS behavior for tests
• Test coverage completeness: Validate all scenarios in DSC_UpdateServicesComputerTargetGroup.Tests.ps1, particularly error cases and nested group dependencies
• Integration test changes: Confirm removal of -Not -Throw assertions across four integration test files does not mask legitimate failures

Possibly related PRs

• dsccommunity/UpdateServicesDsc#86 — Modifies the same test helper file (CommonTestHelper.psm1), affecting Get-WsusServerTemplate and mock object structure for WSUS integration.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Test modifications to integration tests (removing -Not -Throw assertions) appear unrelated to issue #44's core objective of managing computer target groups.	Remove changes to integration test files (updateservicesDsc_WilcardInProduct.Tests.ps1, updateservicesDsc_AllProducts.Tests.ps1, updateservicesDsc_defaultProducts.Tests.ps1, updateservicesDsc_basicInstall.Tests.ps1) as they are not part of the new resource implementation and should be addressed separately.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title accurately summarizes the main change: introduction of a new DSC resource to manage computer target groups.
Description check	✅ Passed	The pull request description is related to the changeset, mentioning it fixes issue #44, references a rebased PR, and includes a comprehensive task checklist.
Linked Issues check	✅ Passed	The PR implements the objective from issue #44 to provide capability for creating and managing computer target groups in WSUS through the new DSC_UpdateServicesComputerTargetGroup resource.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dan-hughes]

I'm amazed this still worked with the latest DscCommunity.* modules. I have done some cleanup in a separate PR #85, which may fix some of the issues you are seeing with old versions of ModuleBuilder and Pester which had their version pinned.

[dan-hughes]

Back to this PR in the mean time, can the resource prefix be changed from MSFT to DSC. Saves doing it later. I assume the original PR had tests originally created for Pester 4?

[Borgquite]

@dan-hughes I think so - the pull still exists under #60. Do you think #85 will solve the problem I'm seeing in tests now, or is it another issue?

(I switched to the new header format for these tests as suggested by CodeRabbit, but now the other tests are failing :( Any advice gratefully appreciated; I'm not anywhere near as experienced in Pester as I would like to be...

(Will get onto the rename once the tests are working)

[dan-hughes]

@dan-hughes I think so - the pull still exists under #60. Do you think #85 will solve the problem I'm seeing in tests now, or is it another issue?

(I switched to the new header format for these tests as suggested by CodeRabbit, but now the other tests are failing :( Any advice gratefully appreciated; I'm not anywhere near as experienced in Pester as I would like to be...

(Will get onto the rename once the tests are working)

It doesn't look like my PR will fix the issues you are having.

Looking at this a little deeper, all of the other tests need to be properly converted to Pester 5 format, especially UpdateServicesServer which seems to be the one that's broken. I commented the whole test file out and everything else passes again.

[Borgquite]

Thanks @dan-hughes. That was what I feared, but didn't want to put the effort in if there was another cause.

I'll do what I can to get it working, hope you won't mind checking my work when I'm done!

[dan-hughes]

Seeing as the other tests need some work, I'd be onboard with commenting them (or just UpdateServicesServer) out and raising issues to rewrite them in the DscCommunity style. They don't look too bad.

It then lets this PR get merged and not let it get too large and moves the remainder forward at a later time.

[Borgquite]

OK @dan-hughes I'm going to step off the CodeRabbit treadmill here and leave this with you until #86 is done now. I think I've got the unit tests related to the new resource passing - the only failure now was created when I tried to rename the resource to DSC_ which I think confused the HQRM module (probably because I only renamed one resource to keep the PR simple, not all of them).

If you could get #86 complete and merged I'll then rebase and hopefully this can go in.

Are you OK to do the DSC_ rename for the remaining resources in #86?

[dan-hughes]

OK @dan-hughes I'm going to step off the CodeRabbit treadmill here and leave this with you until #86 is done now. I think I've got the unit tests related to the new resource passing - the only failure now was created when I tried to rename the resource to DSC_ which I think confused the HQRM module (probably because I only renamed one resource to keep the PR simple, not all of them).

If you could get #86 complete and merged I'll then rebase and hopefully this can go in.

Are you OK to do the DSC_ rename for the remaining resources in #86?

You missed the name in the schema file which is likely why HQRM caught it. Let's do it on a separate PR. If you undo those changes and just keep this PR to the new resource then anything else can be swept up after.

@coderabbit, raise an issue to change the MSFT prefix on the resources to DSC.

[coderabbitai[bot]]

[!TIP] For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue to track the work of changing MSFT prefix to DSC prefix

gh issue create \
  --repo dsccommunity/UpdateServicesDsc \
  --title "Change MSFT prefix on resources to DSC" \
  --body "## Description

Change the resource prefix from MSFT to DSC for all resources in the module to align with DscCommunity naming conventions.

## Context

This work was identified during the review of PR #78 where it was decided to handle the prefix change in a separate PR to keep the scope of #78 focused on adding the new UpdateServicesComputerTargetGroup resource.

## Related

- Discussed in: https://github.com/dsccommunity/UpdateServicesDsc/pull/78#discussion_r1875894951
- Requested by: @dan-hughes

## Acceptance Criteria

- [ ] Rename all resources from MSFT_* to DSC_*
- [ ] Update schema files (.schema.mof) to reflect the new prefix
- [ ] Update all references in documentation
- [ ] Update all references in tests
- [ ] Update examples to use the new prefix
- [ ] Ensure all tests pass after the rename" \
  --assignee dan-hughes

Length of output: 45

---

🏁 Script executed:

#!/bin/bash
# First, try to find information about the user from the PR participants
gh pr view 78 --repo dsccommunity/UpdateServicesDsc --json participants --jq '.participants[] | select(.login | test("dan|hughes"; "i")) | .login'

# Also check all comments to see if we can find the actual username
gh pr view 78 --repo dsccommunity/UpdateServicesDsc --json comments --jq '.comments[] | select(.author.login | test("dan|hughes"; "i")) | .author.login' | head -1

Length of output: 684

---

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue without assignee, but mention the requester in the body

gh issue create \
  --repo dsccommunity/UpdateServicesDsc \
  --title "Change MSFT prefix on resources to DSC" \
  --body "## Description

Change the resource prefix from MSFT to DSC for all resources in the module to align with DscCommunity naming conventions.

## Context

This work was identified during the review of PR #78 where it was decided to handle the prefix change in a separate PR to keep the scope of #78 focused on adding the new UpdateServicesComputerTargetGroup resource.

## Related

- Discussed in: https://github.com/dsccommunity/UpdateServicesDsc/pull/78#discussion_r1875894951
- Requested by: @dan-hughes

## Acceptance Criteria

- [ ] Rename all resources from MSFT_* to DSC_*
- [ ] Update schema files (.schema.mof) to reflect the new prefix
- [ ] Update all references in documentation
- [ ] Update all references in tests
- [ ] Update examples to use the new prefix
- [ ] Ensure all tests pass after the rename

cc @dan-hughes"

Length of output: 59

---

@dan-hughes, I've created issue #87 to track the work of changing the MSFT prefix to DSC across all resources: https://github.com/dsccommunity/UpdateServicesDsc/issues/87

The issue includes:

• Context from this PR discussion
• Acceptance criteria covering schema files, documentation, tests, and examples
• A mention of you so you'll be notified

Feel free to assign it to yourself or anyone else who will be handling this work!

[Borgquite]

If you undo those changes and just keep this PR to the new resource then anything else can be swept up after.

Ta, done!

[dan-hughes]

@Borgquite, my PR's are merged. There will be some conflicts to deal with here.

In this PR remove the changes to the Integration tests and Unit tests not relevant to this new resource PR.

I added a stub module for UpdateServices then renamed the Helper module to make it consistent with other modules.

You will need to update your unit test file to Import and Remove (see the other test files).

[Borgquite]

@coderabbitai please be quiet

[coderabbitai[bot]]

@Borgquite, understood! I'll pause my automatic reviews on this PR. You can resume them anytime by mentioning @coderabbitai resume when you're ready. Good luck with the updates! 👍

✅ Actions performed

Reviews paused.

[Borgquite]

@Borgquite, my PR's are merged. There will be some conflicts to deal with here.

In this PR remove the changes to the Integration tests and Unit tests not relevant to this new resource PR.

I added a stub module for UpdateServices then renamed the Helper module to make it consistent with other modules.

You will need to update your unit test file to Import and Remove (see the other test files).

Will do. Just remembering how much I hate Git sometimes right now. Give me a bit to work out the mess I've got myself into.

[Borgquite]

@coderabbitai resume

[coderabbitai[bot]]

✅ Actions performed

Reviews resumed.

[dan-hughes]

@Borgquite, I'll hold off on any more PR's until this is merged. Everything repo wise is working now though.

[Borgquite]

@coderabbitai pause

[coderabbitai[bot]]

✅ Actions performed

Reviews paused.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 90.12346% with 8 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 55%. Comparing base (84b27b5) to head (12c5c9d). :warning: Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...etGroup/DSC_UpdateServicesComputerTargetGroup.psm1	90%	8 Missing :warning:

Additional details and impacted files

@@        Coverage Diff         @@
##           main   #78   +/-   ##
==================================
+ Coverage    51%   55%   +4%     
==================================
  Files         4     5    +1     
  Lines       668   749   +81     
==================================
+ Hits        345   418   +73     
- Misses      323   331    +8     

Files with missing lines	Coverage Δ
...etGroup/DSC_UpdateServicesComputerTargetGroup.psm1	90% <90%> (ø)

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Borgquite]

@coderabbitai resume