Cleanup connections plugins (#2520) (#2522)

* minor refactors

* minor refactors in plugins/connection/saltstack.py

* minor refactors in plugins/connection/qubes.py

* minor refactor in plugins/connection/lxc.py

* minor refactors in plugins/connection/chroot.py

* minor refactors in plugins/connection/funcd.py

* minor refactors in plugins/connection/iocage.py

* minor refactors in plugins/connection/jail.py

* added changelog fragment

(cherry picked from commit c8f402806f)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

changelogs/fragments/2520-connection-refactors.yml

@@ -0,0 +1,9 @@
+minor_changes:
+  - chroot connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - funcd connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - iocage connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - jail connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - lxc connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - qubes connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - saltstack connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).
+  - zone connection - minor refactor to make lints and IDEs happy (https://github.com/ansible-collections/community.general/pull/2520).

plugins/connection/chroot.py

@@ -62,7 +62,7 @@ display = Display()
 
 
 class Connection(ConnectionBase):
-    ''' Local chroot based connections '''
+    """ Local chroot based connections """
 
     transport = 'community.general.chroot'
     has_pipelining = True
@@ -95,7 +95,7 @@ class Connection(ConnectionBase):
             raise AnsibleError("%s does not look like a chrootable dir (/bin/sh missing)" % self.chroot)
 
     def _connect(self):
-        ''' connect to the chroot '''
+        """ connect to the chroot """
         if os.path.isabs(self.get_option('chroot_exe')):
             self.chroot_cmd = self.get_option('chroot_exe')
         else:
@@ -110,17 +110,17 @@ class Connection(ConnectionBase):
             self._connected = True
 
     def _buffered_exec_command(self, cmd, stdin=subprocess.PIPE):
-        ''' run a command on the chroot.  This is only needed for implementing
+        """ run a command on the chroot.  This is only needed for implementing
         put_file() get_file() so that we don't have to read the whole file
         into memory.
 
         compared to exec_command() it looses some niceties like being able to
         return the process's exit code immediately.
-        '''
+        """
         executable = self.get_option('executable')
         local_cmd = [self.chroot_cmd, self.chroot, executable, '-c', cmd]
 
-        display.vvv("EXEC %s" % (local_cmd), host=self.chroot)
+        display.vvv("EXEC %s" % local_cmd, host=self.chroot)
         local_cmd = [to_bytes(i, errors='surrogate_or_strict') for i in local_cmd]
         p = subprocess.Popen(local_cmd, shell=False, stdin=stdin,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
@@ -128,16 +128,17 @@ class Connection(ConnectionBase):
         return p
 
     def exec_command(self, cmd, in_data=None, sudoable=False):
-        ''' run a command on the chroot '''
+        """ run a command on the chroot """
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         p = self._buffered_exec_command(cmd)
 
         stdout, stderr = p.communicate(in_data)
-        return (p.returncode, stdout, stderr)
+        return p.returncode, stdout, stderr
 
-    def _prefix_login_path(self, remote_path):
+    @staticmethod
-        ''' Make sure that we put files into a standard path
+    def _prefix_login_path(remote_path):
+        """ Make sure that we put files into a standard path
 
             If a path is relative, then we need to choose where to put it.
             ssh chooses $HOME but we aren't guaranteed that a home dir will
@@ -145,13 +146,13 @@ class Connection(ConnectionBase):
             This also happens to be the former default.
 
             Can revisit using $HOME instead if it's a problem
-        '''
+        """
         if not remote_path.startswith(os.path.sep):
             remote_path = os.path.join(os.path.sep, remote_path)
         return os.path.normpath(remote_path)
 
     def put_file(self, in_path, out_path):
-        ''' transfer a file from local to chroot '''
+        """ transfer a file from local to chroot """
         super(Connection, self).put_file(in_path, out_path)
         display.vvv("PUT %s TO %s" % (in_path, out_path), host=self.chroot)
 
@@ -177,7 +178,7 @@ class Connection(ConnectionBase):
             raise AnsibleError("file or module does not exist at: %s" % in_path)
 
     def fetch_file(self, in_path, out_path):
-        ''' fetch a file from chroot to local '''
+        """ fetch a file from chroot to local """
         super(Connection, self).fetch_file(in_path, out_path)
         display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.chroot)
 
@@ -201,6 +202,6 @@ class Connection(ConnectionBase):
                 raise AnsibleError("failed to transfer file %s to %s:\n%s\n%s" % (in_path, out_path, stdout, stderr))
 
     def close(self):
-        ''' terminate the connection; nothing to do here '''
+        """ terminate the connection; nothing to do here """
         super(Connection, self).close()
         self._connected = False

plugins/connection/funcd.py

@@ -44,7 +44,7 @@ display = Display()
 
 
 class Connection(ConnectionBase):
-    ''' Func-based connections '''
+    """ Func-based connections """
 
     has_pipelining = False
 
@@ -53,6 +53,7 @@ class Connection(ConnectionBase):
         self.host = host
         # port is unused, this go on func
         self.port = port
+        self.client = None
 
     def connect(self, port=None):
         if not HAVE_FUNC:
@@ -62,31 +63,32 @@ class Connection(ConnectionBase):
         return self
 
     def exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None):
-        ''' run a command on the remote minion '''
+        """ run a command on the remote minion """
 
         if in_data:
             raise AnsibleError("Internal Error: this module does not support optimized module pipelining")
 
         # totally ignores privlege escalation
-        display.vvv("EXEC %s" % (cmd), host=self.host)
+        display.vvv("EXEC %s" % cmd, host=self.host)
         p = self.client.command.run(cmd)[self.host]
-        return (p[0], p[1], p[2])
+        return p[0], p[1], p[2]
 
-    def _normalize_path(self, path, prefix):
+    @staticmethod
+    def _normalize_path(path, prefix):
         if not path.startswith(os.path.sep):
             path = os.path.join(os.path.sep, path)
         normpath = os.path.normpath(path)
         return os.path.join(prefix, normpath[1:])
 
     def put_file(self, in_path, out_path):
-        ''' transfer a file from local to remote '''
+        """ transfer a file from local to remote """
 
         out_path = self._normalize_path(out_path, '/')
         display.vvv("PUT %s TO %s" % (in_path, out_path), host=self.host)
         self.client.local.copyfile.send(in_path, out_path)
 
     def fetch_file(self, in_path, out_path):
-        ''' fetch a file from remote to local '''
+        """ fetch a file from remote to local """
 
         in_path = self._normalize_path(in_path, '/')
         display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.host)
@@ -99,5 +101,5 @@ class Connection(ConnectionBase):
         shutil.rmtree(tmpdir)
 
     def close(self):
-        ''' terminate the connection; nothing to do here '''
+        """ terminate the connection; nothing to do here """
         pass

plugins/connection/iocage.py

@@ -40,7 +40,7 @@ display = Display()
 
 
 class Connection(Jail):
-    ''' Local iocage based connections '''
+    """ Local iocage based connections """
 
     transport = 'community.general.iocage'
 

plugins/connection/jail.py

@@ -35,7 +35,6 @@ import os
 import os.path
 import subprocess
 import traceback
-import ansible.constants as C
 
 from ansible.errors import AnsibleError
 from ansible.module_utils.six.moves import shlex_quote
@@ -47,7 +46,7 @@ display = Display()
 
 
 class Connection(ConnectionBase):
-    ''' Local BSD Jail based connections '''
+    """ Local BSD Jail based connections """
 
     modified_jailname_key = 'conn_jail_name'
 
@@ -90,20 +89,20 @@ class Connection(ConnectionBase):
         return to_text(stdout, errors='surrogate_or_strict').split()
 
     def _connect(self):
-        ''' connect to the jail; nothing to do here '''
+        """ connect to the jail; nothing to do here """
         super(Connection, self)._connect()
         if not self._connected:
             display.vvv(u"ESTABLISH JAIL CONNECTION FOR USER: {0}".format(self._play_context.remote_user), host=self.jail)
             self._connected = True
 
     def _buffered_exec_command(self, cmd, stdin=subprocess.PIPE):
-        ''' run a command on the jail.  This is only needed for implementing
+        """ run a command on the jail.  This is only needed for implementing
         put_file() get_file() so that we don't have to read the whole file
         into memory.
 
         compared to exec_command() it looses some niceties like being able to
         return the process's exit code immediately.
-        '''
+        """
 
         local_cmd = [self.jexec_cmd]
         set_env = ''
@@ -123,16 +122,17 @@ class Connection(ConnectionBase):
         return p
 
     def exec_command(self, cmd, in_data=None, sudoable=False):
-        ''' run a command on the jail '''
+        """ run a command on the jail """
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         p = self._buffered_exec_command(cmd)
 
         stdout, stderr = p.communicate(in_data)
-        return (p.returncode, stdout, stderr)
+        return p.returncode, stdout, stderr
 
-    def _prefix_login_path(self, remote_path):
+    @staticmethod
-        ''' Make sure that we put files into a standard path
+    def _prefix_login_path(remote_path):
+        """ Make sure that we put files into a standard path
 
             If a path is relative, then we need to choose where to put it.
             ssh chooses $HOME but we aren't guaranteed that a home dir will
@@ -140,13 +140,13 @@ class Connection(ConnectionBase):
             This also happens to be the former default.
 
             Can revisit using $HOME instead if it's a problem
-        '''
+        """
         if not remote_path.startswith(os.path.sep):
             remote_path = os.path.join(os.path.sep, remote_path)
         return os.path.normpath(remote_path)
 
     def put_file(self, in_path, out_path):
-        ''' transfer a file from local to jail '''
+        """ transfer a file from local to jail """
         super(Connection, self).put_file(in_path, out_path)
         display.vvv("PUT %s TO %s" % (in_path, out_path), host=self.jail)
 
@@ -172,7 +172,7 @@ class Connection(ConnectionBase):
             raise AnsibleError("file or module does not exist at: %s" % in_path)
 
     def fetch_file(self, in_path, out_path):
-        ''' fetch a file from jail to local '''
+        """ fetch a file from jail to local """
         super(Connection, self).fetch_file(in_path, out_path)
         display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.jail)
 
@@ -196,6 +196,6 @@ class Connection(ConnectionBase):
                 raise AnsibleError("failed to transfer file %s to %s:\n%s\n%s" % (in_path, out_path, to_native(stdout), to_native(stderr)))
 
     def close(self):
-        ''' terminate the connection; nothing to do here '''
+        """ terminate the connection; nothing to do here """
         super(Connection, self).close()
         self._connected = False

plugins/connection/lxc.py

@@ -42,14 +42,13 @@ try:
 except ImportError:
     pass
 
-from ansible import constants as C
 from ansible import errors
 from ansible.module_utils._text import to_bytes, to_native
 from ansible.plugins.connection import ConnectionBase
 
 
 class Connection(ConnectionBase):
-    ''' Local lxc based connections '''
+    """ Local lxc based connections """
 
     transport = 'community.general.lxc'
     has_pipelining = True
@@ -62,7 +61,7 @@ class Connection(ConnectionBase):
         self.container = None
 
     def _connect(self):
-        ''' connect to the lxc; nothing to do here '''
+        """ connect to the lxc; nothing to do here """
         super(Connection, self)._connect()
 
         if not HAS_LIBLXC:
@@ -77,7 +76,8 @@ class Connection(ConnectionBase):
         if self.container.state == "STOPPED":
             raise errors.AnsibleError("%s is not running" % self.container_name)
 
-    def _communicate(self, pid, in_data, stdin, stdout, stderr):
+    @staticmethod
+    def _communicate(pid, in_data, stdin, stdout, stderr):
         buf = {stdout: [], stderr: []}
         read_fds = [stdout, stderr]
         if in_data:
@@ -111,7 +111,7 @@ class Connection(ConnectionBase):
         return fd
 
     def exec_command(self, cmd, in_data=None, sudoable=False):
-        ''' run a command on the chroot '''
+        """ run a command on the chroot """
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         # python2-lxc needs bytes. python3-lxc needs text.

plugins/connection/qubes.py

@@ -37,15 +37,9 @@ DOCUMENTATION = '''
 #            - name: hosts
 '''
 
-import shlex
-import shutil
-
-import os
-import base64
 import subprocess
 
-import ansible.constants as C
+from ansible.module_utils._text import to_bytes
-from ansible.module_utils._text import to_bytes, to_native
 from ansible.plugins.connection import ConnectionBase, ensure_connect
 from ansible.errors import AnsibleConnectionFailure
 from ansible.utils.display import Display

plugins/connection/saltstack.py

@@ -16,14 +16,11 @@ DOCUMENTATION = '''
         - This allows you to use existing Saltstack infrastructure to connect to targets.
 '''
 
-import re
 import os
-import pty
+import base64
-import codecs
-import subprocess
 
-from ansible.module_utils._text import to_bytes, to_text
+from ansible import errors
-from ansible.module_utils.six.moves import cPickle
+from ansible.plugins.connection import ConnectionBase
 
 HAVE_SALTSTACK = False
 try:
@@ -32,13 +29,9 @@ try:
 except ImportError:
     pass
 
-import os
-from ansible import errors
-from ansible.plugins.connection import ConnectionBase
-
 
 class Connection(ConnectionBase):
-    ''' Salt-based connections '''
+    """ Salt-based connections """
 
     has_pipelining = False
     # while the name of the product is salt, naming that module salt cause
@@ -58,29 +51,30 @@ class Connection(ConnectionBase):
         return self
 
     def exec_command(self, cmd, sudoable=False, in_data=None):
-        ''' run a command on the remote minion '''
+        """ run a command on the remote minion """
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         if in_data:
             raise errors.AnsibleError("Internal Error: this module does not support optimized module pipelining")
 
-        self._display.vvv("EXEC %s" % (cmd), host=self.host)
+        self._display.vvv("EXEC %s" % cmd, host=self.host)
         # need to add 'true;' to work around https://github.com/saltstack/salt/issues/28077
         res = self.client.cmd(self.host, 'cmd.exec_code_all', ['bash', 'true;' + cmd])
         if self.host not in res:
             raise errors.AnsibleError("Minion %s didn't answer, check if salt-minion is running and the name is correct" % self.host)
 
         p = res[self.host]
-        return (p['retcode'], p['stdout'], p['stderr'])
+        return p['retcode'], p['stdout'], p['stderr']
 
-    def _normalize_path(self, path, prefix):
+    @staticmethod
+    def _normalize_path(path, prefix):
         if not path.startswith(os.path.sep):
             path = os.path.join(os.path.sep, path)
         normpath = os.path.normpath(path)
         return os.path.join(prefix, normpath[1:])
 
     def put_file(self, in_path, out_path):
-        ''' transfer a file from local to remote '''
+        """ transfer a file from local to remote """
 
         super(Connection, self).put_file(in_path, out_path)
 
@@ -88,11 +82,11 @@ class Connection(ConnectionBase):
         self._display.vvv("PUT %s TO %s" % (in_path, out_path), host=self.host)
         with open(in_path, 'rb') as in_fh:
             content = in_fh.read()
-        self.client.cmd(self.host, 'hashutil.base64_decodefile', [codecs.encode(content, 'base64'), out_path])
+        self.client.cmd(self.host, 'hashutil.base64_decodefile', [base64.b64encode(content), out_path])
 
     # TODO test it
     def fetch_file(self, in_path, out_path):
-        ''' fetch a file from remote to local '''
+        """ fetch a file from remote to local """
 
         super(Connection, self).fetch_file(in_path, out_path)
 
@@ -102,5 +96,5 @@ class Connection(ConnectionBase):
         open(out_path, 'wb').write(content)
 
     def close(self):
-        ''' terminate the connection; nothing to do here '''
+        """ terminate the connection; nothing to do here """
         pass

plugins/connection/zone.py

@@ -31,7 +31,6 @@ import os.path
 import subprocess
 import traceback
 
-from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.module_utils.six.moves import shlex_quote
 from ansible.module_utils._text import to_bytes
@@ -42,7 +41,7 @@ display = Display()
 
 
 class Connection(ConnectionBase):
-    ''' Local zone based connections '''
+    """ Local zone based connections """
 
     transport = 'community.general.zone'
     has_pipelining = True
@@ -75,9 +74,9 @@ class Connection(ConnectionBase):
                                    stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
         zones = []
-        for l in process.stdout.readlines():
+        for line in process.stdout.readlines():
             # 1:work:running:/zones/work:3126dc59-9a07-4829-cde9-a816e4c5040e:native:shared
-            s = l.split(':')
+            s = line.split(':')
             if s[1] != 'global':
                 zones.append(s[1])
 
@@ -95,20 +94,20 @@ class Connection(ConnectionBase):
         return path + '/root'
 
     def _connect(self):
-        ''' connect to the zone; nothing to do here '''
+        """ connect to the zone; nothing to do here """
         super(Connection, self)._connect()
         if not self._connected:
             display.vvv("THIS IS A LOCAL ZONE DIR", host=self.zone)
             self._connected = True
 
     def _buffered_exec_command(self, cmd, stdin=subprocess.PIPE):
-        ''' run a command on the zone.  This is only needed for implementing
+        """ run a command on the zone.  This is only needed for implementing
         put_file() get_file() so that we don't have to read the whole file
         into memory.
 
         compared to exec_command() it looses some niceties like being able to
         return the process's exit code immediately.
-        '''
+        """
         # NOTE: zlogin invokes a shell (just like ssh does) so we do not pass
         # this through /bin/sh -c here.  Instead it goes through the shell
         # that zlogin selects.
@@ -122,16 +121,16 @@ class Connection(ConnectionBase):
         return p
 
     def exec_command(self, cmd, in_data=None, sudoable=False):
-        ''' run a command on the zone '''
+        """ run a command on the zone """
         super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
 
         p = self._buffered_exec_command(cmd)
 
         stdout, stderr = p.communicate(in_data)
-        return (p.returncode, stdout, stderr)
+        return p.returncode, stdout, stderr
 
     def _prefix_login_path(self, remote_path):
-        ''' Make sure that we put files into a standard path
+        """ Make sure that we put files into a standard path
 
             If a path is relative, then we need to choose where to put it.
             ssh chooses $HOME but we aren't guaranteed that a home dir will
@@ -139,13 +138,13 @@ class Connection(ConnectionBase):
             This also happens to be the former default.
 
             Can revisit using $HOME instead if it's a problem
-        '''
+        """
         if not remote_path.startswith(os.path.sep):
             remote_path = os.path.join(os.path.sep, remote_path)
         return os.path.normpath(remote_path)
 
     def put_file(self, in_path, out_path):
-        ''' transfer a file from local to zone '''
+        """ transfer a file from local to zone """
         super(Connection, self).put_file(in_path, out_path)
         display.vvv("PUT %s TO %s" % (in_path, out_path), host=self.zone)
 
@@ -171,7 +170,7 @@ class Connection(ConnectionBase):
             raise AnsibleError("file or module does not exist at: %s" % in_path)
 
     def fetch_file(self, in_path, out_path):
-        ''' fetch a file from zone to local '''
+        """ fetch a file from zone to local """
         super(Connection, self).fetch_file(in_path, out_path)
         display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.zone)
 
@@ -195,6 +194,6 @@ class Connection(ConnectionBase):
                 raise AnsibleError("failed to transfer file %s to %s:\n%s\n%s" % (in_path, out_path, stdout, stderr))
 
     def close(self):
-        ''' terminate the connection; nothing to do here '''
+        """ terminate the connection; nothing to do here """
         super(Connection, self).close()
         self._connected = False