From e16e6313c712825789a421bc0bd6779cab3ae0ba Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 16 Nov 2017 10:04:03 +1000 Subject: [PATCH] windows: fix for checking locked system files (#30665) * fix for checking locked system files * moved functions to share module util and created tests * fixed windows-paths test based on win_stat changes --- .../Ansible.ModuleUtils.FileUtil.psm1 | 41 +++++++++++++ lib/ansible/modules/windows/win_command.ps1 | 5 +- lib/ansible/modules/windows/win_shell.ps1 | 5 +- lib/ansible/modules/windows/win_stat.ps1 | 30 +++------- lib/ansible/modules/windows/win_stat.py | 3 +- lib/ansible/modules/windows/win_wait_for.ps1 | 5 +- .../targets/win_command/tasks/main.yml | 12 ++++ .../library/file_util_test.ps1 | 59 +++++++++++++++++++ .../targets/win_module_utils/tasks/main.yml | 8 +++ .../targets/win_shell/tasks/main.yml | 12 ++++ .../targets/win_stat/tasks/main.yml | 13 ++++ .../targets/windows-paths/tasks/main.yml | 4 +- 12 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 create mode 100644 test/integration/targets/win_module_utils/library/file_util_test.ps1 diff --git a/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 b/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 new file mode 100644 index 00000000000..f1f954d1296 --- /dev/null +++ b/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 @@ -0,0 +1,41 @@ +# Copyright (c) 2017 Ansible Project +# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause) + +<# +Test-Path/Get-Item cannot find/return info on files that are locked like +C:\pagefile.sys. These 2 functions are designed to work with these files and +provide similar functionality with the normal cmdlets with as minimal overhead +as possible. They work by using Get-ChildItem with a filter and return the +result from that. +#> + +Function Test-FilePath($path) { + # Basic replacement for Test-Path that tests if the file/folder exists at the path + $directory = Split-Path -Path $path -Parent + $filename = Split-Path -Path $path -Leaf + + $file = Get-ChildItem -Path $directory -Filter $filename -Force -ErrorAction SilentlyContinue + if ($file -ne $null) { + if ($file -is [Array] -and $file.Count -gt 1) { + throw "found multiple files at path '$path', make sure no wildcards are set in the path" + } + return $true + } else { + return $false + } +} + +Function Get-FileItem($path) { + # Replacement for Get-Item + $directory = Split-Path -Path $path -Parent + $filename = Split-Path -Path $path -Leaf + + $file = Get-ChildItem -Path $directory -Filter $filename -Force -ErrorAction SilentlyContinue + if ($file -is [Array] -and $file.Count -gt 1) { + throw "found multiple files at path '$path', make sure no wildcards are set in the path" + } + + return $file +} + +Export-ModuleMember -Function Test-FilePath, Get-FileItem diff --git a/lib/ansible/modules/windows/win_command.ps1 b/lib/ansible/modules/windows/win_command.ps1 index 9cdd125df32..ff98771b10e 100644 --- a/lib/ansible/modules/windows/win_command.ps1 +++ b/lib/ansible/modules/windows/win_command.ps1 @@ -6,6 +6,7 @@ #Requires -Module Ansible.ModuleUtils.Legacy #Requires -Module Ansible.ModuleUtils.CommandUtil +#Requires -Module Ansible.ModuleUtils.FileUtil # TODO: add check mode support @@ -27,11 +28,11 @@ $result = @{ cmd = $raw_command_line } -If($creates -and $(Test-Path -Path $creates)) { +if ($creates -and $(Test-FilePath -path $creates)) { Exit-Json @{msg="skipped, since $creates exists";cmd=$raw_command_line;changed=$false;skipped=$true;rc=0} } -If($removes -and -not $(Test-Path -Path $removes)) { +if ($removes -and -not $(Test-FilePath -path $removes)) { Exit-Json @{msg="skipped, since $removes does not exist";cmd=$raw_command_line;changed=$false;skipped=$true;rc=0} } diff --git a/lib/ansible/modules/windows/win_shell.ps1 b/lib/ansible/modules/windows/win_shell.ps1 index 0e293b5bba2..6605190494f 100644 --- a/lib/ansible/modules/windows/win_shell.ps1 +++ b/lib/ansible/modules/windows/win_shell.ps1 @@ -6,6 +6,7 @@ #Requires -Module Ansible.ModuleUtils.Legacy #Requires -Module Ansible.ModuleUtils.CommandUtil +#Requires -Module Ansible.ModuleUtils.FileUtil # TODO: add check mode support @@ -55,11 +56,11 @@ $result = @{ cmd = $raw_command_line } -If($creates -and $(Test-Path $creates)) { +if ($creates -and $(Test-FilePath -path $creates)) { Exit-Json @{msg="skipped, since $creates exists";cmd=$raw_command_line;changed=$false;skipped=$true;rc=0} } -If($removes -and -not $(Test-Path $removes)) { +if ($removes -and -not $(Test-FilePath -path $removes)) { Exit-Json @{msg="skipped, since $removes does not exist";cmd=$raw_command_line;changed=$false;skipped=$true;rc=0} } diff --git a/lib/ansible/modules/windows/win_stat.ps1 b/lib/ansible/modules/windows/win_stat.ps1 index b837820dd44..103aa0802ba 100644 --- a/lib/ansible/modules/windows/win_stat.ps1 +++ b/lib/ansible/modules/windows/win_stat.ps1 @@ -1,21 +1,11 @@ #!powershell # This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# WANT_JSON -# POWERSHELL_COMMON + +# Copyright (c) 2017 Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +#Requires -Module Ansible.ModuleUtils.Legacy +#Requires -Module Ansible.ModuleUtils.FileUtil # C# code to determine link target, copied from http://chrisbensen.blogspot.com.au/2010/06/getfinalpathnamebyhandle.html $symlink_util = @" @@ -85,10 +75,11 @@ $result = @{ # Backward compatibility if ($get_md5 -eq $true -and (Get-Member -inputobject $params -name "get_md5") ) { - Add-DeprecationWarning $result "The parameter 'get_md5' is being replaced with 'checksum_algorithm: md5'" + Add-DeprecationWarning -obj $result -message "The parameter 'get_md5' is being replaced with 'checksum_algorithm: md5'" -version 2.7 } -If (Test-Path -Path $path) +$info = Get-FileItem -path $path +If ($info -ne $null) { $result.stat.exists = $true @@ -98,9 +89,6 @@ If (Test-Path -Path $path) $result.stat.isreg = $false $result.stat.isshared = $false - # Need to use -Force so it picks up hidden files - $info = Get-Item -Force $path - $epoch_date = Get-Date -Date "01/01/1970" $result.stat.creationtime = (Date_To_Timestamp $epoch_date $info.CreationTime) $result.stat.lastaccesstime = (Date_To_Timestamp $epoch_date $info.LastAccessTime) diff --git a/lib/ansible/modules/windows/win_stat.py b/lib/ansible/modules/windows/win_stat.py index aca399999d1..a1c4cc4070c 100644 --- a/lib/ansible/modules/windows/win_stat.py +++ b/lib/ansible/modules/windows/win_stat.py @@ -44,6 +44,7 @@ options: use specified algorithm. - This option is deprecated in Ansible 2.3 and is replaced with C(checksum_algorithm=md5). + - This option will be removed in Ansible 2.7 required: no default: True get_checksum: @@ -199,7 +200,7 @@ stat: type: string sample: C:\temp md5: - description: The MD5 checksum of a file (Between Ansible 1.9 and 2.2 this was returned as a SHA1 hash) + description: The MD5 checksum of a file (Between Ansible 1.9 and 2.2 this was returned as a SHA1 hash), will be removed in 2.7 returned: success, path exist, path is a file, get_md5 == True, md5 is supported type: string sample: 09cb79e8fc7453c84a07f644e441fd81623b7f98 diff --git a/lib/ansible/modules/windows/win_wait_for.ps1 b/lib/ansible/modules/windows/win_wait_for.ps1 index 280e7480d12..63bb3b18867 100644 --- a/lib/ansible/modules/windows/win_wait_for.ps1 +++ b/lib/ansible/modules/windows/win_wait_for.ps1 @@ -5,6 +5,7 @@ # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) #Requires -Module Ansible.ModuleUtils.Legacy +#Requires -Module Ansible.ModuleUtils.FileUtil $ErrorActionPreference = "Stop" @@ -118,7 +119,7 @@ if ($path -eq $null -and $port -eq $null -and $state -eq "drained") { $complete = $false while (((Get-Date) - $start_time).TotalSeconds -lt $timeout) { $attempts += 1 - if (Test-Path -Path $path) { + if (Test-FilePath -path $path) { if ($search_regex -eq $null) { $complete = $true break @@ -149,7 +150,7 @@ if ($path -eq $null -and $port -eq $null -and $state -eq "drained") { $complete = $false while (((Get-Date) - $start_time).TotalSeconds -lt $timeout) { $attempts += 1 - if (Test-Path -Path $path) { + if (Test-FilePath -path $path) { if ($search_regex -ne $null) { $file_contents = Get-Content -Path $path -Raw if ($file_contents -notmatch $search_regex) { diff --git a/test/integration/targets/win_command/tasks/main.yml b/test/integration/targets/win_command/tasks/main.yml index aaa8ea3fb94..8a71a0a8817 100644 --- a/test/integration/targets/win_command/tasks/main.yml +++ b/test/integration/targets/win_command/tasks/main.yml @@ -77,6 +77,18 @@ - cmdout|skipped - cmdout.msg is search('exists') +- name: test creates with hidden system file, should skip + win_command: echo no + args: + creates: C:\pagefile.sys + register: cmdout + +- name: validate result + assert: + that: + - cmdout|skipped + - cmdout.msg is search('exists') + - name: ensure testfile is still present win_stat: path: c:\testfile.txt diff --git a/test/integration/targets/win_module_utils/library/file_util_test.ps1 b/test/integration/targets/win_module_utils/library/file_util_test.ps1 new file mode 100644 index 00000000000..3626c91bcad --- /dev/null +++ b/test/integration/targets/win_module_utils/library/file_util_test.ps1 @@ -0,0 +1,59 @@ +#!powershell + +#Requires -Module Ansible.ModuleUtils.Legacy +#Requires -Module Ansible.ModuleUtils.FileUtil + +Function Assert-Equals($actual, $expected) { + if ($actual -cne $expected) { + Fail-Json @{} "actual != expected`nActual: $actual`nExpected: $expected" + } +} + +# Test-FilePath Hidden system file +$actual = Test-FilePath -path C:\pagefile.sys +Assert-Equals -actual $actual -expected $true + +# Test-FilePath File that doesn't exist +$actual = Test-FilePath -path C:\fakefile +Assert-Equals -actual $actual -expected $false + +# Test-FilePath Normal directory +$actual = Test-FilePath -path C:\Windows +Assert-Equals -actual $actual -expected $true + +# Test-FilePath Normal file +$actual = Test-FilePath -path C:\Windows\System32\kernel32.dll + +# Test-FilePath fails with wildcard +try { + Test-FilePath -Path C:\Windows\*.exe + Fail-Json @{} "exception was not thrown with wildcard search for Test-FilePath" +} catch { + Assert-Equals -actual $_.Exception.Message -expected "found multiple files at path 'C:\Windows\*.exe', make sure no wildcards are set in the path" +} + +# Get-FileItem file +$actual = Get-FileItem -path C:\pagefile.sys +Assert-Equals -actual $actual.FullName -expected C:\pagefile.sys +Assert-Equals -actual $actual.PSIsContainer -expected $false +Assert-Equals -actual $actual.Exists -expected $true + +# Get-FileItem directory +$actual = Get-FileItem -path C:\Windows +Assert-Equals -actual $actual.FullName -expected C:\Windows +Assert-Equals -actual $actual.PSIsContainer -expected $true +Assert-Equals -actual $actual.Exists -expected $true + +# Get-FileItem doesn't exists +$actual = Get-FileItem -path C:\fakefile +Assert-Equals -actual $actual -expected $null + +# Get-FileItem fails with wildcard +try { + Get-FileItem -Path C:\Windows\*.exe + Fail-Json @{} "exception was not thrown with wildcard search for Get-FileItem" +} catch { + Assert-Equals -actual $_.Exception.Message -expected "found multiple files at path 'C:\Windows\*.exe', make sure no wildcards are set in the path" +} + +Exit-Json @{ data = 'success' } diff --git a/test/integration/targets/win_module_utils/tasks/main.yml b/test/integration/targets/win_module_utils/tasks/main.yml index f00f08e1e9a..05ad17dd634 100644 --- a/test/integration/targets/win_module_utils/tasks/main.yml +++ b/test/integration/targets/win_module_utils/tasks/main.yml @@ -80,3 +80,11 @@ win_file: path: C:\ansible testing state: absent + +- name: call module with FileUtil tests + file_util_test: + register: file_util_test + +- assert: + that: + - file_util_test.data == 'success' diff --git a/test/integration/targets/win_shell/tasks/main.yml b/test/integration/targets/win_shell/tasks/main.yml index 2716b948e3f..aeaf0eb373b 100644 --- a/test/integration/targets/win_shell/tasks/main.yml +++ b/test/integration/targets/win_shell/tasks/main.yml @@ -105,6 +105,18 @@ - shellout|skipped - shellout.msg is search('exists') +- name: test creates with hidden system file, should skip + win_shell: echo test + args: + creates: C:\pagefile.sys + register: shellout + +- name: validate result + assert: + that: + - shellout|skipped + - shellout.msg is search('exists') + - name: ensure testfile is still present win_stat: path: c:\testfile.txt diff --git a/test/integration/targets/win_stat/tasks/main.yml b/test/integration/targets/win_stat/tasks/main.yml index bd1e3d53a9f..89594be38d4 100644 --- a/test/integration/targets/win_stat/tasks/main.yml +++ b/test/integration/targets/win_stat/tasks/main.yml @@ -295,6 +295,9 @@ - stat_readonly.stat.size == 3 # Requires more work once modular powershell utils are in +- name: weird issue, need to access the file in anyway to get the correct date stats + win_command: powershell.exe Test-Path {{win_output_dir}}\win_stat\nested\hard-link.ps1 + - name: test win_stat on hard link file win_stat: path: "{{win_output_dir}}\\win_stat\\nested\\hard-link.ps1" @@ -541,3 +544,13 @@ win_file: path: "{{win_output_dir}}\\win_stat" state: absent + +- name: get stat of pagefile + win_stat: + path: C:\pagefile.sys + register: pagefile_stat + +- name: assert get stat of pagefile + assert: + that: + - pagefile_stat.stat.exists == True diff --git a/test/integration/targets/windows-paths/tasks/main.yml b/test/integration/targets/windows-paths/tasks/main.yml index 075428980ca..9610f39339d 100644 --- a/test/integration/targets/windows-paths/tasks/main.yml +++ b/test/integration/targets/windows-paths/tasks/main.yml @@ -82,7 +82,7 @@ - trailing_result|success - trailing_result.stat.attributes == 'Directory' - trailing_result.stat.exists == true - - trailing_result.stat.path == trailing + - trailing_result.stat.path == no_quotes_single # path is without the trailing \ - name: Set variables in key=value syntax set_fact: @@ -178,7 +178,7 @@ - trailing_result|success - trailing_result.stat.attributes == 'Directory' - trailing_result.stat.exists == true - - trailing_result.stat.path == trailing + - trailing_result.stat.path == no_quotes_single # path is without the trailing \ - name: Test tab path {{ tab }} win_stat: