From 75c3482a8685151407c186ce5b1f3b8af3db49d4 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 14 Nov 2012 19:22:38 -0800 Subject: Fix sudoers being written multiple times when strings are used. LP: #1079002 --- cloudinit/distros/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index ea0bac23..24e6f637 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -24,7 +24,6 @@ from StringIO import StringIO import abc -import collections import itertools import os import re @@ -421,7 +420,7 @@ class Distro(object): '', "# User rules for %s" % user, ] - if isinstance(rules, collections.Iterable): + if isinstance(rules, (list, tuple)): for rule in rules: lines.append("%s %s" % (user, rule)) else: -- cgit v1.2.3 From 7b9540b0a17cfcdb94bb133f1413b3a3f68433ef Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 14 Nov 2012 22:40:01 -0800 Subject: Add a test to make sure this doesn't happen again. --- tests/unittests/test_distros/test_generic.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py index 704699b5..63a4af29 100644 --- a/tests/unittests/test_distros/test_generic.py +++ b/tests/unittests/test_distros/test_generic.py @@ -55,6 +55,29 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase): # Make a temp directoy for tests to use. self.tmp = self.makeDir() + def test_sudoers_ensure_rules(self): + rules = ['ALL=(ALL:ALL) ALL'] + rules.append('B-ALL=(ALL:ALL) ALL') + cls = distros.fetch("ubuntu") + d = cls("ubuntu", {}, None) + os.makedirs(os.path.join(self.tmp, "etc")) + os.makedirs(os.path.join(self.tmp, "etc", 'sudoers.d')) + self.patchOS(self.tmp) + self.patchUtils(self.tmp) + d.write_sudo_rules("harlowja", rules) + contents = util.load_file(d.ci_sudoers_fn) + self.restore() + lines = contents.splitlines() + found_amount = 0 + expected = ['harlowja ALL=(ALL:ALL) ALL'] + expected.append('harlowja B-ALL=(ALL:ALL) ALL') + for e in expected: + for line in lines: + line = line.strip() + if line == e: + found_amount += 1 + self.assertEquals(2, found_amount) + def test_sudoers_ensure_new(self): cls = distros.fetch("ubuntu") d = cls("ubuntu", {}, None) -- cgit v1.2.3 From ae6c2665d51fffe5fdc2a15b7f97af0d8c1484af Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 14 Nov 2012 23:11:20 -0800 Subject: Add the string sudoers rule test case as well. --- tests/unittests/test_distros/test_generic.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py index 63a4af29..8ceb141a 100644 --- a/tests/unittests/test_distros/test_generic.py +++ b/tests/unittests/test_distros/test_generic.py @@ -56,6 +56,27 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase): self.tmp = self.makeDir() def test_sudoers_ensure_rules(self): + rules = 'ALL=(ALL:ALL) ALL' + cls = distros.fetch("ubuntu") + d = cls("ubuntu", {}, None) + os.makedirs(os.path.join(self.tmp, "etc")) + os.makedirs(os.path.join(self.tmp, "etc", 'sudoers.d')) + self.patchOS(self.tmp) + self.patchUtils(self.tmp) + d.write_sudo_rules("harlowja", rules) + contents = util.load_file(d.ci_sudoers_fn) + self.restore() + lines = contents.splitlines() + found_amount = 0 + expected = ['harlowja ALL=(ALL:ALL) ALL'] + for e in expected: + for line in lines: + line = line.strip() + if line == e: + found_amount += 1 + self.assertEquals(1, found_amount) + + def test_sudoers_ensure_rules_list(self): rules = ['ALL=(ALL:ALL) ALL'] rules.append('B-ALL=(ALL:ALL) ALL') cls = distros.fetch("ubuntu") -- cgit v1.2.3 From 6aaa0482f44f66f9bb3c89e133c25b3bde755a5e Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 14 Nov 2012 23:24:09 -0800 Subject: Cleanup the tests slightly. --- tests/unittests/test_distros/test_generic.py | 65 ++++++++++++++++------------ 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py index 8ceb141a..3ca769b4 100644 --- a/tests/unittests/test_distros/test_generic.py +++ b/tests/unittests/test_distros/test_generic.py @@ -55,8 +55,7 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase): # Make a temp directoy for tests to use. self.tmp = self.makeDir() - def test_sudoers_ensure_rules(self): - rules = 'ALL=(ALL:ALL) ALL' + def _write_load_sudoers(self, user, rules): cls = distros.fetch("ubuntu") d = cls("ubuntu", {}, None) os.makedirs(os.path.join(self.tmp, "etc")) @@ -66,38 +65,48 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase): d.write_sudo_rules("harlowja", rules) contents = util.load_file(d.ci_sudoers_fn) self.restore() - lines = contents.splitlines() + return contents + + def _count_in(self, lines_look_for, text_content): found_amount = 0 - expected = ['harlowja ALL=(ALL:ALL) ALL'] - for e in expected: - for line in lines: + for e in lines_look_for: + for line in text_content.splitlines(): line = line.strip() if line == e: found_amount += 1 - self.assertEquals(1, found_amount) + return found_amount - def test_sudoers_ensure_rules_list(self): - rules = ['ALL=(ALL:ALL) ALL'] - rules.append('B-ALL=(ALL:ALL) ALL') - cls = distros.fetch("ubuntu") - d = cls("ubuntu", {}, None) - os.makedirs(os.path.join(self.tmp, "etc")) - os.makedirs(os.path.join(self.tmp, "etc", 'sudoers.d')) - self.patchOS(self.tmp) - self.patchUtils(self.tmp) - d.write_sudo_rules("harlowja", rules) - contents = util.load_file(d.ci_sudoers_fn) - self.restore() - lines = contents.splitlines() - found_amount = 0 + def test_sudoers_ensure_rules(self): + rules = 'ALL=(ALL:ALL) ALL' + contents = self._write_load_sudoers('harlowja', rules) expected = ['harlowja ALL=(ALL:ALL) ALL'] - expected.append('harlowja B-ALL=(ALL:ALL) ALL') - for e in expected: - for line in lines: - line = line.strip() - if line == e: - found_amount += 1 - self.assertEquals(2, found_amount) + self.assertEquals(len(expected), self._count_in(expected, contents)) + not_expected = [ + 'harlowja A', + 'harlowja L', + 'harlowja L', + ] + self.assertEquals(0, self._count_in(not_expected, contents)) + + def test_sudoers_ensure_rules_list(self): + rules = [ + 'ALL=(ALL:ALL) ALL', + 'B-ALL=(ALL:ALL) ALL', + 'C-ALL=(ALL:ALL) ALL', + ] + contents = self._write_load_sudoers('harlowja', rules) + expected = [ + 'harlowja ALL=(ALL:ALL) ALL', + 'harlowja B-ALL=(ALL:ALL) ALL', + 'harlowja C-ALL=(ALL:ALL) ALL', + ] + self.assertEquals(len(expected), self._count_in(expected, contents)) + not_expected = [ + 'harlowja A', + 'harlowja L', + 'harlowja L', + ] + self.assertEquals(0, self._count_in(not_expected, contents)) def test_sudoers_ensure_new(self): cls = distros.fetch("ubuntu") -- cgit v1.2.3 From ef915a6ec712d89b9e0b3672947571976a49b68f Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 15 Nov 2012 12:32:05 -0800 Subject: Raise a type error when a sudoers rule is not an accepted type. --- cloudinit/distros/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 24e6f637..e724a418 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -423,8 +423,11 @@ class Distro(object): if isinstance(rules, (list, tuple)): for rule in rules: lines.append("%s %s" % (user, rule)) - else: + elif isinstance(rules, (basestring, str)): lines.append("%s %s" % (user, rules)) + else: + msg = "Can not create sudoers rule addition with type %r" + raise TypeError(msg % (util.obj_name(rules))) content = "\n".join(lines) self.ensure_sudo_dir(os.path.dirname(sudo_file)) -- cgit v1.2.3