[IBP] Compare image checksums

If checksum and size provided in image meta data, fuel-agent will do
checksum comparing to ensure that data written on disk and downloaded
image are identical.

Partially implements blueprint: ibp-image-checksums
Depends on: Ia9c8325a2f71af2a1af2ad54a1ae414edf49a7c6
Change-Id: Id120848537cfc4ed908b9e524624855fb7308603

fuel_agent/drivers/nailgun.py

@@ -19,6 +19,9 @@ from fuel_agent import errors
 from fuel_agent import objects
 from fuel_agent.openstack.common import log as logging
 from fuel_agent.utils import hardware_utils as hu
+from fuel_agent.utils import utils
+
+import yaml
 
 LOG = logging.getLogger(__name__)
 
@@ -351,6 +354,28 @@ class Nailgun(object):
         LOG.debug('--- Preparing image scheme ---')
         data = self.data
         image_scheme = objects.ImageScheme()
+        #FIXME(agordeev): this piece of code for fetching additional image
+        # meta data should be factored out of this particular nailgun driver
+        # into more common and absract data getter which should be able to deal
+        # with various data sources (local file, http(s), etc.) and different
+        # data formats ('blob', json, yaml, etc.).
+        # So, the manager will combine and manipulate all those multiple data
+        # getter instances.
+        # Also, the initial data source should be set to sort out chicken/egg
+        # problem. Command line option may be useful for such a case.
+        # BUG: https://bugs.launchpad.net/fuel/+bug/1430418
+        metadata_url = data['ks_meta']['image_data']['/']['uri'].\
+            split('.')[0] + '.yaml'
+        image_meta = {}
+        raw_image_meta = None
+        try:
+            raw_image_meta = yaml.load(
+                utils.init_http_request(metadata_url).text)
+        except Exception as e:
+            LOG.exception(e)
+            LOG.debug('Failed to fetch/decode image meta data')
+        if raw_image_meta:
+            [image_meta.update(img_info) for img_info in raw_image_meta]
         # We assume for every file system user may provide a separate
         # file system image. For example if partitioning scheme has
         # /, /boot, /var/lib file systems then we will try to get images
@@ -373,5 +398,7 @@ class Nailgun(object):
                 # from provision.json, but currently it is hard coded.
                 format=image_data['format'],
                 container=image_data['container'],
+                size=image_meta.get(fs.mount, {}).get('size'),
+                md5=image_meta.get(fs.mount, {}).get('md5'),
             )
         return image_scheme

fuel_agent/errors.py

@@ -142,3 +142,7 @@ class HttpUrlConnectionError(BaseError):
 
 class HttpUrlInvalidContentLength(BaseError):
     pass
+
+
+class ImageChecksumMismatchError(BaseError):
+    pass

fuel_agent/manager.py

@@ -242,11 +242,15 @@ class Manager(object):
             raise errors.WrongPartitionSchemeError(
                 'Error while trying to get configdrive device: '
                 'configdrive device not found')
+        size = os.path.getsize(CONF.config_drive_path)
+        md5 = utils.calculate_md5(CONF.config_drive_path, size)
         self.image_scheme.add_image(
             uri='file://%s' % CONF.config_drive_path,
             target_device=configdrive_device,
             format='iso9660',
-            container='raw'
+            container='raw',
+            size=size,
+            md5=md5,
         )
 
     def do_copyimage(self):
@@ -275,6 +279,22 @@ class Manager(object):
             LOG.debug('Launching image processing chain')
             processing.process()
 
+            if image.size and image.md5:
+                LOG.debug('Trying to compare image checksum')
+                actual_md5 = utils.calculate_md5(image.target_device,
+                                                 image.size)
+                if actual_md5 == image.md5:
+                    LOG.debug('Checksum matches successfully: md5=%s' %
+                              actual_md5)
+                else:
+                    raise errors.ImageChecksumMismatchError(
+                        'Actual checksum %s mismatches with expected %s for '
+                        'file %s' % (actual_md5, image.md5,
+                                     image.target_device))
+            else:
+                LOG.debug('Skipping image checksum comparing. '
+                          'Ether size or hash have been missed')
+
             LOG.debug('Extending image file systems')
             if image.format in ('ext2', 'ext3', 'ext4', 'xfs'):
                 LOG.debug('Extending %s %s' %

fuel_agent/objects/image.py

@@ -19,7 +19,7 @@ class Image(object):
     SUPPORTED_CONTAINERS = ['raw', 'gzip']
 
     def __init__(self, uri, target_device,
-                 format, container, size=None):
+                 format, container, size=None, md5=None):
         # uri is something like
         # http://host:port/path/to/image.img or
         # file:///tmp/image.img
@@ -33,6 +33,7 @@ class Image(object):
                 'unsupported image container')
         self.container = container
         self.size = size
+        self.md5 = md5
 
 
 class ImageScheme(object):

fuel_agent/tests/test_artifact_utils.py

@@ -34,8 +34,9 @@ class TestTarget(test_base.BaseTestCase):
     def test_target_next(self):
         self.assertRaises(StopIteration, self.tgt.next)
 
+    @mock.patch('os.fsync')
     @mock.patch.object(au.Target, '__iter__')
-    def test_target_target(self, mock_iter):
+    def test_target_target(self, mock_iter, mock_os_sync):
         mock_iter.return_value = iter(['chunk1', 'chunk2', 'chunk3'])
         m = mock.mock_open()
         with mock.patch('six.moves.builtins.open', m):

fuel_agent/tests/test_manager.py

@@ -38,8 +38,10 @@ class TestManager(test_base.BaseTestCase):
         super(TestManager, self).setUp()
         self.mgr = manager.Manager(test_nailgun.PROVISION_SAMPLE_DATA)
 
+    @mock.patch('yaml.load')
+    @mock.patch.object(utils, 'init_http_request')
     @mock.patch.object(hu, 'list_block_devices')
-    def test_do_parsing(self, mock_lbd):
+    def test_do_parsing(self, mock_lbd, mock_http_req, mock_yaml):
         mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE
         self.mgr.do_parsing()
         #NOTE(agordeev): there's no need for deeper assertions as all schemes
@@ -139,10 +141,17 @@ class TestManager(test_base.BaseTestCase):
             mock.call('xfs', '', '', '/dev/mapper/image-glance')]
         self.assertEqual(mock_fu_mf_expected_calls, mock_fu_mf.call_args_list)
 
+    @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(utils, 'execute')
     @mock.patch.object(utils, 'render_and_save')
     @mock.patch.object(hu, 'list_block_devices')
-    def test_do_configdrive(self, mock_lbd, mock_u_ras, mock_u_e):
+    def test_do_configdrive(self, mock_lbd, mock_u_ras, mock_u_e,
+                            mock_http_req, mock_yaml, mock_get_size, mock_md5):
+        mock_get_size.return_value = 123
+        mock_md5.return_value = 'fakemd5'
         mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE
         self.mgr.do_parsing()
         self.assertEqual(1, len(self.mgr.image_scheme.images))
@@ -188,19 +197,28 @@ class TestManager(test_base.BaseTestCase):
                          self.mgr.partition_scheme.configdrive_device())
         self.assertEqual('iso9660', cf_drv_img.format)
         self.assertEqual('raw', cf_drv_img.container)
+        self.assertEqual('fakemd5', cf_drv_img.md5)
+        self.assertEqual(123, cf_drv_img.size)
 
+    @mock.patch('yaml.load')
+    @mock.patch.object(utils, 'init_http_request')
     @mock.patch.object(partition.PartitionScheme, 'configdrive_device')
     @mock.patch.object(utils, 'execute')
     @mock.patch.object(utils, 'render_and_save')
     @mock.patch.object(hu, 'list_block_devices')
     def test_do_configdrive_no_configdrive_device(self, mock_lbd, mock_u_ras,
-                                                  mock_u_e, mock_p_ps_cd):
+                                                  mock_u_e, mock_p_ps_cd,
+                                                  mock_http_req, mock_yaml):
         mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE
         self.mgr.do_parsing()
         mock_p_ps_cd.return_value = None
         self.assertRaises(errors.WrongPartitionSchemeError,
                           self.mgr.do_configdrive)
 
+    @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')
@@ -210,7 +228,8 @@ class TestManager(test_base.BaseTestCase):
     @mock.patch.object(utils, 'render_and_save')
     @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_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 = []
@@ -246,3 +265,82 @@ class TestManager(test_base.BaseTestCase):
         mock_fu_ef_expected_calls = [
             mock.call('ext4', '/dev/mapper/os-root')]
         self.assertEqual(mock_fu_ef_expected_calls, mock_fu_ef.call_args_list)
+
+    @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_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_get_size.return_value = 123
+        mock_md5.side_effect = ['fakemd5', 'really_fakemd5', 'fakemd5']
+        mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE
+        mock_au_c.return_value = FakeChain()
+        self.mgr.do_parsing()
+        self.mgr.image_scheme.images[0].size = 1234
+        self.mgr.image_scheme.images[0].md5 = 'really_fakemd5'
+        self.mgr.do_configdrive()
+        self.assertEqual(2, len(self.mgr.image_scheme.images))
+        self.mgr.do_copyimage()
+        expected_md5_calls = [mock.call('/tmp/config-drive.img', 123),
+                              mock.call('/dev/mapper/os-root', 1234),
+                              mock.call('/dev/sda7', 123)]
+        self.assertEqual(expected_md5_calls, mock_md5.call_args_list)
+
+    @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_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_get_size.return_value = 123
+        mock_md5.side_effect = ['fakemd5', 'really_fakemd5', 'fakemd5']
+        mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE
+        mock_au_c.return_value = FakeChain()
+        self.mgr.do_parsing()
+        self.mgr.image_scheme.images[0].size = 1234
+        self.mgr.image_scheme.images[0].md5 = 'fakemd5'
+        self.mgr.do_configdrive()
+        self.assertEqual(2, len(self.mgr.image_scheme.images))
+        self.assertRaises(errors.ImageChecksumMismatchError,
+                          self.mgr.do_copyimage)

fuel_agent/tests/test_nailgun.py

@@ -15,10 +15,13 @@
 import mock
 from oslotest import base as test_base
 
+import yaml
+
 from fuel_agent.drivers import nailgun
 from fuel_agent import errors
 from fuel_agent.objects import image
 from fuel_agent.utils import hardware_utils as hu
+from fuel_agent.utils import utils
 
 
 CEPH_JOURNAL = {
@@ -538,8 +541,10 @@ class TestNailgun(test_base.BaseTestCase):
         self.assertEqual(1, len(p_scheme.mds))
         self.assertEqual(3, len(p_scheme.parteds))
 
+    @mock.patch('yaml.load')
+    @mock.patch.object(utils, 'init_http_request')
     @mock.patch.object(hu, 'list_block_devices')
-    def test_image_scheme(self, mock_lbd):
+    def test_image_scheme(self, mock_lbd, mock_http_req, mock_yaml):
         mock_lbd.return_value = LIST_BLOCK_DEVICES_SAMPLE
         p_scheme = self.drv.partition_scheme()
         i_scheme = self.drv.image_scheme(p_scheme)
@@ -563,6 +568,40 @@ class TestNailgun(test_base.BaseTestCase):
                              expected_images[i].format)
             self.assertEqual(img.container,
                              expected_images[i].container)
+            self.assertIsNone(img.size)
+            self.assertIsNone(img.md5)
+
+    @mock.patch.object(utils, 'init_http_request')
+    @mock.patch.object(hu, 'list_block_devices')
+    def test_image_scheme_with_checksums(self, mock_lbd, mock_http_req):
+        fake_image_meta = [{'/': {'md5': 'fakeroot', 'size': 1}}]
+        prop_mock = mock.PropertyMock(return_value=yaml.dump(fake_image_meta))
+        type(mock_http_req.return_value).text = prop_mock
+        mock_lbd.return_value = LIST_BLOCK_DEVICES_SAMPLE
+        p_scheme = self.drv.partition_scheme()
+        i_scheme = self.drv.image_scheme(p_scheme)
+        expected_images = []
+        for fs in p_scheme.fss:
+            if fs.mount not in PROVISION_SAMPLE_DATA['ks_meta']['image_data']:
+                continue
+            i_data = PROVISION_SAMPLE_DATA['ks_meta']['image_data'][fs.mount]
+            expected_images.append(image.Image(
+                uri=i_data['uri'],
+                target_device=fs.device,
+                format=i_data['format'],
+                container=i_data['container'],
+            ))
+        expected_images = sorted(expected_images, key=lambda x: x.uri)
+        for i, img in enumerate(sorted(i_scheme.images, key=lambda x: x.uri)):
+            self.assertEqual(img.uri, expected_images[i].uri)
+            self.assertEqual(img.target_device,
+                             expected_images[i].target_device)
+            self.assertEqual(img.format,
+                             expected_images[i].format)
+            self.assertEqual(img.container,
+                             expected_images[i].container)
+            self.assertEqual(img.size, fake_image_meta[0]['/']['size'])
+            self.assertEqual(img.md5, fake_image_meta[0]['/']['md5'])
 
     def test_getlabel(self):
         self.assertEqual('', self.drv._getlabel(None))

fuel_agent/tests/test_utils.py

@@ -86,6 +86,23 @@ class ExecuteTestCase(testtools.TestCase):
                               'fake_file_name')
         mock_open.assert_called_once_with('fake_file_name', 'w')
 
+    def test_calculate_md5_ok(self):
+        # calculated by 'printf %10000s | md5sum'
+        with mock.patch('six.moves.builtins.open',
+                        mock.mock_open(read_data=' ' * 10000), create=True):
+            self.assertEqual('f38898bb69bb02bccb9594dfe471c5c0',
+                             utils.calculate_md5('fake', 10000))
+            self.assertEqual('6934d9d33cd2d0c005994e7d96d2e0d9',
+                             utils.calculate_md5('fake', 1000))
+            self.assertEqual('1e68934346ee57858834a205017af8b7',
+                             utils.calculate_md5('fake', 100))
+            self.assertEqual('41b394758330c83757856aa482c79977',
+                             utils.calculate_md5('fake', 10))
+            self.assertEqual('7215ee9c7d9dc229d2921a40e899ec5f',
+                             utils.calculate_md5('fake', 1))
+            self.assertEqual('d41d8cd98f00b204e9800998ecf8427e',
+                             utils.calculate_md5('fake', 0))
+
     @mock.patch.object(requests, 'get')
     def test_init_http_request_ok(self, mock_req):
         utils.init_http_request('fake_url')

fuel_agent/utils/artifact_utils.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import abc
+import os
 import tarfile
 import tempfile
 import zlib
@@ -56,6 +57,8 @@ class Target(object):
                 count += 1
             LOG.debug('Flushing file: %s' % filename)
             f.flush()
+            # ensure data to be written to disk
+            os.fsync(f.fileno())
         LOG.debug('File is written: %s' % filename)
 
 

fuel_agent/utils/utils.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hashlib
 import locale
 import math
 import os
@@ -48,6 +49,11 @@ u_opts = [
         default=2.0,
         help='Delay in seconds before the next http request retry',
     ),
+    cfg.IntOpt(
+        'read_chunk_size',
+        default=1048576,
+        help='Block size of data to read for calculating checksum',
+    ),
 ]
 
 CONF = cfg.CONF
@@ -143,6 +149,25 @@ def render_and_save(tmpl_dir, tmpl_names, tmpl_data, file_name):
             'templated data to {0}'.format(file_name))
 
 
+def calculate_md5(filename, size):
+    hash = hashlib.md5()
+    processed = 0
+    with open(filename, "rb") as f:
+        while processed < size:
+            block = f.read(CONF.read_chunk_size)
+            if block:
+                block_len = len(block)
+                if processed + block_len < size:
+                    hash.update(block)
+                    processed += block_len
+                else:
+                    hash.update(block[:size - processed])
+                    break
+            else:
+                break
+    return hash.hexdigest()
+
+
 def init_http_request(url, byte_range=0):
     LOG.debug('Trying to initialize http request object %s, byte range: %s'
               % (url, byte_range))

requirements.txt

@@ -9,3 +9,4 @@ Jinja2
 stevedore>=0.15
 requests>=1.2.3
 urllib3>=1.7
+PyYAML==3.10