Relax checksum fields validation

In stein, ironic added the new os_hash_algo and os_hash_value checksum
fields provided by glance, but the checksum field is still mandatory,
which is inconvenient for standalone use case.

We could relax the checksum checking and proceed as long as there is at
least one of checksum mechanism available.

Change-Id: Ia90197416f76ada0422681044a16f1c07d7049a1
Story: 2005773
Task: 33490

ironic_python_agent/extensions/standby.py

@@ -268,9 +268,18 @@ class ImageDownload(object):
         if algo and algo in hashlib.algorithms_available:
             self._hash_algo = hashlib.new(algo)
             self._expected_hash_value = image_info.get('os_hash_value')
-        else:
+        elif image_info.get('checksum'):
             self._hash_algo = hashlib.md5()
             self._expected_hash_value = image_info['checksum']
+        else:
+            message = ('Unable to verify image {} with available checksums. '
+                       'Please make sure the specified \'os_hash_algo\' '
+                       '(currently {}) is supported by this ramdisk, or '
+                       'provide a md5 checksum via the \'checksum\' '
+                       'field'.format(image_info['id'],
+                                      image_info.get('os_hash_algo')))
+            LOG.error(message)
+            raise errors.RESTError(details=message)
 
         self._expected_hash_value = _fetch_checksum(self._expected_hash_value,
                                                     image_info)
@@ -370,7 +379,10 @@ def _validate_image_info(ext, image_info=None, **kwargs):
     """
     image_info = image_info or {}
 
-    for field in ['id', 'urls', 'checksum']:
+    md5sum_avail = False
+    os_hash_checksum_avail = False
+
+    for field in ['id', 'urls']:
         if field not in image_info:
             msg = 'Image is missing \'{}\' field.'.format(field)
             raise errors.InvalidCommandParamsError(msg)
@@ -379,10 +391,12 @@ def _validate_image_info(ext, image_info=None, **kwargs):
         raise errors.InvalidCommandParamsError(
             'Image \'urls\' must be a list with at least one element.')
 
-    if (not isinstance(image_info['checksum'], six.string_types)
-            or not image_info['checksum']):
-        raise errors.InvalidCommandParamsError(
-            'Image \'checksum\' must be a non-empty string.')
+    if 'checksum' in image_info:
+        if (not isinstance(image_info['checksum'], six.string_types)
+                or not image_info['checksum']):
+            raise errors.InvalidCommandParamsError(
+                'Image \'checksum\' must be a non-empty string.')
+        md5sum_avail = True
 
     os_hash_algo = image_info.get('os_hash_algo')
     os_hash_value = image_info.get('os_hash_value')
@@ -395,6 +409,13 @@ def _validate_image_info(ext, image_info=None, **kwargs):
                 not os_hash_value):
             raise errors.InvalidCommandParamsError(
                 'Image \'os_hash_value\' must be a non-empty string.')
+        os_hash_checksum_avail = True
+
+    if not (md5sum_avail or os_hash_checksum_avail):
+        raise errors.InvalidCommandParamsError(
+            'Image checksum is not available, either the \'checksum\' field '
+            'or the \'os_hash_algo\' and \'os_hash_value\' fields pair must '
+            'be set for image verification.')
 
 
 def _validate_partitioning(device):

ironic_python_agent/tests/unit/extensions/test_standby.py

@@ -72,6 +72,13 @@ class TestStandbyExtension(base.IronicAgentTest):
         image_info['os_hash_value'] = 'fake-checksum'
         standby._validate_image_info(None, image_info)
 
+    def test_validate_image_info_success_without_md5(self):
+        image_info = _build_fake_image_info()
+        del image_info['checksum']
+        image_info['os_hash_algo'] = 'sha512'
+        image_info['os_hash_value'] = 'fake-checksum'
+        standby._validate_image_info(None, image_info)
+
     def test_validate_image_info_missing_field(self):
         for field in ['id', 'urls', 'checksum']:
             invalid_info = _build_fake_image_info()
@@ -436,6 +443,24 @@ class TestStandbyExtension(base.IronicAgentTest):
         image_download.verify_image(image_location)
         hashlib_mock.assert_called_with('sha512')
 
+    @mock.patch('hashlib.new', autospec=True)
+    @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch('requests.get', autospec=True)
+    def test_verify_image_success_without_md5(self, requests_mock,
+                                              open_mock, hashlib_mock):
+        image_info = _build_fake_image_info()
+        del image_info['checksum']
+        image_info['os_hash_algo'] = 'sha512'
+        image_info['os_hash_value'] = 'fake-sha512-value'
+        response = requests_mock.return_value
+        response.status_code = 200
+        hexdigest_mock = hashlib_mock.return_value.hexdigest
+        hexdigest_mock.return_value = image_info['os_hash_value']
+        image_location = '/foo/bar'
+        image_download = standby.ImageDownload(image_info)
+        image_download.verify_image(image_location)
+        hashlib_mock.assert_called_with('sha512')
+
     @mock.patch('hashlib.md5', autospec=True)
     @mock.patch('six.moves.builtins.open', autospec=True)
     @mock.patch('requests.get', autospec=True)
@@ -487,6 +512,23 @@ class TestStandbyExtension(base.IronicAgentTest):
                           image_download.verify_image,
                           image_location)
 
+    @mock.patch('hashlib.new', autospec=True)
+    @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch('requests.get', autospec=True)
+    def test_verify_image_failure_without_fallback(self, requests_mock,
+                                                   open_mock, hashlib_mock):
+        image_info = _build_fake_image_info()
+        del image_info['checksum']
+        image_info['os_hash_algo'] = 'unsupported-algorithm'
+        image_info['os_hash_value'] = 'fake-value'
+        response = requests_mock.return_value
+        response.status_code = 200
+        self.assertRaisesRegex(errors.RESTError,
+                               'Unable to verify image.*'
+                               'unsupported-algorithm',
+                               standby.ImageDownload,
+                               image_info)
+
     @mock.patch('ironic_lib.disk_utils.get_disk_identifier',
                 lambda dev: 'ROOT')
     @mock.patch('ironic_python_agent.hardware.dispatch_to_managers',

releasenotes/notes/relax-checksum-feeding-11044ae02b411a07.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes an issue that md5 checksum is still required in the image
+    information when os_hash_algo and os_hash_value are present. The
+    ``checksum`` field is now optional, while ``os_hash_algo`` and
+    ``os_hash_value`` fields must be set if the ``checksum`` field is
+    not provided.