From 4b2cdadc98d823350d51cacb6e814b03067298b4 Mon Sep 17 00:00:00 2001 From: Chris Church Date: Sat, 22 Aug 2015 18:19:43 -0400 Subject: [PATCH] Add PowerShell exception handling and turn on strict mode. * Add exception handling when running PowerShell modules to provide exception message and stack trace. * Enable strict mode for all PowerShell modules and internal commands. * Update common PowerShell code to fix strict mode errors. * Fix an issue with Set-Attr where it would not replace an existing property if already set. * Add tests for exception handling using modified win_ping modules. --- lib/ansible/module_utils/powershell.ps1 | 45 +++++++++---- lib/ansible/plugins/connections/winrm.py | 2 +- lib/ansible/plugins/shell/powershell.py | 45 +++++++++++-- .../library/win_ping_set_attr.ps1 | 31 +++++++++ .../library/win_ping_strict_mode_error.ps1 | 30 +++++++++ .../library/win_ping_syntax_error.ps1 | 30 +++++++++ .../test_win_ping/library/win_ping_throw.ps1 | 30 +++++++++ .../library/win_ping_throw_string.ps1 | 30 +++++++++ .../roles/test_win_ping/tasks/main.yml | 65 +++++++++++++++++++ 9 files changed, 286 insertions(+), 22 deletions(-) create mode 100644 test/integration/roles/test_win_ping/library/win_ping_set_attr.ps1 create mode 100644 test/integration/roles/test_win_ping/library/win_ping_strict_mode_error.ps1 create mode 100644 test/integration/roles/test_win_ping/library/win_ping_syntax_error.ps1 create mode 100644 test/integration/roles/test_win_ping/library/win_ping_throw.ps1 create mode 100644 test/integration/roles/test_win_ping/library/win_ping_throw_string.ps1 diff --git a/lib/ansible/module_utils/powershell.ps1 b/lib/ansible/module_utils/powershell.ps1 index ee659162162..5756d360f1f 100644 --- a/lib/ansible/module_utils/powershell.ps1 +++ b/lib/ansible/module_utils/powershell.ps1 @@ -26,6 +26,8 @@ # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # +Set-StrictMode -Version Latest + # Ansible v2 will insert the module arguments below as a string containing # JSON; assign them to an environment variable and redefine $args so existing # modules will continue to work. @@ -47,7 +49,14 @@ Function Set-Attr($obj, $name, $value) $obj = New-Object psobject } - $obj | Add-Member -Force -MemberType NoteProperty -Name $name -Value $value + Try + { + $obj.$name = $value + } + Catch + { + $obj | Add-Member -Force -MemberType NoteProperty -Name $name -Value $value + } } # Helper function to convert a powershell object to JSON to echo it, exiting @@ -78,7 +87,7 @@ Function Fail-Json($obj, $message = $null) $obj = New-Object psobject } # If the first args is undefined or not an object, make it an object - ElseIf (-not $obj.GetType -or $obj.GetType().Name -ne "PSCustomObject") + ElseIf (-not $obj -or -not $obj.GetType -or $obj.GetType().Name -ne "PSCustomObject") { $obj = New-Object psobject } @@ -94,24 +103,32 @@ Function Fail-Json($obj, $message = $null) # slightly more pythonic # Example: $attr = Get-Attr $response "code" -default "1" #Note that if you use the failifempty option, you do need to specify resultobject as well. -Function Get-Attr($obj, $name, $default = $null,$resultobj, $failifempty=$false, $emptyattributefailmessage) +Function Get-Attr($obj, $name, $default = $null, $resultobj, $failifempty=$false, $emptyattributefailmessage) { - # Check if the provided Member $name exists in $obj and return it or the - # default - If ($obj.$name.GetType) + # Check if the provided Member $name exists in $obj and return it or the default. + Try { + If (-not $obj.$name.GetType) + { + throw + } $obj.$name } - Elseif($failifempty -eq $false) - { - $default - } - else + Catch { - if (!$emptyattributefailmessage) {$emptyattributefailmessage = "Missing required argument: $name"} - Fail-Json -obj $resultobj -message $emptyattributefailmessage + If ($failifempty -eq $false) + { + $default + } + Else + { + If (!$emptyattributefailmessage) + { + $emptyattributefailmessage = "Missing required argument: $name" + } + Fail-Json -obj $resultobj -message $emptyattributefailmessage + } } - return } # Helper filter/pipeline function to convert a value to boolean following current diff --git a/lib/ansible/plugins/connections/winrm.py b/lib/ansible/plugins/connections/winrm.py index 0e19b93ac24..28bac3285a9 100644 --- a/lib/ansible/plugins/connections/winrm.py +++ b/lib/ansible/plugins/connections/winrm.py @@ -167,7 +167,7 @@ class Connection(ConnectionBase): elif '-EncodedCommand' not in cmd_parts: script = ' '.join(cmd_parts) if script: - cmd_parts = self._shell._encode_script(script, as_list=True) + cmd_parts = self._shell._encode_script(script, as_list=True, strict_mode=False) if '-EncodedCommand' in cmd_parts: encoded_cmd = cmd_parts[cmd_parts.index('-EncodedCommand') + 1] decoded_cmd = to_unicode(base64.b64decode(encoded_cmd)) diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index 0e16d34e160..aba3183e768 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -112,12 +112,41 @@ class ShellModule(object): cmd_parts.insert(0, '&') elif shebang and shebang.startswith('#!'): cmd_parts.insert(0, shebang[2:]) - catch = ''' - $_obj = @{ failed = $true; $msg = $_ } - echo $_obj | ConvertTo-Json -Compress -Depth 99 - Exit 1 - ''' - script = 'Try { %s }\nCatch { %s }' % (' '.join(cmd_parts), 'throw') + script = ''' + Try + { + %s + } + Catch + { + $_obj = @{ failed = $true } + If ($_.Exception.GetType) + { + $_obj.Add('msg', $_.Exception.Message) + } + Else + { + $_obj.Add('msg', $_.ToString()) + } + If ($_.InvocationInfo.PositionMessage) + { + $_obj.Add('exception', $_.InvocationInfo.PositionMessage) + } + ElseIf ($_.ScriptStackTrace) + { + $_obj.Add('exception', $_.ScriptStackTrace) + } + Try + { + $_obj.Add('error_record', ($_ | ConvertTo-Json | ConvertFrom-Json)) + } + Catch + { + } + Echo $_obj | ConvertTo-Json -Compress -Depth 99 + Exit 1 + } + ''' % (' '.join(cmd_parts)) if rm_tmp: rm_tmp = self._escape(self._unquote(rm_tmp)) rm_cmd = 'Remove-Item "%s" -Force -Recurse -ErrorAction SilentlyContinue' % rm_tmp @@ -149,9 +178,11 @@ class ShellModule(object): replace = lambda m: substs[m.lastindex - 1] return re.sub(pattern, replace, value) - def _encode_script(self, script, as_list=False): + def _encode_script(self, script, as_list=False, strict_mode=True): '''Convert a PowerShell script to a single base64-encoded command.''' script = to_unicode(script) + if strict_mode: + script = u'Set-StrictMode -Version Latest\r\n%s' % script script = '\n'.join([x.strip() for x in script.splitlines() if x.strip()]) encoded_script = base64.b64encode(script.encode('utf-16-le')) cmd_parts = _common_args + ['-EncodedCommand', encoded_script] diff --git a/test/integration/roles/test_win_ping/library/win_ping_set_attr.ps1 b/test/integration/roles/test_win_ping/library/win_ping_set_attr.ps1 new file mode 100644 index 00000000000..8279b4b414a --- /dev/null +++ b/test/integration/roles/test_win_ping/library/win_ping_set_attr.ps1 @@ -0,0 +1,31 @@ +#!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 . + +# POWERSHELL_COMMON + +$params = Parse-Args $args $true; + +$data = Get-Attr $params "data" "pong"; + +$result = New-Object psobject @{ + changed = $false + ping = "pong" +}; + +# Test that Set-Attr will replace an existing attribute. +Set-Attr $result "ping" $data + +Exit-Json $result; diff --git a/test/integration/roles/test_win_ping/library/win_ping_strict_mode_error.ps1 b/test/integration/roles/test_win_ping/library/win_ping_strict_mode_error.ps1 new file mode 100644 index 00000000000..d498cbcf17c --- /dev/null +++ b/test/integration/roles/test_win_ping/library/win_ping_strict_mode_error.ps1 @@ -0,0 +1,30 @@ +#!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 . + +# POWERSHELL_COMMON + +$params = Parse-Args $args $true; + +$x = $params.thisPropertyDoesNotExist + +$data = Get-Attr $params "data" "pong"; + +$result = New-Object psobject @{ + changed = $false + ping = $data +}; + +Exit-Json $result; diff --git a/test/integration/roles/test_win_ping/library/win_ping_syntax_error.ps1 b/test/integration/roles/test_win_ping/library/win_ping_syntax_error.ps1 new file mode 100644 index 00000000000..6bfe621a804 --- /dev/null +++ b/test/integration/roles/test_win_ping/library/win_ping_syntax_error.ps1 @@ -0,0 +1,30 @@ +#!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 . + +# POWERSHELL_COMMON + +$blah = 'I can't quote my strings correctly.' + +$params = Parse-Args $args $true; + +$data = Get-Attr $params "data" "pong"; + +$result = New-Object psobject @{ + changed = $false + ping = $data +}; + +Exit-Json $result; diff --git a/test/integration/roles/test_win_ping/library/win_ping_throw.ps1 b/test/integration/roles/test_win_ping/library/win_ping_throw.ps1 new file mode 100644 index 00000000000..f0b32186d80 --- /dev/null +++ b/test/integration/roles/test_win_ping/library/win_ping_throw.ps1 @@ -0,0 +1,30 @@ +#!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 . + +# POWERSHELL_COMMON + +throw + +$params = Parse-Args $args $true; + +$data = Get-Attr $params "data" "pong"; + +$result = New-Object psobject @{ + changed = $false + ping = $data +}; + +Exit-Json $result; diff --git a/test/integration/roles/test_win_ping/library/win_ping_throw_string.ps1 b/test/integration/roles/test_win_ping/library/win_ping_throw_string.ps1 new file mode 100644 index 00000000000..e1f3ca60657 --- /dev/null +++ b/test/integration/roles/test_win_ping/library/win_ping_throw_string.ps1 @@ -0,0 +1,30 @@ +#!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 . + +# POWERSHELL_COMMON + +throw "no ping for you" + +$params = Parse-Args $args $true; + +$data = Get-Attr $params "data" "pong"; + +$result = New-Object psobject @{ + changed = $false + ping = $data +}; + +Exit-Json $result; diff --git a/test/integration/roles/test_win_ping/tasks/main.yml b/test/integration/roles/test_win_ping/tasks/main.yml index f17a4a92278..aa5d03c908f 100644 --- a/test/integration/roles/test_win_ping/tasks/main.yml +++ b/test/integration/roles/test_win_ping/tasks/main.yml @@ -79,3 +79,68 @@ - "not win_ping_extra_args_result|failed" - "not win_ping_extra_args_result|changed" - "win_ping_extra_args_result.ping == 'bloop'" + +- name: test modified win_ping that throws an exception + action: win_ping_throw + register: win_ping_throw_result + ignore_errors: true + +- name: check win_ping_throw result + assert: + that: + - "win_ping_throw_result|failed" + - "not win_ping_throw_result|changed" + - "win_ping_throw_result.msg == 'ScriptHalted'" + - "win_ping_throw_result.exception" + - "win_ping_throw_result.error_record" + +- name: test modified win_ping that throws a string exception + action: win_ping_throw_string + register: win_ping_throw_string_result + ignore_errors: true + +- name: check win_ping_throw_string result + assert: + that: + - "win_ping_throw_string_result|failed" + - "not win_ping_throw_string_result|changed" + - "win_ping_throw_string_result.msg == 'no ping for you'" + - "win_ping_throw_string_result.exception" + - "win_ping_throw_string_result.error_record" + +- name: test modified win_ping that has a syntax error + action: win_ping_syntax_error + register: win_ping_syntax_error_result + ignore_errors: true + +- name: check win_ping_syntax_error result + assert: + that: + - "win_ping_syntax_error_result|failed" + - "not win_ping_syntax_error_result|changed" + - "win_ping_syntax_error_result.msg" + - "win_ping_syntax_error_result.exception" + +- name: test modified win_ping that has an error that only surfaces when strict mode is on + action: win_ping_strict_mode_error + register: win_ping_strict_mode_error_result + ignore_errors: true + +- name: check win_ping_strict_mode_error result + assert: + that: + - "win_ping_strict_mode_error_result|failed" + - "not win_ping_strict_mode_error_result|changed" + - "win_ping_strict_mode_error_result.msg" + - "win_ping_strict_mode_error_result.exception" + +- name: test modified win_ping to verify a Set-Attr fix + action: win_ping_set_attr data="fixed" + register: win_ping_set_attr_result + +- name: check win_ping_set_attr_result result + assert: + that: + - "not win_ping_set_attr_result|failed" + - "not win_ping_set_attr_result|changed" + - "win_ping_set_attr_result.ping == 'fixed'"