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

(GH-61) Validate exit code when installing package #168

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

(GH-61) Validate exit code when installing package #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
142 changes: 126 additions & 16 deletions DSCResources/cChocoPackageInstall/cChocoPackageInstall.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ function Test-TargetResource

Return $result
}

function Test-ChocoInstalled
{
Write-Verbose -Message 'Test-ChocoInstalled'
Expand Down Expand Up @@ -254,7 +255,6 @@ Function Test-Command

function InstallPackage
{
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingInvokeExpression','')]
param(
[Parameter(Position=0,Mandatory)]
[string]$pName,
Expand Down Expand Up @@ -288,9 +288,9 @@ function InstallPackage
$chocoParams += " --no-progress"
}

$cmd = "choco install $pName $chocoParams"
Write-Verbose -Message "Install command: '$cmd'"
$packageInstallOuput = Invoke-Expression -Command $cmd
$cmd = "install $pName $chocoParams"
Write-Verbose -Message "Install command: 'choco $cmd'"
$packageInstallOuput = Invoke-Chocolatey $cmd
Write-Verbose -Message "Package output $packageInstallOuput"

# Clear Package Cache
Expand All @@ -302,7 +302,6 @@ function InstallPackage

function UninstallPackage
{
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingInvokeExpression','')]
param(
[Parameter(Position=0,Mandatory)]
[string]$pName,
Expand All @@ -324,9 +323,9 @@ function UninstallPackage
$chocoParams += " --no-progress"
}

$cmd = "choco uninstall $pName $chocoParams"
Write-Verbose -Message "Uninstalling $pName with: '$cmd'"
$packageUninstallOuput = Invoke-Expression -Command $cmd
$cmd = "uninstall $pName $chocoParams"
Write-Verbose -Message "Uninstalling $pName with: 'choco $cmd'"
$packageUninstallOuput = Invoke-Chocolatey -Command $cmd

Write-Verbose -Message "Package uninstall output $packageUninstallOuput "

Expand Down Expand Up @@ -397,7 +396,6 @@ function IsPackageInstalled
}

Function Test-LatestVersionInstalled {
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingInvokeExpression','')]
param(
[Parameter(Mandatory)]
[string]$pName,
Expand All @@ -410,10 +408,10 @@ Function Test-LatestVersionInstalled {
$chocoParams += " --source=`"$pSource`""
}

$cmd = "choco upgrade $pName $chocoParams"
Write-Verbose -Message "Testing if $pName can be upgraded: '$cmd'"
$cmd = "upgrade $pName $chocoParams"
Write-Verbose -Message "Testing if $pName can be upgraded: 'choco $cmd'"

$packageUpgradeOuput = Invoke-Expression -Command $cmd
$packageUpgradeOuput = Invoke-Chocolatey $cmd
$packageUpgradeOuput | ForEach-Object {Write-Verbose -Message $_}

if ($packageUpgradeOuput -match "$pName.*is the latest version available based on your source") {
Expand Down Expand Up @@ -445,7 +443,6 @@ function global:Write-Host

Function Upgrade-Package {
[Diagnostics.CodeAnalysis.SuppressMessage('PSUseApprovedVerbs','')]
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingInvokeExpression','')]
param(
[Parameter(Position=0,Mandatory)]
[string]$pName,
Expand Down Expand Up @@ -475,15 +472,15 @@ Function Upgrade-Package {
$chocoParams += " --no-progress"
}

$cmd = "choco upgrade $pName $chocoParams"
Write-Verbose -Message "Upgrade command: '$cmd'"
$cmd = "upgrade $pName $chocoParams"
Write-Verbose -Message "Upgrade command: 'choco $cmd'"

if (-not (IsPackageInstalled -pName $pName))
{
throw "$pName is not installed, you cannot upgrade"
}

$packageUpgradeOuput = Invoke-Expression -Command $cmd
$packageUpgradeOuput = Invoke-Chocolatey $cmd
$packageUpgradeOuput | ForEach-Object { Write-Verbose -Message $_ }

# Clear Package Cache
Expand Down Expand Up @@ -521,6 +518,119 @@ function Get-ChocoInstalledPackage {
Return $res
}

<#
.Synopsis
Run Chocolatey executable and throws error on failure
.DESCRIPTION
Run Chocolatey executable and throws error on failure
.EXAMPLE
Invoke-Chocolatey "list -lo"
.EXAMPLE
Invoke-Chocolatey -arguments "list -lo"
#>
function Invoke-Chocolatey {
[CmdletBinding()]
Param
(
# Chocolatey arguments."
[Parameter(Position = 0)]
[string]$arguments
)

$result = Invoke-ChocoProcess -arguments $arguments

#exit codes reference: https://docs.chocolatey.org/en-us/choco/commands/install#exit-codes
switch ($result.exitcode) {
0 {
#most widely used success exit code
$result.output.Split("`n")
break
}
350 {
# pending reboot detected, no action has occurred
$result.output.Split("`n")
throw "Error: Chocolatey detected a pending reboot from a previous installation. You can modify your DSC and use the PendingReboot DSC resource to reboot before running this command."
break
}
1605 {
#(MSI uninstall) - the product is not found, could have already been uninstalled
$result.output.Split("`n")
break
}
1614 {
#(MSI uninstall) - the product is uninstalled
$result.output.Split("`n")
break
}
1641 {
#(MSI) - restart initiated
$result.output.Split("`n")
Request-DscReboot
break
}
3010 {
#(MSI, InnoSetup can be passed to provide this) - restart required
$result.output.Split("`n")
Request-DscReboot
break
}
default {
#when error, throw output as error, contains errormessage
throw "Error: Chocolatey command failed with exit code $($result.exitcode).`n$($result.output)"
}
}
}

function Invoke-ChocoProcess {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any particular reason to use this method to create the Chocolatey process instead of & or Start-Process? Or even the Invoke-Expression that was originally being used?

Copy link
Author

Choose a reason for hiding this comment

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

From what I remember from 13 months ago, I had some issues getting a reliable last exit code with Invoke-Expression so I used to the code from the very old and possibly abandoned PR #103 which address the same issue.

[CmdletBinding()]
[OutputType([hashtable])]
Param
(
# Chocolatey arguments."
[Parameter(Position = 0)]
[string]$arguments
)

Write-Verbose -Message "command: 'choco $arguments'"

$pinfo = New-Object System.Diagnostics.ProcessStartInfo
$pinfo.FileName = 'choco'
$pinfo.RedirectStandardError = $true
$pinfo.RedirectStandardOutput = $true
$pinfo.UseShellExecute = $false
$pinfo.Arguments = "$arguments"

$p = New-Object System.Diagnostics.Process
$p.StartInfo = $pinfo
$p.Start() | Out-Null

$output = $p.StandardOutput.ReadToEnd()
$p.WaitForExit()
$exitcode = $p.ExitCode
$p.Dispose()

#Set $LASTEXITCODE variable.
powershell.exe -NoLogo -NoProfile -Noninteractive "exit $exitcode"
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to instantiate a powershell.exe instance instead of just setting $LastExitCode directly?

Copy link
Author

Choose a reason for hiding this comment

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

This PR is basically a "port" of the old PR #103 from 2017 to the latest code base so I cannot tell you exactly why they did it that way.

The only reason I can see to do it that way is to ensure that the real $LastExitCode is set. Setting $LastExitCode directly would create a variable named $LastExitCode which is scoped to the function Invoke-ChocoProcess and it would not set the global $LastExitCode variable.

You can reproduce the behavior with this code

function Test {
    $LASTEXITCODE = 3 # this will create a variable scoped to the function and will not impact its parent scope

   # you can uncomment this line to set the global $LastExitCode 
   # powershell.exe -NoLogo -NoProfile -Noninteractive "exit 3"
}

#fake a lastexitcode
powershell.exe -NoLogo -NoProfile -Noninteractive "exit 123"

write-Host "Value before Test function: $LASTEXITCODE" # will print 123 for the exit code (set by the previous line)
Test
write-Host "Value after Test function: $LASTEXITCODE" # value stays at 123 because the exit code variable in the Test function is scope to that function


return @{
exitCode = $exitcode
output = $output
}
}

function Request-DscReboot {
[Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidGlobalVars','')]
[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments','')]
[CmdletBinding()]
param()

# Setting the flag below allows LCM, if configured, to restart the node
# Reference: https://docs.microsoft.com/en-us/powershell/dsc/configurations/reboot-a-node?view=dsc-1.1

Write-Verbose "A reboot is required for this Chocolatey package"
$global:DSCMachineStatus = 1 # Signal to the LCM to reboot the node
}

function Get-ChocoVersion {
[CmdletBinding()]
param (
Expand Down
62 changes: 62 additions & 0 deletions Tests/cChocoPackageInstall_Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,68 @@ Describe -Name "Testing $ResourceName loaded from $ResourceFile" -Fixture {
}
}

Context -Name "Package cannot be found" -Fixture {
Copy link
Member

Choose a reason for hiding this comment

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

All of these tests added will likely require adjustments when #163 is merged due to the upgrade to Pester 5. (Not suggesting anything needs to be done around this at this particular moment. Just to keep in mind.)

$Scenario1 = @{
Name = 'NonExistentPackage'
Ensure = 'Present'
}

It -name "Set-TargeResource -ensure 'present' should throw" -test {
{ Set-TargetResource @Scenario1 } | Should throw "Error: Chocolatey command failed with exit code 1"
}
}

Context -Name "Choco exit code validation" -Fixture {
BeforeEach {
$global:DSCMachineStatus = $null
}

$Scenario = @{
Name = 'NonExistentPackage'
Ensure = 'Present'
}

It -name "Install package successfully" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = 0 ; 'output' = "command`noutput" } }
Set-TargetResource @Scenario | Should Be $true
$global:DSCMachineStatus | Should Be $null
}

It -name "Package installation fails with exit code -1" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = -1 ; 'output' = "command`noutput" } }
{ Set-TargetResource @Scenario } | Should -Throw "Error: Chocolatey command failed with exit code -1.`ncommand`noutput"
}

It -name "Package installation fails with exit code 350 (pending reboot)" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = 350 ; 'output' = "command`noutput" } }
{ Set-TargetResource @Scenario } | Should -Throw "Error: Chocolatey detected a pending reboot from a previous installation. You can modify your DSC and use the PendingReboot DSC resource to reboot before running this command."
}

It -name "Package installation fails with exit code 1605 (MSI uninstall - product not found)" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = 1605 ; 'output' = "command`noutput" } }
Set-TargetResource @Scenario | Should Be $true
$global:DSCMachineStatus | Should Be $null
}

It -name "Package installation fails with exit code 1614 (MSI uninstall - product is uninstalled)" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = 1614 ; 'output' = "command`noutput" } }
Set-TargetResource @Scenario | Should Be $true
$global:DSCMachineStatus | Should Be $null
}

It -name "Package installation fails with exit code 1641 (MSI uninstall - restart initiated)" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = 1641 ; 'output' = "command`noutput" } }
Set-TargetResource @Scenario | Should Be $true
$global:DSCMachineStatus | Should Be 1
}

It -name "Package installation fails with exit code 3010 (MSI InnoSetup - restart initiated)" -test {
Mock -CommandName 'Invoke-ChocoProcess' -ModuleName 'cChocoPackageInstall' -MockWith { return [hashtable]@{ 'exitCode' = 3010 ; 'output' = "command`noutput" } }
Set-TargetResource @Scenario | Should Be $true
$global:DSCMachineStatus | Should Be 1
}
}

Context -Name "Package is installed with prerelease version 1.0.0-1" -Fixture {
Mock -CommandName 'Get-ChocoInstalledPackage' -ModuleName 'cChocoPackageInstall' -MockWith {
return [pscustomobject]@{
Expand Down