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

[WinGet DSC] Add WinGetConfigRoot cmdlet #4990

Closed
wants to merge 4 commits into from
Closed

[WinGet DSC] Add WinGetConfigRoot cmdlet #4990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29af060
add wingetconfigroot dsc
ryfu-msft Nov 19, 2024
42254f8
add WinGetconfigRoot
ryfu-msft Nov 19, 2024
d79a85a
fix tests
ryfu-msft Nov 19, 2024
2e4e68d
fix tests
ryfu-msft Nov 19, 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 src/PowerShell/Microsoft.WinGet.DSC/Microsoft.WinGet.DSC.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
'WinGetSource'
'WinGetPackageManager'
'WinGetPackage'
'WinGetConfigRoot'
)

# List of all modules packaged with this module
Expand Down
78 changes: 78 additions & 0 deletions src/PowerShell/Microsoft.WinGet.DSC/Microsoft.WinGet.DSC.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ enum WinGetTrustLevel
Trusted
}

enum WinGetScope
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this needs to be a subset of EnvironmentVariableTarget. I didn't know you could use enums like that

Small nit: if it is related to the other enum, I would add a comment for the future

{
User
Machine
}

#endregion enums

#region DscResources
Expand Down Expand Up @@ -446,6 +452,78 @@ class WinGetPackageManager
}
}

[DSCResource()]
class WinGetConfigRoot
{
[DscProperty(Key)]
[string]$WinGetConfigRoot = "WinGetConfigRoot"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this property is settable by a user, there should be a test for what happens if it isn't this specific string.
If it always needs to be that string, then the Path should be the key (since Get() doesn't cause the key to be required, and you would expect Test() and Set() would always have the path, and the string should be an internal string


[DscProperty()]
[string]$Path = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be validation that Exist: $true cannot be used with an empty path in Set, otherwise that results in a state conflict where the user has requested a Exist to be true, but the result of the setter will always result in Exist being false


[DscProperty()]
[bool]$Exist = $false

[DscProperty()]
[WinGetScope]$WinGetScope = [WinGetScope]::User

[WinGetConfigRoot] Get()
{
$currentState = [WinGetConfigRoot]::new()
$wingetConfigRootValue = [System.Environment]::GetEnvironmentVariable($this.WinGetConfigRoot, $this.WinGetScope)

if (-not [string]::IsNullOrEmpty($wingetConfigRootValue))
{
$currentState.Exist = $true
$currentState.Path = $wingetConfigRootValue
}

$currentState.WinGetScope = $this.WinGetScope
return $currentState
}

[bool] Test()
{
$currentState = $this.Get()
if ($this.Exist -ne $currentState.Exist)
{
return $false
}

if ($this.Path -ne $currentState.Path)
{
return $false
}

if ($this.WinGetScope -ne $currentState.WinGetScope)
{
return $false
}

if ($this.WinGetConfigRoot -ne $currentState.WinGetConfigRoot)
{
return $false
}

return $true
}

[void] Set()
{
if (-not $this.Test())
{
if ($this.Exist)
{
[System.Environment]::SetEnvironmentVariable($this.WinGetConfigRoot, $this.Path, $this.WinGetScope)
}
else
{
[System.Environment]::SetEnvironmentVariable($this.WinGetConfigRoot, "", $this.WinGetScope)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: this could set the variable to $null to delete it instead of setting it to the empty string. According to the docs, "" does not delete the variable since .NET 9 (no idea which one this uses...)

}
}
}
}

[DSCResource()]
class WinGetPackage
{
Expand Down
37 changes: 35 additions & 2 deletions src/PowerShell/tests/Microsoft.WinGet.DSC.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ BeforeAll {

Describe 'List available DSC resources'{
It 'Shows DSC Resources'{
$expectedDSCResources = "WinGetAdminSettings", "WinGetPackage", "WinGetPackageManager", "WinGetSource", "WinGetUserSettings"
$expectedDSCResources = "WinGetAdminSettings", "WinGetPackage", "WinGetPackageManager", "WinGetSource", "WinGetUserSettings", "WinGetConfigRoot"
$availableDSCResources = (Get-DscResource -Module Microsoft.WinGet.DSC).Name
$availableDSCResources.length | Should -Be 5
$availableDSCResources.length | Should -Be 6
$availableDSCResources | Where-Object {$expectedDSCResources -notcontains $_} | Should -BeNullOrEmpty -ErrorAction Stop
}
}
Expand Down Expand Up @@ -242,3 +242,36 @@ Describe 'WinGetPackageManager' {

# TODO: Add test to verify Set method for WinGetPackageManager
}

Describe 'WinGetConfigRoot' {
It 'Sets and removes $WinGetConfigRoot by scope' -ForEach 'User','Machine' {
$get = InvokeWinGetDSC -Name WinGetConfigRoot -Method Get -Property @{ WinGetScope = $_ }
$get.WinGetConfigRoot = "WinGetConfigRoot"
$get.Exist | Should -Be $false
$get.Path | Should -Be ""
$get.WinGetScope | Should -Be $_

$test = InvokeWinGetDSC -Name WinGetConfigRoot -Method Test -Property @{ Exist = $true; Path = 'C:\Foo\Bar'; WinGetScope = $_ }
$test.InDesiredState | Should -Be $false

InvokeWinGetDSC -Name WinGetConfigRoot -Method Set -Property @{ Exist = $true; Path = 'C:\Foo\Bar'; WinGetScope = $_ }

# Verify that $WinGetConfigRoot variable is set.
$result = InvokeWinGetDSC -Name WinGetConfigRoot -Method Get -Property @{WinGetScope = $_}
$result.Exist | Should -Be $true
$result.Path | Should -Be 'C:\Foo\Bar'
$result.WinGetScope | Should -Be $_
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the expected result of test if the path is not specified, but the user has requested Exist: true?


# Remove $WinGetConfigRoot variable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Before you remove it, it may be a good idea to check that if a different Path is specified while the variable exists, then the test returns false.

$test2 = InvokeWinGetDSC -Name WinGetConfigRoot -Method Test -Property @{ Exist = $false; WinGetScope = $_ }
$test2.InDesiredState | Should -Be $false

InvokeWinGetDSC -Name WinGetConfigRoot -Method Set -Property @{ Exist = $false; WinGetScope = $_ }

$result2 = InvokeWinGetDSC -Name WinGetConfigRoot -Method Get -Property @{WinGetScope = $_}
$result2.WinGetConfigRoot = "WinGetConfigRoot"
$result2.Exist | Should -Be $false
$result2.Path | Should -Be ""
$result2.WinGetScope | Should -Be $_
}
}
Loading