winrm connection tweaks for pywinrm (#15584)

added warnings for invalid kwargs
sniff supported authtypes (for new pywinrm)
use default authtypes (for old pywinrm)
error on unsupported authtype
allow no username/password to be specified (kerb SSO)
tested w/ old and new pywinrm
hacky CLIXML parsing of stderr

lib/ansible/plugins/connection/winrm.py

@@ -29,8 +29,9 @@ import xmltodict
 
 from ansible.compat.six.moves.urllib.parse import urlunsplit
 
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleError, AnsibleConnectionFailure
 try:
+    import winrm
     from winrm import Response
     from winrm.exceptions import WinRMTransportError
     from winrm.protocol import Protocol
@@ -74,7 +75,7 @@ class Connection(ConnectionBase):
         self.delegate         = None
         self._shell_type      = 'powershell'
 
-        # TODO: Add runas support
+        # FUTURE: Add runas support
 
         super(Connection, self).__init__(*args, **kwargs)
 
@@ -91,15 +92,16 @@ class Connection(ConnectionBase):
         self._winrm_user = self._play_context.remote_user
         self._winrm_pass = self._play_context.password
 
-        if '@' in self._winrm_user:
-            self._winrm_realm = self._winrm_user.split('@', 1)[1].strip() or None
+        if hasattr(winrm, 'FEATURE_SUPPORTED_AUTHTYPES'):
+            self._winrm_supported_authtypes = set(winrm.FEATURE_SUPPORTED_AUTHTYPES)
         else:
-            self._winrm_realm = None
-        self._winrm_realm = host_vars.get('ansible_winrm_realm', self._winrm_realm) or None
+            # for legacy versions of pywinrm, use the values we know are supported
+            self._winrm_supported_authtypes = set(['plaintext','ssl','kerberos'])
 
+        # TODO: figure out what we want to do with auto-transport selection in the face of NTLM/Kerb/CredSSP/Cert/Basic
         transport_selector = 'ssl' if self._winrm_scheme == 'https' else 'plaintext'
 
-        if HAVE_KERBEROS and ('@' in self._winrm_user or self._winrm_realm):
+        if HAVE_KERBEROS and ((self._winrm_user and '@' in self._winrm_user)):
             self._winrm_transport = 'kerberos,%s' % transport_selector
         else:
             self._winrm_transport = transport_selector
@@ -107,13 +109,27 @@ class Connection(ConnectionBase):
         if isinstance(self._winrm_transport, basestring):
             self._winrm_transport = [x.strip() for x in self._winrm_transport.split(',') if x.strip()]
 
-        self._winrm_kwargs = dict(username=self._winrm_user, password=self._winrm_pass, realm=self._winrm_realm)
+        unsupported_transports = set(self._winrm_transport).difference(self._winrm_supported_authtypes)
+
+        if unsupported_transports:
+            raise AnsibleError('The installed version of WinRM does not support transport(s) %s' % list(unsupported_transports))
+
+        self._winrm_kwargs = dict(username=self._winrm_user, password=self._winrm_pass)
         argspec = inspect.getargspec(Protocol.__init__)
-        for arg in argspec.args:
-            if arg in ('self', 'endpoint', 'transport', 'username', 'password', 'realm'):
-                continue
-            if 'ansible_winrm_%s' % arg in host_vars:
-                self._winrm_kwargs[arg] = host_vars['ansible_winrm_%s' % arg]
+        supported_winrm_args = set(argspec.args)
+        passed_winrm_args = set([v.replace('ansible_winrm_', '') for v in host_vars if v.startswith('ansible_winrm_')])
+        unsupported_args = passed_winrm_args.difference(supported_winrm_args)
+
+        # warn for kwargs unsupported by the installed version of pywinrm
+        for arg in unsupported_args:
+            display.warning("ansible_winrm_{0} unsupported by pywinrm (are you running the right pywinrm version?)".format(arg))
+
+        # arg names we're going passing directly
+        internal_kwarg_mask = set(['self', 'endpoint', 'transport', 'username', 'password'])
+
+        # pass through matching kwargs, excluding the list we want to treat specially
+        for arg in passed_winrm_args.difference(internal_kwarg_mask).intersection(supported_winrm_args):
+            self._winrm_kwargs[arg] = host_vars['ansible_winrm_%s' % arg]
 
     def _winrm_connect(self):
         '''
@@ -131,7 +147,13 @@ class Connection(ConnectionBase):
             display.vvvvv('WINRM CONNECT: transport=%s endpoint=%s' % (transport, endpoint), host=self._winrm_host)
             try:
                 protocol = Protocol(endpoint, transport=transport, **self._winrm_kwargs)
-                protocol.send_message('')
+                # send keepalive message to ensure we're awake
+                # TODO: is this necessary?
+                # protocol.send_message(xmltodict.unparse(rq))
+                if not self.shell_id:
+                    self.shell_id = protocol.open_shell(codepage=65001) # UTF-8
+                    display.vvvvv('WINRM OPEN SHELL: %s' % self.shell_id, host=self._winrm_host)
+
                 return protocol
             except Exception as e:
                 err_msg = to_unicode(e).strip()
@@ -147,7 +169,7 @@ class Connection(ConnectionBase):
                 errors.append(u'%s: %s' % (transport, err_msg))
                 display.vvvvv(u'WINRM CONNECTION ERROR: %s\n%s' % (err_msg, to_unicode(traceback.format_exc())), host=self._winrm_host)
         if errors:
-            raise AnsibleError(', '.join(map(to_str, errors)))
+            raise AnsibleConnectionFailure(', '.join(map(to_str, errors)))
         else:
             raise AnsibleError('No transport found for WinRM connection')
 
@@ -169,9 +191,6 @@ class Connection(ConnectionBase):
         if not self.protocol:
             self.protocol = self._winrm_connect()
             self._connected = True
-        if not self.shell_id:
-            self.shell_id = self.protocol.open_shell(codepage=65001) # UTF-8
-            display.vvvvv('WINRM OPEN SHELL: %s' % self.shell_id, host=self._winrm_host)
         if from_exec:
             display.vvvvv("WINRM EXEC %r %r" % (command, args), host=self._winrm_host)
         else:
@@ -186,12 +205,20 @@ class Connection(ConnectionBase):
                 if stdin_iterator:
                     for (data, is_last) in stdin_iterator:
                         self._winrm_send_input(self.protocol, self.shell_id, command_id, data, eof=is_last)
-            except:
+
+            except Exception as ex:
+                from traceback import format_exc
+                display.warning("FATAL ERROR DURING FILE TRANSFER: %s" % format_exc(ex))
                 stdin_push_failed = True
 
-            # NB: this could hang if the receiver is still running (eg, network failed a Send request but the server's still happy).
+            if stdin_push_failed:
+                raise AnsibleError('winrm send_input failed')
+
+            # NB: this can hang if the receiver is still running (eg, network failed a Send request but the server's still happy).
             # FUTURE: Consider adding pywinrm status check/abort operations to see if the target is still running after a failure.
             response = Response(self.protocol.get_command_output(self.shell_id, command_id))
+
+            # TODO: check result from response and set stdin_push_failed if we have nonzero
             if from_exec:
                 display.vvvvv('WINRM RESULT %r' % to_unicode(response), host=self._winrm_host)
             else:
@@ -199,6 +226,7 @@ class Connection(ConnectionBase):
             display.vvvvvv('WINRM STDOUT %s' % to_unicode(response.std_out), host=self._winrm_host)
             display.vvvvvv('WINRM STDERR %s' % to_unicode(response.std_err), host=self._winrm_host)
 
+
             if stdin_push_failed:
                 raise AnsibleError('winrm send_input failed; \nstdout: %s\nstderr %s' % (response.std_out, response.std_err))
 
@@ -245,11 +273,30 @@ class Connection(ConnectionBase):
             result = self._winrm_exec(cmd_parts[0], cmd_parts[1:], from_exec=True)
         except Exception:
             traceback.print_exc()
-            raise AnsibleError("failed to exec cmd %s" % cmd)
+            raise AnsibleConnectionFailure("failed to exec cmd %s" % cmd)
         result.std_out = to_bytes(result.std_out)
         result.std_err = to_bytes(result.std_err)
+
+        # parse just stderr from CLIXML output
+        if self.is_clixml(result.std_err):
+            try:
+                result.std_err = self.parse_clixml_stream(result.std_err)
+            except:
+                # unsure if we're guaranteed a valid xml doc- keep original output just in case
+                pass
+
         return (result.status_code, result.std_out, result.std_err)
 
+    def is_clixml(self, value):
+        return value.startswith("#< CLIXML")
+
+    # hacky way to get just stdout- not always sure of doc framing here, so use with care
+    def parse_clixml_stream(self, clixml_doc, stream_name='Error'):
+        clear_xml = clixml_doc.replace('#< CLIXML\r\n', '')
+        doc = xmltodict.parse(clear_xml)
+        lines = [l.get('#text', '') for l in doc.get('Objs', {}).get('S', {}) if l.get('@S') == stream_name]
+        return '\r\n'.join(lines)
+
     # FUTURE: determine buffer size at runtime via remote winrm config?
     def _put_file_stdin_iterator(self, in_path, out_path, buffer_size=250000):
         in_size = os.path.getsize(to_bytes(in_path, errors='strict'))