Merge "Warn when the provided checksum algorithm does not match the detected" into stable/2023.2

ironic_python_agent/extensions/standby.py

@@ -477,6 +477,25 @@ class ImageDownload(object):
                 self._expected_hash_value,
                 image_info)
 
+        # NOTE(dtantsur): verify that the user's input does not obviously
+        # contradict the actual value. It is especially easy to make such
+        # a mistake when providing a checksum URL.
+        if algo:
+            try:
+                detected_algo = _get_algorithm_by_length(
+                    self._expected_hash_value)
+            except ValueError:
+                pass  # an exotic algorithm?
+            else:
+                if detected_algo.name != algo:
+                    LOG.warning("Provided checksum algorithm %(provided)s "
+                                "does not match the detected algorithm "
+                                "%(detected)s. It may be a sign of a user "
+                                "error when providing the algorithm or the "
+                                "checksum URL.",
+                                {'provided': algo,
+                                 'detected': detected_algo.name})
+
         details = []
         for url in image_info['urls']:
             try:

ironic_python_agent/tests/unit/extensions/test_standby.py

@@ -620,11 +620,13 @@ class TestStandbyExtension(base.IronicAgentTest):
         image_download.verify_image(image_location)
         hashlib_mock.assert_called_with('sha512')
 
+    @mock.patch.object(standby.LOG, 'warning', autospec=True)
     @mock.patch('hashlib.new', autospec=True)
     @mock.patch('builtins.open', autospec=True)
     @mock.patch('requests.get', autospec=True)
     def test_verify_image_success_with_md5_fallback(self, requests_mock,
-                                                    open_mock, hash_mock):
+                                                    open_mock, hash_mock,
+                                                    warn_mock):
         CONF.set_override('md5_enabled', True)
         image_info = _build_fake_image_info()
         image_info['os_hash_algo'] = 'algo-beyond-milky-way'
@@ -632,6 +634,7 @@ class TestStandbyExtension(base.IronicAgentTest):
         image_info['checksum'] = 'd41d8cd98f00b204e9800998ecf8427e'
         response = requests_mock.return_value
         response.status_code = 200
+        hash_mock.return_value.name = 'md5'
         hexdigest_mock = hash_mock.return_value.hexdigest
         hexdigest_mock.return_value = image_info['checksum']
         image_location = '/foo/bar'
@@ -643,7 +646,11 @@ class TestStandbyExtension(base.IronicAgentTest):
         hash_mock.assert_has_calls([
             mock.call('md5'),
             mock.call().__bool__(),
+            mock.call('md5'),
             mock.call().hexdigest()])
+        warn_mock.assert_called_once_with(
+            mock.ANY,
+            {'provided': 'algo-beyond-milky-way', 'detected': 'md5'})
 
     @mock.patch('hashlib.new', autospec=True)
     @mock.patch('builtins.open', autospec=True)
@@ -1706,7 +1713,9 @@ class TestImageDownload(base.IronicAgentTest):
         sleep_mock.assert_called_with(10)
         self.assertEqual(2, sleep_mock.call_count)
 
-    def test_download_image_and_checksum(self, requests_mock, hash_mock):
+    @mock.patch.object(standby.LOG, 'warning', autospec=True)
+    def test_download_image_and_checksum(self, warn_mock, requests_mock,
+                                         hash_mock):
         content = ['SpongeBob', 'SquarePants']
         fake_cs = "019fe036425da1c562f2e9f5299820bf"
         cs_response = mock.Mock()
@@ -1721,6 +1730,7 @@ class TestImageDownload(base.IronicAgentTest):
         image_info['os_hash_algo'] = 'sha512'
         image_info['os_hash_value'] = 'http://example.com/checksum'
         hash_mock.return_value.hexdigest.return_value = fake_cs
+        hash_mock.return_value.name = 'sha512'
         image_download = standby.ImageDownload(image_info)
 
         self.assertEqual(content, list(image_download))
@@ -1731,6 +1741,39 @@ class TestImageDownload(base.IronicAgentTest):
                       stream=True, proxies={}, timeout=60),
         ])
         self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+        warn_mock.assert_not_called()
+
+    @mock.patch.object(standby.LOG, 'warning', autospec=True)
+    def test_download_image_and_checksum_warning_on_mismatch(
+            self, warn_mock, requests_mock, hash_mock):
+        content = ['SpongeBob', 'SquarePants']
+        fake_cs = "019fe036425da1c562f2e9f5299820bf"
+        cs_response = mock.Mock()
+        cs_response.status_code = 200
+        cs_response.text = fake_cs + '\n'
+        response = mock.Mock()
+        response.status_code = 200
+        response.iter_content.return_value = content
+        requests_mock.side_effect = [cs_response, response]
+
+        image_info = _build_fake_image_info()
+        image_info['os_hash_algo'] = 'sha512'
+        image_info['os_hash_value'] = 'http://example.com/checksum'
+        hash_mock.return_value.hexdigest.return_value = fake_cs
+        hash_mock.return_value.name = 'md5'
+        image_download = standby.ImageDownload(image_info)
+
+        self.assertEqual(content, list(image_download))
+        requests_mock.assert_has_calls([
+            mock.call('http://example.com/checksum', cert=None, verify=True,
+                      stream=True, proxies={}, timeout=60),
+            mock.call(image_info['urls'][0], cert=None, verify=True,
+                      stream=True, proxies={}, timeout=60),
+        ])
+        self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+        warn_mock.assert_called_once_with(
+            mock.ANY,
+            {'provided': 'sha512', 'detected': 'md5'})
 
     def test_download_image_and_checksum_md5(self, requests_mock, hash_mock):