Refactored ephemeral directory use (#2089)

Avoid hardcoded temporary directories which can create conflicts with with multi-user environments. Signed-off-by: Sorin Sbarnea <[email protected]> Fixes: #1506
ansible · Jun 22, 2019 · b44fe71 · b44fe71
1 parent f51efaf
commit b44fe71
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 32 deletions.
diff --git a/molecule/scenario.py b/molecule/scenario.py
@@ -18,9 +18,15 @@
 #  FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 #  DEALINGS IN THE SOFTWARE.
 
+import getpass
 import os
+import platform
 import fnmatch
 import tempfile
+try:
+    from pathlib import Path
+except ImportError:
+    from pathlib2 import Path
 
 from molecule import logger
 from molecule import scenarios
@@ -226,8 +232,24 @@ def _setup(self):
             os.makedirs(self.inventory_directory)
 
 
-def ephemeral_directory(path):
+def ephemeral_directory(path=None):
+    """
+    Returns temporary directory to be used by molecule. Molecule users should
+    not make any assumptions about its location, permissions or its content as
+    this may change in future release.
+    """
     d = os.getenv('MOLECULE_EPHEMERAL_DIRECTORY')
-    if d:
-        return os.path.join(tempfile.gettempdir(), d)
-    return os.path.join(tempfile.gettempdir(), path)
+    if not d:
+        # Darwin is the only known platform that returns user-isolated tempdirs
+        # so we do not need to make them unique to each user.
+        if platform.system() == 'Darwin':
+            d = tempfile.gettempdir()
+        else:
+            d = os.path.join(tempfile.gettempdir(), 'mol.' + getpass.getuser())
+    else:
+        d = os.path.abspath(d)
+    if path:
+        d = os.path.abspath(os.path.join(d, path))
+        if not os.path.isdir(d):
+            Path(d).mkdir(mode=0o700, parents=True, exist_ok=True)
+    return d
diff --git a/setup.cfg b/setup.cfg
@@ -80,6 +80,7 @@ install_requires =
     python-gilt >= 1.2.1, < 2
     Jinja2 >= 2.10.1
     paramiko >= 2.5.0, < 3
+    pathlib2; python_version<"3.2"
     pexpect >= 4.6.0, < 5
     psutil == 5.4.6; sys_platform!="win32" and sys_platform!="cygwin"
     PyYAML >= 5.1, < 6

diff --git a/test/conftest.py b/test/conftest.py
@@ -22,13 +22,13 @@
 import os
 import random
 import string
-import tempfile
 
 import pytest
 
 from molecule import config
 from molecule import logger
 from molecule import util
+from molecule.scenario import ephemeral_directory
 
 LOG = logger.get_logger(__name__)
 
@@ -101,8 +101,7 @@ def molecule_ephemeral_directory():
     project_directory = 'test-project'
     scenario_name = 'test-instance'
 
-    return os.path.join(tempfile.gettempdir(), 'molecule', project_directory,
-                        scenario_name)
+    return ephemeral_directory(os.path.join(project_directory, scenario_name))
 
 
 def pytest_addoption(parser):

diff --git a/test/functional/test_command.py b/test/functional/test_command.py
@@ -19,7 +19,7 @@
 #  DEALINGS IN THE SOFTWARE.
 
 import os
-import tempfile
+from molecule.scenario import ephemeral_directory
 
 import pytest
 import sh
@@ -193,9 +193,9 @@ def test_command_dependency_ansible_galaxy(request, scenario_to_test,
     cmd = sh.molecule.bake('dependency', **options)
     pytest.helpers.run_command(cmd)
 
-    dependency_role = os.path.join(tempfile.gettempdir(), 'molecule',
-                                   'dependency', 'ansible-galaxy', 'roles',
-                                   'timezone')
+    dependency_role = os.path.join(
+        ephemeral_directory('molecule'), 'dependency', 'ansible-galaxy',
+        'roles', 'timezone')
     assert os.path.isdir(dependency_role)
 
 
@@ -227,8 +227,9 @@ def test_command_dependency_gilt(request, scenario_to_test, with_scenario,
     cmd = sh.molecule.bake('dependency', **options)
     pytest.helpers.run_command(cmd)
 
-    dependency_role = os.path.join(tempfile.gettempdir(), 'molecule',
-                                   'dependency', 'gilt', 'roles', 'timezone')
+    dependency_role = os.path.join(
+        ephemeral_directory('molecule'), 'dependency', 'gilt', 'roles',
+        'timezone')
     assert os.path.isdir(dependency_role)
 
 
@@ -260,8 +261,9 @@ def test_command_dependency_shell(request, scenario_to_test, with_scenario,
     cmd = sh.molecule.bake('dependency', **options)
     pytest.helpers.run_command(cmd)
 
-    dependency_role = os.path.join(tempfile.gettempdir(), 'molecule',
-                                   'dependency', 'shell', 'roles', 'timezone')
+    dependency_role = os.path.join(
+        ephemeral_directory('molecule'), 'dependency', 'shell', 'roles',
+        'timezone')
     assert os.path.isdir(dependency_role)
 
 

diff --git a/test/unit/conftest.py b/test/unit/conftest.py
@@ -24,14 +24,14 @@
 import os
 import re
 import shutil
-import tempfile
 
 import pytest
 
 from molecule import util
 from molecule import config
+from molecule.scenario import ephemeral_directory
 
-for d in glob.glob(os.path.join(tempfile.gettempdir(), 'molecule', '*')):
+for d in glob.glob(os.path.join(ephemeral_directory('molecule'), '*')):
     if re.search('[A-Z]{5}$', d):
         shutil.rmtree(d)
 

diff --git a/test/unit/test_scenario.py b/test/unit/test_scenario.py
@@ -20,7 +20,6 @@
 
 import os
 import shutil
-import tempfile
 
 import pytest
 
@@ -109,13 +108,7 @@ def test_directory_property(molecule_scenario_directory_fixture, _instance):
 
 
 def test_ephemeral_directory_property(_instance):
-    project_directory = os.path.basename(_instance.config.project_directory)
-    scenario_name = _instance.name
-    project_scenario_directory = os.path.join('molecule', project_directory,
-                                              scenario_name)
-    e_dir = os.path.join(tempfile.gettempdir(), project_scenario_directory)
-
-    assert e_dir == _instance.ephemeral_directory
+    assert os.access(_instance.ephemeral_directory, os.W_OK)
 
 
 def test_inventory_directory_property(_instance):
@@ -235,20 +228,18 @@ def test_setup_creates_ephemeral_and_inventory_directories(_instance):
 
 
 def test_ephemeral_directory():
-    e_dir = os.path.join(tempfile.gettempdir(), 'foo/bar')
-
-    assert e_dir == scenario.ephemeral_directory('foo/bar')
+    # assure we can write to ephemeral directory
+    assert os.access(scenario.ephemeral_directory('foo/bar'), os.W_OK)
 
 
 def test_ephemeral_directory_overriden_via_env_var(monkeypatch):
     monkeypatch.setenv('MOLECULE_EPHEMERAL_DIRECTORY', 'foo/bar')
-    e_dir = os.path.join(tempfile.gettempdir(), 'foo/bar')
 
-    assert e_dir == scenario.ephemeral_directory('foo/bar')
+    assert os.access(scenario.ephemeral_directory('foo/bar'), os.W_OK)
 
 
 def test_ephemeral_directory_overriden_via_env_var_uses_absolute_path(
         monkeypatch):
-    monkeypatch.setenv('MOLECULE_EPHEMERAL_DIRECTORY', '/foo/bar')
+    monkeypatch.setenv('MOLECULE_EPHEMERAL_DIRECTORY', "foo/bar")
 
-    assert '/foo/bar' == scenario.ephemeral_directory('foo/bar')
+    assert os.path.isabs(scenario.ephemeral_directory())