win_unzip: Add integration tests, check-mode, various (#25335)

CHANGELOG.md

@@ -77,6 +77,8 @@ Ansible Changes By Release
 * Add an encoding parameter for the replace module so that it can operate on non-utf-8 files
 * By default, Ansible now uses the cryptography module to implement vault instead of the older pycrypto module.
 * Changed task state resulting from both `rc` and `failed` fields returned, 'rc' no longer overrides 'failed'. Test plugins have also been updated accordingly.
+* The win_unzip module no longer includes dictionary 'win_unzip' in its results,
+  the content is now directly in the resulting output, like pretty much every other module.
 
 #### New Callbacks:
 - profile_roles

lib/ansible/modules/windows/win_unzip.ps1

@@ -19,72 +19,81 @@
 # WANT_JSON
 # POWERSHELL_COMMON
 
+# TODO: This module is not idempotent (it will always unzip and report change)
 
-$params = Parse-Args $args;
+$ErrorActionPreference = "Stop"
 
+$pcx_extensions = @('.bz2', '.gz', '.msu', '.tar', '.zip')
+
+$params = Parse-Args $args -supports_check_mode $true
+$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
+
+$src = Get-AnsibleParam -obj $params -name "src" -type "path" -failifempty $true
+$dest = Get-AnsibleParam -obj $params -name "dest" -type "path" -failifempty $true
+$creates = Get-AnsibleParam -obj $params -name "creates" -type "path"
+$recurse = Get-AnsibleParam -obj $params -name "recurse" -type "bool" -default $false
+$delete_archive = Get-AnsibleParam -obj $params -name "delete_archive" -type "bool" -default $false -aliases 'rm'
+
+# Fixes a fail error message (when the task actually succeeds) for a
+# "Convert-ToJson: The converted JSON string is in bad format"
+# This happens when JSON is parsing a string that ends with a "\",
+# which is possible when specifying a directory to download to.
+# This catches that possible error, before assigning the JSON $result
 $result = @{
-    win_unzip = @{}
     changed = $false
+    dest = $dest -replace '\$',''
+    removed = $false
+    src = $src -replace '\$',''
 }
 
-$creates = Get-AnsibleParam -obj $params -name "creates" -type "path"
+If ($creates -and (Test-Path -LiteralPath $creates)) {
-If ($creates -ne $null) {
+    $result.skipped = $true
-    If (Test-Path $creates) {
+    $result.msg = "The file or directory '$creates' already exists."
-        $result.msg = "The 'creates' file or directory ($creates) already exists."
+    Exit-Json -obj $result
-        Exit-Json $result
-    }
 }
 
-$src = Get-AnsibleParam -obj $params -name "src" -type "path" -failifempty $true
+If (-Not (Test-Path -LiteralPath $src)) {
-If (-Not (Test-Path -path $src)){
+    Fail-Json -obj $result -message "File '$src' does not exist."
-    Fail-Json $result "src file: $src does not exist."
 }
 
 $ext = [System.IO.Path]::GetExtension($src)
 
-
+If (-Not (Test-Path -LiteralPath $dest -PathType Container)){
-$dest = Get-AnsibleParam -obj $params -name "dest" -type "path" -failifempty $true
-If (-Not (Test-Path $dest -PathType Container)){
     Try{
-        New-Item -itemtype directory -path $dest
+        New-Item -ItemType "directory" -path $dest -WhatIf:$check_mode
-    }
+    } Catch {
-    Catch {
+        Fail-Json -obj $result -message "Error creating '$dest' directory! Msg: $($_.Exception.Message)"
-        $err_msg = $_.Exception.Message
-        Fail-Json $result "Error creating $dest directory! Msg: $err_msg"
     }
 }
 
-$recurse = ConvertTo-Bool (Get-AnsibleParam -obj $params -name "recurse" -default "false")
-$rm = ConvertTo-Bool (Get-AnsibleParam -obj $params -name "rm" -default "false")
-
 If ($ext -eq ".zip" -And $recurse -eq $false) {
     Try {
         $shell = New-Object -ComObject Shell.Application
         $zipPkg = $shell.NameSpace([IO.Path]::GetFullPath($src))
         $destPath = $shell.NameSpace([IO.Path]::GetFullPath($dest))
-        # From Folder.CopyHere documentation (https://msdn.microsoft.com/en-us/library/windows/desktop/bb787866.aspx)
+
-        # 1044 means do not display any error dialog (1024), progress dialog (4) and overwrite any file (16)
+        if (-not $check_mode) {
-        $destPath.CopyHere($zipPkg.Items(), 1044)
+            # https://msdn.microsoft.com/en-us/library/windows/desktop/bb787866.aspx
+            # From Folder.CopyHere documentation, 1044 means:
+            #  - 1024: do not display a user interface if an error occurs
+            #  -   16: respond with "yes to all" for any dialog box that is displayed
+            #  -    4: do not display a progress dialog box
+            $destPath.CopyHere($zipPkg.Items(), 1044)
+        }
         $result.changed = $true
+    } Catch {
+        Fail-Json -obj $result -message "Error unzipping '$src' to $dest! Msg: $($_.Exception.Message)"
     }
-    Catch {
+} Else {
-        $err_msg = $_.Exception.Message
-        Fail-Json $result "Error unzipping $src to $dest! Msg: $err_msg"
-    }
-}
-# Requires PSCX
-Else {
     # Check if PSCX is installed
     $list = Get-Module -ListAvailable
 
     If (-Not ($list -match "PSCX")) {
-        Fail-Json $result "PowerShellCommunityExtensions PowerShell Module (PSCX) is required for non-'.zip' compressed archive types."
+        Fail-Json -obj $result -message "PowerShellCommunityExtensions PowerShell Module (PSCX) is required for non-'.zip' compressed archive types."
-    }
+    } Else {
-    Else {
+        $result.pscx_status = "present"
-        $result.win_unzip.pscx_status = "present"
     }
 
-    # Import
     Try {
         Import-Module PSCX
     }
@@ -93,53 +102,31 @@ Else {
     }
 
     Try {
-        If ($recurse) {
+        Expand-Archive -Path $src -OutputPath $dest -Force -WhatIf:$check_mode
-            Expand-Archive -Path $src -OutputPath $dest -Force
+    } Catch {
-
+        Fail-Json -obj $result -message "Error expanding '$src' to '$dest'! Msg: $($_.Exception.Message)"
-            If ($rm -eq $true) {
+    }
-                Get-ChildItem $dest -recurse | Where {$_.extension -eq ".gz" -Or $_.extension -eq ".zip" -Or $_.extension -eq ".bz2" -Or $_.extension -eq ".tar" -Or $_.extension -eq ".msu"} | % {
+
-                    Expand-Archive $_.FullName -OutputPath $dest  -Force
+    If ($recurse) {
-                    Remove-Item $_.FullName -Force
+        Get-ChildItem $dest -recurse | Where {$pcx_extensions -contains $_.extension} | % {
-                }
+            Try {
+                Expand-Archive $_.FullName -OutputPath $dest -Force -WhatIf:$check_mode
+            } Catch {
+                Fail-Json -obj $result -message "Error recursively expanding '$src' to '$dest'! Msg: $($_.Exception.Message)"
             }
-            Else {
+            If ($delete_archive) {
-                Get-ChildItem $dest -recurse | Where {$_.extension -eq ".gz" -Or $_.extension -eq ".zip" -Or $_.extension -eq ".bz2" -Or $_.extension -eq ".tar" -Or $_.extension -eq ".msu"} | % {
+                Remove-Item $_.FullName -Force -WhatIf:$check_mode
-                    Expand-Archive $_.FullName -OutputPath $dest  -Force
+                $result.removed = $true
-                }
             }
         }
-        Else {
-            Expand-Archive -Path $src -OutputPath $dest -Force
-        }
-        $result.changed = $true
-    }
-    Catch {
-        $err_msg = $_.Exception.Message
-        If ($recurse) {
-            Fail-Json $result "Error recursively expanding $src to $dest! Msg: $err_msg"
-        }
-        Else {
-            Fail-Json $result "Error expanding $src to $dest! Msg: $err_msg"
-        }
     }
-}
 
-If ($rm -eq $true){
+    $result.changed = $true
-    Remove-Item $src -Recurse -Force
-    $result.win_unzip.rm = "true"
 }
 
-# Fixes a fail error message (when the task actually succeeds) for a "Convert-ToJson: The converted JSON string is in bad format"
+If ($delete_archive){
-# This happens when JSON is parsing a string that ends with a "\", which is possible when specifying a directory to download to.
+    Remove-Item $src -Recurse -Force -WhatIf:$check_mode
-# This catches that possible error, before assigning the JSON $result
+    $result.removed = $true
-If ($src[$src.length-1] -eq "\") {
-    $src = $src.Substring(0, $src.length-1)
-}
-If ($dest[$dest.length-1] -eq "\") {
-    $dest = $dest.Substring(0, $dest.length-1)
 }
-$result.win_unzip.src = $src.toString()
-$result.win_unzip.dest = $dest.toString()
-$result.win_unzip.recurse = $recurse.toString()
 
 Exit-Json $result

lib/ansible/modules/windows/win_unzip.py

@@ -25,7 +25,6 @@ ANSIBLE_METADATA = {'metadata_version': '1.0',
                     'status': ['preview'],
                     'supported_by': 'community'}
 
-
 DOCUMENTATION = r'''
 ---
 module: win_unzip
@@ -41,59 +40,52 @@ requirements:
 options:
   src:
     description:
-      - File to be unzipped (provide absolute path)
+      - File to be unzipped (provide absolute path).
     required: true
   dest:
     description:
       - Destination of zip file (provide absolute path of directory). If it does not exist, the directory will be created.
     required: true
-  rm:
+  delete_archive:
     description:
-      - Remove the zip file, after unzipping
+      - Remove the zip file, after unzipping.
-    required: no
+    type: bool
-    choices:
+    default: 'no'
-      - true
+    aliases: [ rm ]
-      - false
-      - yes
-      - no
-    default: false
   recurse:
     description:
       - Recursively expand zipped files within the src file.
-    required: no
+    type: bool
-    default: false
+    default: 'no'
-    choices:
-      - true
-      - false
-      - yes
-      - no
   creates:
     description:
       - If this file or directory exists the specified src will not be extracted.
-    required: no
-    default: null
 notes:
+- This module is not really idempotent, it will extract the archive every time, and report a change.
 - For extracting any compression types other than .zip, the PowerShellCommunityExtensions (PSCX) Module is required.  This module (in conjunction with PSCX)
   has the ability to recursively unzip files within the src zip file provided and also functionality for many other compression types. If the destination
   directory does not exist, it will be created before unzipping the file.  Specifying rm parameter will force removal of the src file after extraction.
 - For non-Windows targets, use the M(unarchive) module instead.
-author: Phil Schwartz
+author:
+- Phil Schwartz (@schwartzmx)
 '''
 
 EXAMPLES = r'''
 # This unzips a library that was downloaded with win_get_url, and removes the file after extraction
-# $ ansible -i hosts -m win_unzip -a "src=C:\\LibraryToUnzip.zip dest=C:\\Lib rm=true" all
+# $ ansible -i hosts -m win_unzip -a "src=C:\LibraryToUnzip.zip dest=C:\Lib remove=true" all
-# Playbook example
 
-# Simple unzip
----
 - name: Unzip a bz2 (BZip) file
   win_unzip:
     src: C:\Users\Phil\Logs.bz2
     dest: C:\Users\Phil\OldLogs
     creates: C:\Users\Phil\OldLogs
 
-# This playbook example unzips a .zip file and recursively decompresses the contained .gz files and removes all unneeded compressed files after completion.
+- name: Unzip gz log
+  win_unzip:
+    src: C:\Logs\application-error-logs.gz
+    dest: C:\ExtractedLogs\application-error-logs
+
+# Unzip .zip file, recursively decompresses the contained .gz files and removes all unneeded compressed files after completion.
 - name: Unzip ApplicationLogs.zip and decompress all GZipped log files
   hosts: all
   gather_facts: false
@@ -103,18 +95,33 @@ EXAMPLES = r'''
         src: C:\Downloads\ApplicationLogs.zip
         dest: C:\Application\Logs
         recurse: yes
-        rm: true
+        delete_archive: yes
 
 # Install PSCX to use for extracting a gz file
 - name: Grab PSCX msi
   win_get_url:
     url: http://download-codeplex.sec.s-msft.com/Download/Release?ProjectName=pscx&DownloadId=923562&FileTime=130585918034470000&Build=20959
-    dest: C:\pscx.msi
+    dest: C:\Windows\Temp\pscx.msi
+
 - name: Install PSCX
   win_msi:
-    path: C:\pscx.msi
+    path: C:\Windows\Temp\pscx.msi
-- name: Unzip gz log
+'''
-  win_unzip:
+
-    src: C:\Logs\application-error-logs.gz
+RETURN = r'''
-    dest: C:\ExtractedLogs\application-error-logs
+dest:
+    description: The provided destination path
+    returned: always
+    type: string
+    sample: C:\ExtractedLogs\application-error-logs
+removed:
+    description: Whether the module did remove any files during task run
+    returned: always
+    type: boolean
+    sample: True
+src:
+    description: The provided source path
+    returned: always
+    type: string
+    sample: C:\Logs\application-error-logs.gz
 '''

test/integration/targets/win_unzip/aliases

@@ -0,0 +1 @@
+windows/ci/group2

test/integration/targets/win_unzip/tasks/clean.yml

@@ -0,0 +1,15 @@
+- name: Remove leftover directory
+  win_file:
+    path: C:\Program Files\sysinternals
+    state: absent
+
+- name: Create new directory
+  win_file:
+    path: C:\Program Files\sysinternals
+    state: directory
+
+- name: Download sysinternals archive
+  win_get_url:
+    url: https://download.sysinternals.com/files/SysinternalsSuite.zip
+    dest: C:\Windows\Temp\SysinternalsSuite.zip
+    skip_certificate_validation: yes

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

@@ -0,0 +1,16 @@
+- name: Clean slate
+  include: clean.yml
+
+- name: Test in normal mode
+  include: tests.yml
+  vars:
+    in_check_mode: no
+
+- name: Clean slate
+  include: clean.yml
+
+- name: Test in check-mode
+  include: tests.yml
+  vars:
+    in_check_mode: yes
+  check_mode: yes

test/integration/targets/win_unzip/tasks/tests.yml

@@ -0,0 +1,85 @@
+- name: Unarchive sysinternals archive
+  win_unzip:
+    src: C:\Windows\Temp\SysinternalsSuite.zip
+    dest: C:\Program Files\sysinternals
+  register: unzip_archive
+
+- name: Test unzip_archive
+  assert:
+    that:
+    - unzip_archive|changed == true
+    - unzip_archive.removed == false
+
+
+- name: Unarchive sysinternals archive again, use creates
+  win_unzip:
+    src: C:\Windows\Temp\SysinternalsSuite.zip
+    dest: C:\Program Files\sysinternals
+    creates: C:\Program Files\sysinternals\procexp.exe
+  register: unzip_archive_again_creates
+
+# NOTE: This module is not idempotent, it always extracts, except if we use creates !
+- name: Test unzip_archive_again_creates (normal mode)
+  assert:
+    that:
+    - unzip_archive_again_creates|changed == false
+    - unzip_archive_again_creates.removed == false
+  when: not in_check_mode
+
+- name: Test unzip_archive_again_creates (check-mode)
+  assert:
+    that:
+    - unzip_archive_again_creates|changed == true
+    - unzip_archive_again_creates.removed == false
+  when: in_check_mode
+
+
+- name: Unarchive sysinternals archive again
+  win_unzip:
+    src: C:\Windows\Temp\SysinternalsSuite.zip
+    dest: C:\Program Files\sysinternals
+    delete_archive: yes
+  register: unzip_archive_again
+
+# NOTE/ This module is not idempotent, it always extracts
+- name: Test unzip_archive_again
+  assert:
+    that:
+    - unzip_archive_again|changed == true
+    - unzip_archive_again.removed == true
+
+
+- name: Test whether archive is removed
+  win_stat:
+    path: C:\Windows\Temp\SysinternalsSuite.zip
+  register: stat_archive
+
+- name: Test stat_archive (normal mode)
+  assert:
+    that:
+    - stat_archive.stat.exists == false
+  when: not in_check_mode
+
+- name: Test stat_archive (check-mode)
+  assert:
+    that:
+    - stat_archive.stat.exists == true
+  when: in_check_mode
+
+
+- name: Test extracted files
+  win_stat:
+    path: C:\Program Files\sysinternals\procexp.exe
+  register: stat_procexp
+
+- name: Test stat_procexp (normal mode)
+  assert:
+    that:
+    - stat_procexp.stat.exists == true
+  when: not in_check_mode
+
+- name: Test stat_procexp (check-mode)
+  assert:
+    that:
+    - stat_procexp.stat.exists == false
+  when: in_check_mode