From 6f9f337a67d228cb1ed7a805c74a4085bbd86410 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 17 Jan 2018 14:16:34 +1000 Subject: [PATCH] standardise the powershell FileUtils (#34969) --- .../Ansible.ModuleUtils.FileUtil.psm1 | 54 ++++++++------ lib/ansible/modules/windows/win_command.ps1 | 4 +- lib/ansible/modules/windows/win_shell.ps1 | 4 +- lib/ansible/modules/windows/win_stat.ps1 | 15 ++-- lib/ansible/modules/windows/win_wait_for.ps1 | 4 +- .../library/file_util_test.ps1 | 74 ++++++++++--------- .../targets/win_stat/tasks/main.yml | 2 +- .../targets/win_stat/tasks/tests.yml | 2 +- 8 files changed, 86 insertions(+), 73 deletions(-) diff --git a/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 b/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 index f1f954d129..2e905593eb 100644 --- a/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 +++ b/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1 @@ -9,33 +9,39 @@ 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 { +Function Test-AnsiblePath { + [CmdletBinding()] + Param( + [Parameter(Mandatory=$true)][string]$Path + ) + # Replacement for Test-Path + try { + $file_attributes = [System.IO.File]::GetAttributes($Path) + } catch [System.IO.FileNotFoundException] { 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" + if ([Int32]$file_attributes -eq -1) { + return $false + } else { + return $true } - - return $file } -Export-ModuleMember -Function Test-FilePath, Get-FileItem +Function Get-AnsibleItem { + [CmdletBinding()] + Param( + [Parameter(Mandatory=$true)][string]$Path + ) + # Replacement for Get-Item + $file_attributes = [System.IO.File]::GetAttributes($Path) + if ([Int32]$file_attributes -eq -1) { + throw New-Object -TypeName System.Management.Automation.ItemNotFoundException -ArgumentList "Cannot find path '$Path' because it does not exist." + } elseif ($file_attributes.HasFlag([System.IO.FileAttributes]::Directory)) { + return New-Object -TypeName System.IO.DirectoryInfo -ArgumentList $Path + } else { + return New-Object -TypeName System.IO.FileInfo -ArgumentList $Path + } +} + +Export-ModuleMember -Function Test-AnsiblePath, Get-AnsibleItem diff --git a/lib/ansible/modules/windows/win_command.ps1 b/lib/ansible/modules/windows/win_command.ps1 index ff98771b10..5d88374f55 100644 --- a/lib/ansible/modules/windows/win_command.ps1 +++ b/lib/ansible/modules/windows/win_command.ps1 @@ -28,11 +28,11 @@ $result = @{ cmd = $raw_command_line } -if ($creates -and $(Test-FilePath -path $creates)) { +if ($creates -and $(Test-AnsiblePath -Path $creates)) { Exit-Json @{msg="skipped, since $creates exists";cmd=$raw_command_line;changed=$false;skipped=$true;rc=0} } -if ($removes -and -not $(Test-FilePath -path $removes)) { +if ($removes -and -not $(Test-AnsiblePath -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 6605190494..9120df02d2 100644 --- a/lib/ansible/modules/windows/win_shell.ps1 +++ b/lib/ansible/modules/windows/win_shell.ps1 @@ -56,11 +56,11 @@ $result = @{ cmd = $raw_command_line } -if ($creates -and $(Test-FilePath -path $creates)) { +if ($creates -and $(Test-AnsiblePath -Path $creates)) { Exit-Json @{msg="skipped, since $creates exists";cmd=$raw_command_line;changed=$false;skipped=$true;rc=0} } -if ($removes -and -not $(Test-FilePath -path $removes)) { +if ($removes -and -not $(Test-AnsiblePath -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 37f9d32190..b705b8db8b 100644 --- a/lib/ansible/modules/windows/win_stat.ps1 +++ b/lib/ansible/modules/windows/win_stat.ps1 @@ -34,7 +34,7 @@ if (Get-Member -inputobject $params -name "get_md5") { Add-DepreactionWarning -obj $result -message "get_md5 has been deprecated along with the md5 return value, use get_checksum=True and checksum_algorithm=md5 instead" -version 2.9 } -$info = Get-FileItem -path $path +$info = Get-AnsibleItem -Path $path -ErrorAction SilentlyContinue If ($info -ne $null) { $epoch_date = Get-Date -Date "01/01/1970" $attributes = @() @@ -74,7 +74,7 @@ If ($info -ne $null) { $stat.owner = $info.GetAccessControl().Owner # values that are set according to the type of file - if ($info.PSIsContainer) { + if ($info.Attributes.HasFlag([System.IO.FileAttributes]::Directory)) { $stat.isdir = $true $share_info = Get-WmiObject -Class Win32_Share -Filter "Path='$($stat.path -replace '\\', '\\')'" if ($share_info -ne $null) { @@ -82,11 +82,14 @@ If ($info -ne $null) { $stat.sharename = $share_info.Name } - $dir_files_sum = Get-ChildItem $stat.path -Recurse | Measure-Object -property length -sum - if ($dir_files_sum -eq $null) { + try { + $size = 0 + foreach ($file in $info.EnumerateFiles("*", [System.IO.SearchOption]::AllDirectories)) { + $size += $file.Length + } + $stat.size = $size + } catch { $stat.size = 0 - } else { - $stat.size = $dir_files_sum.Sum } } else { $stat.extension = $info.Extension diff --git a/lib/ansible/modules/windows/win_wait_for.ps1 b/lib/ansible/modules/windows/win_wait_for.ps1 index 63bb3b1886..3eaac979c9 100644 --- a/lib/ansible/modules/windows/win_wait_for.ps1 +++ b/lib/ansible/modules/windows/win_wait_for.ps1 @@ -119,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-FilePath -path $path) { + if (Test-AnsiblePath -Path $path) { if ($search_regex -eq $null) { $complete = $true break @@ -150,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-FilePath -path $path) { + if (Test-AnsiblePath -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_module_utils/library/file_util_test.ps1 b/test/integration/targets/win_module_utils/library/file_util_test.ps1 index 3626c91bca..e8f101428c 100644 --- a/test/integration/targets/win_module_utils/library/file_util_test.ps1 +++ b/test/integration/targets/win_module_utils/library/file_util_test.ps1 @@ -3,57 +3,61 @@ #Requires -Module Ansible.ModuleUtils.Legacy #Requires -Module Ansible.ModuleUtils.FileUtil +$ErrorActionPreference = "Stop" + +$result = @{ + changed = $false +} + Function Assert-Equals($actual, $expected) { if ($actual -cne $expected) { - Fail-Json @{} "actual != expected`nActual: $actual`nExpected: $expected" + $call_stack = (Get-PSCallStack)[1] + $error_msg = "AssertionError:`r`nActual: `"$actual`" != Expected: `"$expected`"`r`nLine: $($call_stack.ScriptLineNumber), Method: $($call_stack.Position.Text)" + Fail-Json -obj $result -message $error_msg } } -# Test-FilePath Hidden system file -$actual = Test-FilePath -path C:\pagefile.sys +# Test-AnsiblePath Hidden system file +$actual = Test-AnsiblePath -Path C:\pagefile.sys Assert-Equals -actual $actual -expected $true -# Test-FilePath File that doesn't exist -$actual = Test-FilePath -path C:\fakefile +# Test-AnsiblePath File that doesn't exist +$actual = Test-AnsiblePath -Path C:\fakefile Assert-Equals -actual $actual -expected $false -# Test-FilePath Normal directory -$actual = Test-FilePath -path C:\Windows +# Test-AnsiblePath Normal directory +$actual = Test-AnsiblePath -Path C:\Windows Assert-Equals -actual $actual -expected $true -# Test-FilePath Normal file -$actual = Test-FilePath -path C:\Windows\System32\kernel32.dll +# Test-AnsiblePath Normal file +$actual = Test-AnsiblePath -Path C:\Windows\System32\kernel32.dll -# Test-FilePath fails with wildcard +# Test-AnsiblePath fails with wildcard +$failed = $false try { - Test-FilePath -Path C:\Windows\*.exe - Fail-Json @{} "exception was not thrown with wildcard search for Test-FilePath" + Test-AnsiblePath -Path C:\Windows\*.exe } catch { - Assert-Equals -actual $_.Exception.Message -expected "found multiple files at path 'C:\Windows\*.exe', make sure no wildcards are set in the path" + $failed = $true + Assert-Equals -actual $_.Exception.Message -expected "Exception calling `"GetAttributes`" with `"1`" argument(s): `"Illegal characters in path.`"" } +Assert-Equals -actual $failed -expected $true -# 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 +# Get-AnsibleItem doesn't exist with -ErrorAction SilentlyContinue param +$actual = Get-AnsibleItem -Path C:\fakefile -ErrorAction SilentlyContinue 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' } +# Get-AnsibleItem file +$actual = Get-AnsibleItem -Path C:\pagefile.sys +Assert-Equals -actual $actual.FullName -expected C:\pagefile.sys +Assert-Equals -actual $actual.Attributes.HasFlag([System.IO.FileAttributes]::Directory) -expected $false +Assert-Equals -actual $actual.Exists -expected $true + +# Get-AnsibleItem directory +$actual = Get-AnsibleItem -Path C:\Windows +Assert-Equals -actual $actual.FullName -expected C:\Windows +Assert-Equals -actual $actual.Attributes.HasFlag([System.IO.FileAttributes]::Directory) -expected $true +Assert-Equals -actual $actual.Exists -expected $true + +$result.data = "success" +Exit-Json -obj $result diff --git a/test/integration/targets/win_stat/tasks/main.yml b/test/integration/targets/win_stat/tasks/main.yml index d61097b6da..51d2aa4da0 100644 --- a/test/integration/targets/win_stat/tasks/main.yml +++ b/test/integration/targets/win_stat/tasks/main.yml @@ -107,7 +107,7 @@ name: '{{win_stat_user}}' state: absent - - name: ensure testy user profile is deleted + - name: ensure test user profile is deleted win_shell: rmdir /S /Q {{profile_dir_out.stdout_lines[0]}} args: executable: cmd.exe diff --git a/test/integration/targets/win_stat/tasks/tests.yml b/test/integration/targets/win_stat/tasks/tests.yml index a702ddbba5..21a24d19bb 100644 --- a/test/integration/targets/win_stat/tasks/tests.yml +++ b/test/integration/targets/win_stat/tasks/tests.yml @@ -196,7 +196,7 @@ - stat_directory.stat.nlink == 1 - stat_directory.stat.owner == 'BUILTIN\Administrators' - stat_directory.stat.path == win_stat_dir + '\\nested' - - stat_directory.stat.size == 21 + - stat_directory.stat.size == 24 - name: test win_stat on empty directory win_stat: