Skip to content

fix(dsc): make PSResourceList test, set and what-if agree on desired state - #2048

Open
Gijs Reijn (Gijsreyn) wants to merge 1 commit into
PowerShell:masterfrom
Gijsreyn:dsc/main/fix-psresourcelist-desired-state
Open

Gijs Reijn (Gijsreyn) wants to merge 1 commit into
PowerShell:masterfrom
Gijsreyn:dsc/main/fix-psresourcelist-desired-state

Conversation

@Gijsreyn

Copy link
Copy Markdown
Contributor

PR Summary

test, set and what-if for psresourcelist each computed desired state on their own and disagreed. They now share one pairing step (GetPSResourceListActions) that matches every desired resource to its current resource by index and decides None, Install or Uninstall.

PR Context

This PR fixes the following points:

  1. test only checked the first resource.

    # PSScriptAnalyzer is installed, Pester is not
    repositoryName: PSGallery
    resources:
      - name: PSScriptAnalyzer
      - name: Pester
    # before: dsc resource test
    _inDesiredState: true
    differingProperties: []
    
    # after
    _inDesiredState: false
    differingProperties: [resources]
    
  2. A resource without version never converged.

    repositoryName: PSGallery
    resources:
      - name: Pester
    # before: dsc resource set
    {"error":"Failed to install resource 'Pester' with version ''. Error: Cannot validate argument on parameter 'Version'. The argument is null or empty."}
    exit code 4
    
    # after
    Install-PSResource -Name Pester -Repository PSGallery      # -Version is left out, latest is installed
    exit code 0
    
  3. set compared every desired resource against every current one.

    # test_local_mod 1.0.0 and 2.0.0 are installed
    repositoryName: LocalRepo
    resources:
      - name: test_local_mod
        version: 1.0.0
      - name: test_local_mod
        version: 2.0.0
        _exist: false
    # before: 1.0.0 was also matched against the second entry and removed
    Uninstall-PSResource -Name test_local_mod -Version 1.0.0
    Uninstall-PSResource -Name test_local_mod -Version 2.0.0
    
    # after: each desired entry is paired with its own current resource
    Uninstall-PSResource -Name test_local_mod -Version 2.0.0
    
  4. All resources were installed or uninstalled with the last resource's scope.

    repositoryName: PSGallery
    resources:
      - name: PSScriptAnalyzer
        scope: CurrentUser
      - name: Pester
        scope: AllUsers
    # before
    Install-PSResource -Name PSScriptAnalyzer -Scope AllUsers
    Install-PSResource -Name Pester           -Scope AllUsers
    
    # after
    Install-PSResource -Name PSScriptAnalyzer -Scope CurrentUser
    Install-PSResource -Name Pester           -Scope AllUsers
    
  5. _exist is compared before version and repository, and those only apply when specified.

    # Pester is not installed
    repositoryName: PSGallery
    resources:
      - name: Pester
        _exist: false
    # before: empty version '' compared against '' failed, so the resource was never in desired state
    _inDesiredState: false
    
    # after
    _inDesiredState: true
    
  6. Uninstall removes the resource from the scope it is installed in.

    Before, Uninstall-PSResource received the desired scope, which defaults to CurrentUser when omitted. A resource installed in AllUsers was therefore never removed and set never converged.

    # Pester is installed in AllUsers
    repositoryName: PSGallery
    resources:
      - name: Pester
        _exist: false
    # before: nothing to remove in CurrentUser, Pester stays installed, exit code 0
    Uninstall-PSResource -Name Pester -Scope CurrentUser
    
    # after
    Uninstall-PSResource -Name Pester -Scope AllUsers
    

Note

scope is still not a filter. GetPSResourceList matches the current resource on name and version across both scopes, so an explicit scope: CurrentUser with _exist: false also removes an AllUsers copy. I'll create another issue for it.

  1. Errors trace at error level, so the real message is visible without --trace-level debug.

    # before: dsc resource set (default trace level)
    ERROR Installation failed
    
    # after
    {"error":"Failed to install resource 'Pester' with version '99.0.0'. Error: Package 'Pester' with version '99.0.0' could not be found in repository 'PSGallery'."}
    ERROR Installation failed
    

This PR also includes a couple of behavioral changes:

  • _exist: false with a version removes only that version (range); without version all versions are removed.
  • What-if reports a scope mismatch as Would install, matching what set does.
  • Error text is shown by default.

PR Checklist

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved issues affect scope-aware convergence, install deduplication, and test isolation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Unifies PSResourceList desired-state evaluation across test, set, and what-if.

Changes:

  • Centralizes resource action pairing.
  • Handles versions, scopes, _exist, and error tracing.
  • Adds DSC regression coverage.
File Description
test/​DscResource/​PSResourceGetDSCResource.Tests.ps1 Adds regression and scope-related tests.
src/​dsc/​psresourceget.ps1 Implements shared desired-state actions and scope-aware operations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/dsc/psresourceget.ps1
Comment on lines +361 to 364
else {
# No version constraint: any installed version means the resource exists.
# Only record the first match so that one input resource maps to one current resource.
Write-Trace -message "No version constraint for input: $($inputResource.Name). Treating installed version $($matchingResources[0].Version) as a match." -level debug
$preferred = $matchingResources | Select-Object -First 1
Comment thread src/dsc/psresourceget.ps1
# Install if resource should exist but doesn't, or exists but not in desired state
elseif ($resourceAction.action -eq 'Install') {
Write-Trace -message "Resource $name needs to be installed." -level debug
$key = $name.ToLowerInvariant() + '-' + $versionStr.ToLowerInvariant()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants