diff options
author | Brian Coca <bcoca@users.noreply.github.com> | 2022-02-16 20:20:33 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-16 17:20:33 -0800 |
commit | 5c2251504c7f985fa586c658247f2973950ed732 (patch) | |
tree | 5b8b5d4798043ecdb1b018ca052b0baedfc19819 | |
parent | f7f27ade2d9443e67805e8a654480b610400a049 (diff) | |
download | ansible-5c2251504c7f985fa586c658247f2973950ed732.tar.gz |
ssh connection: use 'correct' host in all cases (#76017) (#76969)
ssh plugin, use 'correct' information source in all cases
* still fallback to pc
* added inventory to new test
* undef var can still show as parser error on pc
now task_exectuer has a more accurate error handling
(cherry picked from commit be19863e44cc6b78706147b25489a73d7c8fbcb5)
-rw-r--r-- | changelogs/fragments/ssh_use_right_host.yml | 2 | ||||
-rw-r--r-- | lib/ansible/executor/task_executor.py | 17 | ||||
-rw-r--r-- | lib/ansible/plugins/connection/ssh.py | 9 | ||||
-rw-r--r-- | test/integration/targets/delegate_to/inventory | 8 | ||||
-rwxr-xr-x | test/integration/targets/delegate_to/runme.sh | 2 | ||||
-rw-r--r-- | test/integration/targets/delegate_to/test_delegate_to.yml | 24 |
6 files changed, 57 insertions, 5 deletions
diff --git a/changelogs/fragments/ssh_use_right_host.yml b/changelogs/fragments/ssh_use_right_host.yml new file mode 100644 index 0000000000..de1d6ef9e3 --- /dev/null +++ b/changelogs/fragments/ssh_use_right_host.yml @@ -0,0 +1,2 @@ +bugfixes: + - ssh connection now uses more correct host source as play_context can ignore loop/delegation variations. diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index ff48a32d1e..ebf2f0ad9f 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -480,8 +480,19 @@ class TaskExecutor: raise self._loop_eval_error # pylint: disable=raising-bad-type # if we ran into an error while setting up the PlayContext, raise it now, unless is known issue with delegation - if context_validation_error is not None and not (self._task.delegate_to and isinstance(context_validation_error, AnsibleUndefinedVariable)): - raise context_validation_error # pylint: disable=raising-bad-type + # and undefined vars (correct values are in cvars later on and connection plugins, if still error, blows up there) + if context_validation_error is not None: + raiseit = True + if self._task.delegate_to: + if isinstance(context_validation_error, AnsibleUndefinedVariable): + raiseit = False + elif isinstance(context_validation_error, AnsibleParserError): + # parser error, might be cause by undef too + orig_exc = getattr(context_validation_error, 'orig_exc', None) + if isinstance(orig_exc, AnsibleUndefinedVariable): + raiseit = False + if raiseit: + raise context_validation_error # pylint: disable=raising-bad-type # if this task is a TaskInclude, we just return now with a success code so the # main thread can expand the task list for the given host @@ -502,7 +513,7 @@ class TaskExecutor: # Now we do final validation on the task, which sets all fields to their final values. try: self._task.post_validate(templar=templar) - except AnsibleError: + except AnsibleError as e: raise except Exception: return dict(changed=False, failed=True, _ansible_no_log=self._play_context.no_log, exception=to_text(traceback.format_exc())) diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index d498683ecd..488ceea6e0 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -1242,7 +1242,9 @@ class Connection(ConnectionBase): super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) - display.vvv(u"ESTABLISH SSH CONNECTION FOR USER: {0}".format(self.user), host=self._play_context.remote_addr) + self.host = self.get_option('host') or self._play_context.remote_addr + + display.vvv(u"ESTABLISH SSH CONNECTION FOR USER: {0}".format(self.user), host=self.host) if getattr(self._shell, "_IS_WINDOWS", False): # Become method 'runas' is done in the wrapper that is executed, @@ -1286,6 +1288,8 @@ class Connection(ConnectionBase): super(Connection, self).put_file(in_path, out_path) + self.host = self.get_option('host') or self._play_context.remote_addr + display.vvv(u"PUT {0} TO {1}".format(in_path, out_path), host=self.host) if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): raise AnsibleFileNotFound("file or module does not exist: {0}".format(to_native(in_path))) @@ -1300,6 +1304,8 @@ class Connection(ConnectionBase): super(Connection, self).fetch_file(in_path, out_path) + self.host = self.get_option('host') or self._play_context.remote_addr + display.vvv(u"FETCH {0} TO {1}".format(in_path, out_path), host=self.host) # need to add / if path is rooted @@ -1311,6 +1317,7 @@ class Connection(ConnectionBase): def reset(self): run_reset = False + self.host = self.get_option('host') or self._play_context.remote_addr # If we have a persistent ssh connection (ControlPersist), we can ask it to stop listening. # only run the reset if the ControlPath already exists or if it isn't configured and ControlPersist is set diff --git a/test/integration/targets/delegate_to/inventory b/test/integration/targets/delegate_to/inventory index f7ad0a33da..ebc332544c 100644 --- a/test/integration/targets/delegate_to/inventory +++ b/test/integration/targets/delegate_to/inventory @@ -7,3 +7,11 @@ testhost5 ansible_connection=fakelocal [all:vars] ansible_python_interpreter="{{ ansible_playbook_python }}" + +[delegated_vars] +testhost6 myhost=127.0.0.3 +testhost7 myhost=127.0.0.4 + +[delegated_vars:vars] +ansible_host={{myhost}} +ansible_connection=ssh diff --git a/test/integration/targets/delegate_to/runme.sh b/test/integration/targets/delegate_to/runme.sh index af090cdf02..52343163d6 100755 --- a/test/integration/targets/delegate_to/runme.sh +++ b/test/integration/targets/delegate_to/runme.sh @@ -48,7 +48,7 @@ ANSIBLE_SSH_ARGS='-C -o ControlMaster=auto -o ControlPersist=60s -o UserKnownHos # this test is not doing what it says it does, also relies on var that should not be available #ansible-playbook test_loop_control.yml -v "$@" -ansible-playbook test_delegate_to_loop_randomness.yml -v "$@" +ansible-playbook test_delegate_to_loop_randomness.yml -i inventory -v "$@" ansible-playbook delegate_and_nolog.yml -i inventory -v "$@" diff --git a/test/integration/targets/delegate_to/test_delegate_to.yml b/test/integration/targets/delegate_to/test_delegate_to.yml index 05b0536e68..dcfa9d0351 100644 --- a/test/integration/targets/delegate_to/test_delegate_to.yml +++ b/test/integration/targets/delegate_to/test_delegate_to.yml @@ -56,3 +56,27 @@ - name: remove test file file: path={{ output_dir }}/tmp.txt state=absent + + +- name: verify delegation with per host vars + hosts: testhost6 + gather_facts: yes + tasks: + - debug: msg={{ansible_facts['env']}} + + - name: ensure normal facts still work as expected + assert: + that: + - '"127.0.0.3" in ansible_facts["env"]["SSH_CONNECTION"]' + + - name: Test delegate_to with other host defined using same named var + setup: + register: setup_results + delegate_to: testhost7 + + - debug: msg={{setup_results.ansible_facts.ansible_env}} + + - name: verify ssh plugin resolves variable for ansible_host correctly + assert: + that: + - '"127.0.0.4" in setup_results.ansible_facts.ansible_env["SSH_CONNECTION"]' |