Adding --omit-env-files option (#1000) (#1022)

[2.1] [backport] Adding --omit-env-files

Back porting #1000 to 2.1 branch.
Adding --omit-env-files option
Adding an option to not write out files into the env folder as these can contain sensitive information.
Changing write fifo to take either bytes or a string.
Initially based off of #812
Reviewed-by: Shane McDonald me@shanemcd.com
Reviewed-by: John Westcott IV 
Reviewed-by: David Shrewsbury

Reviewed-by: Shane McDonald <me@shanemcd.com>

ansible_runner/__main__.py

@@ -217,6 +217,14 @@
                      "(status and stdout still included for other events)"
             ),
         ),
+        (
+            ("--omit-env-files",),
+            dict(
+                action="store_true",
+                dest='suppress_env_files',
+                help="Add flag to prevent the writing of the env directory"
+            ),
+        ),
         (
             ("-q", "--quiet",),
             dict(
@@ -895,7 +903,8 @@ def main(sys_args=None):
                                    resource_profiling_results_dir=vargs.get('resource_profiling_results_dir'),
                                    cmdline=vargs.get('cmdline'),
                                    limit=vargs.get('limit'),
-                                   streamer=streamer
+                                   streamer=streamer,
+                                   suppress_env_files=vargs.get("suppress_env_files"),
                                    )
                 try:
                     res = run(**run_options)

ansible_runner/config/_base.py

@@ -64,7 +64,8 @@ def __init__(self,
                  project_dir=None, artifact_dir=None, fact_cache_type='jsonfile', fact_cache=None,
                  process_isolation=False, process_isolation_executable=None,
                  container_image=None, container_volume_mounts=None, container_options=None, container_workdir=None, container_auth_data=None,
-                 ident=None, rotate_artifacts=0, timeout=None, ssh_key=None, quiet=False, json_mode=False, check_job_event_data=False):
+                 ident=None, rotate_artifacts=0, timeout=None, ssh_key=None, quiet=False, json_mode=False,
+                 check_job_event_data=False, suppress_env_files=False):
         # common params
         self.host_cwd = host_cwd
         self.envvars = envvars
@@ -91,6 +92,7 @@ def __init__(self,
         self.settings = settings
         self.timeout = timeout
         self.check_job_event_data = check_job_event_data
+        self.suppress_env_files = suppress_env_files
 
         # setup initial environment
         if private_data_dir:
@@ -160,13 +162,18 @@ def _prepare_env(self, runner_mode='pexpect'):
                     self.passwords.update(self.loader.load_file('env/passwords', Mapping))
                 else:
                     self.passwords = self.passwords or self.loader.load_file('env/passwords', Mapping)
-                self.expect_passwords = {
-                    re.compile(pattern, re.M): password
-                    for pattern, password in iteritems(self.passwords)
-                }
             except ConfigurationError:
                 debug('Not loading passwords')
-                self.expect_passwords = dict()
+
+            self.expect_passwords = dict()
+            try:
+                if self.passwords:
+                    self.expect_passwords = {
+                        re.compile(pattern, re.M): password
+                        for pattern, password in iteritems(self.passwords)
+                    }
+            except Exception as e:
+                debug('Failed to compile RE from passwords: {}'.format(e))
 
             self.expect_passwords[pexpect.TIMEOUT] = None
             self.expect_passwords[pexpect.EOF] = None

ansible_runner/interface.py

@@ -165,6 +165,7 @@ def run(**kwargs):
                      be read from ``env/settings`` in ``private_data_dir``.
     :param str ssh_key: The ssh private key passed to ``ssh-agent`` as part of the ansible-playbook run.
     :param str cmdline: Command line options passed to Ansible read from ``env/cmdline`` in ``private_data_dir``
+    :param bool suppress_env_files: Disable the writing of files into the ``env`` which may store sensitive information
     :param str limit: Matches ansible's ``--limit`` parameter to further constrain the inventory to be used
     :param int forks: Control Ansible parallel concurrency
     :param int verbosity: Control how verbose the output of ansible-playbook is

ansible_runner/utils/__init__.py

@@ -222,19 +222,20 @@ def dump_artifacts(kwargs):
             if not os.path.exists(obj):
                 kwargs['inventory'] = dump_artifact(obj, path, 'hosts')
 
-    for key in ('envvars', 'extravars', 'passwords', 'settings'):
-        obj = kwargs.get(key)
-        if obj and not os.path.exists(os.path.join(private_data_dir, 'env', key)):
-            path = os.path.join(private_data_dir, 'env')
-            dump_artifact(json.dumps(obj), path, key)
-            kwargs.pop(key)
-
-    for key in ('ssh_key', 'cmdline'):
-        obj = kwargs.get(key)
-        if obj and not os.path.exists(os.path.join(private_data_dir, 'env', key)):
-            path = os.path.join(private_data_dir, 'env')
-            dump_artifact(str(kwargs[key]), path, key)
-            kwargs.pop(key)
+    if not kwargs.get('suppress_env_files', False):
+        for key in ('envvars', 'extravars', 'passwords', 'settings'):
+            obj = kwargs.get(key)
+            if obj and not os.path.exists(os.path.join(private_data_dir, 'env', key)):
+                path = os.path.join(private_data_dir, 'env')
+                dump_artifact(json.dumps(obj), path, key)
+                kwargs.pop(key)
+
+        for key in ('ssh_key', 'cmdline'):
+            obj = kwargs.get(key)
+            if obj and not os.path.exists(os.path.join(private_data_dir, 'env', key)):
+                path = os.path.join(private_data_dir, 'env')
+                dump_artifact(str(kwargs[key]), path, key)
+                kwargs.pop(key)
 
 
 def collect_new_events(event_path, old_events):
@@ -390,6 +391,9 @@ def open_fifo_write(path, data):
     reads data from the pipe.
     '''
     os.mkfifo(path, stat.S_IRUSR | stat.S_IWUSR)
+    # If the data is a string instead of bytes, convert it before writing the fifo
+    if type(data) == str:
+        data = data.encode()
     threading.Thread(target=lambda p, d: open(p, 'wb').write(d),
                      args=(path, data)).start()
 

test/integration/test_interface.py

@@ -47,6 +47,12 @@ def test_env_accuracy(request, project_fixtures):
     printenv_example = project_fixtures / 'printenv'
     os.environ['SET_BEFORE_TEST'] = 'MADE_UP_VALUE'
 
+    # Remove the envvars file if it exists
+    try:
+        os.remove(printenv_example / "env/envvars")
+    except FileNotFoundError:
+        pass
+
     def remove_test_env_var():
         if 'SET_BEFORE_TEST' in os.environ:
             del os.environ['SET_BEFORE_TEST']
@@ -65,6 +71,31 @@ def remove_test_env_var():
 
     assert actual_env == res.config.env
 
+    # Assert that the env file was properly created
+    assert os.path.exists(printenv_example / "env/envvars") == 1
+
+
+def test_no_env_files(request, project_fixtures):
+    printenv_example = project_fixtures / 'printenv'
+    os.environ['SET_BEFORE_TEST'] = 'MADE_UP_VALUE'
+
+    # Remove the envvars file if it exists
+    try:
+        os.remove(printenv_example / "env/envvars")
+    except FileNotFoundError:
+        pass
+
+    res = run(
+        private_data_dir=printenv_example,
+        playbook='get_environment.yml',
+        inventory=None,
+        envvars={'FROM_TEST': 'FOOBAR'},
+        suppress_env_files=True,
+    )
+    assert res.rc == 0, res.stdout.read()
+    # Assert that the env file was not created
+    assert os.path.exists(printenv_example / "env/envvars") == 0
+
 
 @pytest.mark.test_all_runtimes
 def test_env_accuracy_inside_container(request, project_fixtures, runtime):

test/unit/utils/test_dump_artifacts.py

@@ -147,6 +147,40 @@ def test_dump_artifacts_inventory_object(mocker):
     assert mock_dump_artifact.called_once_with(inv_string, '/tmp/inventory', 'hosts.json')
 
 
+def test_dump_artifacts_passwords(mocker):
+    mock_dump_artifact = mocker.patch('ansible_runner.utils.dump_artifact')
+
+    kwargs = {
+        'private_data_dir': '/tmp',
+        'passwords': {"a": "b"},
+        'envvars': {"abc": "def"},
+        'ssh_key': 'asdfg1234',
+    }
+
+    dump_artifacts(kwargs)
+
+    assert mock_dump_artifact.call_count == 3
+    mock_dump_artifact.assert_any_call('{"a": "b"}', '/tmp/env', 'passwords')
+    mock_dump_artifact.assert_any_call('{"abc": "def"}', '/tmp/env', 'envvars')
+    mock_dump_artifact.assert_called_with('asdfg1234', '/tmp/env', 'ssh_key')
+
+
+def test_dont_dump_artifacts_passwords(mocker):
+    mock_dump_artifact = mocker.patch('ansible_runner.utils.dump_artifact')
+
+    kwargs = {
+        'private_data_dir': '/tmp',
+        'passwords': {"a": "b"},
+        'envvars': {"abd": "def"},
+        'ssh_key': 'asdfg1234',
+        'suppress_env_files': True
+    }
+
+    dump_artifacts(kwargs)
+
+    assert mock_dump_artifact.call_count == 0
+
+
 @pytest.mark.parametrize(
     ('key', 'value', 'value_str'), (
         ('extravars', {'foo': 'bar'}, '{"foo": "bar"}'),

test/unit/utils/test_fifo_pipe.py

@@ -0,0 +1,26 @@
+from ansible_runner.utils import open_fifo_write
+from os import remove
+
+
+def test_fifo_write_bytes(tmp_path):
+    path = tmp_path / "bytes_test"
+    data = "bytes"
+    try:
+        open_fifo_write(path, data.encode())
+        with open(path, 'r') as f:
+            results = f.read()
+        assert results == data
+    finally:
+        remove(path)
+
+
+def test_fifo_write_string(tmp_path):
+    path = tmp_path / "string_test"
+    data = "string"
+    try:
+        open_fifo_write(path, data)
+        with open(path, 'r') as f:
+            results = f.read()
+        assert results == data
+    finally:
+        remove(path)