Merge pull request #4571 from chrismeyersfsu/fix-4506

pass network ssh key via an env var, not ssh-agent

awx/main/tasks.py

@@ -374,9 +374,12 @@ class BaseTask(Task):
                     data += '\n'
                 # For credentials used with ssh-add, write to a named pipe which
                 # will be read then closed, instead of leaving the SSH key on disk.
-                if name in ('credential', 'network_credential', 'scm_credential', 'ad_hoc_credential') and not ssh_too_old:
+                if name in ('credential', 'scm_credential', 'ad_hoc_credential') and not ssh_too_old:
                     path = os.path.join(kwargs.get('private_data_dir', tempfile.gettempdir()), name)
                     self.open_fifo_write(path, data)
+                # Ansible network modules do not yet support ssh-agent.
+                # Instead, ssh private key file is explicitly passed via an
+                # env variable.
                 else:
                     handle, path = tempfile.mkstemp(dir=kwargs.get('private_data_dir', None))
                     f = os.fdopen(handle, 'w')
@@ -875,6 +878,10 @@ class RunJob(BaseTask):
             env['ANSIBLE_NET_USERNAME'] = network_cred.username
             env['ANSIBLE_NET_PASSWORD'] = decrypt_field(network_cred, 'password')
 
+            ssh_keyfile = kwargs.get('private_data_files', {}).get('network_credential', '')
+            if ssh_keyfile:
+                env['ANSIBLE_NET_SSH_KEYFILE'] = ssh_keyfile
+
             authorize = network_cred.authorize
             env['ANSIBLE_NET_AUTHORIZE'] = unicode(int(authorize))
             if authorize:
@@ -1037,8 +1044,15 @@ class RunJob(BaseTask):
         private_data_files = kwargs.get('private_data_files', {})
         if 'credential' in private_data_files:
             return private_data_files.get('credential')
-        elif 'network_credential' in private_data_files:
+        '''
-            return private_data_files.get('network_credential')
+        Note: Don't inject network ssh key data into ssh-agent for network
+        credentials because the ansible modules do not yet support it.
+        We will want to add back in support when/if Ansible network modules
+        support this.
+        '''
+        #elif 'network_credential' in private_data_files:
+        #    return private_data_files.get('network_credential')
+
         return ''
 
     def should_use_proot(self, instance, **kwargs):

awx/main/tests/unit/test_network_credential.py

@@ -1,3 +1,5 @@
+import pytest
+
 from awx.main.models.credential import Credential
 from awx.main.models.jobs import Job
 from awx.main.models.inventory import Inventory
@@ -36,50 +38,84 @@ def test_net_cred_parse(mocker):
             'password':'test',
             'authorize': True,
             'authorize_password': 'passwd',
+            'ssh_key_data': """-----BEGIN PRIVATE KEY-----\nstuff==\n-----END PRIVATE KEY-----""",
+        }
+        private_data_files = {
+            'network_credential': '/tmp/this_file_does_not_exist_during_test_but_the_path_is_real',
         }
         job.network_credential = Credential(**options)
 
         run_job = RunJob()
         mocker.patch.object(run_job, 'should_use_proot', return_value=False)
 
-        env = run_job.build_env(job, private_data_dir='/tmp')
+        env = run_job.build_env(job, private_data_dir='/tmp', private_data_files=private_data_files)
         assert env['ANSIBLE_NET_USERNAME'] == options['username']
         assert env['ANSIBLE_NET_PASSWORD'] == options['password']
         assert env['ANSIBLE_NET_AUTHORIZE'] == '1'
         assert env['ANSIBLE_NET_AUTH_PASS'] == options['authorize_password']
+        assert env['ANSIBLE_NET_SSH_KEYFILE'] == private_data_files['network_credential']
 
 
-def test_net_cred_ssh_agent(mocker, get_ssh_version):
+@pytest.fixture
-    with mocker.patch('django.db.ConnectionRouter.db_for_write'):
+def mock_job(mocker):
-        run_job = RunJob()
+    options = {
+        'username':'test',
+        'password':'test',
+        'ssh_key_data': """-----BEGIN PRIVATE KEY-----\nstuff==\n-----END PRIVATE KEY-----""",
+        'authorize': True,
+        'authorize_password': 'passwd',
+    }
 
-        options = {
+    mock_job_attrs = {'forks': False, 'id': 1, 'cancel_flag': False, 'status': 'running', 'job_type': 'normal',
-            'username':'test',
+                      'credential': None, 'cloud_credential': None, 'network_credential': Credential(**options),
-            'password':'test',
+                      'become_enabled': False, 'become_method': None, 'become_username': None,
-            'ssh_key_data': """-----BEGIN PRIVATE KEY-----\nstuff==\n-----END PRIVATE KEY-----""",
+                      'inventory': mocker.MagicMock(spec=Inventory, id=2), 'force_handlers': False,
-            'authorize': True,
+                      'limit': None, 'verbosity': None, 'job_tags': None, 'skip_tags': None,
-            'authorize_password': 'passwd',
+                      'start_at_task': None, 'pk': 1, 'launch_type': 'normal', 'job_template':None,
-        }
+                      'created_by': None, 'extra_vars_dict': None, 'project':None, 'playbook': 'test.yml'}
-        mock_job_attrs = {'forks': False, 'id': 1, 'cancel_flag': False, 'status': 'running', 'job_type': 'normal',
+    mock_job = mocker.MagicMock(spec=Job, **mock_job_attrs)
-                          'credential': None, 'cloud_credential': None, 'network_credential': Credential(**options),
+    return mock_job
-                          'become_enabled': False, 'become_method': None, 'become_username': None,
-                          'inventory': mocker.MagicMock(spec=Inventory, id=2), 'force_handlers': False,
-                          'limit': None, 'verbosity': None, 'job_tags': None, 'skip_tags': None,
-                          'start_at_task': None, 'pk': 1, 'launch_type': 'normal', 'job_template':None,
-                          'created_by': None, 'extra_vars_dict': None, 'project':None, 'playbook': 'test.yml'}
-        mock_job = mocker.MagicMock(spec=Job, **mock_job_attrs)
 
-        mocker.patch.object(run_job, 'update_model', return_value=mock_job)
-        mocker.patch.object(run_job, 'build_cwd', return_value='/tmp')
-        mocker.patch.object(run_job, 'should_use_proot', return_value=False)
-        mocker.patch.object(run_job, 'run_pexpect', return_value=('successful', 0))
-        mocker.patch.object(run_job, 'open_fifo_write', return_value=None)
-        mocker.patch.object(run_job, 'post_run_hook', return_value=None)
 
-        run_job.run(mock_job.id)
+@pytest.fixture
-        assert run_job.update_model.call_count == 4
+def run_job_net_cred(mocker, get_ssh_version, mock_job):
+    mocker.patch('django.db.ConnectionRouter.db_for_write')
+    run_job = RunJob()
+
+    mocker.patch.object(run_job, 'update_model', return_value=mock_job)
+    mocker.patch.object(run_job, 'build_cwd', return_value='/tmp')
+    mocker.patch.object(run_job, 'should_use_proot', return_value=False)
+    mocker.patch.object(run_job, 'run_pexpect', return_value=('successful', 0))
+    mocker.patch.object(run_job, 'open_fifo_write', return_value=None)
+    mocker.patch.object(run_job, 'post_run_hook', return_value=None)
+
+    return run_job
+
+
+@pytest.mark.skip(reason="Note: Ansible network modules don't yet support ssh-agent added keys.")
+def test_net_cred_ssh_agent(run_job_net_cred, mock_job):
+    run_job = run_job_net_cred
+    run_job.run(mock_job.id)
+
+    assert run_job.update_model.call_count == 4
+
+    job_args = run_job.update_model.call_args_list[1][1].get('job_args')
+    assert 'ssh-add' in job_args
+    assert 'ssh-agent' in job_args
+    assert 'network_credential' in job_args
+
+
+def test_net_cred_job_model_env(run_job_net_cred, mock_job):
+    run_job = run_job_net_cred
+    run_job.run(mock_job.id)
+
+    assert run_job.update_model.call_count == 4
+
+    job_args = run_job.update_model.call_args_list[1][1].get('job_env')
+    assert 'ANSIBLE_NET_USERNAME' in job_args
+    assert 'ANSIBLE_NET_PASSWORD' in job_args
+    assert 'ANSIBLE_NET_AUTHORIZE' in job_args
+    assert 'ANSIBLE_NET_AUTH_PASS' in job_args
+    assert 'ANSIBLE_NET_SSH_KEYFILE' in job_args
+
 
-        job_args = run_job.update_model.call_args_list[1][1].get('job_args')
-        assert 'ssh-add' in job_args
-        assert 'ssh-agent' in job_args
-        assert 'network_credential' in job_args