Remove deprecated scp_if_ssh from SSH connection plugin (#82072)

* removed deprecated scp_if_ssh feature from SSH connection
  plugin

Fixes: #81715

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

changelogs/fragments/scp_if_ssh.yml

@@ -0,0 +1,3 @@
+---
+removed_features:
+  - Remove deprecated ``scp_if_ssh`` from ssh connection plugin (https://github.com/ansible/ansible/issues/81715).

lib/ansible/plugins/connection/ssh.py

@@ -306,10 +306,10 @@ DOCUMENTATION = '''
             version_added: '2.7'
       ssh_transfer_method:
         description:
-            - "Preferred method to use when transferring files over ssh"
+            - Preferred method to use when transferring files over SSH.
-            - Setting to 'smart' (default) will try them in order, until one succeeds or they all fail
+            - Setting to 'smart' (default) will try them in order until one succeeds or they all fail.
-            - For OpenSSH >=9.0 you must add an additional option to enable scp (scp_extra_args="-O")
+            - For OpenSSH >=9.0 you must add an additional option to enable scp (scp_extra_args="-O").
-            - Using 'piped' creates an ssh pipe with C(dd) on either side to copy the data
+            - Using 'piped' creates an SSH pipe with C(dd) on either side to copy the data.
         choices: ['sftp', 'scp', 'piped', 'smart']
         type: string
         env: [{name: ANSIBLE_SSH_TRANSFER_METHOD}]
@@ -318,24 +318,6 @@ DOCUMENTATION = '''
         vars:
             - name: ansible_ssh_transfer_method
               version_added: '2.12'
-      scp_if_ssh:
-        deprecated:
-              why: In favor of the O(ssh_transfer_method) option.
-              version: "2.17"
-              alternatives: O(ssh_transfer_method)
-        default: smart
-        description:
-          - "Preferred method to use when transferring files over SSH."
-          - When set to V(smart), Ansible will try them until one succeeds or they all fail.
-          - If set to V(True), it will force 'scp', if V(False) it will use 'sftp'.
-          - For OpenSSH >=9.0 you must add an additional option to enable scp (C(scp_extra_args="-O"))
-          - This setting will overridden by O(ssh_transfer_method) if set.
-        env: [{name: ANSIBLE_SCP_IF_SSH}]
-        ini:
-        - {key: scp_if_ssh, section: ssh_connection}
-        vars:
-          - name: ansible_scp_if_ssh
-            version_added: '2.7'
       use_tty:
         version_added: '2.5'
         default: true
@@ -403,10 +385,8 @@ from ansible.errors import (
     AnsibleError,
     AnsibleFileNotFound,
 )
-from ansible.errors import AnsibleOptionsError
 from ansible.module_utils.six import PY3, text_type, binary_type
 from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text
-from ansible.module_utils.parsing.convert_bool import BOOLEANS, boolean
 from ansible.plugins.connection import ConnectionBase, BUFSIZE
 from ansible.plugins.shell.powershell import _parse_clixml
 from ansible.utils.display import Display
@@ -1241,10 +1221,9 @@ class Connection(ConnectionBase):
         # Transfer methods to try
         methods = []
 
-        # Use the transfer_method option if set, otherwise use scp_if_ssh
+        # Use the transfer_method option if set
         ssh_transfer_method = self.get_option('ssh_transfer_method')
-        scp_if_ssh = self.get_option('scp_if_ssh')
+        if ssh_transfer_method is None:
-        if ssh_transfer_method is None and scp_if_ssh == 'smart':
             ssh_transfer_method = 'smart'
 
         if ssh_transfer_method is not None:
@@ -1252,20 +1231,6 @@ class Connection(ConnectionBase):
                 methods = smart_methods
             else:
                 methods = [ssh_transfer_method]
-        else:
-            # since this can be a non-bool now, we need to handle it correctly
-            if not isinstance(scp_if_ssh, bool):
-                scp_if_ssh = scp_if_ssh.lower()
-                if scp_if_ssh in BOOLEANS:
-                    scp_if_ssh = boolean(scp_if_ssh, strict=False)
-                elif scp_if_ssh != 'smart':
-                    raise AnsibleOptionsError('scp_if_ssh needs to be one of [smart|True|False]')
-            if scp_if_ssh == 'smart':
-                methods = smart_methods
-            elif scp_if_ssh is True:
-                methods = ['scp']
-            else:
-                methods = ['sftp']
 
         for method in methods:
             returncode = stdout = stderr = None

test/integration/targets/connection_ssh/runme.sh

@@ -66,7 +66,7 @@ fi
 # sftp
 ./posix.sh "$@"
 # scp
-ANSIBLE_SCP_IF_SSH=true ./posix.sh "$@" "${scp_args[@]}"
+ANSIBLE_SSH_TRANSFER_METHOD=scp ./posix.sh "$@" "${scp_args[@]}"
 # piped
 ANSIBLE_SSH_TRANSFER_METHOD=piped ./posix.sh "$@"
 

test/units/plugins/connection/test_ssh.py

@@ -26,7 +26,7 @@ import pytest
 from ansible.errors import AnsibleAuthenticationFailure
 import unittest
 from unittest.mock import patch, MagicMock, PropertyMock
-from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNotFound
+from ansible.errors import AnsibleConnectionFailure
 import shlex
 from ansible.module_utils.common.text.converters import to_bytes
 from ansible.playbook.play_context import PlayContext
@@ -241,11 +241,9 @@ class TestConnectionBaseClass(unittest.TestCase):
         conn.host = "some_host"
 
         conn.set_option('reconnection_retries', 9)
-        conn.set_option('ssh_transfer_method', None)  # unless set to None scp_if_ssh is ignored
+        conn.set_option('ssh_transfer_method', None)  # unless set to None
 
-        # Test with SCP_IF_SSH set to smart
         # Test when SFTP works
-        conn.set_option('scp_if_ssh', 'smart')
         expected_in_data = b' '.join((b'put', to_bytes(shlex.quote('/path/to/in/file')), to_bytes(shlex.quote('/path/to/dest/file')))) + b'\n'
         conn.put_file('/path/to/in/file', '/path/to/dest/file')
         conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False)
@@ -256,35 +254,6 @@ class TestConnectionBaseClass(unittest.TestCase):
         conn._bare_run.assert_called_with('some command to run', None, checkrc=False)
         conn._bare_run.side_effect = None
 
-        # test with SCP_IF_SSH enabled
-        conn.set_option('scp_if_ssh', True)
-        conn.put_file('/path/to/in/file', '/path/to/dest/file')
-        conn._bare_run.assert_called_with('some command to run', None, checkrc=False)
-
-        conn.put_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩')
-        conn._bare_run.assert_called_with('some command to run', None, checkrc=False)
-
-        # test with SCPP_IF_SSH disabled
-        conn.set_option('scp_if_ssh', False)
-        expected_in_data = b' '.join((b'put', to_bytes(shlex.quote('/path/to/in/file')), to_bytes(shlex.quote('/path/to/dest/file')))) + b'\n'
-        conn.put_file('/path/to/in/file', '/path/to/dest/file')
-        conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False)
-
-        expected_in_data = b' '.join((b'put',
-                                      to_bytes(shlex.quote('/path/to/in/file/with/unicode-fö〩')),
-                                      to_bytes(shlex.quote('/path/to/dest/file/with/unicode-fö〩')))) + b'\n'
-        conn.put_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩')
-        conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False)
-
-        # test that a non-zero rc raises an error
-        conn._bare_run.return_value = (1, 'stdout', 'some errors')
-        self.assertRaises(AnsibleError, conn.put_file, '/path/to/bad/file', '/remote/path/to/file')
-
-        # test that a not-found path raises an error
-        mock_ospe.return_value = False
-        conn._bare_run.return_value = (0, 'stdout', '')
-        self.assertRaises(AnsibleFileNotFound, conn.put_file, '/path/to/bad/file', '/remote/path/to/file')
-
     @patch('time.sleep')
     def test_plugins_connection_ssh_fetch_file(self, mock_sleep):
         pc = PlayContext()
@@ -299,11 +268,9 @@ class TestConnectionBaseClass(unittest.TestCase):
         conn.host = "some_host"
 
         conn.set_option('reconnection_retries', 9)
-        conn.set_option('ssh_transfer_method', None)  # unless set to None scp_if_ssh is ignored
+        conn.set_option('ssh_transfer_method', None)  # unless set to None
 
-        # Test with SCP_IF_SSH set to smart
         # Test when SFTP works
-        conn.set_option('scp_if_ssh', 'smart')
         expected_in_data = b' '.join((b'get', to_bytes(shlex.quote('/path/to/in/file')), to_bytes(shlex.quote('/path/to/dest/file')))) + b'\n'
         conn.set_options({})
         conn.fetch_file('/path/to/in/file', '/path/to/dest/file')
@@ -314,32 +281,6 @@ class TestConnectionBaseClass(unittest.TestCase):
         conn.fetch_file('/path/to/in/file', '/path/to/dest/file')
         conn._bare_run.assert_called_with('some command to run', None, checkrc=False)
 
-        # test with SCP_IF_SSH enabled
-        conn._bare_run.side_effect = None
-        conn.set_option('ssh_transfer_method', None)  # unless set to None scp_if_ssh is ignored
-        conn.set_option('scp_if_ssh', 'True')
-        conn.fetch_file('/path/to/in/file', '/path/to/dest/file')
-        conn._bare_run.assert_called_with('some command to run', None, checkrc=False)
-
-        conn.fetch_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩')
-        conn._bare_run.assert_called_with('some command to run', None, checkrc=False)
-
-        # test with SCP_IF_SSH disabled
-        conn.set_option('scp_if_ssh', False)
-        expected_in_data = b' '.join((b'get', to_bytes(shlex.quote('/path/to/in/file')), to_bytes(shlex.quote('/path/to/dest/file')))) + b'\n'
-        conn.fetch_file('/path/to/in/file', '/path/to/dest/file')
-        conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False)
-
-        expected_in_data = b' '.join((b'get',
-                                      to_bytes(shlex.quote('/path/to/in/file/with/unicode-fö〩')),
-                                      to_bytes(shlex.quote('/path/to/dest/file/with/unicode-fö〩')))) + b'\n'
-        conn.fetch_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩')
-        conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False)
-
-        # test that a non-zero rc raises an error
-        conn._bare_run.return_value = (1, 'stdout', 'some errors')
-        self.assertRaises(AnsibleError, conn.fetch_file, '/path/to/bad/file', '/remote/path/to/file')
-
 
 class MockSelector(object):
     def __init__(self):