ssh connection reset and ssh tokens in controlpath

[apatard]

SUMMARY

When investigating ansible-community/molecule-vagrant#1, I found out that meta: reset_connection is not working due to the reset() function of the ssh.py connection plugin not being able to find the connection socket. It turns out that molecule is using control_path = %(directory)s/%%h-%%p-%%r. For instance, it's translated into ControlPath=/home/vagrant/.ansible/cp/%h-%p-%r on the ssh command line.

The reset code does :

        run_reset = False
        if controlpersist and len(cp_arg) > 0:
            cp_path = cp_arg[0].split(b"=", 1)[-1]
            if os.path.exists(cp_path):
                run_reset = True
        elif controlpersist:
            run_reset = True

Due to the content of the ControlPath argument, it will set cp_path to /home/vagrant/.ansible/cp/%h-%p-%r and of course, the os.path.exists(cp_path) will fail, making "meta: reset_connection" useless.
fwiw, looks like this bug as been introduced by the fix for #42991

A crude workaround would be changing the test to if b'%' in cp_path or os.path.exists(cp_path). It may be better to interpolate the ssh tokens but I've no idea if it's really possible and how to do that.

ISSUE TYPE

• Bug Report

COMPONENT NAME

ssh connection plugin

ANSIBLE VERSION

ansible --version
ansible 2.9.6
  config file = /home/rtp/devel/hupstream/ansible/molecule-vagrant-zuul/t/ansible.cfg
  configured module search path = ['/home/rtp/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/rtp/.local/lib/python3.7/site-packages/ansible
  executable location = /home/rtp/.local/bin/ansible
  python version = 3.7.3 (default, Dec 20 2019, 18:57:59) [GCC 8.3.0]

CONFIGURATION

[defaults]
ansible_managed = Ansible managed: Do NOT edit this file manually!
display_failed_stderr = True
forks = 50
retry_files_enabled = False
host_key_checking = False
nocows = 1
interpreter_python = auto
[ssh_connection]
scp_if_ssh = True
control_path = %(directory)s/%%h-%%p-%%r
pipelining = True

OS / ENVIRONMENT

Debian stable

STEPS TO REPRODUCE

Since I was testing that for molecule-vagrant, I've a Vagrantfile and a converge.yml file:

Vagrantfile:

Vagrant.configure("2") do |c|
  c.vm.define 'test2' do |v|
    v.vm.hostname = 'test2'
    v.vm.box = 'debian/buster64'
  end
  c.vm.provision :ansible do |ansible|
    ansible.playbook = "converge.yml"
  end
end

converge.yml

---
- name: Converge
  hosts: all
  gather_facts: false
  tasks:
    - name: Create test group
      group:
        name: testgroup
      become: true
    - name: Add vagrant user to test group
      user:
        name: vagrant
        groups: testgroup
        append: yes
      become: true
    - name: reset connection
      meta: reset_connection
    - name: Get vagrant user info
      command: id -nG
      register: user_grps
    - name: Print user_grps
      debug:
        var: user_grps
    - name: Check user in vagrant group
      assert:
        that:
          - "'testgroup' in user_grps.stdout.split(' ')"

ansible.cfg

[ssh_connection]
control_path = %(directory)s/%%h-%%p-%%r

EXPECTED RESULTS

The run of the converge.yml should work

ACTUAL RESULTS

TASK [Check user in vagrant group] *********************************************
fatal: [test2]: FAILED! => {
    "assertion": "'testgroup' in user_grps.stdout.split(' ')",
    "changed": false,
    "evaluated_to": false,
    "msg": "Assertion failed"
}

[ansibot]

Files identified in the description:

• lib/ansible/plugins/connection/ssh.py

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

[apatard]

Any idea on how to fix this ?

[sivel]

I think the code based fix will be something roughly like this:

diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py
index ed44a035fe..fa5ad29cf2 100644
--- a/lib/ansible/plugins/connection/ssh.py
+++ b/lib/ansible/plugins/connection/ssh.py
@@ -1266,7 +1266,7 @@ class Connection(ConnectionBase):
         run_reset = False
         if controlpersist and len(cp_arg) > 0:
             cp_path = cp_arg[0].split(b"=", 1)[-1]
-            if os.path.exists(cp_path):
+            if os.path.exists(cp_path) or re.search(r'%[hprC]', cp_path):
                 run_reset = True
         elif controlpersist:
             run_reset = True

I haven't tested this, and am not actively pursuing this fix any further. If you want to test this, and get together a pull request, please feel free to do so.

[apatard]

thanks. I'll test that. I guess this would work but I'm still worried about re-introducing bug #42991.