From 7bec03bca0e02230572917372a8bb7f73d389c7d Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 21 Jun 2012 13:36:33 -0700 Subject: This is now functional and all passing again --- tests/unittests/test_util.py | 91 ++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 42 deletions(-) (limited to 'tests/unittests/test_util.py') diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index e8f5885c..27c0fbd6 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1,13 +1,13 @@ -from unittest import TestCase -from mocker import MockerTestCase -from tempfile import mkdtemp from shutil import rmtree +from tempfile import mkdtemp + import os import stat -from cloudinit.util import (mergedict, get_cfg_option_list_or_str, write_file, - delete_dir_contents, get_cmdline, - keyval_str_to_dict) +from unittest import TestCase +from mocker import MockerTestCase + +from cloudinit import util class TestMergeDict(TestCase): @@ -15,14 +15,14 @@ class TestMergeDict(TestCase): """Test simple non-conflict merge.""" source = {"key1": "value1"} candidate = {"key2": "value2"} - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual({"key1": "value1", "key2": "value2"}, result) def test_nested_merge(self): """Test nested merge.""" source = {"key1": {"key1.1": "value1.1"}} candidate = {"key1": {"key1.2": "value1.2"}} - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual( {"key1": {"key1.1": "value1.1", "key1.2": "value1.2"}}, result) @@ -30,42 +30,42 @@ class TestMergeDict(TestCase): """Test that candidate doesn't override source.""" source = {"key1": "value1", "key2": "value2"} candidate = {"key1": "value2", "key2": "NEW VALUE"} - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual(source, result) def test_empty_candidate(self): """Test empty candidate doesn't change source.""" source = {"key": "value"} candidate = {} - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual(source, result) def test_empty_source(self): """Test empty source is replaced by candidate.""" source = {} candidate = {"key": "value"} - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual(candidate, result) def test_non_dict_candidate(self): """Test non-dict candidate is discarded.""" source = {"key": "value"} candidate = "not a dict" - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual(source, result) def test_non_dict_source(self): """Test non-dict source is not modified with a dict candidate.""" source = "not a dict" candidate = {"key": "value"} - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual(source, result) def test_neither_dict(self): """Test if neither candidate or source is dict source wins.""" source = "source" candidate = "candidate" - result = mergedict(source, candidate) + result = util.mergedict(source, candidate) self.assertEqual(source, result) @@ -73,31 +73,31 @@ class TestGetCfgOptionListOrStr(TestCase): def test_not_found_no_default(self): """None is returned if key is not found and no default given.""" config = {} - result = get_cfg_option_list_or_str(config, "key") + result = util.get_cfg_option_list(config, "key") self.assertIsNone(result) def test_not_found_with_default(self): """Default is returned if key is not found.""" config = {} - result = get_cfg_option_list_or_str(config, "key", default=["DEFAULT"]) + result = util.get_cfg_option_list(config, "key", default=["DEFAULT"]) self.assertEqual(["DEFAULT"], result) def test_found_with_default(self): """Default is not returned if key is found.""" config = {"key": ["value1"]} - result = get_cfg_option_list_or_str(config, "key", default=["DEFAULT"]) + result = util.get_cfg_option_list(config, "key", default=["DEFAULT"]) self.assertEqual(["value1"], result) def test_found_convert_to_list(self): """Single string is converted to one element list.""" config = {"key": "value1"} - result = get_cfg_option_list_or_str(config, "key") + result = util.get_cfg_option_list(config, "key") self.assertEqual(["value1"], result) def test_value_is_none(self): """If value is None empty list is returned.""" config = {"key": None} - result = get_cfg_option_list_or_str(config, "key") + result = util.get_cfg_option_list(config, "key") self.assertEqual([], result) @@ -117,7 +117,7 @@ class TestWriteFile(MockerTestCase): path = os.path.join(self.tmp, "NewFile.txt") contents = "Hey there" - write_file(path, contents) + util.write_file(path, contents) self.assertTrue(os.path.exists(path)) self.assertTrue(os.path.isfile(path)) @@ -133,7 +133,7 @@ class TestWriteFile(MockerTestCase): path = os.path.join(dirname, "NewFile.txt") contents = "Hey there" - write_file(path, contents) + util.write_file(path, contents) self.assertTrue(os.path.isdir(dirname)) self.assertTrue(os.path.isfile(path)) @@ -143,7 +143,7 @@ class TestWriteFile(MockerTestCase): path = os.path.join(self.tmp, "NewFile.txt") contents = "Hey there" - write_file(path, contents, mode=0666) + util.write_file(path, contents, mode=0666) self.assertTrue(os.path.exists(path)) self.assertTrue(os.path.isfile(path)) @@ -158,7 +158,7 @@ class TestWriteFile(MockerTestCase): # Create file first with basic content with open(path, "wb") as f: f.write("LINE1\n") - write_file(path, contents, omode="a") + util.write_file(path, contents, omode="a") self.assertTrue(os.path.exists(path)) self.assertTrue(os.path.isfile(path)) @@ -167,17 +167,24 @@ class TestWriteFile(MockerTestCase): self.assertEqual("LINE1\nHey there", create_contents) def test_restorecon_if_possible_is_called(self): - """Make sure the restorecon_if_possible is called correctly.""" - path = os.path.join(self.tmp, "NewFile.txt") - contents = "Hey there" - - # Mock out the restorecon_if_possible call to test if it's called. - mock_restorecon = self.mocker.replace( - "cloudinit.util.restorecon_if_possible", passthrough=False) - mock_restorecon(path) - self.mocker.replay() - - write_file(path, contents) + """Make sure the selinux guard is called correctly.""" + try: + # We can only mock these out if selinux actually + # exists, so thats why we catch the import + mock_restorecon = self.mocker.replace( + "selinux.restorecon", passthrough=False) + mock_is_selinux_enabled = self.mocker.replace( + "selinux.is_selinux_enabled", passthrough=False) + mock_is_selinux_enabled.result(True) + mock_restorecon(path) + self.mocker.replay() + old = util.HAVE_LIBSELINUX + util.HAVE_LIBSELINUX = True + with util.SeLinuxGuard(self.tmp) as is_on: + self.assertTrue(is_on) + util.HAVE_LIBSELINUX = old + except ImportError: + pass class TestDeleteDirContents(TestCase): @@ -196,7 +203,7 @@ class TestDeleteDirContents(TestCase): def test_does_not_delete_dir(self): """Ensure directory itself is not deleted.""" - delete_dir_contents(self.tmp) + util.delete_dir_contents(self.tmp) self.assertTrue(os.path.isdir(self.tmp)) self.assertDirEmpty(self.tmp) @@ -206,7 +213,7 @@ class TestDeleteDirContents(TestCase): with open(os.path.join(self.tmp, "new_file.txt"), "wb") as f: f.write("DELETE ME") - delete_dir_contents(self.tmp) + util.delete_dir_contents(self.tmp) self.assertDirEmpty(self.tmp) @@ -214,7 +221,7 @@ class TestDeleteDirContents(TestCase): """Empty directories should be deleted.""" os.mkdir(os.path.join(self.tmp, "new_dir")) - delete_dir_contents(self.tmp) + util.delete_dir_contents(self.tmp) self.assertDirEmpty(self.tmp) @@ -223,7 +230,7 @@ class TestDeleteDirContents(TestCase): os.mkdir(os.path.join(self.tmp, "new_dir")) os.mkdir(os.path.join(self.tmp, "new_dir", "new_subdir")) - delete_dir_contents(self.tmp) + util.delete_dir_contents(self.tmp) self.assertDirEmpty(self.tmp) @@ -234,7 +241,7 @@ class TestDeleteDirContents(TestCase): with open(f_name, "wb") as f: f.write("DELETE ME") - delete_dir_contents(self.tmp) + util.delete_dir_contents(self.tmp) self.assertDirEmpty(self.tmp) @@ -246,7 +253,7 @@ class TestDeleteDirContents(TestCase): f.write("DELETE ME") os.symlink(file_name, link_name) - delete_dir_contents(self.tmp) + util.delete_dir_contents(self.tmp) self.assertDirEmpty(self.tmp) @@ -255,12 +262,12 @@ class TestKeyValStrings(TestCase): def test_keyval_str_to_dict(self): expected = {'1': 'one', '2': 'one+one', 'ro': True} cmdline = "1=one ro 2=one+one" - self.assertEqual(expected, keyval_str_to_dict(cmdline)) + self.assertEqual(expected, util.keyval_str_to_dict(cmdline)) class TestGetCmdline(TestCase): def test_cmdline_reads_debug_env(self): os.environ['DEBUG_PROC_CMDLINE'] = 'abcd 123' - self.assertEqual(os.environ['DEBUG_PROC_CMDLINE'], get_cmdline()) + self.assertEqual(os.environ['DEBUG_PROC_CMDLINE'], util.get_cmdline()) # vi: ts=4 expandtab -- cgit v1.2.3 From 0f1c493aa032b60210fc63c6b120b833ba4c10de Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 21 Jun 2012 15:57:09 -0700 Subject: Use the mocker built-in method for creating temporary dirs (which it will cleanup) --- tests/unittests/test_util.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'tests/unittests/test_util.py') diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 27c0fbd6..aea1aabb 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1,6 +1,3 @@ -from shutil import rmtree -from tempfile import mkdtemp - import os import stat @@ -10,7 +7,7 @@ from mocker import MockerTestCase from cloudinit import util -class TestMergeDict(TestCase): +class TestMergeDict(MockerTestCase): def test_simple_merge(self): """Test simple non-conflict merge.""" source = {"key1": "value1"} @@ -105,12 +102,10 @@ class TestWriteFile(MockerTestCase): def setUp(self): super(TestWriteFile, self).setUp() # Make a temp directoy for tests to use. - self.tmp = mkdtemp(prefix="unittest_") + self.tmp = self.makeDir(prefix="unittest_") def tearDown(self): super(TestWriteFile, self).tearDown() - # Clean up temp directory - rmtree(self.tmp) def test_basic_usage(self): """Verify basic usage with default args.""" -- cgit v1.2.3 From 51181b8540372b9b83bbab51cae548992c497682 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 21 Jun 2012 15:59:57 -0700 Subject: Missed some other mkdtemp and rmtree calls no longer needed --- tests/unittests/test_util.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) (limited to 'tests/unittests/test_util.py') diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index aea1aabb..ba565b29 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -101,12 +101,8 @@ class TestGetCfgOptionListOrStr(TestCase): class TestWriteFile(MockerTestCase): def setUp(self): super(TestWriteFile, self).setUp() - # Make a temp directoy for tests to use. self.tmp = self.makeDir(prefix="unittest_") - def tearDown(self): - super(TestWriteFile, self).tearDown() - def test_basic_usage(self): """Verify basic usage with default args.""" path = os.path.join(self.tmp, "NewFile.txt") @@ -182,16 +178,10 @@ class TestWriteFile(MockerTestCase): pass -class TestDeleteDirContents(TestCase): +class TestDeleteDirContents(MockerTestCase): def setUp(self): super(TestDeleteDirContents, self).setUp() - # Make a temp directoy for tests to use. - self.tmp = mkdtemp(prefix="unittest_") - - def tearDown(self): - super(TestDeleteDirContents, self).tearDown() - # Clean up temp directory - rmtree(self.tmp) + self.tmp = self.makeDir(prefix="unittest_") def assertDirEmpty(self, dirname): self.assertEqual([], os.listdir(dirname)) -- cgit v1.2.3 From ba5fb03646f6318a0ace286da746b4bb32f75d5a Mon Sep 17 00:00:00 2001 From: harlowja Date: Thu, 21 Jun 2012 23:35:07 -0700 Subject: Fixup python selinux guards, only try to restore after we check if its useful to restore, fix test to work with selinux enabled sysystems --- cloudinit/stages.py | 20 ++++++++++++-------- cloudinit/util.py | 26 +++++++++++++++++--------- tests/unittests/test_util.py | 13 ++++++------- 3 files changed, 35 insertions(+), 24 deletions(-) (limited to 'tests/unittests/test_util.py') diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 25f13fd4..cf5e6924 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -221,11 +221,12 @@ class Init(object): (cfg_list, pkg_list) = self._get_datasources() # Deep copy so that user-data handlers can not modify # (which will affect user-data handlers down the line...) - sys_cfg = copy.deepcopy(self.cfg) - ds_deps = copy.deepcopy(self.ds_deps) - (ds, dsname) = sources.find_source(sys_cfg, self.distro, + (ds, dsname) = sources.find_source(self.cfg, + self.distro, self.paths, - ds_deps, cfg_list, pkg_list) + copy.deepcopy(self.ds_deps), + cfg_list, + pkg_list) LOG.debug("Loaded datasource %s - %s", dsname, ds) if ds: self.datasource = ds @@ -408,7 +409,7 @@ class Modules(object): def __init__(self, init, cfg_files=None): self.datasource = init.datasource self.cfg_files = cfg_files - self.base_cfg = copy.deepcopy(init.cfg) + self.base_cfg = init.cfg self.init = init # Created on first use self._cached_cfg = None @@ -419,7 +420,8 @@ class Modules(object): if self._cached_cfg is None: self._cached_cfg = self._get_config() LOG.debug("Loading 'module' config %s", self._cached_cfg) - return self._cached_cfg + # Only give out a copy so that others can't modify this... + return copy.deepcopy(self._cached_cfg) def _get_config(self): t_cfgs = [] @@ -531,9 +533,11 @@ class Modules(object): LOG.warn(("Module %s is verified on %s distros" " but not on %s distro. It may or may not work" " correctly."), name, worked_distros, d_name) - # Deep copy the config so that modules can't alter it # Use the configs logger and not our own - func_args = [name, copy.deepcopy(self.cfg), + # TODO: possibly check the module + # for having a LOG attr and just give it back + # its own logger? + func_args = [name, self.cfg, cc, config.LOG, args] # Mark it as having started running am_ran += 1 diff --git a/cloudinit/util.py b/cloudinit/util.py index 3aa4e462..332b8379 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -35,6 +35,7 @@ import pwd import random import shutil import socket +import stat import string # pylint: disable=W0402 import subprocess import sys @@ -132,14 +133,24 @@ class SeLinuxGuard(object): self.enabled = True def __enter__(self): - # TODO: Should we try to engage selinux here?? return self.enabled def __exit__(self, excp_type, excp_value, excp_traceback): if self.enabled: - LOG.debug("Restoring selinux mode for %s (recursive=%s)", - self.path, self.recursive) - selinux.restorecon(self.path, recursive=self.recursive) + path = os.path.realpath(os.path.expanduser(self.path)) + do_restore = False + try: + # See if even worth restoring?? + stats = os.lstat(path) + if stat.ST_MODE in stats: + selinux.matchpathcon(path, stats[stat.ST_MODE]) + do_restore = True + except OSError: + pass + if do_restore: + LOG.debug("Restoring selinux mode for %s (recursive=%s)", + path, self.recursive) + selinux.restorecon(path, recursive=self.recursive) class MountFailedError(Exception): @@ -1067,8 +1078,7 @@ def ensure_dir(path, mode=None): if not os.path.isdir(path): # Make the dir and adjust the mode LOG.debug("Ensuring directory exists at path %s", path) - # TODO: check if guard needed?? - with SeLinuxGuard(path=os.path.dirname(path)): + with SeLinuxGuard(os.path.dirname(path), recursive=True): os.makedirs(path) chmod(path, mode) else: @@ -1222,8 +1232,7 @@ def chmod(path, mode): if path and real_mode: LOG.debug("Adjusting the permissions of %s (perms=%o)", path, real_mode) - # TODO: check if guard needed?? - with SeLinuxGuard(path=path): + with SeLinuxGuard(path): os.chmod(path, real_mode) @@ -1239,7 +1248,6 @@ def write_file(filename, content, mode=0644, omode="wb"): """ ensure_dir(os.path.dirname(filename)) LOG.debug("Writing to %s - %s, %s bytes", filename, omode, len(content)) - # TODO: check if guard needed?? with SeLinuxGuard(path=filename): with open(filename, omode) as fh: fh.write(content) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index ba565b29..3be6e186 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -71,7 +71,7 @@ class TestGetCfgOptionListOrStr(TestCase): """None is returned if key is not found and no default given.""" config = {} result = util.get_cfg_option_list(config, "key") - self.assertIsNone(result) + self.assertEqual(None, result) def test_not_found_with_default(self): """Default is returned if key is not found.""" @@ -166,14 +166,13 @@ class TestWriteFile(MockerTestCase): "selinux.restorecon", passthrough=False) mock_is_selinux_enabled = self.mocker.replace( "selinux.is_selinux_enabled", passthrough=False) - mock_is_selinux_enabled.result(True) - mock_restorecon(path) + mock_is_selinux_enabled() + self.mocker.result(True) + mock_restorecon("/etc/hosts", recursive=False) + self.mocker.result(True) self.mocker.replay() - old = util.HAVE_LIBSELINUX - util.HAVE_LIBSELINUX = True - with util.SeLinuxGuard(self.tmp) as is_on: + with util.SeLinuxGuard("/etc/hosts") as is_on: self.assertTrue(is_on) - util.HAVE_LIBSELINUX = old except ImportError: pass -- cgit v1.2.3 From a0740928d0f4738792e478dad845b30eb8c61c41 Mon Sep 17 00:00:00 2001 From: harlowja Date: Fri, 29 Jun 2012 20:37:46 -0700 Subject: Refactor the selinux guard to aid in mocking 1. Adjust the test_util after this mocking to be cleaner --- cloudinit/util.py | 28 +++++++++++++-------------- tests/unittests/test_util.py | 46 +++++++++++++++++++++++++++++--------------- 2 files changed, 44 insertions(+), 30 deletions(-) (limited to 'tests/unittests/test_util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index 3ff3835a..0c592656 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -46,19 +46,13 @@ import urlparse import yaml +from cloudinit import importer from cloudinit import log as logging from cloudinit import url_helper as uhelp from cloudinit.settings import (CFG_BUILTIN, CLOUD_CONFIG) -try: - import selinux - HAVE_LIBSELINUX = True -except ImportError: - HAVE_LIBSELINUX = False - - LOG = logging.getLogger(__name__) # Helps cleanup filenames to ensure they aren't FS incompatible @@ -126,31 +120,37 @@ class ProcessExecutionError(IOError): class SeLinuxGuard(object): def __init__(self, path, recursive=False): + # Late import since it might not always + # be possible to use this + try: + self.selinux = importer.import_module('selinux') + except ImportError: + self.selinux = None self.path = path self.recursive = recursive - self.enabled = False - if HAVE_LIBSELINUX and selinux.is_selinux_enabled(): - self.enabled = True def __enter__(self): - return self.enabled + if self.selinux: + return True + else: + return False def __exit__(self, excp_type, excp_value, excp_traceback): - if self.enabled: + if self.selinux: path = os.path.realpath(os.path.expanduser(self.path)) do_restore = False try: # See if even worth restoring?? stats = os.lstat(path) if stat.ST_MODE in stats: - selinux.matchpathcon(path, stats[stat.ST_MODE]) + self.selinux.matchpathcon(path, stats[stat.ST_MODE]) do_restore = True except OSError: pass if do_restore: LOG.debug("Restoring selinux mode for %s (recursive=%s)", path, self.recursive) - selinux.restorecon(path, recursive=self.recursive) + self.selinux.restorecon(path, recursive=self.recursive) class MountFailedError(Exception): diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 3be6e186..93979f06 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -5,6 +5,26 @@ from unittest import TestCase from mocker import MockerTestCase from cloudinit import util +from cloudinit import importer + + +class FakeSelinux(object): + + def __init__(self, match_what): + self.match_what = match_what + self.restored = [] + + def matchpathcon(self, path, mode): + if path == self.match_what: + return + else: + raise OSError("No match!") + + def is_selinux_enabled(self): + return True + + def restorecon(self, path, recursive): + self.restored.append(path) class TestMergeDict(MockerTestCase): @@ -159,22 +179,16 @@ class TestWriteFile(MockerTestCase): def test_restorecon_if_possible_is_called(self): """Make sure the selinux guard is called correctly.""" - try: - # We can only mock these out if selinux actually - # exists, so thats why we catch the import - mock_restorecon = self.mocker.replace( - "selinux.restorecon", passthrough=False) - mock_is_selinux_enabled = self.mocker.replace( - "selinux.is_selinux_enabled", passthrough=False) - mock_is_selinux_enabled() - self.mocker.result(True) - mock_restorecon("/etc/hosts", recursive=False) - self.mocker.result(True) - self.mocker.replay() - with util.SeLinuxGuard("/etc/hosts") as is_on: - self.assertTrue(is_on) - except ImportError: - pass + import_mock = self.mocker.replace(importer.import_module, + passthrough=False) + import_mock('selinux') + fake_se = FakeSelinux('/etc/hosts') + self.mocker.result(fake_se) + self.mocker.replay() + with util.SeLinuxGuard("/etc/hosts") as is_on: + self.assertTrue(is_on) + self.assertEqual(1, len(fake_se.restored)) + self.assertEqual('/etc/hosts', fake_se.restored[0]) class TestDeleteDirContents(MockerTestCase): -- cgit v1.2.3