win_package: add support for arguments as list (#32024)

* win_package: add support for arguments as list

* re-added failure tests as they were accidentally commented out

* changed exit_code in failure messages to rc

lib/ansible/modules/windows/win_package.ps1

@@ -5,15 +5,16 @@
 # Copyright (c) 2017 Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-# WANT_JSON
-# POWERSHELL_COMMON
+#Requires -Module Ansible.ModuleUtils.Legacy
+#Requires -Module Ansible.ModuleUtils.CommandUtil
+#Requires -Module Ansible.ModuleUtils.ArgvParser
 
 $ErrorActionPreference = 'Stop'
 
 $params = Parse-Args -arguments $args -supports_check_mode $true
 $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
 
-$arguments = Get-AnsibleParam -obj $params -name "arguments" -type "str"
+$arguments = Get-AnsibleParam -obj $params -name "arguments"
 $expected_return_code = Get-AnsibleParam -obj $params -name "expected_return_code" -type "list" -default @(0, 3010)
 $name = Get-AnsibleParam -obj $params -name "name" -type "str"
 $path = Get-AnsibleParam -obj $params -name "path" -type "str"
@@ -32,6 +33,13 @@ $result = @{
     restart_required = $false # deprecate in 2.6
 }
 
+if ($arguments -ne $null) {
+    # convert a list to a string and escape the values
+    if ($arguments -is [array]) {
+        $arguments = Argv-ToString -arguments $arguments
+    }
+}
+
 if (-not $validate_certs) {
     [System.Net.ServicePointManager]::ServerCertificateValidationCallback = { $true }
 }
@@ -81,42 +89,6 @@ if ($creates_version -ne $null -and $creates_path -eq $null) {
     Fail-Json -obj $result -Message "creates_path must be set when creates_version is set"
 }
 
-# run module
-# used when installing a local process
-$process_util = @"
-using System;
-using System.IO;
-using System.Threading;
-
-namespace Ansible {
-    public static class ProcessUtil {
-
-        public static void GetProcessOutput(StreamReader stdoutStream, StreamReader stderrStream, out string stdout, out string stderr) {
-            var sowait = new EventWaitHandle(false, EventResetMode.ManualReset);
-            var sewait = new EventWaitHandle(false, EventResetMode.ManualReset);
-
-            string so = null, se = null;
-
-            ThreadPool.QueueUserWorkItem((s) => {
-                so = stdoutStream.ReadToEnd();
-                sowait.Set();
-            });
-
-            ThreadPool.QueueUserWorkItem((s) => {
-                se = stderrStream.ReadToEnd();
-                sewait.Set();
-            });
-
-            foreach (var wh in new WaitHandle[] { sowait, sewait })
-                wh.WaitOne();
-
-            stdout = so;
-            stderr = se;
-        }
-    }
-}
-"@
-
 $msi_tools = @"
 using System;
 using System.Runtime.InteropServices;
@@ -171,36 +143,6 @@ Function Download-File($url, $path) {
     }
 }
 
-Function Run-Process($executable, $arguments) {
-    Add-Type -TypeDefinition $process_util
-    $proc = New-Object -TypeName System.Diagnostics.Process
-    $psi = $proc.StartInfo
-    $psi.FileName = $executable
-    $psi.Arguments = $arguments
-    $psi.RedirectStandardOutput = $true
-    $psi.RedirectStandardError = $true
-    $psi.UseShellExecute = $false
-
-    try {
-        $proc.Start() | Out-Null
-    } catch [System.ComponentModel.Win32Exception] {
-        Fail-Json $result "failed to start executable $($executable): $($_.Exception.Message)"
-    }
-
-    $stdout = [string]$null
-    $stderr = [string]$null
-    [Ansible.ProcessUtil]::GetProcessOutput($proc.StandardOutput, $proc.StandardError, [ref] $stdout, [ref] $stderr) | Out-Null
-    $proc.WaitForExit() | Out-Null
-
-    $process_result = @{
-        stdout = $stdout
-        stderr = $stderr
-        rc = $proc.ExitCode
-    }
-
-    return $process_result
-}
-
 Function Test-RegistryProperty($path, $name) {
     # will validate if the registry key contains the property, returns true
     # if the property exists and false if the property does not
@@ -394,8 +336,7 @@ if ($state -eq "absent") {
             }
 
             if ($program_metadata.msi -eq $true) {
-                # we are installing an msi
-                $uninstall_exe = "$env:windir\system32\msiexec.exe"
+                # we are uninstalling an msi
                 $temp_path = [System.IO.Path]::GetTempPath()
                 $log_file = [System.IO.Path]::GetRandomFileName()
                 $log_path = Join-Path -Path $temp_path -ChildPath $log_file
@@ -404,28 +345,27 @@ if ($state -eq "absent") {
                 if ($program_metadata.product_id -ne $null) {
                     $id = $program_metadata.product_id
                 } else {
-                    $id = "`"$local_path`""`
+                    $id = $local_path
                 }
 
-                $uninstall_arguments = @("/x", $id, "/L*V", "`"$log_path`"", "/qn", "/norestart")
-                if ($arguments -ne $null) {
-                    $uninstall_arguments += $arguments
-                }
-                $uninstall_arguments = $uninstall_arguments -join " "
+                $uninstall_arguments = @("$env:windir\system32\msiexec.exe", "/x", $id, "/L*V", $log_path, "/qn", "/norestart")
             } else {
                 $log_path = $null
-
-                $uninstall_exe = $local_path
-                if ($arguments -ne $null) {
-                    $uninstall_arguments = $arguments
-                } else {
-                    $uninstall_arguments = ""
-                }
+                $uninstall_arguments = @($local_path)
             }
 
             if (-not $check_mode) {
-                $process_result = Run-Process -executable $uninstall_exe -arguments $uninstall_arguments
+                $uninstall_command = Argv-ToString -arguments $uninstall_arguments
+                if ($arguments -ne $null) {
+                    $uninstall_command += " $arguments"
+                }
                 
+                try {
+                    $process_result = Run-Command -command $uninstall_command
+                } catch {
+                    Fail-Json -obj $result -message "failed to run uninstall process ($uninstall_command): $($_.Exception.Message)"
+                }
+
                 if (($log_path -ne $null) -and (Test-Path -Path $log_path)) {
                     $log_content = Get-Content -Path $log_path | Out-String
                 } else {
@@ -440,7 +380,7 @@ if ($state -eq "absent") {
                     if ($log_content -ne $null) {
                         $result.log = $log_content
                     }
-                    Fail-Json -obj $result -message "unexpected rc from uninstall $uninstall_exe $($uninstall_arguments): see exit_code, stdout and stderr for more details"
+                    Fail-Json -obj $result -message "unexpected rc from uninstall $uninstall_exe $($uninstall_arguments): see rc, stdout and stderr for more details"
                 } else {
                     $result.failed = $false
                 }
@@ -484,32 +424,30 @@ if ($state -eq "absent") {
                 $local_path = $path
             }
 
-
             if ($program_metadata.msi -eq $true) {
                 # we are installing an msi
-                $install_exe = "$env:windir\system32\msiexec.exe"
                 $temp_path = [System.IO.Path]::GetTempPath()
                 $log_file = [System.IO.Path]::GetRandomFileName()
                 $log_path = Join-Path -Path $temp_path -ChildPath $log_file
                 
                 $cleanup_artifacts += $log_path
-                $install_arguments = @("/i", "`"$($local_path)`"", "/L*V", "`"$log_path`"", "/qn", "/norestart")
-                if ($arguments -ne $null) {
-                    $install_arguments += $arguments
-                }
-                $install_arguments = $install_arguments -join " "
+                $install_arguments = @("$env:windir\system32\msiexec.exe", "/i", $local_path, "/L*V", $log_path, "/qn", "/norestart")
             } else {
                 $log_path = $null                
-                $install_exe = $local_path
-                if ($arguments -ne $null) {
-                    $install_arguments = $arguments
-                } else {
-                    $install_arguments = ""
-                }
+                $install_arguments = @($local_path)
             }
 
             if (-not $check_mode) {
-                $process_result = Run-Process -executable $install_exe -arguments $install_arguments
+                $install_command = Argv-ToString -arguments $install_arguments
+                if ($arguments -ne $null) {
+                    $install_command += " $arguments"
+                }
+                
+                try {
+                    $process_result = Run-Command -command $install_command
+                } catch {
+                    Fail-Json -obj $result -message "failed to run install process ($install_command): $($_.Exception.Message)"
+                }
                 
                 if (($log_path -ne $null) -and (Test-Path -Path $log_path)) {
                     $log_content = Get-Content -Path $log_path | Out-String
@@ -525,7 +463,7 @@ if ($state -eq "absent") {
                     if ($log_content -ne $null) {
                         $result.log = $log_content
                     }
-                    Fail-Json -obj $result -message "unexpected rc from install $install_exe $($install_arguments): see exit_code, stdout and stderr for more details"
+                    Fail-Json -obj $result -message "unexpected rc from install $install_exe $($install_arguments): see rc, stdout and stderr for more details"
                 } else {
                     $result.failed = $false
                 }

lib/ansible/modules/windows/win_package.py

@@ -42,6 +42,10 @@ options:
       package.
     - If the package is an MSI do not supply the C(/qn), C(/log) or
       C(/norestart) arguments.
+    - As of Ansible 2.5, this parameter can be a list of arguments and the
+      module will escape the arguments as necessary, it is recommended to use a
+      string when dealing with MSI packages due to the unique escaping issues
+      with msiexec.
   creates_path:
     description:
     - Will check the existance of the path specified and use the result to
@@ -158,6 +162,15 @@ EXAMPLES = r'''
     product_id: '{CF2BEA3C-26EA-32F8-AA9B-331F7E34BA97}'
     arguments: /install /passive /norestart
 
+- name: Install Visual C thingy with list of arguments instead of a string
+  win_package:
+    path: http://download.microsoft.com/download/1/6/B/16B06F60-3B20-4FF2-B699-5E9B7962F9AE/VSU_4/vcredist_x64.exe
+    product_id: '{CF2BEA3C-26EA-32F8-AA9B-331F7E34BA97}'
+    arguments:
+    - /install
+    - /passive
+    - /norestart
+
 - name: Install Remote Desktop Connection Manager from msi
   win_package:
     path: https://download.microsoft.com/download/A/F/0/AF0071F3-B198-4A35-AA90-C68D103BDCCF/rdcman.msi

test/integration/targets/win_package/defaults/main.yml

@@ -1,5 +1,6 @@
 ---
 # spaces are tricky, let's have one by default
+test_win_package_path_safe: C:\ansible\win_package
 test_win_package_path: C:\ansible\win package
 test_win_package_good_url: https://s3.amazonaws.com/ansible-ci-files/test/integration/roles/test_win_package/good.msi
 test_win_package_reboot_url: https://s3.amazonaws.com/ansible-ci-files/test/integration/roles/test_win_package/reboot.msi

test/integration/targets/win_package/tasks/exe_tests.yml

@@ -38,7 +38,7 @@
     that:
     - install_local_exe|changed
     - install_local_exe.reboot_required == False
-    - install_local_exe.exit_code == 0
+    - install_local_exe.rc == 0
     - install_local_exe_actual.exists == True
 
 - name: install local exe (idempotent)
@@ -93,7 +93,7 @@
     that:
     - uninstall_path_local_exe|changed
     - uninstall_path_local_exe.reboot_required == False
-    - uninstall_path_local_exe.exit_code == 0
+    - uninstall_path_local_exe.rc == 0
     - uninstall_path_local_exe_actual.exists == False
 
 - name: uninstall local exe with path (idempotent)
@@ -148,7 +148,7 @@
     that:
     - install_url_exe|changed
     - install_url_exe.reboot_required == False
-    - install_url_exe.exit_code == 0
+    - install_url_exe.rc == 0
     - install_url_exe_actual.exists == True
 
 - name: install url exe (idempotent)
@@ -201,7 +201,7 @@
     that:
     - uninstall_id_local_exe|changed
     - uninstall_id_local_exe.reboot_required == False
-    - uninstall_id_local_exe.exit_code == 0
+    - uninstall_id_local_exe.rc == 0
     - uninstall_id_local_exe_actual.exists == False
 
 - name: uninstall local exe with product_id (idempotent)

test/integration/targets/win_package/tasks/main.yml

@@ -1,8 +1,11 @@
 ---
-- name: ensure testing folder exists
+- name: ensure testing folders exists
   win_file:
-    path: '{{test_win_package_path}}'
+    path: '{{item}}'
     state: directory
+  with_items:
+  - '{{test_win_package_path}}'
+  - '{{test_win_package_path_safe}}'
 
 - name: download msi files from S3 bucket
   win_get_url:
@@ -50,3 +53,11 @@
     - { id: '{{test_win_package_good_id}}' }
     - { id: '{{test_win_package_reboot_id}}' }
     # - { id: '{{test_win_package_exe_id}}', args: '/S' }
+  
+  - name: cleanup test artifacts
+    win_file:
+      path: '{{item}}'
+      state: absent
+    with_items:
+    - '{{test_win_package_path}}'
+    - '{{test_win_package_path_safe}}'

test/integration/targets/win_package/tasks/msi_tests.yml

@@ -42,7 +42,7 @@
     that:
     - install_local_msi|changed
     - install_local_msi.reboot_required == False
-    - install_local_msi.exit_code == 0
+    - install_local_msi.rc == 0
     - install_local_msi_actual.exists == True
 
 - name: install local msi (idempotent)
@@ -91,7 +91,7 @@
     that:
     - uninstall_path_local_msi|changed
     - uninstall_path_local_msi.reboot_required == False
-    - uninstall_path_local_msi.exit_code == 0
+    - uninstall_path_local_msi.rc == 0
     - uninstall_path_local_msi_actual.exists == False
 
 - name: uninstall local msi with path (idempotent)
@@ -142,7 +142,7 @@
     that:
     - install_url_msi|changed
     - install_url_msi.reboot_required == False
-    - install_url_msi.exit_code == 0
+    - install_url_msi.rc == 0
     - install_url_msi_actual.exists == True
 
 - name: install url msi (idempotent)
@@ -192,7 +192,7 @@
     that:
     - uninstall_id_local_msi|changed
     - uninstall_id_local_msi.reboot_required == False
-    - uninstall_id_local_msi.exit_code == 0
+    - uninstall_id_local_msi.rc == 0
     - uninstall_id_local_msi_actual.exists == False
 
 - name: uninstall local msi with product_id (idempotent)
@@ -241,7 +241,7 @@
     that:
     - install_local_reboot_msi|changed
     - install_local_reboot_msi.reboot_required == True
-    - install_local_reboot_msi.exit_code == 3010
+    - install_local_reboot_msi.rc == 3010
     - install_local_reboot_msi_actual.exists == True
 
 - name: install local reboot msi (idempotent)
@@ -313,7 +313,7 @@
     that:
     - install_msi_argument|changed
     - install_msi_argument.reboot_required == False
-    - install_msi_argument.exit_code == 0
+    - install_msi_argument.rc == 0
     - install_msi_argument_moo.stat.exists == False
     - install_msi_argument_cow.stat.exists == True
 
@@ -333,3 +333,72 @@
   win_package:
     path: '{{test_win_package_path}}\good.msi'
     state: absent
+
+- name: create custom install directory for msi install
+  win_file:
+    path: '{{test_win_package_path_safe}}\good'
+    state: directory
+
+- name: install msi to custom path using string arguments
+  win_package:
+    path: '{{test_win_package_path}}\good.msi'
+    state: present
+    arguments: ADDLOCAL=Cow INSTALLDIR={{test_win_package_path_safe}}\install
+  register: install_msi_string_arguments
+
+- name: get result of moo file after install local msi with string arguments
+  win_stat:
+    path: '{{test_win_package_path_safe}}\install\moo.exe'
+  register: install_msi_string_arguments_moo
+
+- name: get result of cow file after install local msi with string arguments
+  win_stat:
+    path: '{{test_win_package_path_safe}}\install\cow.exe'
+  register: install_msi_string_arguments_cow
+
+- name: assert results of install msi to custom path using string arguments
+  assert:
+    that:
+    - install_msi_string_arguments|changed
+    - install_msi_string_arguments.reboot_required == False
+    - install_msi_string_arguments.rc == 0
+    - install_msi_string_arguments_moo.stat.exists == False
+    - install_msi_string_arguments_cow.stat.exists == True
+
+- name: uninstall good msi after string argument test
+  win_package:
+    path: '{{test_win_package_path}}\good.msi'
+    state: absent
+
+- name: install msi to custom path using list arguments
+  win_package:
+    path: '{{test_win_package_path}}\good.msi'
+    state: present
+    arguments:
+    - ADDLOCAL=Moo
+    - INSTALLDIR={{test_win_package_path_safe}}\install
+  register: install_msi_list_arguments
+
+- name: get result of moo file after install local msi with list arguments
+  win_stat:
+    path: '{{test_win_package_path_safe}}\install\moo.exe'
+  register: install_msi_list_arguments_moo
+
+- name: get result of cow file after install local msi with list arguments
+  win_stat:
+    path: '{{test_win_package_path_safe}}\install\cow.exe'
+  register: install_msi_list_arguments_cow
+
+- name: assert results of install msi to custom path using list arguments
+  assert:
+    that:
+    - install_msi_list_arguments|changed
+    - install_msi_list_arguments.reboot_required == False
+    - install_msi_list_arguments.rc == 0
+    - install_msi_list_arguments_moo.stat.exists == True
+    - install_msi_list_arguments_cow.stat.exists == False
+
+- name: uninstall good msi after list argument test
+  win_package:
+    path: '{{test_win_package_path}}\good.msi'
+    state: absent

test/integration/targets/win_package/tasks/network_tests.yml

@@ -40,7 +40,7 @@
     that:
     - install_network_msi|changed
     - install_network_msi.reboot_required == False
-    - install_network_msi.exit_code == 0
+    - install_network_msi.rc == 0
     - install_network_msi_actual.exists == True
 
 - name: install network msi (idempotent)
@@ -98,7 +98,7 @@
     that:
     - uninstall_path_network_msi|changed
     - uninstall_path_network_msi.reboot_required == False
-    - uninstall_path_network_msi.exit_code == 0
+    - uninstall_path_network_msi.rc == 0
     - uninstall_path_network_msi_actual.exists == False
 
 - name: uninstall network msi with path (idempotent)
@@ -156,7 +156,7 @@
     that:
     - install_network_reboot_msi|changed
     - install_network_reboot_msi.reboot_required == True
-    - install_network_reboot_msi.exit_code == 3010
+    - install_network_reboot_msi.rc == 3010
     - install_network_reboot_msi_actual.exists == True
 
 - name: install network reboot msi (idempotent)
@@ -210,7 +210,7 @@
     that:
     - uninstall_id_network_msi|changed
     - uninstall_id_network_msi.reboot_required == True
-    - uninstall_id_network_msi.exit_code == 3010
+    - uninstall_id_network_msi.rc == 3010
     - uninstall_id_network_msi_actual.exists == False
 
 - name: uninstall network msi with product_id (idempotent)
@@ -283,7 +283,7 @@
     that:
     - install_network_msi_argument|changed
     - install_network_msi_argument.reboot_required == False
-    - install_network_msi_argument.exit_code == 0
+    - install_network_msi_argument.rc == 0
     - install_network_msi_argument_moo.stat.exists == False
     - install_network_msi_argument_cow.stat.exists == True
 
@@ -350,7 +350,7 @@
     that:
     - install_network_exe|changed
     - install_network_exe.reboot_required == False
-    - install_network_exe.exit_code == 0
+    - install_network_exe.rc == 0
     - install_network_exe_actual.exists == True
 
 - name: install network exe (idempotent)
@@ -409,7 +409,7 @@
     that:
     - uninstall_network_exe|changed
     - uninstall_network_exe.reboot_required == False
-    - uninstall_network_exe.exit_code == 0
+    - uninstall_network_exe.rc == 0
     - uninstall_network_exe_actual.exists == False
 
 - name: uninstall network exe (idempotent)