Skip to content
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.

Sign up for GitHub

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

Allow module without version #600

Closed
wants to merge 16 commits into from
Closed

Allow module without version #600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e8e1180
Allow modules to be added to PSModulePath without version
Gijsreyn Nov 19, 2024
ee03947
Add tests
Gijsreyn Nov 27, 2024
ced2421
Fix incorrect discovery on include
Gijsreyn Nov 27, 2024
8edeb32
Added description from Andrew
Gijsreyn Dec 7, 2024
b7e94a8
Fix tests from rebase
Dec 7, 2024
1a488e3
Resolve remarks
Dec 9, 2024
704186d
Allow modules to be added to PSModulePath without version
Gijsreyn Nov 19, 2024
db4268c
Add tests
Gijsreyn Nov 27, 2024
64c8ea9
Fix incorrect discovery on include
Gijsreyn Nov 27, 2024
c090754
Added description from Andrew
Gijsreyn Dec 7, 2024
b513548
Fix tests from rebase
Dec 7, 2024
58b9d3d
Resolve remarks
Dec 9, 2024
dd460eb
no cmdlets
Dec 9, 2024
9fc330a
Merge branch 'allow-module-without-version' of https://github.com/Gij…
Dec 9, 2024
4b0e9d4
Merge branch 'main' into allow-module-without-version
Gijsreyn Dec 9, 2024
bd42eaf
Merge branch 'main' into allow-module-without-version
Gijsreyn Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"./y2j/Cargo.toml"
],
"rust-analyzer.showUnlinkedFileNotification": true,
"powershell.codeFormatting.preset": "OTBS",
"json.schemas": [
{
"fileMatch": ["**/*.dsc.resource.json"],
Expand Down
52 changes: 52 additions & 0 deletions powershell-adapter/Tests/TestClassNoVersion/TestClassNoVersion.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

@{

# Script module or binary module file associated with this manifest.
RootModule = 'TestClassNoVersion.psm1'

# Version number of this module.
ModuleVersion = '0.0.1'

# Supported PSEditions
# CompatiblePSEditions = @()

# ID used to uniquely identify this module
GUID = 'ec985d60-82f4-4d45-83e0-b6f935654350'

# Author of this module
Author = 'Microsoft'

# Company or vendor of this module
CompanyName = 'Microsoft Corporation'

# Copyright statement for this module
Copyright = '(c) Microsoft. All rights reserved.'

# Functions to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no functions to export.
FunctionsToExport = @()

# Cmdlets to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no cmdlets to export.
CmdletsToExport = @()

# Variables to export from this module
VariablesToExport = @()

# Aliases to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no aliases to export.
AliasesToExport = @()

# DSC resources to export from this module
DscResourcesToExport = 'TestClassNoVersion'

# Private data to pass to the module specified in RootModule/ModuleToProcess. This may also contain a PSData hashtable with additional module metadata used by PowerShell.
PrivateData = @{

PSData = @{
DscCapabilities = @('Get', 'Test', 'Set')

} # End of PSData hashtable

} # End of PrivateData hashtable
}

20 changes: 20 additions & 0 deletions powershell-adapter/Tests/TestClassNoVersion/TestClassNoVersion.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[DscResource()]
class TestClassNoVersion
{
[DscProperty(Key)]
[string] $Name

[void] Set()
{
}

[bool] Test()
{
return $true
}

[TestClassNoVersion] Get()
{
return $this
}
}
67 changes: 42 additions & 25 deletions powershell-adapter/Tests/powershellgroup.resource.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
Describe 'PowerShell adapter resource tests' {

BeforeAll {
$OldPSModulePath = $env:PSModulePath
$OldPSModulePath = $env:PSModulePath
$env:PSModulePath += [System.IO.Path]::PathSeparator + $PSScriptRoot

if ($IsLinux -or $IsMacOS) {
$cacheFilePath = Join-Path $env:HOME ".dsc" "PSAdapterCache.json"
}
else
{
} else {
$cacheFilePath = Join-Path $env:LocalAppData "dsc" "PSAdapterCache.json"
}
}
Expand All @@ -28,7 +26,7 @@ Describe 'PowerShell adapter resource tests' {
$r = dsc resource list '*' -a Microsoft.DSC/PowerShell
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
($resources | ? {$_.Type -eq 'TestClassResource/TestClassResource'}).Count | Should -Be 1
($resources | ? { $_.Type -in @('TestClassResource/TestClassResource', 'TestClassNoVersion/TestClassNoVersion') }).Count | Should -Be 2
}

It 'Get works on class-based resource' {
Expand All @@ -44,6 +42,14 @@ Describe 'PowerShell adapter resource tests' {
$propertiesNames | Should -Not -Contain 'HiddenNonDscProperty'
}

It 'Get works on class-based resource without sub-directory version' {

$r = "{'Name':'TestClassNoVersion'}" | dsc resource get -r 'TestClassNoVersion/TestClassNoVersion' -f -
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.actualState.result.properties.Name | Should -BeExactly 'TestClassNoVersion'
}

It 'Get uses enum names on class-based resource' {

$r = "{'Name':'TestClassResource1'}" | dsc resource get -r 'TestClassResource/TestClassResource' -f -
Expand All @@ -66,6 +72,15 @@ Describe 'PowerShell adapter resource tests' {
$propertiesNames | Should -Not -Contain 'HiddenNonDscProperty'
}

It 'Test works on class-based resource without sub-directory version' {

$r = "{'Name':'TestClassNoVersion'}" | dsc resource test -r 'TestClassNoVersion/TestClassNoVersion' -f -
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.actualState.result.properties.InDesiredState | Should -Be $True
$res.actualState.result.properties.InDesiredState.GetType().Name | Should -Be "Boolean"
}

It 'Set works on class-based resource' {

$r = "{'Name':'TestClassResource1','Prop1':'ValueForProp1'}" | dsc resource set -r 'TestClassResource/TestClassResource' -f -
Expand All @@ -74,6 +89,14 @@ Describe 'PowerShell adapter resource tests' {
$res.afterState.result | Should -Not -BeNull
}

It 'Set works on class-based resource without sub-directory version' {

$r = "{'Name':'TestClassNoVersion'}" | dsc resource set -r 'TestClassNoVersion/TestClassNoVersion' -f -
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.afterState.result | Should -Not -BeNull
}

It 'Export works on PS class-based resource' {

$r = dsc resource export -r TestClassResource/TestClassResource
Expand All @@ -84,7 +107,7 @@ Describe 'PowerShell adapter resource tests' {
$res.resources[0].properties.result[0].Prop1 | Should -Be "Property of object1"

# verify that only properties with DscProperty attribute are returned
$res.resources[0].properties.result | %{
$res.resources[0].properties.result | % {
$propertiesNames = $_ | Get-Member -MemberType NoteProperty | % Name
$propertiesNames | Should -Not -Contain 'NonDscProperty'
$propertiesNames | Should -Not -Contain 'HiddenNonDscProperty'
Expand All @@ -97,7 +120,7 @@ Describe 'PowerShell adapter resource tests' {
$LASTEXITCODE | Should -Be 0
$res = $r | ConvertFrom-Json
$res.actualState.result.count | Should -Be 5
$res.actualState.result| % {$_.Name | Should -Not -BeNullOrEmpty}
$res.actualState.result | % { $_.Name | Should -Not -BeNullOrEmpty }
}

It 'Verify that ClearCache works in PSAdapter' {
Expand Down Expand Up @@ -158,8 +181,7 @@ Describe 'PowerShell adapter resource tests' {
$LASTEXITCODE | Should -Be 0
"$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Detected non-existent cache entry'
"$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Constructing Get-DscResource cache'
}
finally {
} finally {
$env:PSModulePath = $oldPath
Copy-Item -Recurse -Force -Path "$PSScriptRoot/Backup/TestClassResource" -Destination "$PSScriptRoot"
Remove-Item -Recurse -Force -Path "$PSScriptRoot/Backup"
Expand All @@ -171,7 +193,7 @@ Describe 'PowerShell adapter resource tests' {
$r = dsc resource list '*' -a Microsoft.DSC/PowerShell
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
$t = $resources | ? {$_.Type -eq 'TestClassResource/TestClassResource'}
$t = $resources | ? { $_.Type -eq 'TestClassResource/TestClassResource' }
$t.properties | Should -Contain "BaseProperty"
}

Expand Down Expand Up @@ -214,11 +236,10 @@ Describe 'PowerShell adapter resource tests' {
$r = dsc resource list '*' -a Microsoft.DSC/PowerShell
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
$r = @($resources | ? {$_.Type -eq 'TestClassResource/TestClassResource'})
$r = @($resources | ? { $_.Type -eq 'TestClassResource/TestClassResource' })
$r.Count | Should -Be 1
$r[0].Version | Should -Be '2.0.1'
}
finally {
} finally {
$env:PSModulePath = $oldPath
}
}
Expand All @@ -229,12 +250,11 @@ Describe 'PowerShell adapter resource tests' {
$adapterPath = Join-Path $PSScriptRoot 'TestAdapter'
$env:PATH += [System.IO.Path]::PathSeparator + $adapterPath

$r = '{TestCaseId: 1}'| dsc resource get -r 'Test/TestCase' -f -
$r = '{TestCaseId: 1}' | dsc resource get -r 'Test/TestCase' -f -
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
$resources.actualState.result | Should -Be $True
}
finally {
} finally {
$env:PATH = $oldPath
}
}
Expand All @@ -245,13 +265,12 @@ Describe 'PowerShell adapter resource tests' {
$adapterPath = Join-Path $PSScriptRoot 'TestAdapter'
$env:PATH += [System.IO.Path]::PathSeparator + $adapterPath

$r = '{TestCaseId: 1}'| dsc resource set -r 'Test/TestCase' -f -
$r = '{TestCaseId: 1}' | dsc resource set -r 'Test/TestCase' -f -
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
$resources.beforeState.result | Should -Be $True
$resources.afterState.result | Should -Be $True
}
finally {
} finally {
$env:PATH = $oldPath
}
}
Expand All @@ -262,12 +281,11 @@ Describe 'PowerShell adapter resource tests' {
$adapterPath = Join-Path $PSScriptRoot 'TestAdapter'
$env:PATH += [System.IO.Path]::PathSeparator + $adapterPath

$r = '{TestCaseId: 1}'| dsc resource test -r 'Test/TestCase' -f -
$r = '{TestCaseId: 1}' | dsc resource test -r 'Test/TestCase' -f -
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
$resources.actualState.result | Should -Be $True
}
finally {
} finally {
$env:PATH = $oldPath
}
}
Expand All @@ -282,8 +300,7 @@ Describe 'PowerShell adapter resource tests' {
$LASTEXITCODE | Should -Be 0
$resources = $r | ConvertFrom-Json
$resources.resources[0].properties.result | Should -Be $True
}
finally {
} finally {
$env:PATH = $oldPath
}
}
Expand Down Expand Up @@ -321,7 +338,7 @@ Describe 'PowerShell adapter resource tests' {
"$TestDrive/tracing.txt" | Should -FileContentMatchExactly 'Constructing Get-DscResource cache'

# next executions following shortly after should Not rebuild the cache
1..3 | %{
1..3 | % {
dsc -l trace resource list -a Microsoft.DSC/PowerShell 2> $TestDrive/tracing.txt
"$TestDrive/tracing.txt" | Should -Not -FileContentMatchExactly 'Constructing Get-DscResource cache'
}
Expand Down
18 changes: 10 additions & 8 deletions powershell-adapter/psDscAdapter/psDscAdapter.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ function Get-DSCResourceModules
{
continue
}
# Partially re-used code from the original DSCv2 Get-DSCResource code
# Get-Module -ListAvailable is slow and has a bug causing incorrect reporting
$moduleFolders = Get-ChildItem $folder -Directory
SteveL-MSFT marked this conversation as resolved.
Show resolved Hide resolved
if (-not $moduleFolders) {
$moduleFolders = Get-ChildItem (Split-Path $folder -Parent) | Where-Object { $_.Name -eq (Split-Path $folder -Leaf) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

The -Depth 2 parameter in original code was supposed to handle both cases: modules with version subdirs and without.
Can you please give an example directory/module structure that was not handled correctly by old code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When working in the winget-dsc repository, I often found myself doing something like:

$env:PSModulePath += ";C:\source\winget-dsc\resources\Microsoft.DotNet.Dsc"

# Call function
Get-DscResourcesModules
# Old code did not list out the module

Hope that sheds some light on it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please post an output of
Get-ChildItem -Recurse -Path C:\source\winget-dsc\resources\Microsoft.DotNet.Dsc | %{$_.FullName}
Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There you go:

image

I have added both. The left is the main branch, and the right is the current one in this pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you; in the right picture - what is the full path to NpmDsc.psd1 that is returned by Get-DSCResourceModules ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I think I got what's happening.
The module discovery in PowerShell (and consequently DSC PS adapter) requires that all module files should be in the module directory; and $env:PSModulePath should point to the parent of that module directory.
In your case, the line should be:
$env:PSModulePath += ";C:\source\winget-dsc\resources"
rather than
$env:PSModulePath += ";C:\source\winget-dsc\resources\NpmDsc".
This should work with existing code. Please try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation, Andrew. I tried it in the past, and that works. But sometimes, I want only particular modules to be found, with the structure mentioned in this PR and in the test. What are your thoughts on those?

Copy link
Member

Choose a reason for hiding this comment

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

@Gijsreyn I think the problem is that this is contrary to how module discovery works with PS7 and it would be preferable to not deviate from that. Is this only for development purposes or do you have a real scenario where you want to limit the discovery?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can understand, and this was the use case. Feel free to abandon the pull request. I appreciated the time you guys looked into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @SteveL-MSFT. After testing for a while, I got a scenario that breaks the current testing cycle.

Imagine the following use case found while developing in winget-dsc. To test out both cases by either using Invoke-DscResource or the class directly, we declare at the top of the test the using module statement.

When modules are being discovered in the lower levels, so say C:\source\winget-dsc\resources\NpmDsc, it correctly loads the using module statement, but it doesn't do it for PSDesiredStateConfiguration. If I now add it on higher level in the same call, I get an error stating:

PS C:\Users\User> $env:PSModulePath += ";C:\source\winget-dsc\resources\Microsoft.Windows.Setting.Language"
PS C:\Users\User> Get-DscResource
Exception: Exception setting "Module": "Cannot convert the "System.Object[]" value of type "System.Object[]" to type
"System.Management.Automation.PSModuleInfo"."

I know you've closed down the ticket, but perhaps we can still consider it. Thanks.

}

foreach($moduleFolder in Get-ChildItem $folder -Directory)
{
$addModule = $false
foreach($psd1 in Get-ChildItem -Recurse -Filter "$($moduleFolder.Name).psd1" -Path $moduleFolder.fullname -Depth 2)
{
$containsDSCResource = select-string -LiteralPath $psd1 -pattern '^[^#]*\bDscResourcesToExport\b.*'
if($null -ne $containsDSCResource)
{
foreach ($moduleFolder in $moduleFolders) {
foreach ($psd1 in Get-ChildItem -Recurse -Filter "$($moduleFolder.Name).psd1" -Path $moduleFolder.fullname -Depth 2) {
$containsDSCResource = Select-String -LiteralPath $psd1 -Pattern '^[^#]*\bDscResourcesToExport\b.*'
if ($null -ne $containsDSCResource) {
$dscModulePsd1List.Add($psd1) | Out-Null
}
}
Expand Down
Loading