Merge pull request #1115 from moreati/pr956

Initial support for templated `ansible_ssh_args`,   `ansible_ssh_common_args`, and `ansible_ssh_extra_args`

.ci/ansible_tests.py

@@ -6,11 +6,13 @@ import glob
 import os
 import signal
 import sys
-import textwrap
+
+import jinja2
 
 import ci_lib
 
 
+TEMPLATES_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible/templates')
 TESTS_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible')
 HOSTS_DIR = os.path.join(ci_lib.TMP, 'hosts')
 
@@ -52,37 +54,19 @@ with ci_lib.Fold('job_setup'):
         distros[container['distro']].append(container['name'])
         families[container['family']].append(container['name'])
 
+    jinja_env = jinja2.Environment(
+        loader=jinja2.FileSystemLoader(searchpath=TEMPLATES_DIR),
+        lstrip_blocks=True,  # Remove spaces and tabs from before a block
+        trim_blocks=True,  # Remove first newline after a block
+    )
+    inventory_template = jinja_env.get_template('test-targets.j2')
     inventory_path = os.path.join(HOSTS_DIR, 'target')
+
     with open(inventory_path, 'w') as fp:
-        fp.write('[test-targets]\n')
-        fp.writelines(
-            "%(name)s "
-            "ansible_host=%(hostname)s "
-            "ansible_port=%(port)s "
-            "ansible_python_interpreter=%(python_path)s "
-            "ansible_user=mitogen__has_sudo_nopw "
-            "ansible_password=has_sudo_nopw_password"
-            "\n"
-            % container
-            for container in containers
-        )
-
-        for distro, hostnames in sorted(distros.items(), key=lambda t: t[0]):
-            fp.write('\n[%s]\n' % distro)
-            fp.writelines('%s\n' % name for name in hostnames)
-
-        for family, hostnames in sorted(families.items(), key=lambda t: t[0]):
-            fp.write('\n[%s]\n' % family)
-            fp.writelines('%s\n' % name for name in hostnames)
-
-        fp.write(textwrap.dedent(
-            '''
-            [linux:children]
-            test-targets
-
-            [linux_containers:children]
-            test-targets
-            '''
+        fp.write(inventory_template.render(
+            containers=containers,
+            distros=distros,
+            families=families,
         ))
 
     ci_lib.dump_file(inventory_path)

ansible_mitogen/transport_config.py

@@ -498,12 +498,13 @@ class PlayContextSpec(Spec):
         )
 
     def ssh_args(self):
+        local_vars = self._task_vars.get("hostvars", {}).get(self._inventory_name, {})
         return [
             mitogen.core.to_text(term)
             for s in (
-                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
-                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
-                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {}))
+                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
+                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
+                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=local_vars)
             )
             for term in ansible.utils.shlex.shlex_split(s or '')
         ]
@@ -738,12 +739,13 @@ class MitogenViaSpec(Spec):
         )
 
     def ssh_args(self):
+        local_vars = self._task_vars.get("hostvars", {}).get(self._inventory_name, {})
         return [
             mitogen.core.to_text(term)
             for s in (
-                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
-                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
-                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {}))
+                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
+                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
+                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=local_vars)
             )
             for term in ansible.utils.shlex.shlex_split(s)
             if s

docs/changelog.rst

@@ -26,6 +26,9 @@ Unreleased
 * :gh:issue:`1110` Fix :exc:`mitogen.core.StreamError` when Ansible copy
   module is called with a file larger than 124 kibibytes
   (:data:`ansible_mitogen.connection.Connection.SMALL_FILE_LIMIT`)
+* :gh:issue:`905` Initial support for templated ``ansible_ssh_args``,
+  ``ansible_ssh_common_args``, and ``ansible_ssh_extra_args`` variables.
+  NB: play or task scoped variables will probably still fail.
 
 
 v0.3.9 (2024-08-13)

docs/contributors.rst

@@ -116,6 +116,7 @@ sponsorship and outstanding future-thinking of its early adopters.
 
     <ul>
     <li>Alex Willmer</li>
+    <li><a href="https://github.com/momiji">Christian Bourgeois </a></li>
     <li><a href="https://underwhelm.net/">Dan Dorman</a> &mdash; - <em>When I truly understand my enemy … then in that very moment I also love him.</em></li>
     <li>Daniel Foerster</li>
     <li><a href="https://www.deps.co/">Deps</a> &mdash; <em>Private Maven Repository Hosting for Java, Scala, Groovy, Clojure</em></li>

tests/ansible/hosts/default.hosts

@@ -12,3 +12,10 @@ target ansible_host=localhost ansible_user="{{ lookup('pipe', 'whoami') }}"
 target
 
 [linux_containers]
+
+[issue905]
+ssh-common-args ansible_host=localhost ansible_user="{{ lookup('pipe', 'whoami') }}"
+
+[issue905:vars]
+ansible_ssh_common_args=-o PermitLocalCommand=yes -o LocalCommand="touch {{ ssh_args_canary_file }}"
+ssh_args_canary_file=/tmp/ssh_args_{{ inventory_hostname }}

tests/ansible/integration/ssh/all.yml

@@ -1,3 +1,4 @@
+- import_playbook: args.yml
 - import_playbook: config.yml
 - import_playbook: password.yml
 - import_playbook: timeouts.yml

tests/ansible/integration/ssh/args.yml

@@ -0,0 +1,48 @@
+- name: integration/ssh/args.yml
+  hosts: issue905
+  gather_facts: false
+  tasks:
+    # Test that ansible_ssh_common_args are templated; ansible_ssh_args &
+    # ansible_ssh_extra_args aren't directly tested, we assume they're similar.
+    # FIXME This test currently relies on variables set in the host group.
+    #       Ideally they'd be set here, and the host group eliminated, but
+    #       Mitogen currently fails to template when defined in the play.
+    # TODO Replace LocalCommand canary with SetEnv canary, to simplify test.
+    #      Requires modification of sshd_config files to add AcceptEnv ...
+    - name: Test templating of ansible_ssh_common_args et al
+      block:
+        - name: Ensure no lingering canary files
+          file:
+            path: "{{ ssh_args_canary_file }}"
+            state: absent
+          delegate_to: localhost
+
+        - name: Reset connections to force new ssh execution
+          meta: reset_connection
+
+        - name: Perform SSH connection, to trigger side effect
+          ping:
+
+        # LocalCommand="touch {{ ssh_args_canary_file }}" in ssh_*_args
+        - name: Stat for canary file created by side effect
+          stat:
+            path: "{{ ssh_args_canary_file }}"
+          delegate_to: localhost
+          register: ssh_args_canary_stat
+
+        - assert:
+            that:
+              - ssh_args_canary_stat.stat.exists == true
+            quiet: true
+            success_msg: "Canary found: {{ ssh_args_canary_file }}"
+            fail_msg: |
+              ssh_args_canary_file={{ ssh_args_canary_file }}
+              ssh_args_canary_stat={{ ssh_args_canary_stat }}
+      always:
+        - name: Cleanup canary files
+          file:
+            path: "{{ ssh_args_canary_file }}"
+            state: absent
+          delegate_to: localhost
+  tags:
+    - issue_905

tests/ansible/templates/test-targets.j2

@@ -0,0 +1,39 @@
+[test-targets]
+{% for c in containers %}
+{{ c.name }} ansible_host={{ c.hostname }} ansible_port={{ c.port }} ansible_python_interpreter={{ c.python_path }}
+{% endfor %}
+
+[test-targets:vars]
+ansible_user=mitogen__has_sudo_nopw
+ansible_password=has_sudo_nopw_password
+
+{% for distro, hostnames in distros | dictsort %}
+[{{ distro }}]
+{% for hostname in hostnames %}
+{{ hostname }}
+{% endfor %}
+{% endfor %}
+
+{% for family, hostnames in families | dictsort %}
+[{{ family }}]
+{% for hostname in hostnames %}
+{{ hostname }}
+{% endfor %}
+{% endfor %}
+
+[linux:children]
+test-targets
+
+[linux_containers:children]
+test-targets
+
+[issue905]
+{% for c in containers[:1] %}
+ssh-common-args ansible_host={{ c.hostname }} ansible_port={{ c.port }} ansible_python_interpreter={{ c.python_path }}
+{% endfor %}
+
+[issue905:vars]
+ansible_user=mitogen__has_sudo_nopw
+ansible_password=has_sudo_nopw_password
+ansible_ssh_common_args=-o PermitLocalCommand=yes -o LocalCommand="touch {{ '{{' }} ssh_args_canary_file {{ '}}' }}"
+ssh_args_canary_file=/tmp/ssh_args_{{ '{{' }} inventory_hostname {{ '}}' }}