From 092ec680e6bdd4ed2fde9f8921172d1de97c268b Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Fri, 28 Aug 2020 03:57:24 +1000 Subject: [PATCH] Ensure -k is set to delegated hosts without a pass (#71136) (#71168) * Ensure -k is set to delegated hosts without a pass * Fix up some broken tests * Update task_executor.py one possible fix, the other is updating winrm to normalize on 'password' like the other connection plugins * Add alias for winrm and fix incorrect assumption * Make sure aliases are used for keyword options * Conditionally run test if sshpass is present, fix sanity Co-authored-by: Brian Coca (cherry picked from commit 3f22f79e73af4398d03b0c1676bb8efde32ea607) --- changelogs/fragments/delegation_password.yml | 2 + lib/ansible/config/manager.py | 21 +++++++-- lib/ansible/plugins/connection/psrp.py | 3 +- lib/ansible/plugins/connection/winrm.py | 2 + .../action_plugins/delegation_action.py | 12 +++++ .../targets/connection_delegation/aliases | 4 ++ .../delegation_connection.py | 45 +++++++++++++++++++ .../connection_delegation/inventory.ini | 1 + .../targets/connection_delegation/runme.sh | 9 ++++ .../targets/connection_delegation/test.yml | 23 ++++++++++ 10 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/delegation_password.yml create mode 100644 test/integration/targets/connection_delegation/action_plugins/delegation_action.py create mode 100644 test/integration/targets/connection_delegation/aliases create mode 100644 test/integration/targets/connection_delegation/connection_plugins/delegation_connection.py create mode 100644 test/integration/targets/connection_delegation/inventory.ini create mode 100755 test/integration/targets/connection_delegation/runme.sh create mode 100644 test/integration/targets/connection_delegation/test.yml diff --git a/changelogs/fragments/delegation_password.yml b/changelogs/fragments/delegation_password.yml new file mode 100644 index 00000000000..c68191b25df --- /dev/null +++ b/changelogs/fragments/delegation_password.yml @@ -0,0 +1,2 @@ +bugfixes: +- Ensure password passed in by -k is used on delegated hosts that do not have ansible_password set diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 3cbfb38d448..5cd87acfc94 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -429,10 +429,12 @@ class ConfigManager(object): defs = self.get_configuration_definitions(plugin_type, plugin_name) if config in defs: + aliases = defs[config].get('aliases', []) + # direct setting via plugin arguments, can set to None so we bypass rest of processing/defaults direct_aliases = [] if direct: - direct_aliases = [direct[alias] for alias in defs[config].get('aliases', []) if alias in direct] + direct_aliases = [direct[alias] for alias in aliases if alias in direct] if direct and config in direct: value = direct[config] origin = 'Direct' @@ -447,9 +449,20 @@ class ConfigManager(object): origin = 'var: %s' % origin # use playbook keywords if you have em - if value is None and keys and config in keys: - value, origin = keys[config], 'keyword' - origin = 'keyword: %s' % origin + if value is None and keys: + if config in keys: + value = keys[config] + keyword = config + + elif aliases: + for alias in aliases: + if alias in keys: + value = keys[alias] + keyword = alias + break + + if value is not None: + origin = 'keyword: %s' % keyword # env vars are next precedence if value is None and defs[config].get('env'): diff --git a/lib/ansible/plugins/connection/psrp.py b/lib/ansible/plugins/connection/psrp.py index 915a433d664..728689ea600 100644 --- a/lib/ansible/plugins/connection/psrp.py +++ b/lib/ansible/plugins/connection/psrp.py @@ -39,7 +39,8 @@ options: - name: ansible_password - name: ansible_winrm_pass - name: ansible_winrm_password - aliases: [ password ] + aliases: + - password # Needed for --ask-pass to come through on delegation port: description: - The port for PSRP to connect on the remote target. diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 66c22fa515a..7f9e3658c66 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -40,6 +40,8 @@ DOCUMENTATION = """ - name: ansible_winrm_pass - name: ansible_winrm_password type: str + aliases: + - password # Needed for --ask-pass to come through on delegation port: description: - port for winrm to connect on remote target diff --git a/test/integration/targets/connection_delegation/action_plugins/delegation_action.py b/test/integration/targets/connection_delegation/action_plugins/delegation_action.py new file mode 100644 index 00000000000..9d419e75b08 --- /dev/null +++ b/test/integration/targets/connection_delegation/action_plugins/delegation_action.py @@ -0,0 +1,12 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + + def run(self, tmp=None, task_vars=None): + return { + 'remote_password': self._connection.get_option('remote_password'), + } diff --git a/test/integration/targets/connection_delegation/aliases b/test/integration/targets/connection_delegation/aliases new file mode 100644 index 00000000000..9950a08844a --- /dev/null +++ b/test/integration/targets/connection_delegation/aliases @@ -0,0 +1,4 @@ +shippable/posix/group1 +skip/freebsd # No sshpass +skip/osx # No sshpass +skip/rhel # No sshpass diff --git a/test/integration/targets/connection_delegation/connection_plugins/delegation_connection.py b/test/integration/targets/connection_delegation/connection_plugins/delegation_connection.py new file mode 100644 index 00000000000..f61846cfab3 --- /dev/null +++ b/test/integration/targets/connection_delegation/connection_plugins/delegation_connection.py @@ -0,0 +1,45 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +DOCUMENTATION = """ +author: Ansible Core Team +connection: delegation_connection +short_description: Test connection for delegated host check +description: +- Some further description that you don't care about. +options: + remote_password: + description: The remote password + type: str + vars: + - name: ansible_password + # Tests that an aliased key gets the -k option which hardcodes the value to password + aliases: + - password +""" + +from ansible.plugins.connection import ConnectionBase + + +class Connection(ConnectionBase): + + transport = 'delegation_connection' + has_pipelining = True + + def __init__(self, *args, **kwargs): + super(Connection, self).__init__(*args, **kwargs) + + def _connect(self): + super(Connection, self)._connect() + + def exec_command(self, cmd, in_data=None, sudoable=True): + super(Connection, self).exec_command(cmd, in_data, sudoable) + + def put_file(self, in_path, out_path): + super(Connection, self).put_file(in_path, out_path) + + def fetch_file(self, in_path, out_path): + super(Connection, self).fetch_file(in_path, out_path) + + def close(self): + super(Connection, self).close() diff --git a/test/integration/targets/connection_delegation/inventory.ini b/test/integration/targets/connection_delegation/inventory.ini new file mode 100644 index 00000000000..e7f846d3ee9 --- /dev/null +++ b/test/integration/targets/connection_delegation/inventory.ini @@ -0,0 +1 @@ +my_host ansible_host=127.0.0.1 ansible_connection=delegation_connection diff --git a/test/integration/targets/connection_delegation/runme.sh b/test/integration/targets/connection_delegation/runme.sh new file mode 100755 index 00000000000..eb26f7c5377 --- /dev/null +++ b/test/integration/targets/connection_delegation/runme.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +set -ux + +echo "Checking if sshpass is present" +which sshpass 2>&1 || exit 0 +echo "sshpass is present, continuing with test" + +sshpass -p my_password ansible-playbook -i inventory.ini test.yml -k "$@" diff --git a/test/integration/targets/connection_delegation/test.yml b/test/integration/targets/connection_delegation/test.yml new file mode 100644 index 00000000000..678bef518b7 --- /dev/null +++ b/test/integration/targets/connection_delegation/test.yml @@ -0,0 +1,23 @@ +--- +- hosts: localhost + gather_facts: no + tasks: + - name: test connection receives -k from play_context when delegating + delegation_action: + delegate_to: my_host + register: result + + - assert: + that: + - result.remote_password == 'my_password' + + - name: ensure vars set for that host take precedence over -k + delegation_action: + delegate_to: my_host + vars: + ansible_password: other_password + register: result + + - assert: + that: + - result.remote_password == 'other_password'