From 5c39c3b2d16acc0415de146c31c4b8ecaf01fb7d Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 17 May 2018 09:52:46 +1000 Subject: [PATCH] Module basic.py to create parent dirs of tmpdir if needed (#40201) * Module basic.py to create parent dirs of tmpdir if needed * Added warning to dir creation * Assert if make_dirs was called or not in unit tests --- lib/ansible/module_utils/basic.py | 8 +++++ test/units/module_utils/basic/test_tmpdir.py | 31 ++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index f05dc367e8..10c8f2a0fe 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -938,6 +938,14 @@ class AnsibleModule(object): # clean it up once finished. if self._tmpdir is None: basedir = os.path.expanduser(os.path.expandvars(self._remote_tmp)) + if not os.path.exists(basedir): + self.warn("Module remote_tmp %s did not exist and was created " + "with a mode of 0700, this may cause issues when " + "running as another user. To avoid this, create the " + "remote_tmp dir with the correct permissions " + "manually" % basedir) + os.makedirs(basedir, mode=0o700) + basefile = "ansible-moduletmp-%s-" % time.time() tmpdir = tempfile.mkdtemp(prefix=basefile, dir=basedir) if not self._keep_remote_files: diff --git a/test/units/module_utils/basic/test_tmpdir.py b/test/units/module_utils/basic/test_tmpdir.py index 0d7996719f..95c864c2cd 100644 --- a/test/units/module_utils/basic/test_tmpdir.py +++ b/test/units/module_utils/basic/test_tmpdir.py @@ -24,6 +24,7 @@ class TestAnsibleModuleTmpDir: "_ansible_remote_tmp": "/path/tmpdir", "_ansible_keep_remote_files": False, }, + True, "/path/to/dir" ), ( @@ -32,6 +33,16 @@ class TestAnsibleModuleTmpDir: "_ansible_remote_tmp": "/path/tmpdir", "_ansible_keep_remote_files": False }, + False, + "/path/tmpdir/ansible-moduletmp-42-" + ), + ( + { + "_ansible_tmpdir": None, + "_ansible_remote_tmp": "/path/tmpdir", + "_ansible_keep_remote_files": False + }, + True, "/path/tmpdir/ansible-moduletmp-42-" ), ( @@ -40,18 +51,31 @@ class TestAnsibleModuleTmpDir: "_ansible_remote_tmp": "$HOME/.test", "_ansible_keep_remote_files": False }, + False, os.path.join(os.environ['HOME'], ".test/ansible-moduletmp-42-") ), ) # pylint bug: https://github.com/PyCQA/pylint/issues/511 # pylint: disable=undefined-variable - @pytest.mark.parametrize('stdin, expected', ((s, e) for s, e in DATA), + @pytest.mark.parametrize('stdin, expected, stat_exists', ((s, e, t) for s, t, e in DATA), indirect=['stdin']) - def test_tmpdir_property(self, am, monkeypatch, expected): + def test_tmpdir_property(self, am, monkeypatch, expected, stat_exists): + makedirs = {'called': False} + def mock_mkdtemp(prefix, dir): return os.path.join(dir, prefix) + + def mock_makedirs(path, mode): + makedirs['called'] = True + expected = os.path.expanduser(os.path.expandvars(am._remote_tmp)) + assert path == expected + assert mode == 0o700 + return + monkeypatch.setattr(tempfile, 'mkdtemp', mock_mkdtemp) + monkeypatch.setattr(os.path, 'exists', lambda x: stat_exists) + monkeypatch.setattr(os, 'makedirs', mock_makedirs) monkeypatch.setattr(shutil, 'rmtree', lambda x: None) with patch('time.time', return_value=42): @@ -60,3 +84,6 @@ class TestAnsibleModuleTmpDir: # verify subsequent calls always produces the same tmpdir assert am.tmpdir == actual_tmpdir + + if not stat_exists: + assert makedirs['called']