From 8e19ab178a858fef6bc5e4feadf67c339ca6f922 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Fri, 29 May 2020 10:00:21 -0500 Subject: [PATCH] connection plugins: try config, then play_context (#69751) Change: Rather than only using config, have base connection plugins fall back to play_context. Test Plan: - Tested ansible-connection logic against an IOS device - Tested -k against a VM - CI Signed-off-by: Rick Elrod --- .../plugins/connection/paramiko_ssh.py | 6 +++-- lib/ansible/plugins/connection/ssh.py | 22 +++++++++++-------- test/integration/targets/network_cli/aliases | 3 +++ .../targets/network_cli/passworded_user.yml | 14 ++++++++++++ test/integration/targets/network_cli/runme.sh | 19 ++++++++++++++++ .../integration/targets/network_cli/setup.yml | 14 ++++++++++++ .../targets/network_cli/teardown.yml | 14 ++++++++++++ 7 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 test/integration/targets/network_cli/aliases create mode 100644 test/integration/targets/network_cli/passworded_user.yml create mode 100755 test/integration/targets/network_cli/runme.sh create mode 100644 test/integration/targets/network_cli/setup.yml create mode 100644 test/integration/targets/network_cli/teardown.yml diff --git a/lib/ansible/plugins/connection/paramiko_ssh.py b/lib/ansible/plugins/connection/paramiko_ssh.py index 2460b4accc0..96a76d67461 100644 --- a/lib/ansible/plugins/connection/paramiko_ssh.py +++ b/lib/ansible/plugins/connection/paramiko_ssh.py @@ -324,9 +324,11 @@ class Connection(ConnectionBase): ssh.set_missing_host_key_policy(MyAddPolicy(self._new_stdin, self)) + conn_password = self.get_option('password') or self._play_context.password + allow_agent = True - if self.get_option('password') is not None: + if conn_password is not None: allow_agent = False try: @@ -344,7 +346,7 @@ class Connection(ConnectionBase): allow_agent=allow_agent, look_for_keys=self.get_option('look_for_keys'), key_filename=key_filename, - password=self.get_option('password'), + password=conn_password, timeout=self._play_context.timeout, port=port, **ssh_connect_kwargs diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index a566af9e117..d7789bac2d0 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -397,9 +397,10 @@ def _ssh_retry(func): def wrapped(self, *args, **kwargs): remaining_tries = int(C.ANSIBLE_SSH_RETRIES) + 1 cmd_summary = u"%s..." % to_text(args[0]) + conn_password = self.get_option('password') or self._play_context.password for attempt in range(remaining_tries): cmd = args[0] - if attempt != 0 and self.get_option('password') and isinstance(cmd, list): + if attempt != 0 and conn_password and isinstance(cmd, list): # If this is a retry, the fd/pipe for sshpass is closed, and we need a new one self.sshpass_pipe = os.pipe() cmd[1] = b'-d' + to_bytes(self.sshpass_pipe[0], nonstring='simplerepr', errors='surrogate_or_strict') @@ -417,7 +418,7 @@ def _ssh_retry(func): except (AnsibleControlPersistBrokenPipeError): # Retry one more time because of the ControlPersist broken pipe (see #16731) cmd = args[0] - if self.get_option('password') and isinstance(cmd, list): + if conn_password and isinstance(cmd, list): # This is a retry, so the fd/pipe for sshpass is closed, and we need a new one self.sshpass_pipe = os.pipe() cmd[1] = b'-d' + to_bytes(self.sshpass_pipe[0], nonstring='simplerepr', errors='surrogate_or_strict') @@ -564,6 +565,7 @@ class Connection(ConnectionBase): ''' b_command = [] + conn_password = self.get_option('password') or self._play_context.password # # First, the command to invoke @@ -572,7 +574,7 @@ class Connection(ConnectionBase): # If we want to use password authentication, we have to set up a pipe to # write the password to sshpass. - if self.get_option('password'): + if conn_password: if not self._sshpass_available(): raise AnsibleError("to use the 'ssh' connection type with passwords, you must install the sshpass program") @@ -597,7 +599,7 @@ class Connection(ConnectionBase): # sftp batch mode does not prompt for passwords so it must be disabled # if not using controlpersist and using sshpass if binary == 'sftp' and C.DEFAULT_SFTP_BATCH_MODE: - if self.get_option('password'): + if conn_password: b_args = [b'-o', b'BatchMode=no'] self._add_args(b_command, b_args, u'disable batch mode for sshpass') b_command += [b'-b', b'-'] @@ -631,7 +633,7 @@ class Connection(ConnectionBase): b_args = (b"-o", b'IdentityFile="' + to_bytes(os.path.expanduser(key), errors='surrogate_or_strict') + b'"') self._add_args(b_command, b_args, u"ANSIBLE_PRIVATE_KEY_FILE/private_key_file/ansible_ssh_private_key_file set") - if not self.get_option('password'): + if not conn_password: self._add_args( b_command, ( b"-o", b"KbdInteractiveAuthentication=no", @@ -799,11 +801,13 @@ class Connection(ConnectionBase): else: cmd = list(map(to_bytes, cmd)) + conn_password = self.get_option('password') or self._play_context.password + if not in_data: try: # Make sure stdin is a proper pty to avoid tcgetattr errors master, slave = pty.openpty() - if PY3 and self.get_option('password'): + if PY3 and conn_password: # pylint: disable=unexpected-keyword-arg p = subprocess.Popen(cmd, stdin=slave, stdout=subprocess.PIPE, stderr=subprocess.PIPE, pass_fds=self.sshpass_pipe) else: @@ -814,7 +818,7 @@ class Connection(ConnectionBase): p = None if not p: - if PY3 and self.get_option('password'): + if PY3 and conn_password: # pylint: disable=unexpected-keyword-arg p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, pass_fds=self.sshpass_pipe) else: @@ -824,10 +828,10 @@ class Connection(ConnectionBase): # If we are using SSH password authentication, write the password into # the pipe we opened in _build_command. - if self.get_option('password'): + if conn_password: os.close(self.sshpass_pipe[0]) try: - os.write(self.sshpass_pipe[1], to_bytes(self.get_option('password')) + b'\n') + os.write(self.sshpass_pipe[1], to_bytes(conn_password) + b'\n') except OSError as e: # Ignore broken pipe errors if the sshpass process has exited. if e.errno != errno.EPIPE or p.poll() is None: diff --git a/test/integration/targets/network_cli/aliases b/test/integration/targets/network_cli/aliases new file mode 100644 index 00000000000..6a739c96bda --- /dev/null +++ b/test/integration/targets/network_cli/aliases @@ -0,0 +1,3 @@ +# Keeping incidental for efficiency, to avoid spinning up another VM +shippable/vyos/incidental +network/vyos diff --git a/test/integration/targets/network_cli/passworded_user.yml b/test/integration/targets/network_cli/passworded_user.yml new file mode 100644 index 00000000000..20781a2af59 --- /dev/null +++ b/test/integration/targets/network_cli/passworded_user.yml @@ -0,0 +1,14 @@ +- hosts: vyos-1-1-8 + gather_facts: false + + tasks: + - name: Run whoami + vyos.vyos.vyos_command: + commands: + - whoami + register: whoami + + - assert: + that: + - whoami is successful + - whoami.stdout_lines[0][0] == 'atester' diff --git a/test/integration/targets/network_cli/runme.sh b/test/integration/targets/network_cli/runme.sh new file mode 100755 index 00000000000..783016a925d --- /dev/null +++ b/test/integration/targets/network_cli/runme.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +set -ux # no -e because we want to run teardown no matter waht +export ANSIBLE_ROLES_PATH=../ + +ansible-playbook setup.yml -i "$INVENTORY_PATH" "$@" + +# We need a nonempty file to override key with (empty file gives a +# lovely "list index out of range" error) +foo=$(mktemp) +echo hello > "$foo" + +ansible-playbook \ + -i "$INVENTORY_PATH" \ + -e ansible_user=atester \ + -e ansible_password=testymctest \ + -e ansible_ssh_private_key_file="$foo" \ + passworded_user.yml + +ansible-playbook teardown.yml -i "$INVENTORY_PATH" "$@" diff --git a/test/integration/targets/network_cli/setup.yml b/test/integration/targets/network_cli/setup.yml new file mode 100644 index 00000000000..bd61eb41faa --- /dev/null +++ b/test/integration/targets/network_cli/setup.yml @@ -0,0 +1,14 @@ +- hosts: vyos-1-1-8 + connection: ansible.netcommon.network_cli + become: true + gather_facts: false + + tasks: + - name: Create user with password + register: result + vyos.vyos.vyos_config: + lines: + - set system login user atester full-name "Ansible Tester" + - set system login user atester authentication plaintext-password testymctest + - set system login user jsmith level admin + - delete service ssh disable-password-authentication diff --git a/test/integration/targets/network_cli/teardown.yml b/test/integration/targets/network_cli/teardown.yml new file mode 100644 index 00000000000..b7b66a49655 --- /dev/null +++ b/test/integration/targets/network_cli/teardown.yml @@ -0,0 +1,14 @@ +- hosts: vyos-1-1-8 + connection: ansible.netcommon.network_cli + become: true + gather_facts: false + + tasks: + - name: Get rid of user (undo everything from setup.yml) + register: result + vyos.vyos.vyos_config: + lines: + - delete system login user atester full-name "Ansible Tester" + - delete system login user atester authentication plaintext-password testymctest + - delete system login user jsmith level admin + - set service ssh disable-password-authentication