Allow image checksum to be a URL

We allow image_source to be a URL, let us also support URLs for checksums.
This change copies handling of multi-file checksum files from metalsmith.

Change-Id: Ie4d7e5c79b76bdd72d50eeb384cf10519278a80c
Story: #2005061
Task: #29605

ironic_python_agent/errors.py

@@ -142,7 +142,7 @@ class ImageDownloadError(RESTError):
     message = 'Error downloading image'
 
     def __init__(self, image_id, msg):
-        details = 'Download of image id {} failed: {}'.format(image_id, msg)
+        details = 'Download of image {} failed: {}'.format(image_id, msg)
         self.secondary_message = msg
         super(ImageDownloadError, self).__init__(details)
 

ironic_python_agent/extensions/standby.py

@@ -23,6 +23,7 @@ from oslo_config import cfg
 from oslo_log import log
 import requests
 import six
+from six.moves.urllib import parse as urlparse
 
 from ironic_python_agent import errors
 from ironic_python_agent.extensions import base
@@ -54,6 +55,60 @@ def _path_to_script(script):
     return os.path.join(cwd, '..', script)
 
 
+def _download_with_proxy(image_info, url, image_id):
+    """Opens a download stream for the given URL.
+
+    :param image_info: Image information dictionary.
+    :param url: The URL string to request the image from.
+    :param image_id: Image ID or URL for logging.
+
+    :raises: ImageDownloadError if the download stream was not started
+             properly.
+    """
+    no_proxy = image_info.get('no_proxy')
+    if no_proxy:
+        os.environ['no_proxy'] = no_proxy
+    proxies = image_info.get('proxies', {})
+    verify, cert = utils.get_ssl_client_options(CONF)
+    resp = requests.get(url, stream=True, proxies=proxies,
+                        verify=verify, cert=cert)
+    if resp.status_code != 200:
+        msg = ('Received status code {} from {}, expected 200. Response '
+               'body: {}').format(resp.status_code, url, resp.text)
+        raise errors.ImageDownloadError(image_id, msg)
+    return resp
+
+
+def _fetch_checksum(checksum, image_info):
+    """Fetch checksum from remote location, if needed."""
+    if not (checksum.startswith('http://') or checksum.startswith('https://')):
+        # Not a remote checksum, return as it is.
+        return checksum
+
+    LOG.debug('Downloading checksums file from %s', checksum)
+    resp = _download_with_proxy(image_info, checksum, checksum).text
+    lines = [line.strip() for line in resp.split('\n') if line.strip()]
+    if not lines:
+        raise errors.ImageDownloadError(checksum, "Empty checksum file")
+    elif len(lines) == 1:
+        # Special case - checksums file with only the checksum itself
+        if ' ' not in lines[0]:
+            return lines[0]
+
+    # FIXME(dtantsur): can we assume the same name for all images?
+    expected_fname = os.path.basename(
+        urlparse.urlparse(image_info['urls'][0]).path)
+    for line in lines:
+        checksum, fname = line.strip().split(None, 1)
+        # The star symbol designates binary mode, which is the same as text
+        # mode on GNU systems.
+        if fname.strip().lstrip('*') == expected_fname:
+            return checksum.strip()
+
+    raise errors.ImageDownloadError(
+        checksum, "Checksum file does not contain name %s" % expected_fname)
+
+
 def _write_partition_image(image, image_info, device):
     """Call disk_util to create partition and write the partition image.
 
@@ -217,11 +272,15 @@ class ImageDownload(object):
             self._hash_algo = hashlib.md5()
             self._expected_hash_value = image_info['checksum']
 
+        self._expected_hash_value = _fetch_checksum(self._expected_hash_value,
+                                                    image_info)
+
         details = []
         for url in image_info['urls']:
             try:
                 LOG.info("Attempting to download image from {}".format(url))
-                self._request = self._download_file(image_info, url)
+                self._request = _download_with_proxy(image_info, url,
+                                                     image_info['id'])
             except errors.ImageDownloadError as e:
                 failtime = time.time() - self._time
                 log_msg = ('URL: {}; time: {} '
@@ -236,28 +295,6 @@ class ImageDownload(object):
             details = '\n '.join(details)
             raise errors.ImageDownloadError(image_info['id'], details)
 
-    def _download_file(self, image_info, url):
-        """Opens a download stream for the given URL.
-
-        :param image_info: Image information dictionary.
-        :param url: The URL string to request the image from.
-
-        :raises: ImageDownloadError if the download stream was not started
-                 properly.
-        """
-        no_proxy = image_info.get('no_proxy')
-        if no_proxy:
-            os.environ['no_proxy'] = no_proxy
-        proxies = image_info.get('proxies', {})
-        verify, cert = utils.get_ssl_client_options(CONF)
-        resp = requests.get(url, stream=True, proxies=proxies,
-                            verify=verify, cert=cert)
-        if resp.status_code != 200:
-            msg = ('Received status code {} from {}, expected 200. Response '
-                   'body: {}').format(resp.status_code, url, resp.text)
-            raise errors.ImageDownloadError(image_info['id'], msg)
-        return resp
-
     def __iter__(self):
         """Downloads and returns the next chunk of the image.
 

ironic_python_agent/tests/unit/extensions/test_standby.py

@@ -23,13 +23,11 @@ from ironic_python_agent import hardware
 from ironic_python_agent.tests.unit import base
 
 
-def _build_fake_image_info():
+def _build_fake_image_info(url='http://example.org'):
     return {
         'id': 'fake_id',
         'node_uuid': '1be26c0b-03f2-4d2e-ae87-c02d7f33c123',
-        'urls': [
+        'urls': [url],
-            'http://example.org',
-        ],
         'checksum': 'abc123',
         'image_type': 'whole-disk-image',
     }
@@ -1119,10 +1117,10 @@ class TestStandbyExtension(base.IronicAgentTest):
         self.assertEqual(expected_msg, result_msg)
 
 
-class TestImageDownload(base.IronicAgentTest):
-
 @mock.patch('hashlib.md5', autospec=True)
 @mock.patch('requests.get', autospec=True)
+class TestImageDownload(base.IronicAgentTest):
+
     def test_download_image(self, requests_mock, md5_mock):
         content = ['SpongeBob', 'SquarePants']
         response = requests_mock.return_value
@@ -1140,16 +1138,14 @@ class TestImageDownload(base.IronicAgentTest):
         self.assertEqual(image_info['checksum'],
                          image_download._hash_algo.hexdigest())
 
-    @mock.patch('time.time', autospec=True)
-    @mock.patch('requests.get', autospec=True)
     def test_download_image_fail(self, requests_mock, time_mock):
         response = requests_mock.return_value
         response.status_code = 401
         response.text = 'Unauthorized'
         time_mock.return_value = 0.0
         image_info = _build_fake_image_info()
-        msg = ('Error downloading image: Download of image id fake_id failed: '
+        msg = ('Error downloading image: Download of image fake_id failed: '
-               'URL: http://example.org; time: 0.0 seconds. Error: '
+               'URL: http://example.org; time: .* seconds. Error: '
                'Received status code 401 from http://example.org, expected '
                '200. Response body: Unauthorized')
         self.assertRaisesRegex(errors.ImageDownloadError, msg,
@@ -1157,3 +1153,117 @@ class TestImageDownload(base.IronicAgentTest):
         requests_mock.assert_called_once_with(image_info['urls'][0],
                                               cert=None, verify=True,
                                               stream=True, proxies={})
+
+    def test_download_image_and_checksum(self, requests_mock, md5_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['checksum'] = 'http://example.com/checksum'
+        md5_mock.return_value.hexdigest.return_value = fake_cs
+        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={}),
+            mock.call(image_info['urls'][0], cert=None, verify=True,
+                      stream=True, proxies={}),
+        ])
+        self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+
+    def test_download_image_and_checksum_multiple(self, requests_mock,
+                                                  md5_mock):
+        content = ['SpongeBob', 'SquarePants']
+        fake_cs = "019fe036425da1c562f2e9f5299820bf"
+        cs_response = mock.Mock()
+        cs_response.status_code = 200
+        cs_response.text = """
+foobar  irrelevant file.img
+%s  image.img
+""" % fake_cs
+        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(
+            'http://example.com/path/image.img')
+        image_info['checksum'] = 'http://example.com/checksum'
+        md5_mock.return_value.hexdigest.return_value = fake_cs
+        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={}),
+            mock.call(image_info['urls'][0], cert=None, verify=True,
+                      stream=True, proxies={}),
+        ])
+        self.assertEqual(fake_cs, image_download._hash_algo.hexdigest())
+
+    def test_download_image_and_checksum_unknown_file(self, requests_mock,
+                                                      md5_mock):
+        content = ['SpongeBob', 'SquarePants']
+        fake_cs = "019fe036425da1c562f2e9f5299820bf"
+        cs_response = mock.Mock()
+        cs_response.status_code = 200
+        cs_response.text = """
+foobar  irrelevant file.img
+%s  not-my-image.img
+""" % fake_cs
+        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(
+            'http://example.com/path/image.img')
+        image_info['checksum'] = 'http://example.com/checksum'
+        md5_mock.return_value.hexdigest.return_value = fake_cs
+        self.assertRaisesRegex(errors.ImageDownloadError,
+                               'Checksum file does not contain name image.img',
+                               standby.ImageDownload, image_info)
+
+    def test_download_image_and_checksum_empty_file(self, requests_mock,
+                                                    md5_mock):
+        content = ['SpongeBob', 'SquarePants']
+        cs_response = mock.Mock()
+        cs_response.status_code = 200
+        cs_response.text = " "
+        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(
+            'http://example.com/path/image.img')
+        image_info['checksum'] = 'http://example.com/checksum'
+        self.assertRaisesRegex(errors.ImageDownloadError,
+                               'Empty checksum file',
+                               standby.ImageDownload, image_info)
+
+    def test_download_image_and_checksum_failed(self, requests_mock, md5_mock):
+        content = ['SpongeBob', 'SquarePants']
+        cs_response = mock.Mock()
+        cs_response.status_code = 400
+        cs_response.text = " "
+        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(
+            'http://example.com/path/image.img')
+        image_info['checksum'] = 'http://example.com/checksum'
+        self.assertRaisesRegex(errors.ImageDownloadError,
+                               'Received status code 400 from '
+                               'http://example.com/checksum',
+                               standby.ImageDownload, image_info)

releasenotes/notes/image-checksum-39b2ceef40933c28.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Allows image checksum to be a URL pointing at a file with the image
+    checksum or several checksums.