Merge "Enhanced checksum support"
This commit is contained in:
commit
5ad766ebcb
@ -198,9 +198,21 @@ class ImageDownload(object):
|
||||
:raises: ImageDownloadError if starting the image download fails for
|
||||
any reason.
|
||||
"""
|
||||
self._md5checksum = hashlib.md5()
|
||||
self._time = time_obj or time.time()
|
||||
self._image_info = image_info
|
||||
self._request = None
|
||||
|
||||
# Determine the hash algorithm and value will be used for calculation
|
||||
# and verification, fallback to md5 if algorithm is not set or not
|
||||
# supported.
|
||||
algo = image_info.get('os_hash_algo')
|
||||
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:
|
||||
self._hash_algo = hashlib.md5()
|
||||
self._expected_hash_value = image_info['checksum']
|
||||
|
||||
details = []
|
||||
for url in image_info['urls']:
|
||||
try:
|
||||
@ -249,41 +261,31 @@ class ImageDownload(object):
|
||||
which is a constant in this module.
|
||||
"""
|
||||
for chunk in self._request.iter_content(IMAGE_CHUNK_SIZE):
|
||||
self._md5checksum.update(chunk)
|
||||
self._hash_algo.update(chunk)
|
||||
yield chunk
|
||||
|
||||
def md5sum(self):
|
||||
"""Computes and returns the md5 checksum of the downloaded image.
|
||||
def verify_image(self, image_location):
|
||||
"""Verifies the checksum of the local images matches expectations.
|
||||
|
||||
Note that md5sum will not return the true checksum of the image until
|
||||
the download has been fully completed through this object's
|
||||
iterator inferface.
|
||||
If this function does not raise ImageChecksumError then it is very
|
||||
likely that the local copy of the image was transmitted and stored
|
||||
correctly.
|
||||
|
||||
:returns: The md5 checksum of the image as a string in hexadecimal.
|
||||
:param image_location: The location of the local image.
|
||||
:raises: ImageChecksumError if the checksum of the local image does
|
||||
not match the checksum as reported by glance in image_info.
|
||||
"""
|
||||
return self._md5checksum.hexdigest()
|
||||
|
||||
|
||||
def _verify_image(image_info, image_location, checksum):
|
||||
"""Verifies the checksum of the local images matches expectations.
|
||||
|
||||
If this function does not raise ImageChecksumError then it is very likely
|
||||
that the local copy of the image was transmitted and stored correctly.
|
||||
|
||||
:param image_info: Image information dictionary.
|
||||
:param image_location: The location of the local image.
|
||||
:param checksum: The computed checksum of the local image.
|
||||
:raises: ImageChecksumError if the checksum of the local image does not
|
||||
match the checksum as reported by glance in image_info.
|
||||
"""
|
||||
LOG.debug('Verifying image at {} against MD5 checksum '
|
||||
'{}'.format(image_location, checksum))
|
||||
if checksum != image_info['checksum']:
|
||||
LOG.error(errors.ImageChecksumError.details_str.format(
|
||||
image_location, image_info['id'],
|
||||
image_info['checksum'], checksum))
|
||||
raise errors.ImageChecksumError(image_location, image_info['id'],
|
||||
image_info['checksum'], checksum)
|
||||
checksum = self._hash_algo.hexdigest()
|
||||
LOG.debug('Verifying image at {} against {} checksum '
|
||||
'{}'.format(image_location, self._hash_algo.name, checksum))
|
||||
if checksum != self._expected_hash_value:
|
||||
LOG.error(errors.ImageChecksumError.details_str.format(
|
||||
image_location, self._image_info['id'],
|
||||
self._expected_hash_value, checksum))
|
||||
raise errors.ImageChecksumError(image_location,
|
||||
self._image_info['id'],
|
||||
self._expected_hash_value,
|
||||
checksum)
|
||||
|
||||
|
||||
def _download_image(image_info):
|
||||
@ -310,7 +312,7 @@ def _download_image(image_info):
|
||||
totaltime = time.time() - starttime
|
||||
LOG.info("Image downloaded from {} in {} seconds".format(image_location,
|
||||
totaltime))
|
||||
_verify_image(image_info, image_location, image_download.md5sum())
|
||||
image_download.verify_image(image_location)
|
||||
|
||||
|
||||
def _validate_image_info(ext, image_info=None, **kwargs):
|
||||
@ -341,6 +343,18 @@ def _validate_image_info(ext, image_info=None, **kwargs):
|
||||
raise errors.InvalidCommandParamsError(
|
||||
'Image \'checksum\' must be a non-empty string.')
|
||||
|
||||
os_hash_algo = image_info.get('os_hash_algo')
|
||||
os_hash_value = image_info.get('os_hash_value')
|
||||
if os_hash_algo or os_hash_value:
|
||||
if (not isinstance(os_hash_algo, six.string_types) or
|
||||
not os_hash_algo):
|
||||
raise errors.InvalidCommandParamsError(
|
||||
'Image \'os_hash_algo\' must be a non-empty string.')
|
||||
if (not isinstance(os_hash_value, six.string_types) or
|
||||
not os_hash_value):
|
||||
raise errors.InvalidCommandParamsError(
|
||||
'Image \'os_hash_value\' must be a non-empty string.')
|
||||
|
||||
|
||||
class StandbyExtension(base.BaseAgentExtension):
|
||||
"""Extension which adds stand-by related functionality to agent."""
|
||||
@ -397,7 +411,7 @@ class StandbyExtension(base.BaseAgentExtension):
|
||||
LOG.info("Image streamed onto device {} in {} "
|
||||
"seconds".format(device, totaltime))
|
||||
# Verify if the checksum of the streamed image is correct
|
||||
_verify_image(image_info, device, image_download.md5sum())
|
||||
image_download.verify_image(device)
|
||||
|
||||
@base.async_command('cache_image', _validate_image_info)
|
||||
def cache_image(self, image_info=None, force=False):
|
||||
|
@ -68,6 +68,12 @@ class TestStandbyExtension(base.IronicAgentTest):
|
||||
def test_validate_image_info_success(self):
|
||||
standby._validate_image_info(None, _build_fake_image_info())
|
||||
|
||||
def test_validate_image_info_success_with_new_hash_fields(self):
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['os_hash_algo'] = 'md5'
|
||||
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()
|
||||
@ -109,6 +115,22 @@ class TestStandbyExtension(base.IronicAgentTest):
|
||||
standby._validate_image_info,
|
||||
invalid_info)
|
||||
|
||||
def test_validate_image_info_no_hash_value(self):
|
||||
invalid_info = _build_fake_image_info()
|
||||
invalid_info['os_hash_algo'] = 'sha512'
|
||||
|
||||
self.assertRaises(errors.InvalidCommandParamsError,
|
||||
standby._validate_image_info,
|
||||
invalid_info)
|
||||
|
||||
def test_validate_image_info_no_hash_algo(self):
|
||||
invalid_info = _build_fake_image_info()
|
||||
invalid_info['os_hash_value'] = 'fake-checksum'
|
||||
|
||||
self.assertRaises(errors.InvalidCommandParamsError,
|
||||
standby._validate_image_info,
|
||||
invalid_info)
|
||||
|
||||
def test_cache_image_invalid_image_list(self):
|
||||
self.assertRaises(errors.InvalidCommandParamsError,
|
||||
self.agent_extension.cache_image,
|
||||
@ -385,19 +407,87 @@ class TestStandbyExtension(base.IronicAgentTest):
|
||||
standby._download_image,
|
||||
image_info)
|
||||
|
||||
def test_verify_image_success(self):
|
||||
@mock.patch('hashlib.md5', autospec=True)
|
||||
@mock.patch('six.moves.builtins.open', autospec=True)
|
||||
@mock.patch('requests.get', autospec=True)
|
||||
def test_verify_image_success(self, requests_mock, open_mock, md5_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
response = requests_mock.return_value
|
||||
response.status_code = 200
|
||||
hexdigest_mock = md5_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = image_info['checksum']
|
||||
image_location = '/foo/bar'
|
||||
checksum = image_info['checksum']
|
||||
standby._verify_image(image_info, image_location, checksum)
|
||||
image_download = standby.ImageDownload(image_info)
|
||||
image_download.verify_image(image_location)
|
||||
|
||||
def test_verify_image_failure(self):
|
||||
@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_with_new_hash_fields(self, requests_mock,
|
||||
open_mock,
|
||||
hashlib_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
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'
|
||||
checksum = 'invalid-checksum'
|
||||
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)
|
||||
def test_verify_image_success_with_md5_fallback(self, requests_mock,
|
||||
open_mock, md5_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['os_hash_algo'] = 'algo-beyond-milky-way'
|
||||
image_info['os_hash_value'] = 'mysterious-alien-codes'
|
||||
response = requests_mock.return_value
|
||||
response.status_code = 200
|
||||
hexdigest_mock = md5_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = image_info['checksum']
|
||||
image_location = '/foo/bar'
|
||||
image_download = standby.ImageDownload(image_info)
|
||||
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_with_new_hash_fields(self, requests_mock,
|
||||
open_mock,
|
||||
hashlib_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['os_hash_algo'] = 'sha512'
|
||||
image_info['os_hash_value'] = 'fake-sha512-value'
|
||||
response = requests_mock.return_value
|
||||
response.status_code = 200
|
||||
image_download = standby.ImageDownload(image_info)
|
||||
image_location = '/foo/bar'
|
||||
hexdigest_mock = hashlib_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = 'invalid-checksum'
|
||||
self.assertRaises(errors.ImageChecksumError,
|
||||
standby._verify_image,
|
||||
image_info, image_location, checksum)
|
||||
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)
|
||||
def test_verify_image_failure(self, requests_mock, open_mock, md5_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
response = requests_mock.return_value
|
||||
response.status_code = 200
|
||||
image_download = standby.ImageDownload(image_info)
|
||||
image_location = '/foo/bar'
|
||||
hexdigest_mock = md5_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = 'invalid-checksum'
|
||||
self.assertRaises(errors.ImageChecksumError,
|
||||
image_download.verify_image,
|
||||
image_location)
|
||||
|
||||
@mock.patch('ironic_lib.disk_utils.get_disk_identifier',
|
||||
lambda dev: 'ROOT')
|
||||
@ -945,7 +1035,8 @@ class TestImageDownload(base.IronicAgentTest):
|
||||
requests_mock.assert_called_once_with(image_info['urls'][0],
|
||||
cert=None, verify=True,
|
||||
stream=True, proxies={})
|
||||
self.assertEqual(image_info['checksum'], image_download.md5sum())
|
||||
self.assertEqual(image_info['checksum'],
|
||||
image_download._hash_algo.hexdigest())
|
||||
|
||||
@mock.patch('time.time', autospec=True)
|
||||
@mock.patch('requests.get', autospec=True)
|
||||
|
@ -0,0 +1,7 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
Adds enhanced checksum support to IPA, when ``os_hash_algo`` and
|
||||
``os_hash_value`` are passed in via ``image_info``, it will be used
|
||||
for image checksum calculation and verification. The md5 checksum
|
||||
is supported if these information are absent.
|
Loading…
x
Reference in New Issue
Block a user