PowerShell adapter PR

[mgreenegit]

No description provided.

[anmenaga]

Failing tests involve Binary resources.
I was under impression that we do not declare support for those in the fist version of upcoming release of DSC?
If that is the case, then these tests should be removed.

[mgreenegit]

The underlying reason is for v1, it needed to run winrm quickconfig. I don’t think we can do that within a build runner?

…

________________________________ From: Andrew ***@***.***> Sent: Wednesday, April 10, 2024 1:12:34 PM To: PowerShell/DSC ***@***.***> Cc: Michael Greene (POWERSHELL) ***@***.***>; Mention ***@***.***> Subject: Re: [PowerShell/DSC] PowerShell adapter PR (PR #363) Failing tests involve Binary resources. I was under impression that we do not declare support for those in the fist version of upcoming release of DSC? If that is the case, then these tests should be removed. — Reply to this email directly, view it on GitHub<#363 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABHQMO2TFNYH3ZHKED4FTD3Y4WMLFAVCNFSM6AAAAABEQ3DVPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBYGM2TQOJWGI>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[anmenaga]

The underlying reason is for v1, it needed to run winrm quickconfig. I don’t think we can do that within a build runner?

I don't see a technical problem with that; we can run anything within a build script;
However, the real question is - do we intend to declare support for old binary resources? That will require additional work (and we already have plenty with the need to remove script-based and class resources dependency on PSDesiredStateConfiguration module/PS DSC subsystem.

[mgreenegit]

I didn’t know you could perform admin level actions. Thanks @anmenaga. Ready for review.

[mgreenegit]

Good point

________________________________ From: Steve Lee ***@***.***> Sent: Thursday, April 11, 2024 9:10:44 AM To: PowerShell/DSC ***@***.***> Cc: Michael Greene (POWERSHELL) ***@***.***>; Mention ***@***.***> Subject: Re: [PowerShell/DSC] PowerShell adapter PR (PR #363) @SteveL-MSFT requested changes on this pull request.

________________________________ In build.ps1<#363 (comment)>:

@@ -14,6 +14,10 @@ param(

[switch]$SkipLinkCheck ) +if ($isWindows) { + winrm quickconfig -quiet We should do this in the test and not the build script. Probably in a BeforeAll {} block

________________________________ In powershell-adapter/Tests/TestClassResource/0.0.1/TestClassResource.psd1<#363 (comment)>:

+GUID = 'b267fa32-e77d-48e6-9248-676cc6f2327f'

+ +# 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 = '*' ⬇️ Suggested change

-CmdletsToExport = '*' +# this wildcard is required due to PowerShell/PSDesiredStateConfiguration#101 +CmdletsToExport = '*'

________________________________ In powershell-adapter/Tests/powershellgroup.resource.tests.ps1<#363 (comment)>:

($resources | ? {$_.Type -eq 'PSTestModule/TestPSRepository'}).Count | Should -Be 1

} + It 'Windows PowerShell adapter supports File resource' -Skip:(!$IsWindows){ + + $r = dsc resource list --adapter Microsoft.DSC/WindowsPowerShell + $LASTEXITCODE | Should -Be 0 + $resources = $r | ConvertFrom-Json + ($resources | ? {$_.Type -eq 'PSDesiredStateConfiguration/File'}).Count | Should -Be 1 + } + + It 'Get works on Binary "File" resource' -Skip:(!$IsWindows){ + + $testFile = 'c:\test.txt' ⬇️ Suggested change - $testFile = 'c:\test.txt' + $testFile = '$testdrive\test.txt'

________________________________ In powershell-adapter/Tests/powershellgroup.resource.tests.ps1<#363 (comment)>:

+ ($resources | ? {$_.Type -eq 'PSDesiredStateConfiguration/File'}).Count | Should -Be 1

+ } + + It 'Get works on Binary "File" resource' -Skip:(!$IsWindows){ + + $testFile = 'c:\test.txt' + 'test' | Set-Content -Path $testFile -Force + $r = '{"DestinationPath":"' + $testFile.replace('\','\\') + '"}' | dsc resource get -r 'PSDesiredStateConfiguration/File' + $LASTEXITCODE | Should -Be 0 + $res = $r | ConvertFrom-Json + $res.actualState.result.properties.DestinationPath | Should -Be "$testFile" + } + + It 'Get works on traditional "Script" resource' -Skip:(!$IsWindows){ + + $testFile = 'c:\test.txt' ⬇️ Suggested change - $testFile = 'c:\test.txt' + $testFile = '$testdrive\test.txt'

________________________________ In powershell-adapter/Tests/winps_resource.dsc.yaml<#363 (comment)>:

$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/10/config/document.json

resources: - name: Get info from classic DSC resources - type: Microsoft.Windows/WindowsPowerShell + type: Microsoft.DSC/WindowsPowerShell Since WindowsPowerShell is part of Windows, we want the typename of this resource to be Microsoft.Windows/WindowsPowerShell whereas PowerShell 7 being cross platform would be Microsoft.DSC/PowerShell

________________________________ In powershell-adapter/windowspowershell.dsc.resource.json<#363 (comment)>:

@@ -1,8 +1,12 @@

{ - "manifestVersion": "1.0", - "type": "Microsoft.Windows/WindowsPowerShell", + "$schema": "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2023/08/bundled/resource/manifest.json", + "type": "Microsoft.DSC/WindowsPowerShell", As noted above ⬇️ Suggested change - "type": "Microsoft.DSC/WindowsPowerShell", + "type": "Microsoft.Windows/WindowsPowerShell",

________________________________ In powershell-adapter/~/.config/powershell/Microsoft.PowerShell_profile.ps1<#363 (comment)>:

@@ -0,0 +1 @@

+$env:psmodulepath = $env:psmodulepath.trimend(';') Why is this file here? seems accidentally included? — Reply to this email directly, view it on GitHub<#363 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABHQMOZI3GXLN74P4IYZK2LY42YYJAVCNFSM6AAAAABEQ3DVPCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSOJUGYZDCNRQGI>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[mgreenegit]

Addressed all comments