Sudoers.d creation cleanups + tests.

4 files changed, 45 insertions, 10 deletions

diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 2d01efc3..d2cb0a8b 100644
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -283,8 +283,10 @@ class Distro(object):
         # Ensure the dir is included and that
         # it actually exists as a directory
         sudoers_contents = ''
+        base_exists = False
         if os.path.exists(sudo_base):
             sudoers_contents = util.load_file(sudo_base)
+            base_exists = True
         found_include = False
         for line in sudoers_contents.splitlines():
             line = line.strip()
@@ -299,18 +301,24 @@ class Distro(object):
                 found_include = True
                 break
         if not found_include:
-            sudoers_contents += "\n#includedir %s\n" % (path)
             try:
-                if not os.path.exists(sudo_base):
+                if not base_exists:
+                    lines = [('# See sudoers(5) for more information'
+                              ' on "#include" directives:'), '',
+                             '# Added by cloud-init',
+                             "#includedir %s" % (path), '']
+                    sudoers_contents = "\n".join(lines)
                     util.write_file(sudo_base, sudoers_contents, 0440)
                 else:
-                    with open(sudo_base, 'a') as f:
-                        f.write(sudoers_contents)
-                LOG.debug("added '#includedir %s' to %s" % (path, sudo_base))
+                    lines = ['', '# Added by cloud-init',
+                             "#includedir %s" % (path), '']
+                    sudoers_contents = "\n".join(lines)
+                    util.append_file(sudo_base, sudoers_contents)
+                LOG.debug("Added '#includedir %s' to %s" % (path, sudo_base))
             except IOError as e:
                 util.logexc(LOG, "Failed to write %s" % sudo_base, e)
                 raise e
-        util.ensure_dir(path, 0755)
+        util.ensure_dir(path, 0750)
 
     def write_sudo_rules(self,
         user,
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index 2c5dcad2..e8080668 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -103,6 +103,7 @@ class FilesystemMockingTestCase(ResourceUsingTestCase):
     def patchUtils(self, new_root):
         patch_funcs = {
             util: [('write_file', 1),
+                   ('append_file', 1),
                    ('load_file', 1),
                    ('ensure_dir', 1),
                    ('chmod', 1),
diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py
index 2df4c2f0..704699b5 100644
--- a/tests/unittests/test_distros/test_generic.py
+++ b/tests/unittests/test_distros/test_generic.py
@@ -1,6 +1,9 @@
-from mocker import MockerTestCase
-
 from cloudinit import distros
+from cloudinit import util
+
+from tests.unittests import helpers
+
+import os
 
 unknown_arch_info = {
     'arches': ['default'],
@@ -27,7 +30,7 @@ gpmi = distros._get_package_mirror_info  # pylint: disable=W0212
 gapmi = distros._get_arch_package_mirror_info  # pylint: disable=W0212
 
 
-class TestGenericDistro(MockerTestCase):
+class TestGenericDistro(helpers.FilesystemMockingTestCase):
 
     def return_first(self, mlist):
         if not mlist:
@@ -52,6 +55,29 @@ class TestGenericDistro(MockerTestCase):
         # Make a temp directoy for tests to use.
         self.tmp = self.makeDir()
 
+    def test_sudoers_ensure_new(self):
+        cls = distros.fetch("ubuntu")
+        d = cls("ubuntu", {}, None)
+        self.patchOS(self.tmp)
+        self.patchUtils(self.tmp)
+        d.ensure_sudo_dir("/b")
+        contents = util.load_file("/etc/sudoers")
+        self.assertIn("includedir /b", contents)
+        self.assertTrue(os.path.isdir("/b"))
+
+    def test_sudoers_ensure_append(self):
+        cls = distros.fetch("ubuntu")
+        d = cls("ubuntu", {}, None)
+        self.patchOS(self.tmp)
+        self.patchUtils(self.tmp)
+        util.write_file("/etc/sudoers", "josh, josh\n")
+        d.ensure_sudo_dir("/b")
+        contents = util.load_file("/etc/sudoers")
+        self.assertIn("includedir /b", contents)
+        self.assertTrue(os.path.isdir("/b"))
+        self.assertIn("josh", contents)
+        self.assertEquals(2, contents.count("josh"))
+
     def test_arch_package_mirror_info_unknown(self):
         """for an unknown arch, we should get back that with arch 'default'."""
         arch_mirrors = gapmi(package_mirrors, arch="unknown")
diff --git a/tests/unittests/test_runs/test_merge_run.py b/tests/unittests/test_runs/test_merge_run.py
index 04c03730..36de97ae 100644
--- a/tests/unittests/test_runs/test_merge_run.py
+++ b/tests/unittests/test_runs/test_merge_run.py
@@ -7,7 +7,7 @@ from cloudinit import stages
 from cloudinit import util
 
 
-class TestSimpleRun(helpers.FilesystemMockingTestCase):
+class TestMergeRun(helpers.FilesystemMockingTestCase):
     def _patchIn(self, root):
         self.restore()
         self.patchOS(root)