From b2050bd4ee6b8c8b497a8ef89d4b5262a06f09ec Mon Sep 17 00:00:00 2001 From: Andrey Tykhonov Date: Mon, 28 Sep 2015 18:39:44 +0300 Subject: [PATCH] Check whether given filename is a special device Accept only a special device file, otherwise throw an exception. Change-Id: I23598b52d273091a877896968d17377373322cae Closes-bug: #1492989 --- fuel_agent/manager.py | 12 ++++ fuel_agent/tests/test_manager.py | 120 ++++++++++++++++++++++--------- fuel_agent/utils/hardware.py | 7 ++ 3 files changed, 106 insertions(+), 33 deletions(-) diff --git a/fuel_agent/manager.py b/fuel_agent/manager.py index 8ad0fa2..011ff82 100644 --- a/fuel_agent/manager.py +++ b/fuel_agent/manager.py @@ -26,6 +26,7 @@ from fuel_agent.utils import artifact as au from fuel_agent.utils import build as bu from fuel_agent.utils import fs as fu from fuel_agent.utils import grub as gu +from fuel_agent.utils import hardware as hw from fuel_agent.utils import lvm as lu from fuel_agent.utils import md as mu from fuel_agent.utils import partition as pu @@ -322,6 +323,17 @@ class Manager(object): processing.append(au.GunzipStream) LOG.debug('Appending TARGET processor: %s' % image.target_device) + + error = None + if not os.path.exists(image.target_device): + error = "TARGET processor '{0}' does not exist." + elif not hw.is_block_device(image.target_device): + error = "TARGET processor '{0}' is not a block device." + if error: + error = error.format(image.target_device) + LOG.error(error) + raise errors.WrongDeviceError(error) + processing.append(image.target_device) LOG.debug('Launching image processing chain') diff --git a/fuel_agent/tests/test_manager.py b/fuel_agent/tests/test_manager.py index c48bbea..7c0025f 100644 --- a/fuel_agent/tests/test_manager.py +++ b/fuel_agent/tests/test_manager.py @@ -36,6 +36,16 @@ from fuel_agent.utils import utils CONF = cfg.CONF +class FakeChain(object): + processors = [] + + def append(self, thing): + self.processors.append(thing) + + def process(self): + pass + + class TestManager(unittest2.TestCase): @mock.patch('fuel_agent.drivers.nailgun.Nailgun.parse_image_meta', @@ -488,6 +498,8 @@ class TestManager(unittest2.TestCase): self.assertRaises(errors.WrongPartitionSchemeError, self.mgr.do_configdrive) + @mock.patch.object(manager.os.path, 'exists') + @mock.patch.object(hu, 'is_block_device') @mock.patch.object(utils, 'calculate_md5') @mock.patch('os.path.getsize') @mock.patch('yaml.load') @@ -502,17 +514,10 @@ class TestManager(unittest2.TestCase): @mock.patch.object(hu, 'list_block_devices') def test_do_copyimage(self, mock_lbd, mock_u_ras, mock_u_e, mock_au_c, mock_au_h, mock_au_l, mock_au_g, mock_fu_ef, - mock_http_req, mock_yaml, mock_get_size, mock_md5): - - class FakeChain(object): - processors = [] - - def append(self, thing): - self.processors.append(thing) - - def process(self): - pass - + mock_http_req, mock_yaml, mock_get_size, mock_md5, + mock_ibd, mock_os_path): + mock_os_path.return_value = True + mock_ibd.return_value = True mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE mock_au_c.return_value = FakeChain() self.mgr.do_configdrive() @@ -538,6 +543,67 @@ class TestManager(unittest2.TestCase): mock.call('ext4', '/dev/mapper/os-root')] self.assertEqual(mock_fu_ef_expected_calls, mock_fu_ef.call_args_list) + @mock.patch.object(manager.os.path, 'exists') + @mock.patch.object(hu, 'is_block_device') + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') + @mock.patch.object(fu, 'extend_fs') + @mock.patch.object(au, 'GunzipStream') + @mock.patch.object(au, 'LocalFile') + @mock.patch.object(au, 'HttpUrl') + @mock.patch.object(au, 'Chain') + @mock.patch.object(utils, 'execute') + @mock.patch.object(utils, 'render_and_save') + @mock.patch.object(hu, 'list_block_devices') + def test_do_copyimage_target_doesnt_exist(self, mock_lbd, mock_u_ras, + mock_u_e, mock_au_c, mock_au_h, + mock_au_l, mock_au_g, mock_fu_ef, + mock_http_req, mock_yaml, + mock_get_size, mock_md5, + mock_ibd, mock_os_path): + mock_os_path.return_value = False + mock_ibd.return_value = True + mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE + mock_au_c.return_value = FakeChain() + self.mgr.do_configdrive() + with self.assertRaisesRegexp(errors.WrongDeviceError, + 'TARGET processor .* does not exist'): + self.mgr.do_copyimage() + + @mock.patch.object(manager.os.path, 'exists') + @mock.patch.object(hu, 'is_block_device') + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') + @mock.patch.object(fu, 'extend_fs') + @mock.patch.object(au, 'GunzipStream') + @mock.patch.object(au, 'LocalFile') + @mock.patch.object(au, 'HttpUrl') + @mock.patch.object(au, 'Chain') + @mock.patch.object(utils, 'execute') + @mock.patch.object(utils, 'render_and_save') + @mock.patch.object(hu, 'list_block_devices') + def test_do_copyimage_target_not_block_device(self, mock_lbd, mock_u_ras, + mock_u_e, mock_au_c, + mock_au_h, mock_au_l, + mock_au_g, mock_fu_ef, + mock_http_req, mock_yaml, + mock_get_size, mock_md5, + mock_ibd, mock_os_path): + mock_os_path.return_value = True + mock_ibd.return_value = False + mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE + mock_au_c.return_value = FakeChain() + self.mgr.do_configdrive() + msg = 'TARGET processor .* is not a block device' + with self.assertRaisesRegexp(errors.WrongDeviceError, msg): + self.mgr.do_copyimage() + + @mock.patch.object(manager.os.path, 'exists') + @mock.patch.object(hu, 'is_block_device') @mock.patch.object(utils, 'calculate_md5') @mock.patch('os.path.getsize') @mock.patch('yaml.load') @@ -553,17 +619,10 @@ class TestManager(unittest2.TestCase): def test_do_copyimage_md5_matches(self, mock_lbd, mock_u_ras, mock_u_e, mock_au_c, mock_au_h, mock_au_l, mock_au_g, mock_fu_ef, mock_http_req, - mock_yaml, mock_get_size, mock_md5): - - class FakeChain(object): - processors = [] - - def append(self, thing): - self.processors.append(thing) - - def process(self): - pass - + mock_yaml, mock_get_size, mock_md5, + mock_ibd, mock_os_path): + mock_os_path.return_value = True + mock_ibd.return_value = True mock_get_size.return_value = 123 mock_md5.side_effect = ['fakemd5', 'really_fakemd5', 'fakemd5'] mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE @@ -578,6 +637,8 @@ class TestManager(unittest2.TestCase): mock.call('/dev/sda7', 123)] self.assertEqual(expected_md5_calls, mock_md5.call_args_list) + @mock.patch.object(hu, 'is_block_device') + @mock.patch.object(manager.os.path, 'exists') @mock.patch.object(utils, 'calculate_md5') @mock.patch('os.path.getsize') @mock.patch('yaml.load') @@ -593,17 +654,10 @@ class TestManager(unittest2.TestCase): def test_do_copyimage_md5_mismatch(self, mock_lbd, mock_u_ras, mock_u_e, mock_au_c, mock_au_h, mock_au_l, mock_au_g, mock_fu_ef, mock_http_req, - mock_yaml, mock_get_size, mock_md5): - - class FakeChain(object): - processors = [] - - def append(self, thing): - self.processors.append(thing) - - def process(self): - pass - + mock_yaml, mock_get_size, mock_md5, + mock_os_path, mock_ibd): + mock_os_path.return_value = True + mock_ibd.return_value = True mock_get_size.return_value = 123 mock_md5.side_effect = ['fakemd5', 'really_fakemd5', 'fakemd5'] mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE diff --git a/fuel_agent/utils/hardware.py b/fuel_agent/utils/hardware.py index 1ed7586..8d1ac79 100644 --- a/fuel_agent/utils/hardware.py +++ b/fuel_agent/utils/hardware.py @@ -13,6 +13,7 @@ # limitations under the License. import os +import stat from fuel_agent import errors from fuel_agent.openstack.common import log as logging @@ -245,6 +246,12 @@ def extrareport(dev): return spec +def is_block_device(self, filepath): + """Check whether `filepath` is a block device.""" + mode = os.stat(filepath).st_mode + return stat.S_ISBLK(mode) + + def get_block_devices_from_udev_db(): devs = [] output = utils.execute('udevadm', 'info', '--export-db')[0]