From 69ea487005409a1d90574ca6ab28fc7968a91498 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Sun, 16 May 2021 13:45:26 +0200 Subject: [PATCH] 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 c8f402806fe1f7d8ee8a0a716c986b1b47860cae) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> --- .../fragments/2520-connection-refactors.yml | 9 ++++++ plugins/connection/chroot.py | 27 ++++++++-------- plugins/connection/funcd.py | 18 ++++++----- plugins/connection/iocage.py | 2 +- plugins/connection/jail.py | 26 +++++++-------- plugins/connection/lxc.py | 10 +++--- plugins/connection/qubes.py | 8 +---- plugins/connection/saltstack.py | 32 ++++++++----------- plugins/connection/zone.py | 27 ++++++++-------- 9 files changed, 79 insertions(+), 80 deletions(-) create mode 100644 changelogs/fragments/2520-connection-refactors.yml diff --git a/changelogs/fragments/2520-connection-refactors.yml b/changelogs/fragments/2520-connection-refactors.yml new file mode 100644 index 0000000000..2e5c8273d7 --- /dev/null +++ b/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). diff --git a/plugins/connection/chroot.py b/plugins/connection/chroot.py index ffaea2b198..a18506cb80 100644 --- a/plugins/connection/chroot.py +++ b/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): - ''' Make sure that we put files into a standard path + @staticmethod + 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 diff --git a/plugins/connection/funcd.py b/plugins/connection/funcd.py index 3aed7145cb..109e251146 100644 --- a/plugins/connection/funcd.py +++ b/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 diff --git a/plugins/connection/iocage.py b/plugins/connection/iocage.py index 435c789fd2..beb440eae3 100644 --- a/plugins/connection/iocage.py +++ b/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' diff --git a/plugins/connection/jail.py b/plugins/connection/jail.py index 5252e3c4eb..f5d787b62f 100644 --- a/plugins/connection/jail.py +++ b/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): - ''' Make sure that we put files into a standard path + @staticmethod + 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 diff --git a/plugins/connection/lxc.py b/plugins/connection/lxc.py index 8de1acc35d..6512a87c6d 100644 --- a/plugins/connection/lxc.py +++ b/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. diff --git a/plugins/connection/qubes.py b/plugins/connection/qubes.py index aa0075b674..d3f934b601 100644 --- a/plugins/connection/qubes.py +++ b/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, to_native +from ansible.module_utils._text import to_bytes from ansible.plugins.connection import ConnectionBase, ensure_connect from ansible.errors import AnsibleConnectionFailure from ansible.utils.display import Display diff --git a/plugins/connection/saltstack.py b/plugins/connection/saltstack.py index 6be7a79949..f8e3680aea 100644 --- a/plugins/connection/saltstack.py +++ b/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 codecs -import subprocess +import base64 -from ansible.module_utils._text import to_bytes, to_text -from ansible.module_utils.six.moves import cPickle +from ansible import errors +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 diff --git a/plugins/connection/zone.py b/plugins/connection/zone.py index 7a7a36331d..b101ec5cf3 100644 --- a/plugins/connection/zone.py +++ b/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