Merge pull request ansible#12047 from cchurch/powershell_common_cleanup

Add PowerShell exception handling and turn on strict mode.

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

lib/ansible/plugins/connections/winrm.py

@@ -167,7 +167,7 @@ def exec_command(self, cmd, tmp_path, in_data=None, sudoable=True):
         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))

lib/ansible/plugins/shell/powershell.py

@@ -112,12 +112,41 @@ def build_module_command(self, env_string, shebang, cmd, rm_tmp=None):
             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 @@ def _escape(self, value, include_vars=False):
         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]

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 <http://www.gnu.org/licenses/>.
+
+# 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;

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 <http://www.gnu.org/licenses/>.
+
+# 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;

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 <http://www.gnu.org/licenses/>.
+
+# 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;

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 <http://www.gnu.org/licenses/>.
+
+# 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;

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 <http://www.gnu.org/licenses/>.
+
+# 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;

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'"