Merge ~smoser/cloud-init:bug/1707222-use-run-for-tmpdir into cloud-init:master

[Scott Moser]

Use /run/cloud-init for tempfile operations.

During boot, the usage of /tmp is not safe. In systemd systems,
systemd-tmpfiles-clean may run at any point and clear out a temp file
while cloud-init is using it. The solution was to use
/run/cloud-init/tmp.

LP: #1707222

[Scott Moser (smoser)]

Issues with this currently are
a.) no unit tests of the _TEMPDIR path using /run/cloud-init.
b.) ExtendedTemporaryFile was using util.del_file, but I did not want a circular import to cloudinit.util
c.) could clean up /run/cloud-init via atexit
d.) tempdir() was using del_dir, but circular import.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:4b4e53977755b00b6105dafaeb117c260b0bf7d5
https://jenkins.ubuntu.com/server/job/cloud-init-ci/220/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    FAILED: Ubuntu LTS: Build

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/220/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/222/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/222/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:b6e31f58f80f85b9b5c14bc6f5b0f47d7b71ad5e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/221/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/221/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:6150f71ba59f92b6d5bb179f1b78c6235d37ba93
https://jenkins.ubuntu.com/server/job/cloud-init-ci/223/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/223/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Fixing

189c266... by Scott Moser on 2017-08-30

add 'param' doc for odir

3fcdeca... by Scott Moser on 2017-08-30

address dedent feedback, and actually assign to _TMPDIR

90724e4... by Scott Moser on 2017-08-30

try/except to avoid race of check then remove.

[Scott Moser (smoser)]

i think i've addressed all comments.

5ac4d45... by Scott Moser on 2017-08-30

correct errno usage

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:1020f5eff24fc2be551e027fee982c4895eb20a7
https://jenkins.ubuntu.com/server/job/cloud-init-ci/231/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/231/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:b2c155ef261d584647e7b2d1546507bb6de5de5d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/233/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/233/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Looks good, minor item and one question inline below.

a9ff924... by Scott Moser on 2017-08-31

file header and vi footer on temp_utils.py

7eacf50... by Scott Moser on 2017-08-31

improve comment in _tmpfile_dir_arg

[Scott Moser (smoser)]

adjusted to feedback. i'm not sure what to do about the Paths statement that Chad raised.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:3e8b05f3ec26e4bb97961091c2125f401fbb64b6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/237/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    FAILED: Ubuntu LTS: Build

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/237/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:41ef7485d1589a7c4a379763d4e4fdcc6d88d12b
https://jenkins.ubuntu.com/server/job/cloud-init-ci/239/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/239/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

> adjusted to feedback. i'm not sure what to do about the Paths statement that
> Chad raised.in

Scott, let's not block on my paths comment for now. We can sort it in a followup branch if the need becomes greater. It might involve us reworking how Paths object is initialized which would allow both paths and this to source the same static config variables/globals. It

[Chad Smith (chad.smith)]

It might be nice to cover some of this in unittesting, but I can take a stab at it on a cc_bootcmd schema branch I'm working.

[Scott Moser (smoser)]

marked approved per Chad's last comment there.

[Chad Smith (chad.smith)]

review: Approve

c778531... by Scott Moser on 2017-09-07

docstring fix.

[Ryan Harper (raharper)]

review: Approve

diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py
index 604f93b..9c0476a 100644
--- a/cloudinit/config/cc_bootcmd.py
+++ b/cloudinit/config/cc_bootcmd.py
@@ -37,6 +37,7 @@ specified either as lists or strings. For invocation details, see ``runcmd``.
 import os
 
 from cloudinit.settings import PER_ALWAYS
+from cloudinit import temp_utils
 from cloudinit import util
 
 frequency = PER_ALWAYS
@@ -49,7 +50,7 @@ def handle(name, cfg, cloud, log, _args):
                    " no 'bootcmd' key in configuration"), name)
         return
 
-    with util.ExtendedTemporaryFile(suffix=".sh") as tmpf:
+    with temp_utils.ExtendedTemporaryFile(suffix=".sh") as tmpf:
         try:
             content = util.shellify(cfg["bootcmd"])
             tmpf.write(util.encode_text(content))
diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
index 02c70b1..c192dd3 100644
--- a/cloudinit/config/cc_chef.py
+++ b/cloudinit/config/cc_chef.py
@@ -71,6 +71,7 @@ import itertools
 import json
 import os
 
+from cloudinit import temp_utils
 from cloudinit import templater
 from cloudinit import url_helper
 from cloudinit import util
@@ -303,7 +304,7 @@ def install_chef(cloud, chef_cfg, log):
                                                  "omnibus_url_retries",
                                                  default=OMNIBUS_URL_RETRIES))
         content = url_helper.readurl(url=url, retries=retries).contents
-        with util.tempdir() as tmpd:
+        with temp_utils.tempdir() as tmpd:
             # Use tmpdir over tmpfile to avoid 'text file busy' on execute
             tmpf = "%s/chef-omnibus-install" % tmpd
             util.write_file(tmpf, content, mode=0o700)
diff --git a/cloudinit/config/cc_snappy.py b/cloudinit/config/cc_snappy.py
index a9682f1..eecb817 100644
--- a/cloudinit/config/cc_snappy.py
+++ b/cloudinit/config/cc_snappy.py
@@ -63,11 +63,11 @@ is ``auto``. Options are:
 
 from cloudinit import log as logging
 from cloudinit.settings import PER_INSTANCE
+from cloudinit import temp_utils
 from cloudinit import util
 
 import glob
 import os
-import tempfile
 
 LOG = logging.getLogger(__name__)
 
@@ -183,7 +183,7 @@ def render_snap_op(op, name, path=None, cfgfile=None, config=None):
             #      config
             # Note, however, we do not touch config files on disk.
             nested_cfg = {'config': {shortname: config}}
-            (fd, cfg_tmpf) = tempfile.mkstemp()
+            (fd, cfg_tmpf) = temp_utils.mkstemp()
             os.write(fd, util.yaml_dumps(nested_cfg).encode())
             os.close(fd)
             cfgfile = cfg_tmpf
diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py
index c7febc5..c842c83 100644
--- a/cloudinit/net/dhcp.py
+++ b/cloudinit/net/dhcp.py
@@ -9,6 +9,7 @@ import os
 import re
 
 from cloudinit.net import find_fallback_nic, get_devicelist
+from cloudinit import temp_utils
 from cloudinit import util
 
 LOG = logging.getLogger(__name__)
@@ -47,7 +48,7 @@ def maybe_perform_dhcp_discovery(nic=None):
     if not dhclient_path:
         LOG.debug('Skip dhclient configuration: No dhclient command found.')
         return {}
-    with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir:
+    with temp_utils.tempdir(prefix='cloud-init-dhcp-') as tmpdir:
         return dhcp_discovery(dhclient_path, nic, tmpdir)
 
 
diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
index e22409d..28ed0ae 100644
--- a/cloudinit/sources/helpers/azure.py
+++ b/cloudinit/sources/helpers/azure.py
@@ -6,10 +6,10 @@ import os
 import re
 import socket
 import struct
-import tempfile
 import time
 
 from cloudinit import stages
+from cloudinit import temp_utils
 from contextlib import contextmanager
 from xml.etree import ElementTree
 
@@ -111,7 +111,7 @@ class OpenSSLManager(object):
     }
 
     def __init__(self):
-        self.tmpdir = tempfile.mkdtemp()
+        self.tmpdir = temp_utils.mkdtemp()
         self.certificate = None
         self.generate_certificate()
 
diff --git a/cloudinit/temp_utils.py b/cloudinit/temp_utils.py
new file mode 100644
index 0000000..0355f19
--- /dev/null
+++ b/cloudinit/temp_utils.py
@@ -0,0 +1,93 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+import contextlib
+import errno
+import os
+import shutil
+import tempfile
+
+_TMPDIR = None
+_ROOT_TMPDIR = "/run/cloud-init/tmp"
+
+
+def _tempfile_dir_arg(odir=None):
+    """Return the proper 'dir' argument for tempfile functions.
+
+    When root, cloud-init will use /run/cloud-init/tmp to avoid
+    any cleaning that a distro boot might do on /tmp (such as
+    systemd-tmpfiles-clean).
+
+    If the caller of this function (mkdtemp or mkstemp) was provided
+    with a 'dir' argument, then that is respected.
+
+    @param odir: original 'dir' arg to 'mkdtemp' or other."""
+
+    if odir is not None:
+        return odir
+
+    global _TMPDIR
+    if _TMPDIR:
+        return _TMPDIR
+
+    if os.getuid() == 0:
+        tdir = _ROOT_TMPDIR
+    else:
+        tdir = os.environ.get('TMPDIR', '/tmp')
+    if not os.path.isdir(tdir):
+        os.makedirs(tdir)
+        os.chmod(tdir, 0o1777)
+
+    _TMPDIR = tdir
+    return tdir
+
+
+def ExtendedTemporaryFile(**kwargs):
+    kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None))
+    fh = tempfile.NamedTemporaryFile(**kwargs)
+    # Replace its unlink with a quiet version
+    # that does not raise errors when the
+    # file to unlink has been unlinked elsewhere..
+
+    def _unlink_if_exists(path):
+        try:
+            os.unlink(path)
+        except OSError as e:
+            if e.errno != errno.ENOENT:
+                raise e
+
+    fh.unlink = _unlink_if_exists
+
+    # Add a new method that will unlink
+    # right 'now' but still lets the exit
+    # method attempt to remove it (which will
+    # not throw due to our del file being quiet
+    # about files that are not there)
+    def unlink_now():
+        fh.unlink(fh.name)
+
+    setattr(fh, 'unlink_now', unlink_now)
+    return fh
+
+
[e-mail]
+def tempdir(**kwargs):
+    # This seems like it was only added in python 3.2
+    # Make it since its useful...
+    # See: http://bugs.python.org/file12970/tempdir.patch
+    tdir = mkdtemp(**kwargs)
+    try:
+        yield tdir
+    finally:
+        shutil.rmtree(tdir)
+
+
+def mkdtemp(**kwargs):
+    kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None))
+    return tempfile.mkdtemp(**kwargs)
+
+
+def mkstemp(**kwargs):
+    kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None))
+    return tempfile.mkstemp(**kwargs)
+
+# vi: ts=4 expandtab
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 609e94c..ae5cda8 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -30,7 +30,6 @@ import stat
 import string
 import subprocess
 import sys
-import tempfile
 import time
 
 from errno import ENOENT, ENOEXEC
@@ -45,6 +44,7 @@ from cloudinit import importer
 from cloudinit import log as logging
 from cloudinit import mergers
 from cloudinit import safeyaml
+from cloudinit import temp_utils
 from cloudinit import type_utils
 from cloudinit import url_helper
 from cloudinit import version
@@ -349,26 +349,6 @@ class DecompressionError(Exception):
     pass
 
 
-def ExtendedTemporaryFile(**kwargs):
-    fh = tempfile.NamedTemporaryFile(**kwargs)
-    # Replace its unlink with a quiet version
-    # that does not raise errors when the
-    # file to unlink has been unlinked elsewhere..
-    LOG.debug("Created temporary file %s", fh.name)
-    fh.unlink = del_file
-
-    # Add a new method that will unlink
-    # right 'now' but still lets the exit
-    # method attempt to remove it (which will
-    # not throw due to our del file being quiet
-    # about files that are not there)
-    def unlink_now():
-        fh.unlink(fh.name)
-
-    setattr(fh, 'unlink_now', unlink_now)
-    return fh
-
-
 def fork_cb(child_cb, *args, **kwargs):
     fid = os.fork()
     if fid == 0:
@@ -790,18 +770,6 @@ def umask(n_msk):
         os.umask(old)
 
 
[e-mail]
-def tempdir(**kwargs):
-    # This seems like it was only added in python 3.2
-    # Make it since its useful...
-    # See: http://bugs.python.org/file12970/tempdir.patch
-    tdir = tempfile.mkdtemp(**kwargs)
-    try:
-        yield tdir
-    finally:
-        del_dir(tdir)
-
-
 def center(text, fill, max_len):
     return '{0:{fill}{align}{size}}'.format(text, fill=fill,
                                             align="^", size=max_len)
@@ -1587,7 +1555,7 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True):
         mtypes = ['']
 
     mounted = mounts()
-    with tempdir() as tmpd:
+    with temp_utils.tempdir() as tmpd:
         umount = False
         if os.path.realpath(device) in mounted:
             mountpoint = mounted[os.path.realpath(device)]['mountpoint']
diff --git a/packages/bddeb b/packages/bddeb
index 7c12354..4f2e2dd 100755
--- a/packages/bddeb
+++ b/packages/bddeb
@@ -21,8 +21,9 @@ def find_root():
 if "avoid-pep8-E402-import-not-top-of-file":
     # Use the util functions from cloudinit
     sys.path.insert(0, find_root())
-    from cloudinit import templater
     from cloudinit import util
+    from cloudinit import temp_utils
+    from cloudinit import templater
 
 DEBUILD_ARGS = ["-S", "-d"]
 
@@ -148,7 +149,7 @@ def main():
         capture = False
 
     templ_data = {'debian_release': args.release}
-    with util.tempdir() as tdir:
+    with temp_utils.tempdir() as tdir:
 
         # output like 0.7.6-1022-g36e92d3
         ver_data = read_version()
diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
index 80ce003..44b99ec 100644
--- a/tests/unittests/test_datasource/test_azure_helper.py
+++ b/tests/unittests/test_datasource/test_azure_helper.py
@@ -275,7 +275,7 @@ class TestOpenSSLManager(TestCase):
                 mock.patch('builtins.open'))
 
     @mock.patch.object(azure_helper, 'cd', mock.MagicMock())
-    @mock.patch.object(azure_helper.tempfile, 'mkdtemp')
+    @mock.patch.object(azure_helper.temp_utils, 'mkdtemp')
     def test_openssl_manager_creates_a_tmpdir(self, mkdtemp):
         manager = azure_helper.OpenSSLManager()
         self.assertEqual(mkdtemp.return_value, manager.tmpdir)
@@ -292,7 +292,7 @@ class TestOpenSSLManager(TestCase):
         manager.clean_up()
 
     @mock.patch.object(azure_helper, 'cd', mock.MagicMock())
-    @mock.patch.object(azure_helper.tempfile, 'mkdtemp', mock.MagicMock())
+    @mock.patch.object(azure_helper.temp_utils, 'mkdtemp', mock.MagicMock())
     @mock.patch.object(azure_helper.util, 'del_dir')
     def test_clean_up(self, del_dir):
         manager = azure_helper.OpenSSLManager()
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index c10ef90..f249615 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -9,6 +9,7 @@ from cloudinit.net import network_state
 from cloudinit.net import renderers
 from cloudinit.net import sysconfig
 from cloudinit.sources.helpers import openstack
+from cloudinit import temp_utils
 from cloudinit import util
 
 from cloudinit.tests.helpers import CiTestCase
@@ -2150,7 +2151,7 @@ class TestCmdlineConfigParsing(CiTestCase):
         static['mac_address'] = macs['eth1']
 
         expected = {'version': 1, 'config': [dhcp, static]}
-        with util.tempdir() as tmpd:
+        with temp_utils.tempdir() as tmpd:
             for fname, content in pairs:
                 fp = os.path.join(tmpd, fname)
                 files.append(fp)