Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

WIP: DSC v3 resource for PSResourceGet #1852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
adityapatwardhan wants to merge 16 commits into master
base: master
Choose a base branch
Loading
from DscResource
Draft

Conversation

@adityapatwardhan
Copy link
Member

@adityapatwardhan adityapatwardhan commented Jul 28, 2025

PR Summary

The initial implementation of DSC v3 resource for PSResourceGet. It include two resources, Repository and PSResources.

PR Context

PR Checklist

Gijsreyn reacted with rocket emoji
@adityapatwardhan adityapatwardhan changed the title (削除) DSC v3 resource for PSResourceGet (削除ここまで) (追記) WIP: DSC v3 resource for PSResourceGet (追記ここまで) Jul 28, 2025
@adityapatwardhan adityapatwardhan marked this pull request as draft July 28, 2025 20:56
Copy link

@michaeltlombardi michaeltlombardi left a comment

Choose a reason for hiding this comment

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

Quick review pass on the manifests, looking at resource implementation next.

Comment on lines 83 to 106
Add-Type -AssemblyName "$PSScriptRoot/dependencies/NuGet.Versioning.dll"

foreach ($resource in $allPSResources) {
foreach ($inputResource in $inputObj.resources) {
if ($resource.Name -eq $inputResource.Name) {
if ($inputResource.Version) {
# Use the NuGet.Versioning package if available, otherwise do a simple comparison
try {
$versionRange = [NuGet.Versioning.VersionRange]::Parse($inputResource.Version)
$resourceVersion = [NuGet.Versioning.NuGetVersion]::Parse($resource.Version.ToString())
if ($versionRange.Satisfies($resourceVersion)) {
$resourcesExist += $resource
}
}
catch {
# Fallback: simple string comparison (not full NuGet range support)
if ($resource.Version.ToString() -eq $inputResource.Version) {
$resourcesExist += $resource
}
}
}
}
}
}
Copy link
Member

@SteveL-MSFT SteveL-MSFT Jul 28, 2025

Choose a reason for hiding this comment

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

This code seems very similar across different operations, can it be a helper function?

Copy link
Member Author

@adityapatwardhan adityapatwardhan Jul 29, 2025

Choose a reason for hiding this comment

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

There are subtle differences, but I will see if I can refactor

Comment on lines 221 to 235
if ($inputObj.Name) {
$splatt['Name'] = $inputObj.Name
}

if ($inputObj.Uri) {
$splatt['Uri'] = $inputObj.Uri
}

if ($inputObj.Trusted) {
$splatt['Trusted'] = $inputObj.Trusted
}

if ($null -ne $inputObj.Priority ) {
$splatt['Priority'] = $inputObj.Priority
}
Copy link
Member

@SteveL-MSFT SteveL-MSFT Jul 28, 2025

Choose a reason for hiding this comment

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

Can this just be a loop like:

$properties = @('Name', 'Uri', 'Trusted', 'Priority')
for ($property in $properties) {
 if ($null -ne $inputObject.$property) {
 $splatt[$property] = $inputObject.$property
 }
}

Copy link

@michaeltlombardi michaeltlombardi left a comment

Choose a reason for hiding this comment

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

A few more comments on the schema and implementation.

[string]$ResourceType
)

$inputObj = $stdinput | ConvertFrom-Json -ErrorAction Stop
Copy link

@michaeltlombardi michaeltlombardi Jul 28, 2025

Choose a reason for hiding this comment

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

I would probably recommend defining a class for both the various input types and a function to handle both converting the input into the appropriate type and validating the input.

Right now reading $inputObj.<property> and trying to keep track is very difficult, plus we lose the IntelliSense/etc we would get from stronger typing.

adityapatwardhan reacted with thumbs up emoji
# catch any un-caught exception and write it to the error stream
trap {
Write-Trace -Level Error -message $_.Exception.Message
exit 1
Copy link

@michaeltlombardi michaeltlombardi Jul 28, 2025

Choose a reason for hiding this comment

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

Future enhancement - specific exit codes for specific issues/exceptions.

Copy link
Contributor

Is the plan to add a discover extension in DSC to be able to find these resource manifests? If so, is there an issue/pr for that?

"type": "Microsoft.PowerShell.PSResourceGet/PSResources",
"version": "0.0.1",
"get": {
"executable": "pwsh",
Copy link
Contributor

@ThomasNieto ThomasNieto Jul 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How is Windows PowerShell handled? PSResourceGet supports both. For PSScript DSC v3 resource there are actually two different resources to account for different versions of PowerShell.

"type": "Microsoft.PowerShell.PSResourceGet/Repository",
"version": "0.0.1",
"get": {
"executable": "pwsh",
Copy link
Contributor

@ThomasNieto ThomasNieto Jul 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How is Windows PowerShell handled? PSResourceGet supports both. For PSScript DSC v3 resource there are actually two different resources to account for different versions of PowerShell.

Copy link

Is the plan to add a discover extension in DSC to be able to find these resource manifests? If so, is there an issue/pr for that?

@ThomasNieto yup! PowerShell/DSC#913 proposes a discovery extension for finding resource and extension manifests through the PSModulePath.

Copy link
Contributor

I logged PowerShell/DSC#1024 issue. PSResourceGet will also have the same issue since it uses NuGet version range syntax containing square brackets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT SteveL-MSFT SteveL-MSFT requested changes

@anamnavi anamnavi Awaiting requested review from anamnavi anamnavi is a code owner

@alerickson alerickson Awaiting requested review from alerickson alerickson is a code owner

@SydneyhSmith SydneyhSmith Awaiting requested review from SydneyhSmith SydneyhSmith is a code owner

@mgreenegit mgreenegit Awaiting requested review from mgreenegit

@theJasonHelmick theJasonHelmick Awaiting requested review from theJasonHelmick

+2 more reviewers

@michaeltlombardi michaeltlombardi michaeltlombardi left review comments

@ThomasNieto ThomasNieto ThomasNieto left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /