Add support for Windows hosts in the SSH connection plugin (#47732)

* Add support for Windows hosts in the SSH connection plugin

* fix Python 2.6 unit test and sanity issues

* fix up connection tests in CI, disable SCP for now

* ensure we don't pollute the existing environment during the test

* Add connection_windows_ssh to classifier

* use test dir for inventory file

* Required powershell as default shell and fix tests

* Remove exlicit become_methods on connection

* clarify console encoding comment

* ignore recent SCP errors in integration tests

* Add cmd shell type and added more tests

* Fix some doc issues

* revises windows faq

* add anchors for windows links

* revises windows setup page

* Update changelogs/fragments/windows-ssh.yaml

Co-Authored-By: jborean93 <jborean93@gmail.com>

test/integration/targets/connection_windows_ssh/aliases

@@ -0,0 +1,6 @@
+windows
+shippable/windows/group1
+shippable/windows/smoketest
+skip/windows/2008  # Windows Server 2008 does not support Win32-OpenSSH
+needs/target/connection
+needs/target/setup_remote_tmp_dir

test/integration/targets/connection_windows_ssh/runme.sh

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+
+set -eux
+
+# We need to run these tests with both the powershell and cmd shell type
+
+### cmd tests - no DefaultShell set ###
+ansible -i ../../inventory.winrm localhost \
+    -m template \
+    -a "src=test_connection.inventory.j2 dest=~/ansible_testing/test_connection.inventory" \
+    -e "test_shell_type=cmd" \
+    "$@"
+
+# https://github.com/PowerShell/Win32-OpenSSH/wiki/DefaultShell
+ansible -i ../../inventory.winrm windows \
+    -m win_regedit \
+    -a "path=HKLM:\\\\SOFTWARE\\\\OpenSSH name=DefaultShell state=absent" \
+    "$@"
+
+# Need to flush the connection to ensure we get a new shell for the next tests
+ansible -i ~/ansible_testing/test_connection.inventory windows-ssh \
+    -m meta -a "reset_connection" \
+    "$@"
+
+# sftp
+./windows.sh "$@"
+# scp
+ANSIBLE_SCP_IF_SSH=true ./windows.sh "$@"
+# other tests not part of the generic connection test framework
+ansible-playbook -i ~/ansible_testing/test_connection.inventory tests.yml \
+    "$@"
+
+### powershell tests - explicit DefaultShell set ###
+# we do this last as the default shell on our CI instances is set to PowerShell
+ansible -i ../../inventory.winrm localhost \
+    -m template \
+    -a "src=test_connection.inventory.j2 dest=~/ansible_testing/test_connection.inventory" \
+    -e "test_shell_type=powershell" \
+    "$@"
+
+# ensure the default shell is set to PowerShell
+ansible -i ../../inventory.winrm windows \
+    -m win_regedit \
+    -a "path=HKLM:\\\\SOFTWARE\\\\OpenSSH name=DefaultShell data=C:\\\\Windows\\\\System32\\\\WindowsPowerShell\\\\v1.0\\\\powershell.exe" \
+    "$@"
+
+ansible -i ~/ansible_testing/test_connection.inventory windows-ssh \
+    -m meta -a "reset_connection" \
+    "$@"
+
+./windows.sh "$@"
+ANSIBLE_SCP_IF_SSH=true ./windows.sh "$@"
+ansible-playbook -i ~/ansible_testing/test_connection.inventory tests.yml \
+    "$@"

test/integration/targets/connection_windows_ssh/test_connection.inventory.j2

@@ -0,0 +1,12 @@
+[windows-ssh]
+{% for host in vars.groups.winrm %}
+{{ host }}  ansible_host={{ hostvars[host]['ansible_host'] }} ansible_user={{ hostvars[host]['ansible_user'] }}{{ ' ansible_ssh_private_key_file=' ~ hostvars[host]['ansible_ssh_private_key_file'] if (hostvars[host]['ansible_ssh_private_key_file']|default()) else '' }}
+{% endfor %}
+
+[windows-ssh:vars]
+ansible_shell_type={{ test_shell_type }}
+ansible_connection=ssh
+ansible_port=22
+# used to preserve the existing environment and not touch existing files
+ansible_ssh_extra_args="-o UserKnownHostsFile=/dev/null"
+ansible_ssh_host_key_checking=False

test/integration/targets/connection_windows_ssh/tests.yml

@@ -0,0 +1,32 @@
+---
+- name: test out Windows SSH specific tests
+  hosts: windows-ssh
+  serial: 1
+  gather_facts: no
+
+  tasks:
+  - name: test out become with Windows SSH
+    win_whoami:
+    register: win_ssh_become
+    become: yes
+    become_method: runas
+    become_user: SYSTEM
+
+  - name: assert test out become with Windows SSH
+    assert:
+      that:
+      - win_ssh_become.account.sid == "S-1-5-18"
+
+  - name: test out async with Windows SSH
+    win_shell: Write-Host café
+    async: 20
+    poll: 3
+    register: win_ssh_async
+
+  - name: assert test out async with Windows SSH
+    assert:
+      that:
+      - win_ssh_async is changed
+      - win_ssh_async.rc == 0
+      - win_ssh_async.stdout == "café\n"
+      - win_ssh_async.stderr == ""

test/integration/targets/connection_windows_ssh/tests_fetch.yml

@@ -0,0 +1,41 @@
+# This must be a play as we need to invoke it with the ANSIBLE_SCP_IF_SSH env
+# to control the mechanism used. Unfortunately while ansible_scp_if_ssh is
+# documented, it isn't actually used hence the separate invocation
+---
+- name: further fetch tests with metachar characters in filename
+  hosts: windows-ssh
+  force_handlers: yes
+  serial: 1
+  gather_facts: no
+
+  tasks:
+  - name: setup remote tmp dir
+    import_role:
+      name: ../../setup_remote_tmp_dir
+
+  - name: create remote file with metachar in name
+    win_copy:
+      content: some content
+      dest: '{{ remote_tmp_dir }}\file ^with &whoami'
+
+  - name: test fetch against a file with cmd metacharacters
+    block:
+    - name: fetch file with metachar in name
+      fetch:
+        src: '{{ remote_tmp_dir }}\file ^with &whoami'
+        dest: ansible-test.txt
+        flat: yes
+      register: fetch_res
+
+    - name: assert fetch file with metachar in name
+      assert:
+        that:
+        - fetch_res is changed
+        - fetch_res.checksum == '94e66df8cd09d410c62d9e0dc59d3a884e458e05'
+
+    always:
+    - name: remove local copy of file
+      file:
+        path: ansible-test.txt
+        state: absent
+      delegate_to: localhost

test/integration/targets/connection_windows_ssh/windows.sh

@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+
+set -eux
+
+cd ../connection
+
+# A recent patch to OpenSSH causes a validation error when running through Ansible. It seems like if the path is quoted
+# then it will fail with 'protocol error: filename does not match request'. We currently ignore this by setting
+# 'ansible_scp_extra_args=-T' to ignore this check but this should be removed once that bug is fixed and our test
+# container has been updated.
+# https://unix.stackexchange.com/questions/499958/why-does-scps-strict-filename-checking-reject-quoted-last-component-but-not-oth
+# https://github.com/openssh/openssh-portable/commit/391ffc4b9d31fa1f4ad566499fef9176ff8a07dc
+INVENTORY=~/ansible_testing/test_connection.inventory ./test.sh \
+    -e target_hosts=windows-ssh \
+    -e action_prefix=win_ \
+    -e local_tmp=/tmp/ansible-local \
+    -e remote_tmp=c:/windows/temp/ansible-remote \
+    -e ansible_scp_extra_args=-T \
+    "$@"
+
+cd ../connection_windows_ssh
+
+ansible-playbook -i ~/ansible_testing/test_connection.inventory tests_fetch.yml \
+    -e ansible_scp_extra_args=-T \
+    "$@"

test/runner/lib/classification.py

@@ -474,6 +474,11 @@ class PathMapper(object):
             if integration_name not in self.integration_targets_by_name:
                 integration_name = None
 
+            windows_integration_name = 'connection_windows_%s' % name
+
+            if windows_integration_name not in self.integration_targets_by_name:
+                windows_integration_name = None
+
             # entire integration test commands depend on these connection plugins
 
             if name in ['winrm', 'psrp']:
@@ -506,6 +511,7 @@ class PathMapper(object):
 
             return {
                 'integration': integration_name,
+                'windows-integration': windows_integration_name,
                 'units': units_path,
             }
 

test/runner/lib/executor.py

@@ -694,6 +694,10 @@ def windows_inventory(remotes):
             ansible_port=remote.connection.port,
         )
 
+        # used for the connection_windows_ssh test target
+        if remote.ssh_key:
+            options["ansible_ssh_private_key_file"] = os.path.abspath(remote.ssh_key.key)
+
         hosts.append(
             '%s %s' % (
                 remote.name.replace('/', '_'),

test/units/plugins/shell/test_cmd.py

@@ -0,0 +1,16 @@
+import pytest
+
+from ansible.plugins.shell.cmd import ShellModule
+
+
+@pytest.mark.parametrize('s, expected', [
+    ['arg1', 'arg1'],
+    [None, '""'],
+    ['arg1 and 2', '^"arg1 and 2^"'],
+    ['malicious argument\\"&whoami', '^"malicious argument\\^"^&whoami^"'],
+    ['C:\\temp\\some ^%file% > nul', '^"C:\\temp\\some ^^^%file^% ^> nul^"']
+])
+def test_quote_args(s, expected):
+    cmd = ShellModule()
+    actual = cmd.quote(s)
+    assert actual == expected

test/units/plugins/shell/test_powershell.py

@@ -0,0 +1,53 @@
+from ansible.plugins.shell.powershell import _parse_clixml
+
+
+def test_parse_clixml_empty():
+    empty = b'#< CLIXML\r\n<Objs Version="1.1.0.1" xmlns="http://schemas.microsoft.com/powershell/2004/04"></Objs>'
+    expected = b''
+    actual = _parse_clixml(empty)
+    assert actual == expected
+
+
+def test_parse_clixml_with_progress():
+    progress = b'#< CLIXML\r\n<Objs Version="1.1.0.1" xmlns="http://schemas.microsoft.com/powershell/2004/04">' \
+               b'<Obj S="progress" RefId="0"><TN RefId="0"><T>System.Management.Automation.PSCustomObject</T><T>System.Object</T></TN><MS>' \
+               b'<I64 N="SourceId">1</I64><PR N="Record"><AV>Preparing modules for first use.</AV><AI>0</AI><Nil />' \
+               b'<PI>-1</PI><PC>-1</PC><T>Completed</T><SR>-1</SR><SD> </SD></PR></MS></Obj></Objs>'
+    expected = b''
+    actual = _parse_clixml(progress)
+    assert actual == expected
+
+
+def test_parse_clixml_single_stream():
+    single_stream = b'#< CLIXML\r\n<Objs Version="1.1.0.1" xmlns="http://schemas.microsoft.com/powershell/2004/04">' \
+                    b'<S S="Error">fake : The term \'fake\' is not recognized as the name of a cmdlet. Check _x000D__x000A_</S>' \
+                    b'<S S="Error">the spelling of the name, or if a path was included._x000D__x000A_</S>' \
+                    b'<S S="Error">At line:1 char:1_x000D__x000A_</S>' \
+                    b'<S S="Error">+ fake cmdlet_x000D__x000A_</S><S S="Error">+ ~~~~_x000D__x000A_</S>' \
+                    b'<S S="Error">    + CategoryInfo          : ObjectNotFound: (fake:String) [], CommandNotFoundException_x000D__x000A_</S>' \
+                    b'<S S="Error">    + FullyQualifiedErrorId : CommandNotFoundException_x000D__x000A_</S><S S="Error"> _x000D__x000A_</S>' \
+                    b'</Objs>'
+    expected = b"fake : The term 'fake' is not recognized as the name of a cmdlet. Check \r\n" \
+               b"the spelling of the name, or if a path was included.\r\n" \
+               b"At line:1 char:1\r\n" \
+               b"+ fake cmdlet\r\n" \
+               b"+ ~~~~\r\n" \
+               b"    + CategoryInfo          : ObjectNotFound: (fake:String) [], CommandNotFoundException\r\n" \
+               b"    + FullyQualifiedErrorId : CommandNotFoundException\r\n "
+    actual = _parse_clixml(single_stream)
+    assert actual == expected
+
+
+def test_parse_clixml_multiple_streams():
+    multiple_stream = b'#< CLIXML\r\n<Objs Version="1.1.0.1" xmlns="http://schemas.microsoft.com/powershell/2004/04">' \
+                      b'<S S="Error">fake : The term \'fake\' is not recognized as the name of a cmdlet. Check _x000D__x000A_</S>' \
+                      b'<S S="Error">the spelling of the name, or if a path was included._x000D__x000A_</S>' \
+                      b'<S S="Error">At line:1 char:1_x000D__x000A_</S>' \
+                      b'<S S="Error">+ fake cmdlet_x000D__x000A_</S><S S="Error">+ ~~~~_x000D__x000A_</S>' \
+                      b'<S S="Error">    + CategoryInfo          : ObjectNotFound: (fake:String) [], CommandNotFoundException_x000D__x000A_</S>' \
+                      b'<S S="Error">    + FullyQualifiedErrorId : CommandNotFoundException_x000D__x000A_</S><S S="Error"> _x000D__x000A_</S>' \
+                      b'<S S="Info">hi info</S>' \
+                      b'</Objs>'
+    expected = b"hi info"
+    actual = _parse_clixml(multiple_stream, stream="Info")
+    assert actual == expected