Checksum files before raw conversion
While working another issue, we discovered that support added to the ironic-conductor process combined the image_download_source option of "local" with the "force_raw" option resulted in a case where Ironic had no concept to checksum the files *before* the conductor process triggered an image format conversion and then records new checksum values. In essence, this opened the user requested image file to be suspetible to a theoretical man-in-the-middle attack OR the remote server replacing the content with an unknown file, such as a new major version. The is at odds with Ironic's security model where we do want to ensure the end user of ironic is asserting a known checksum for the image artifact they are deploying, so they are aware of the present state. Due to the risk, we chose to raise this as a CVE, as infrastructure operators should likely apply this patch. As a note, if your *not* forcing all images to be raw format through the conductor, then this issue is likely not a major issue for you, but you should still apply the patch. This is being tracked as CVE-2024-47211. Closes-Bug: 2076289 Change-Id: Id6185b317aa6e4f4363ee49f77e688701995323a Signed-off-by: Julia Kreger <juliaashleykreger@gmail.com>
This commit is contained in:
parent
9fe510a14a
commit
00c5e0faf8
@ -19,6 +19,40 @@ OpenStack deployment.
|
|||||||
|
|
||||||
.. TODO: add "Multi-tenancy Considerations" section
|
.. TODO: add "Multi-tenancy Considerations" section
|
||||||
|
|
||||||
|
Image Checksums
|
||||||
|
===============
|
||||||
|
|
||||||
|
Ironic has long provided a capacity to supply and check a checksum for disk
|
||||||
|
images being deployed. However, one aspect which Ironic has not asserted is
|
||||||
|
"Why?" in terms of "Is it for security?" or "Is it for data integrity?".
|
||||||
|
|
||||||
|
The answer is both to ensure a higher level of security with remote
|
||||||
|
image files, *and* provide faster feedback should a image being transferred
|
||||||
|
happens to be corrupted.
|
||||||
|
|
||||||
|
Normally checksums are verified by the ``ironic-python-agent`` **OR** the
|
||||||
|
deployment interface responsible for overall deployment operation. That being
|
||||||
|
said, not *every* deployment interface relies on disk images which have
|
||||||
|
checksums, and those deployment interfaces are for specific use cases which
|
||||||
|
Ironic users leverage, outside of the "general" use case capabilities provided
|
||||||
|
by the ``direct`` deployment interface.
|
||||||
|
|
||||||
|
.. NOTE::
|
||||||
|
Use of the node ``instance_info/image_checksum`` field is discouraged
|
||||||
|
for integrated OpenStack Users as usage of the matching Glance Image
|
||||||
|
Service field is also deprecated. That being said, Ironic retains this
|
||||||
|
feature by popular demand while also enabling also retain simplified
|
||||||
|
operator interaction.
|
||||||
|
The newer field values supported by Glance are also specifically
|
||||||
|
supported by Ironic as ``instance_info/image_os_hash_value`` for
|
||||||
|
checksum values and ``instance_info/image_os_hash_algo`` field for
|
||||||
|
the checksum algorithm.
|
||||||
|
|
||||||
|
.. WARNING::
|
||||||
|
Setting a checksum value to a URL is supported, *however* doing this is
|
||||||
|
making a "tradeoff" with security as the remote checksum *can* change.
|
||||||
|
Conductor support this functionality can be disabled using the
|
||||||
|
:oslo.config:option:`conductor.disable_support_for_checksum_files` setting.
|
||||||
|
|
||||||
REST API: user roles and policy settings
|
REST API: user roles and policy settings
|
||||||
========================================
|
========================================
|
||||||
|
258
ironic/common/checksum_utils.py
Normal file
258
ironic/common/checksum_utils.py
Normal file
@ -0,0 +1,258 @@
|
|||||||
|
# Copyright (c) 2024 Red Hat, Inc.
|
||||||
|
#
|
||||||
|
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||||
|
# not use this file except in compliance with the License. You may obtain
|
||||||
|
# a copy of the License at
|
||||||
|
#
|
||||||
|
# http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
#
|
||||||
|
# Unless required by applicable law or agreed to in writing, software
|
||||||
|
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||||
|
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||||
|
# License for the specific language governing permissions and limitations
|
||||||
|
# under the License.
|
||||||
|
|
||||||
|
import os
|
||||||
|
import re
|
||||||
|
import time
|
||||||
|
from urllib import parse as urlparse
|
||||||
|
|
||||||
|
from oslo_log import log as logging
|
||||||
|
from oslo_utils import fileutils
|
||||||
|
|
||||||
|
from ironic.common import exception
|
||||||
|
from ironic.common.i18n import _
|
||||||
|
from ironic.common import image_service
|
||||||
|
from ironic.conf import CONF
|
||||||
|
|
||||||
|
LOG = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
# REGEX matches for Checksum file payloads
|
||||||
|
# If this list requires changes, it should be changed in
|
||||||
|
# ironic-python-agent (extensions/standby.py) as well.
|
||||||
|
|
||||||
|
MD5_MATCH = r"^([a-fA-F\d]{32})\s" # MD5 at beginning of line
|
||||||
|
MD5_MATCH_END = r"\s([a-fA-F\d]{32})$" # MD5 at end of line
|
||||||
|
MD5_MATCH_ONLY = r"^([a-fA-F\d]{32})$" # MD5 only
|
||||||
|
SHA256_MATCH = r"^([a-fA-F\d]{64})\s" # SHA256 at beginning of line
|
||||||
|
SHA256_MATCH_END = r"\s([a-fA-F\d]{64})$" # SHA256 at end of line
|
||||||
|
SHA256_MATCH_ONLY = r"^([a-fA-F\d]{64})$" # SHA256 only
|
||||||
|
SHA512_MATCH = r"^([a-fA-F\d]{128})\s" # SHA512 at beginning of line
|
||||||
|
SHA512_MATCH_END = r"\s([a-fA-F\d]{128})$" # SHA512 at end of line
|
||||||
|
SHA512_MATCH_ONLY = r"^([a-fA-F\d]{128})$" # SHA512 only
|
||||||
|
FILENAME_MATCH_END = r"\s[*]?{filename}$" # Filename binary/text end of line
|
||||||
|
FILENAME_MATCH_PARENTHESES = r"\s\({filename}\)\s" # CentOS images
|
||||||
|
|
||||||
|
CHECKSUM_MATCHERS = (MD5_MATCH, MD5_MATCH_END, SHA256_MATCH, SHA256_MATCH_END,
|
||||||
|
SHA512_MATCH, SHA512_MATCH_END)
|
||||||
|
CHECKSUM_ONLY_MATCHERS = (MD5_MATCH_ONLY, SHA256_MATCH_ONLY, SHA512_MATCH_ONLY)
|
||||||
|
FILENAME_MATCHERS = (FILENAME_MATCH_END, FILENAME_MATCH_PARENTHESES)
|
||||||
|
|
||||||
|
|
||||||
|
def validate_checksum(path, checksum, checksum_algo=None):
|
||||||
|
"""Validate image checksum.
|
||||||
|
|
||||||
|
:param path: File path in the form of a string to calculate a checksum
|
||||||
|
which is compared to the checksum field.
|
||||||
|
:param checksum: The supplied checksum value, a string, which will be
|
||||||
|
compared to the file.
|
||||||
|
:param checksum_algo: The checksum type of the algorithm.
|
||||||
|
:raises: ImageChecksumError if the supplied data cannot be parsed or
|
||||||
|
if the supplied value does not match the supplied checksum
|
||||||
|
value.
|
||||||
|
"""
|
||||||
|
# TODO(TheJilia): At some point, we likely need to compare
|
||||||
|
# the incoming checksum algorithm upfront, ut if one is invoked which
|
||||||
|
# is not supported, hashlib will raise ValueError.
|
||||||
|
use_checksum_algo = None
|
||||||
|
if ":" in checksum:
|
||||||
|
# A form of communicating the checksum algorithm is to delimit the
|
||||||
|
# type from the value. See ansible deploy interface where this
|
||||||
|
# is most evident.
|
||||||
|
split_checksum = checksum.split(":")
|
||||||
|
use_checksum = split_checksum[1]
|
||||||
|
use_checksum_algo = split_checksum[0]
|
||||||
|
else:
|
||||||
|
use_checksum = checksum
|
||||||
|
if not use_checksum_algo:
|
||||||
|
use_checksum_algo = checksum_algo
|
||||||
|
# If we have a zero length value, but we split it, we have
|
||||||
|
# invalid input. Also, checksum is what we expect, algorithm is
|
||||||
|
# optional. This guards against the split of a value which is
|
||||||
|
# image_checksum = "sha256:" which is a potential side effect of
|
||||||
|
# splitting the string.
|
||||||
|
if use_checksum == '':
|
||||||
|
raise exception.ImageChecksumError()
|
||||||
|
|
||||||
|
# Make everything lower case since we don't expect mixed case,
|
||||||
|
# but we may have human originated input on the supplied algorithm.
|
||||||
|
try:
|
||||||
|
if not use_checksum_algo:
|
||||||
|
# This is backwards compatible support for a bare checksum.
|
||||||
|
calculated = compute_image_checksum(path)
|
||||||
|
else:
|
||||||
|
calculated = compute_image_checksum(path,
|
||||||
|
use_checksum_algo.lower())
|
||||||
|
except ValueError:
|
||||||
|
# ValueError is raised when an invalid/unsupported/unknown
|
||||||
|
# checksum algorithm is invoked.
|
||||||
|
LOG.error("Failed to generate checksum for file %(path)s, possible "
|
||||||
|
"invalid checksum algorithm: %(algo)s",
|
||||||
|
{"path": path,
|
||||||
|
"algo": use_checksum_algo})
|
||||||
|
raise exception.ImageChecksumAlgorithmFailure()
|
||||||
|
except OSError:
|
||||||
|
LOG.error("Failed to read file %(path)s to compute checksum.",
|
||||||
|
{"path": path})
|
||||||
|
raise exception.ImageChecksumFileReadFailure()
|
||||||
|
if (use_checksum is not None
|
||||||
|
and calculated.lower() != use_checksum.lower()):
|
||||||
|
LOG.error("We were supplied a checksum value of %(supplied)s, but "
|
||||||
|
"calculated a value of %(value)s. This is a fatal error.",
|
||||||
|
{"supplied": use_checksum,
|
||||||
|
"value": calculated})
|
||||||
|
raise exception.ImageChecksumError()
|
||||||
|
|
||||||
|
|
||||||
|
def compute_image_checksum(image_path, algorithm='md5'):
|
||||||
|
"""Compute checksum by given image path and algorithm.
|
||||||
|
|
||||||
|
:param image_path: The path to the file to undergo checksum calculation.
|
||||||
|
:param algorithm: The checksum algorithm to utilize. Defaults
|
||||||
|
to 'md5' due to historical support reasons in Ironic.
|
||||||
|
:returns: The calculated checksum value.
|
||||||
|
:raises: ValueError when the checksum algorithm is not supported
|
||||||
|
by the system.
|
||||||
|
"""
|
||||||
|
|
||||||
|
time_start = time.time()
|
||||||
|
LOG.debug('Start computing %(algo)s checksum for image %(image)s.',
|
||||||
|
{'algo': algorithm, 'image': image_path})
|
||||||
|
|
||||||
|
checksum = fileutils.compute_file_checksum(image_path,
|
||||||
|
algorithm=algorithm)
|
||||||
|
time_elapsed = time.time() - time_start
|
||||||
|
LOG.debug('Computed %(algo)s checksum for image %(image)s in '
|
||||||
|
'%(delta).2f seconds, checksum value: %(checksum)s.',
|
||||||
|
{'algo': algorithm, 'image': image_path, 'delta': time_elapsed,
|
||||||
|
'checksum': checksum})
|
||||||
|
return checksum
|
||||||
|
|
||||||
|
|
||||||
|
def get_checksum_and_algo(instance_info):
|
||||||
|
"""Get and return the image checksum and algo.
|
||||||
|
|
||||||
|
:param instance_info: The node instance info, or newly updated/generated
|
||||||
|
instance_info value.
|
||||||
|
:returns: A tuple containing two values, a checksum and algorithm,
|
||||||
|
if available.
|
||||||
|
"""
|
||||||
|
checksum_algo = None
|
||||||
|
if 'image_os_hash_value' in instance_info.keys():
|
||||||
|
# A value set by image_os_hash_value supersedes other
|
||||||
|
# possible uses as it is specific.
|
||||||
|
checksum = instance_info.get('image_os_hash_value')
|
||||||
|
checksum_algo = instance_info.get('image_os_hash_algo')
|
||||||
|
else:
|
||||||
|
checksum = instance_info.get('image_checksum')
|
||||||
|
if is_checksum_url(checksum):
|
||||||
|
image_source = instance_info.get('image_source')
|
||||||
|
checksum = get_checksum_from_url(checksum, image_source)
|
||||||
|
|
||||||
|
# NOTE(TheJulia): This is all based on SHA-2 lengths.
|
||||||
|
# SHA-3 would require a hint and it would not be a fixed length.
|
||||||
|
# That said, SHA-2 is still valid and has not been withdrawn.
|
||||||
|
checksum_len = len(checksum)
|
||||||
|
if checksum_len == 128:
|
||||||
|
# SHA2-512 is 512 bits, 128 characters.
|
||||||
|
checksum_algo = "sha512"
|
||||||
|
elif checksum_len == 64:
|
||||||
|
checksum_algo = "sha256"
|
||||||
|
|
||||||
|
if checksum_len == 32 and not CONF.agent.allow_md5_checksum:
|
||||||
|
# MD5 not permitted and the checksum is the length of MD5
|
||||||
|
# and not otherwise defined.
|
||||||
|
LOG.error('Cannot compute the checksum as it uses MD5 '
|
||||||
|
'and is disabled by configuration. If the checksum '
|
||||||
|
'is *not* MD5, please specify the algorithm.')
|
||||||
|
raise exception.ImageChecksumAlgorithmFailure()
|
||||||
|
|
||||||
|
return checksum, checksum_algo
|
||||||
|
|
||||||
|
|
||||||
|
def is_checksum_url(checksum):
|
||||||
|
"""Identify if checksum is not a url.
|
||||||
|
|
||||||
|
:param checksum: The user supplied checksum value.
|
||||||
|
:returns: True if the checksum is a url, otherwise False.
|
||||||
|
:raises: ImageChecksumURLNotSupported should the conductor have this
|
||||||
|
support disabled.
|
||||||
|
"""
|
||||||
|
if (checksum.startswith('http://') or checksum.startswith('https://')):
|
||||||
|
if CONF.conductor.disable_support_for_checksum_files:
|
||||||
|
raise exception.ImageChecksumURLNotSupported()
|
||||||
|
return True
|
||||||
|
else:
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def get_checksum_from_url(checksum, image_source):
|
||||||
|
"""Gets a checksum value based upon a remote checksum URL file.
|
||||||
|
|
||||||
|
:param checksum: The URL to the checksum URL content.
|
||||||
|
:param image_soource: The image source utilized to match with
|
||||||
|
the contents of the URL payload file.
|
||||||
|
:raises: ImageDownloadFailed when the checksum file cannot be
|
||||||
|
accessed or cannot be parsed.
|
||||||
|
"""
|
||||||
|
|
||||||
|
LOG.debug('Attempting to download checksum from: %(checksum)s.',
|
||||||
|
{'checksum': checksum})
|
||||||
|
|
||||||
|
# Directly invoke the image service and get the checksum data.
|
||||||
|
resp = image_service.HttpImageService.get(checksum)
|
||||||
|
checksum_url = str(checksum)
|
||||||
|
|
||||||
|
# NOTE(TheJulia): The rest of this method is taken from
|
||||||
|
# ironic-python-agent. If a change is required here, it may
|
||||||
|
# be required in ironic-python-agent (extensions/standby.py).
|
||||||
|
lines = [line.strip() for line in resp.split('\n') if line.strip()]
|
||||||
|
if not lines:
|
||||||
|
raise exception.ImageDownloadFailed(image_href=checksum,
|
||||||
|
reason=_('Checksum file empty.'))
|
||||||
|
elif len(lines) == 1:
|
||||||
|
# Special case - checksums file with only the checksum itself
|
||||||
|
if ' ' not in lines[0]:
|
||||||
|
for matcher in CHECKSUM_ONLY_MATCHERS:
|
||||||
|
checksum = re.findall(matcher, lines[0])
|
||||||
|
if checksum:
|
||||||
|
return checksum[0]
|
||||||
|
raise exception.ImageDownloadFailed(
|
||||||
|
image_href=checksum_url,
|
||||||
|
reason=(
|
||||||
|
_("Invalid checksum file (No valid checksum found)")))
|
||||||
|
# FIXME(dtantsur): can we assume the same name for all images?
|
||||||
|
expected_fname = os.path.basename(urlparse.urlparse(
|
||||||
|
image_source).path)
|
||||||
|
for line in lines:
|
||||||
|
# Ignore comment lines
|
||||||
|
if line.startswith("#"):
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Ignore checksums for other files
|
||||||
|
for matcher in FILENAME_MATCHERS:
|
||||||
|
if re.findall(matcher.format(filename=expected_fname), line):
|
||||||
|
break
|
||||||
|
else:
|
||||||
|
continue
|
||||||
|
|
||||||
|
for matcher in CHECKSUM_MATCHERS:
|
||||||
|
checksum = re.findall(matcher, line)
|
||||||
|
if checksum:
|
||||||
|
return checksum[0]
|
||||||
|
|
||||||
|
raise exception.ImageDownloadFailed(
|
||||||
|
image_href=checksum,
|
||||||
|
reason=(_("Checksum file does not contain name %s")
|
||||||
|
% expected_fname))
|
@ -909,3 +909,25 @@ class BootModeNotAllowed(Invalid):
|
|||||||
|
|
||||||
class InvalidImage(ImageUnacceptable):
|
class InvalidImage(ImageUnacceptable):
|
||||||
_msg_fmt = _("The requested image is not valid for use.")
|
_msg_fmt = _("The requested image is not valid for use.")
|
||||||
|
|
||||||
|
|
||||||
|
class ImageChecksumError(InvalidImage):
|
||||||
|
"""Exception indicating checksum failed to match."""
|
||||||
|
_msg_fmt = _("The supplied image checksum is invalid or does not match.")
|
||||||
|
|
||||||
|
|
||||||
|
class ImageChecksumAlgorithmFailure(InvalidImage):
|
||||||
|
"""Cannot load the requested or required checksum algorithm."""
|
||||||
|
_msg_fmt = _("The requested image checksum algorithm cannot be loaded.")
|
||||||
|
|
||||||
|
|
||||||
|
class ImageChecksumURLNotSupported(InvalidImage):
|
||||||
|
"""Exception indicating we cannot support the remote checksum file."""
|
||||||
|
_msg_fmt = _("Use of remote checksum files is not supported.")
|
||||||
|
|
||||||
|
|
||||||
|
class ImageChecksumFileReadFailure(InvalidImage):
|
||||||
|
"""An OSError was raised when trying to read the file."""
|
||||||
|
_msg_fmt = _("Failed to read the file from local storage "
|
||||||
|
"to perform a checksum operation.")
|
||||||
|
code = http_client.SERVICE_UNAVAILABLE
|
||||||
|
@ -287,6 +287,43 @@ class HttpImageService(BaseImageService):
|
|||||||
'no_cache': no_cache,
|
'no_cache': no_cache,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def get(image_href):
|
||||||
|
"""Downloads content and returns the response text.
|
||||||
|
|
||||||
|
:param image_href: Image reference.
|
||||||
|
:raises: exception.ImageRefValidationFailed if GET request returned
|
||||||
|
response code not equal to 200.
|
||||||
|
:raises: exception.ImageDownloadFailed if:
|
||||||
|
* IOError happened during file write;
|
||||||
|
* GET request failed.
|
||||||
|
"""
|
||||||
|
|
||||||
|
try:
|
||||||
|
|
||||||
|
verify = strutils.bool_from_string(CONF.webserver_verify_ca,
|
||||||
|
strict=True)
|
||||||
|
except ValueError:
|
||||||
|
verify = CONF.webserver_verify_ca
|
||||||
|
|
||||||
|
try:
|
||||||
|
auth = HttpImageService.gen_auth_from_conf_user_pass(image_href)
|
||||||
|
response = requests.get(image_href, stream=False, verify=verify,
|
||||||
|
timeout=CONF.webserver_connection_timeout,
|
||||||
|
auth=auth)
|
||||||
|
if response.status_code != http_client.OK:
|
||||||
|
raise exception.ImageRefValidationFailed(
|
||||||
|
image_href=image_href,
|
||||||
|
reason=_("Got HTTP code %s instead of 200 in response "
|
||||||
|
"to GET request.") % response.status_code)
|
||||||
|
|
||||||
|
return response.text
|
||||||
|
|
||||||
|
except (OSError, requests.ConnectionError, requests.RequestException,
|
||||||
|
IOError) as e:
|
||||||
|
raise exception.ImageDownloadFailed(image_href=image_href,
|
||||||
|
reason=str(e))
|
||||||
|
|
||||||
|
|
||||||
class FileImageService(BaseImageService):
|
class FileImageService(BaseImageService):
|
||||||
"""Provides retrieval of disk images available locally on the conductor."""
|
"""Provides retrieval of disk images available locally on the conductor."""
|
||||||
|
@ -28,6 +28,7 @@ from oslo_log import log as logging
|
|||||||
from oslo_utils import fileutils
|
from oslo_utils import fileutils
|
||||||
import pycdlib
|
import pycdlib
|
||||||
|
|
||||||
|
from ironic.common import checksum_utils
|
||||||
from ironic.common import exception
|
from ironic.common import exception
|
||||||
from ironic.common.glance_service import service_utils as glance_utils
|
from ironic.common.glance_service import service_utils as glance_utils
|
||||||
from ironic.common.i18n import _
|
from ironic.common.i18n import _
|
||||||
@ -386,9 +387,13 @@ def fetch_into(context, image_href, image_file):
|
|||||||
{'image_href': image_href, 'time': time.time() - start})
|
{'image_href': image_href, 'time': time.time() - start})
|
||||||
|
|
||||||
|
|
||||||
def fetch(context, image_href, path, force_raw=False):
|
def fetch(context, image_href, path, force_raw=False,
|
||||||
|
checksum=None, checksum_algo=None):
|
||||||
with fileutils.remove_path_on_error(path):
|
with fileutils.remove_path_on_error(path):
|
||||||
fetch_into(context, image_href, path)
|
fetch_into(context, image_href, path)
|
||||||
|
if (not CONF.conductor.disable_file_checksum
|
||||||
|
and checksum):
|
||||||
|
checksum_utils.validate_checksum(path, checksum, checksum_algo)
|
||||||
if force_raw:
|
if force_raw:
|
||||||
image_to_raw(image_href, path, "%s.part" % path)
|
image_to_raw(image_href, path, "%s.part" % path)
|
||||||
|
|
||||||
|
@ -493,6 +493,28 @@ opts = [
|
|||||||
'permitted for deployment with Ironic. If an image '
|
'permitted for deployment with Ironic. If an image '
|
||||||
'format outside of this list is detected, the image '
|
'format outside of this list is detected, the image '
|
||||||
'validation logic will fail the deployment process.')),
|
'validation logic will fail the deployment process.')),
|
||||||
|
cfg.BoolOpt('disable_file_checksum',
|
||||||
|
default=False,
|
||||||
|
mutable=False,
|
||||||
|
help=_('Deprecated Security option: In the default case, '
|
||||||
|
'image files have their checksums verified before '
|
||||||
|
'undergoing additional conductor side actions such '
|
||||||
|
'as image conversion. '
|
||||||
|
'Enabling this option opens the risk of files being '
|
||||||
|
'replaced at the source without the user\'s '
|
||||||
|
'knowledge.'),
|
||||||
|
deprecated_for_removal=True),
|
||||||
|
cfg.BoolOpt('disable_support_for_checksum_files',
|
||||||
|
default=False,
|
||||||
|
mutable=False,
|
||||||
|
help=_('Security option: By default Ironic will attempt to '
|
||||||
|
'retrieve a remote checksum file via HTTP(S) URL in '
|
||||||
|
'order to validate an image download. This is '
|
||||||
|
'functionality aligning with ironic-python-agent '
|
||||||
|
'support for standalone users. Disabling this '
|
||||||
|
'functionality by setting this option to True will '
|
||||||
|
'create a more secure environment, however it may '
|
||||||
|
'break users in an unexpected fashion.')),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
@ -16,7 +16,6 @@
|
|||||||
|
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
import time
|
|
||||||
|
|
||||||
from ironic_lib import metrics_utils
|
from ironic_lib import metrics_utils
|
||||||
from ironic_lib import utils as il_utils
|
from ironic_lib import utils as il_utils
|
||||||
@ -26,6 +25,7 @@ from oslo_utils import fileutils
|
|||||||
from oslo_utils import strutils
|
from oslo_utils import strutils
|
||||||
|
|
||||||
from ironic.common import async_steps
|
from ironic.common import async_steps
|
||||||
|
from ironic.common import checksum_utils
|
||||||
from ironic.common import context
|
from ironic.common import context
|
||||||
from ironic.common import exception
|
from ironic.common import exception
|
||||||
from ironic.common import faults
|
from ironic.common import faults
|
||||||
@ -60,6 +60,7 @@ RESCUE_LIKE_STATES = (states.RESCUING, states.RESCUEWAIT, states.RESCUEFAIL,
|
|||||||
|
|
||||||
DISK_LAYOUT_PARAMS = ('root_gb', 'swap_mb', 'ephemeral_gb')
|
DISK_LAYOUT_PARAMS = ('root_gb', 'swap_mb', 'ephemeral_gb')
|
||||||
|
|
||||||
|
|
||||||
# All functions are called from deploy() directly or indirectly.
|
# All functions are called from deploy() directly or indirectly.
|
||||||
# They are split for stub-out.
|
# They are split for stub-out.
|
||||||
|
|
||||||
@ -206,7 +207,8 @@ def check_for_missing_params(info_dict, error_msg, param_prefix=''):
|
|||||||
|
|
||||||
|
|
||||||
def fetch_images(ctx, cache, images_info, force_raw=True,
|
def fetch_images(ctx, cache, images_info, force_raw=True,
|
||||||
expected_format=None):
|
expected_format=None, expected_checksum=None,
|
||||||
|
expected_checksum_algo=None):
|
||||||
"""Check for available disk space and fetch images using ImageCache.
|
"""Check for available disk space and fetch images using ImageCache.
|
||||||
|
|
||||||
:param ctx: context
|
:param ctx: context
|
||||||
@ -215,6 +217,10 @@ def fetch_images(ctx, cache, images_info, force_raw=True,
|
|||||||
:param force_raw: boolean value, whether to convert the image to raw
|
:param force_raw: boolean value, whether to convert the image to raw
|
||||||
format
|
format
|
||||||
:param expected_format: The expected format of the image.
|
:param expected_format: The expected format of the image.
|
||||||
|
:param expected_checksum: The expected image checksum, to be used if we
|
||||||
|
need to convert the image to raw prior to deploying.
|
||||||
|
:param expected_checksum_algo: The checksum algo in use, if separately
|
||||||
|
set.
|
||||||
:raises: InstanceDeployFailure if unable to find enough disk space
|
:raises: InstanceDeployFailure if unable to find enough disk space
|
||||||
:raises: InvalidImage if the supplied image metadata or contents are
|
:raises: InvalidImage if the supplied image metadata or contents are
|
||||||
deemed to be invalid, unsafe, or not matching the expectations
|
deemed to be invalid, unsafe, or not matching the expectations
|
||||||
@ -233,9 +239,12 @@ def fetch_images(ctx, cache, images_info, force_raw=True,
|
|||||||
image_list = []
|
image_list = []
|
||||||
for href, path in images_info:
|
for href, path in images_info:
|
||||||
# NOTE(TheJulia): Href in this case can be an image UUID or a URL.
|
# NOTE(TheJulia): Href in this case can be an image UUID or a URL.
|
||||||
image_format = cache.fetch_image(href, path, ctx=ctx,
|
image_format = cache.fetch_image(
|
||||||
|
href, path, ctx=ctx,
|
||||||
force_raw=force_raw,
|
force_raw=force_raw,
|
||||||
expected_format=expected_format)
|
expected_format=expected_format,
|
||||||
|
expected_checksum=expected_checksum,
|
||||||
|
expected_checksum_algo=expected_checksum_algo)
|
||||||
image_list.append((href, path, image_format))
|
image_list.append((href, path, image_format))
|
||||||
return image_list
|
return image_list
|
||||||
|
|
||||||
@ -1072,7 +1081,8 @@ class InstanceImageCache(image_cache.ImageCache):
|
|||||||
|
|
||||||
|
|
||||||
@METRICS.timer('cache_instance_image')
|
@METRICS.timer('cache_instance_image')
|
||||||
def cache_instance_image(ctx, node, force_raw=None, expected_format=None):
|
def cache_instance_image(ctx, node, force_raw=None, expected_format=None,
|
||||||
|
expected_checksum=None, expected_checksum_algo=None):
|
||||||
"""Fetch the instance's image from Glance
|
"""Fetch the instance's image from Glance
|
||||||
|
|
||||||
This method pulls the disk image and writes them to the appropriate
|
This method pulls the disk image and writes them to the appropriate
|
||||||
@ -1082,6 +1092,10 @@ def cache_instance_image(ctx, node, force_raw=None, expected_format=None):
|
|||||||
:param node: an ironic node object
|
:param node: an ironic node object
|
||||||
:param force_raw: whether convert image to raw format
|
:param force_raw: whether convert image to raw format
|
||||||
:param expected_format: The expected format of the disk image contents.
|
:param expected_format: The expected format of the disk image contents.
|
||||||
|
:param expected_checksum: The expected image checksum, to be used if we
|
||||||
|
need to convert the image to raw prior to deploying.
|
||||||
|
:param expected_checksum_algo: The checksum algo in use, if separately
|
||||||
|
set.
|
||||||
:returns: a tuple containing the uuid of the image and the path in
|
:returns: a tuple containing the uuid of the image and the path in
|
||||||
the filesystem where image is cached.
|
the filesystem where image is cached.
|
||||||
:raises: InvalidImage if the requested image is invalid and cannot be
|
:raises: InvalidImage if the requested image is invalid and cannot be
|
||||||
@ -1101,7 +1115,9 @@ def cache_instance_image(ctx, node, force_raw=None, expected_format=None):
|
|||||||
{'image': uuid, 'uuid': node.uuid})
|
{'image': uuid, 'uuid': node.uuid})
|
||||||
|
|
||||||
image_list = fetch_images(ctx, InstanceImageCache(), [(uuid, image_path)],
|
image_list = fetch_images(ctx, InstanceImageCache(), [(uuid, image_path)],
|
||||||
force_raw, expected_format=expected_format)
|
force_raw, expected_format=expected_format,
|
||||||
|
expected_checksum=expected_checksum,
|
||||||
|
expected_checksum_algo=expected_checksum_algo)
|
||||||
return (uuid, image_path, image_list[0][2])
|
return (uuid, image_path, image_list[0][2])
|
||||||
|
|
||||||
|
|
||||||
@ -1119,17 +1135,11 @@ def destroy_images(node_uuid):
|
|||||||
@METRICS.timer('compute_image_checksum')
|
@METRICS.timer('compute_image_checksum')
|
||||||
def compute_image_checksum(image_path, algorithm='md5'):
|
def compute_image_checksum(image_path, algorithm='md5'):
|
||||||
"""Compute checksum by given image path and algorithm."""
|
"""Compute checksum by given image path and algorithm."""
|
||||||
time_start = time.time()
|
# NOTE(TheJulia): This likely wouldn't be removed, but if we do
|
||||||
LOG.debug('Start computing %(algo)s checksum for image %(image)s.',
|
# significant refactoring we could likely just change everything
|
||||||
{'algo': algorithm, 'image': image_path})
|
# over to the images common code, if we don't need the metrics
|
||||||
checksum = fileutils.compute_file_checksum(image_path,
|
# data anymore.
|
||||||
algorithm=algorithm)
|
return checksum_utils.compute_image_checksum(image_path, algorithm)
|
||||||
time_elapsed = time.time() - time_start
|
|
||||||
LOG.debug('Computed %(algo)s checksum for image %(image)s in '
|
|
||||||
'%(delta).2f seconds, checksum value: %(checksum)s.',
|
|
||||||
{'algo': algorithm, 'image': image_path, 'delta': time_elapsed,
|
|
||||||
'checksum': checksum})
|
|
||||||
return checksum
|
|
||||||
|
|
||||||
|
|
||||||
def remove_http_instance_symlink(node_uuid):
|
def remove_http_instance_symlink(node_uuid):
|
||||||
@ -1205,6 +1215,11 @@ def _validate_image_url(node, url, secret=False, inspect_image=None,
|
|||||||
# let it run the image validation checking as it's normal course
|
# let it run the image validation checking as it's normal course
|
||||||
# of action, and save what it tells us the image format is.
|
# of action, and save what it tells us the image format is.
|
||||||
# if there *was* a mismatch, it will raise the error.
|
# if there *was* a mismatch, it will raise the error.
|
||||||
|
|
||||||
|
# NOTE(TheJulia): We don't need to supply the checksum here, because
|
||||||
|
# we are not converting the image. The net result is the deploy
|
||||||
|
# interface or remote agent has the responsibility to checksum the
|
||||||
|
# image.
|
||||||
_, image_path, img_format = cache_instance_image(
|
_, image_path, img_format = cache_instance_image(
|
||||||
ctx,
|
ctx,
|
||||||
node,
|
node,
|
||||||
@ -1226,10 +1241,14 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
|
|||||||
initial_format = instance_info.get('image_disk_format')
|
initial_format = instance_info.get('image_disk_format')
|
||||||
else:
|
else:
|
||||||
initial_format = image_info.get('disk_format')
|
initial_format = image_info.get('disk_format')
|
||||||
|
checksum, checksum_algo = checksum_utils.get_checksum_and_algo(
|
||||||
|
instance_info)
|
||||||
_, image_path, img_format = cache_instance_image(
|
_, image_path, img_format = cache_instance_image(
|
||||||
task.context, task.node,
|
task.context, task.node,
|
||||||
force_raw=force_raw,
|
force_raw=force_raw,
|
||||||
expected_format=initial_format)
|
expected_format=initial_format,
|
||||||
|
expected_checksum=checksum,
|
||||||
|
expected_checksum_algo=checksum_algo)
|
||||||
if force_raw or image_info is None:
|
if force_raw or image_info is None:
|
||||||
if force_raw:
|
if force_raw:
|
||||||
instance_info['image_disk_format'] = 'raw'
|
instance_info['image_disk_format'] = 'raw'
|
||||||
@ -1265,7 +1284,8 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
|
|||||||
'%(node)s due to image conversion',
|
'%(node)s due to image conversion',
|
||||||
{'image': image_path, 'node': task.node.uuid})
|
{'image': image_path, 'node': task.node.uuid})
|
||||||
instance_info['image_checksum'] = None
|
instance_info['image_checksum'] = None
|
||||||
hash_value = compute_image_checksum(image_path, os_hash_algo)
|
hash_value = checksum_utils.compute_image_checksum(image_path,
|
||||||
|
os_hash_algo)
|
||||||
else:
|
else:
|
||||||
instance_info['image_checksum'] = old_checksum
|
instance_info['image_checksum'] = old_checksum
|
||||||
|
|
||||||
@ -1363,6 +1383,11 @@ def build_instance_info_for_deploy(task):
|
|||||||
image_info = glance.show(image_source)
|
image_info = glance.show(image_source)
|
||||||
LOG.debug('Got image info: %(info)s for node %(node)s.',
|
LOG.debug('Got image info: %(info)s for node %(node)s.',
|
||||||
{'info': image_info, 'node': node.uuid})
|
{'info': image_info, 'node': node.uuid})
|
||||||
|
# Values are explicitly set into the instance info field
|
||||||
|
# so IPA have the values available.
|
||||||
|
instance_info['image_checksum'] = image_info['checksum']
|
||||||
|
instance_info['image_os_hash_algo'] = image_info['os_hash_algo']
|
||||||
|
instance_info['image_os_hash_value'] = image_info['os_hash_value']
|
||||||
if image_download_source == 'swift':
|
if image_download_source == 'swift':
|
||||||
# In this case, we are getting a file *from* swift for a glance
|
# In this case, we are getting a file *from* swift for a glance
|
||||||
# image which is backed by swift. IPA downloads the file directly
|
# image which is backed by swift. IPA downloads the file directly
|
||||||
@ -1376,14 +1401,9 @@ def build_instance_info_for_deploy(task):
|
|||||||
validate_results = _validate_image_url(
|
validate_results = _validate_image_url(
|
||||||
node, swift_temp_url, secret=True,
|
node, swift_temp_url, secret=True,
|
||||||
expected_format=image_format)
|
expected_format=image_format)
|
||||||
# Values are explicitly set into the instance info field
|
|
||||||
# so IPA have the values available.
|
|
||||||
instance_info['image_url'] = swift_temp_url
|
instance_info['image_url'] = swift_temp_url
|
||||||
instance_info['image_checksum'] = image_info['checksum']
|
|
||||||
instance_info['image_disk_format'] = \
|
instance_info['image_disk_format'] = \
|
||||||
validate_results.get('disk_format', image_format)
|
validate_results.get('disk_format', image_format)
|
||||||
instance_info['image_os_hash_algo'] = image_info['os_hash_algo']
|
|
||||||
instance_info['image_os_hash_value'] = image_info['os_hash_value']
|
|
||||||
else:
|
else:
|
||||||
# In this case, we're directly downloading the glance image and
|
# In this case, we're directly downloading the glance image and
|
||||||
# hosting it locally for retrieval by the IPA.
|
# hosting it locally for retrieval by the IPA.
|
||||||
|
@ -66,7 +66,8 @@ class ImageCache(object):
|
|||||||
fileutils.ensure_tree(master_dir)
|
fileutils.ensure_tree(master_dir)
|
||||||
|
|
||||||
def fetch_image(self, href, dest_path, ctx=None, force_raw=True,
|
def fetch_image(self, href, dest_path, ctx=None, force_raw=True,
|
||||||
expected_format=None):
|
expected_format=None, expected_checksum=None,
|
||||||
|
expected_checksum_algo=None):
|
||||||
"""Fetch image by given href to the destination path.
|
"""Fetch image by given href to the destination path.
|
||||||
|
|
||||||
Does nothing if destination path exists and is up to date with cache
|
Does nothing if destination path exists and is up to date with cache
|
||||||
@ -82,16 +83,32 @@ class ImageCache(object):
|
|||||||
:param force_raw: boolean value, whether to convert the image to raw
|
:param force_raw: boolean value, whether to convert the image to raw
|
||||||
format
|
format
|
||||||
:param expected_format: The expected image format.
|
:param expected_format: The expected image format.
|
||||||
|
:param expected_checksum: The expected image checksum
|
||||||
|
:param expected_checksum_algo: The expected image checksum algorithm,
|
||||||
|
if needed/supplied.
|
||||||
"""
|
"""
|
||||||
img_download_lock_name = 'download-image'
|
img_download_lock_name = 'download-image'
|
||||||
if self.master_dir is None:
|
if self.master_dir is None:
|
||||||
# NOTE(ghe): We don't share images between instances/hosts
|
# NOTE(ghe): We don't share images between instances/hosts
|
||||||
|
# NOTE(TheJulia): These is a weird code path, because master_dir
|
||||||
|
# has to be None, which by default it never should be unless
|
||||||
|
# an operator forces it to None, which is a path we just never
|
||||||
|
# expect.
|
||||||
|
# TODO(TheJulia): This may be dead-ish code and likely needs
|
||||||
|
# to be removed. Likely originated *out* of just the iscsi
|
||||||
|
# deployment interface and local image caching.
|
||||||
if not CONF.parallel_image_downloads:
|
if not CONF.parallel_image_downloads:
|
||||||
with lockutils.lock(img_download_lock_name):
|
with lockutils.lock(img_download_lock_name):
|
||||||
_fetch(ctx, href, dest_path, force_raw)
|
_fetch(ctx, href, dest_path, force_raw,
|
||||||
|
expected_format=expected_format,
|
||||||
|
expected_checksum=expected_checksum,
|
||||||
|
expected_checksum_algo=expected_checksum_algo)
|
||||||
else:
|
else:
|
||||||
with _concurrency_semaphore:
|
with _concurrency_semaphore:
|
||||||
_fetch(ctx, href, dest_path, force_raw)
|
_fetch(ctx, href, dest_path, force_raw,
|
||||||
|
expected_format=expected_format,
|
||||||
|
expected_checksum=expected_checksum,
|
||||||
|
expected_checksum_algo=expected_checksum_algo)
|
||||||
return
|
return
|
||||||
|
|
||||||
# TODO(ghe): have hard links and counts the same behaviour in all fs
|
# TODO(ghe): have hard links and counts the same behaviour in all fs
|
||||||
@ -143,13 +160,17 @@ class ImageCache(object):
|
|||||||
self._download_image(
|
self._download_image(
|
||||||
href, master_path, dest_path, img_info,
|
href, master_path, dest_path, img_info,
|
||||||
ctx=ctx, force_raw=force_raw,
|
ctx=ctx, force_raw=force_raw,
|
||||||
expected_format=expected_format)
|
expected_format=expected_format,
|
||||||
|
expected_checksum=expected_checksum,
|
||||||
|
expected_checksum_algo=expected_checksum_algo)
|
||||||
|
|
||||||
# NOTE(dtantsur): we increased cache size - time to clean up
|
# NOTE(dtantsur): we increased cache size - time to clean up
|
||||||
self.clean_up()
|
self.clean_up()
|
||||||
|
|
||||||
def _download_image(self, href, master_path, dest_path, img_info,
|
def _download_image(self, href, master_path, dest_path, img_info,
|
||||||
ctx=None, force_raw=True, expected_format=None):
|
ctx=None, force_raw=True, expected_format=None,
|
||||||
|
expected_checksum=None,
|
||||||
|
expected_checksum_algo=None):
|
||||||
"""Download image by href and store at a given path.
|
"""Download image by href and store at a given path.
|
||||||
|
|
||||||
This method should be called with uuid-specific lock taken.
|
This method should be called with uuid-specific lock taken.
|
||||||
@ -162,6 +183,8 @@ class ImageCache(object):
|
|||||||
:param force_raw: boolean value, whether to convert the image to raw
|
:param force_raw: boolean value, whether to convert the image to raw
|
||||||
format
|
format
|
||||||
:param expected_format: The expected original format for the image.
|
:param expected_format: The expected original format for the image.
|
||||||
|
:param expected_checksum: The expected image checksum.
|
||||||
|
:param expected_checksum_algo: The expected image checksum algorithm.
|
||||||
:raise ImageDownloadFailed: when the image cache and the image HTTP or
|
:raise ImageDownloadFailed: when the image cache and the image HTTP or
|
||||||
TFTP location are on different file system,
|
TFTP location are on different file system,
|
||||||
causing hard link to fail.
|
causing hard link to fail.
|
||||||
@ -173,7 +196,9 @@ class ImageCache(object):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
with _concurrency_semaphore:
|
with _concurrency_semaphore:
|
||||||
_fetch(ctx, href, tmp_path, force_raw, expected_format)
|
_fetch(ctx, href, tmp_path, force_raw, expected_format,
|
||||||
|
expected_checksum=expected_checksum,
|
||||||
|
expected_checksum_algo=expected_checksum_algo)
|
||||||
|
|
||||||
if img_info.get('no_cache'):
|
if img_info.get('no_cache'):
|
||||||
LOG.debug("Caching is disabled for image %s", href)
|
LOG.debug("Caching is disabled for image %s", href)
|
||||||
@ -337,13 +362,16 @@ def _free_disk_space_for(path):
|
|||||||
return stat.f_frsize * stat.f_bavail
|
return stat.f_frsize * stat.f_bavail
|
||||||
|
|
||||||
|
|
||||||
def _fetch(context, image_href, path, force_raw=False, expected_format=None):
|
def _fetch(context, image_href, path, force_raw=False, expected_format=None,
|
||||||
|
expected_checksum=None, expected_checksum_algo=None):
|
||||||
"""Fetch image and convert to raw format if needed."""
|
"""Fetch image and convert to raw format if needed."""
|
||||||
path_tmp = "%s.part" % path
|
path_tmp = "%s.part" % path
|
||||||
if os.path.exists(path_tmp):
|
if os.path.exists(path_tmp):
|
||||||
LOG.warning("%s exist, assuming it's stale", path_tmp)
|
LOG.warning("%s exist, assuming it's stale", path_tmp)
|
||||||
os.remove(path_tmp)
|
os.remove(path_tmp)
|
||||||
images.fetch(context, image_href, path_tmp, force_raw=False)
|
images.fetch(context, image_href, path_tmp, force_raw=False,
|
||||||
|
checksum=expected_checksum,
|
||||||
|
checksum_algo=expected_checksum_algo)
|
||||||
# By default, the image format is unknown
|
# By default, the image format is unknown
|
||||||
image_format = None
|
image_format = None
|
||||||
disable_dii = CONF.conductor.disable_deep_image_inspection
|
disable_dii = CONF.conductor.disable_deep_image_inspection
|
||||||
|
211
ironic/tests/unit/common/test_checksum_utils.py
Normal file
211
ironic/tests/unit/common/test_checksum_utils.py
Normal file
@ -0,0 +1,211 @@
|
|||||||
|
# coding=utf-8
|
||||||
|
|
||||||
|
# Copyright 2024 Red Hat, Inc.
|
||||||
|
#
|
||||||
|
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||||
|
# not use this file except in compliance with the License. You may obtain
|
||||||
|
# a copy of the License at
|
||||||
|
#
|
||||||
|
# http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
#
|
||||||
|
# Unless required by applicable law or agreed to in writing, software
|
||||||
|
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||||
|
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||||
|
# License for the specific language governing permissions and limitations
|
||||||
|
# under the License.
|
||||||
|
|
||||||
|
from unittest import mock
|
||||||
|
|
||||||
|
from oslo_config import cfg
|
||||||
|
|
||||||
|
from ironic.common import checksum_utils
|
||||||
|
from ironic.common import exception
|
||||||
|
from ironic.common import image_service
|
||||||
|
from ironic.tests import base
|
||||||
|
|
||||||
|
CONF = cfg.CONF
|
||||||
|
|
||||||
|
|
||||||
|
@mock.patch.object(checksum_utils, 'compute_image_checksum',
|
||||||
|
autospec=True)
|
||||||
|
class IronicChecksumUtilsValidateTestCase(base.TestCase):
|
||||||
|
|
||||||
|
def test_validate_checksum(self, mock_compute):
|
||||||
|
mock_compute.return_value = 'f00'
|
||||||
|
checksum_utils.validate_checksum('path', 'f00', 'algo')
|
||||||
|
mock_compute.assert_called_once_with('path', 'algo')
|
||||||
|
|
||||||
|
def test_validate_checksum_mixed_case(self, mock_compute):
|
||||||
|
mock_compute.return_value = 'f00'
|
||||||
|
checksum_utils.validate_checksum('path', 'F00', 'ALGO')
|
||||||
|
mock_compute.assert_called_once_with('path', 'algo')
|
||||||
|
|
||||||
|
def test_validate_checksum_mixed_md5(self, mock_compute):
|
||||||
|
mock_compute.return_value = 'f00'
|
||||||
|
checksum_utils.validate_checksum('path', 'F00')
|
||||||
|
mock_compute.assert_called_once_with('path')
|
||||||
|
|
||||||
|
def test_validate_checksum_mismatch(self, mock_compute):
|
||||||
|
mock_compute.return_value = 'a00'
|
||||||
|
self.assertRaises(exception.ImageChecksumError,
|
||||||
|
checksum_utils.validate_checksum,
|
||||||
|
'path', 'f00', 'algo')
|
||||||
|
mock_compute.assert_called_once_with('path', 'algo')
|
||||||
|
|
||||||
|
def test_validate_checksum_hashlib_not_supports_algo(self, mock_compute):
|
||||||
|
mock_compute.side_effect = ValueError()
|
||||||
|
self.assertRaises(exception.ImageChecksumAlgorithmFailure,
|
||||||
|
checksum_utils.validate_checksum,
|
||||||
|
'path', 'f00', 'algo')
|
||||||
|
mock_compute.assert_called_once_with('path', 'algo')
|
||||||
|
|
||||||
|
def test_validate_checksum_file_not_found(self, mock_compute):
|
||||||
|
mock_compute.side_effect = OSError()
|
||||||
|
self.assertRaises(exception.ImageChecksumFileReadFailure,
|
||||||
|
checksum_utils.validate_checksum,
|
||||||
|
'path', 'f00', 'algo')
|
||||||
|
mock_compute.assert_called_once_with('path', 'algo')
|
||||||
|
|
||||||
|
def test_validate_checksum_mixed_case_delimited(self, mock_compute):
|
||||||
|
mock_compute.return_value = 'f00'
|
||||||
|
checksum_utils.validate_checksum('path', 'algo:F00')
|
||||||
|
mock_compute.assert_called_once_with('path', 'algo')
|
||||||
|
|
||||||
|
|
||||||
|
class IronicChecksumUtilsTestCase(base.TestCase):
|
||||||
|
|
||||||
|
def test_is_checksum_url_string(self):
|
||||||
|
self.assertFalse(checksum_utils.is_checksum_url('f00'))
|
||||||
|
|
||||||
|
def test_is_checksum_url_file(self):
|
||||||
|
self.assertFalse(checksum_utils.is_checksum_url('file://foo'))
|
||||||
|
|
||||||
|
def test_is_checksum_url(self):
|
||||||
|
urls = ['http://foo.local/file',
|
||||||
|
'https://foo.local/file']
|
||||||
|
for url in urls:
|
||||||
|
self.assertTrue(checksum_utils.is_checksum_url(url))
|
||||||
|
|
||||||
|
def test_get_checksum_and_algo_image_checksum(self):
|
||||||
|
value = 'c46f2c98efe1cd246be1796cd842246e'
|
||||||
|
i_info = {'image_checksum': value}
|
||||||
|
csum, algo = checksum_utils.get_checksum_and_algo(i_info)
|
||||||
|
self.assertEqual(value, csum)
|
||||||
|
self.assertIsNone(algo)
|
||||||
|
|
||||||
|
def test_get_checksum_and_algo_image_checksum_not_allowed(self):
|
||||||
|
CONF.set_override('allow_md5_checksum', False, group='agent')
|
||||||
|
value = 'c46f2c98efe1cd246be1796cd842246e'
|
||||||
|
i_info = {'image_checksum': value}
|
||||||
|
self.assertRaises(exception.ImageChecksumAlgorithmFailure,
|
||||||
|
checksum_utils.get_checksum_and_algo,
|
||||||
|
i_info)
|
||||||
|
|
||||||
|
def test_get_checksum_and_algo_image_checksum_glance(self):
|
||||||
|
value = 'c46f2c98efe1cd246be1796cd842246e'
|
||||||
|
i_info = {'image_os_hash_value': value,
|
||||||
|
'image_os_hash_algo': 'foobar'}
|
||||||
|
csum, algo = checksum_utils.get_checksum_and_algo(i_info)
|
||||||
|
self.assertEqual(value, csum)
|
||||||
|
self.assertEqual('foobar', algo)
|
||||||
|
|
||||||
|
def test_get_checksum_and_algo_image_checksum_sha256(self):
|
||||||
|
value = 'a' * 64
|
||||||
|
i_info = {'image_checksum': value}
|
||||||
|
csum, algo = checksum_utils.get_checksum_and_algo(i_info)
|
||||||
|
self.assertEqual(value, csum)
|
||||||
|
self.assertEqual('sha256', algo)
|
||||||
|
|
||||||
|
def test_get_checksum_and_algo_image_checksum_sha512(self):
|
||||||
|
value = 'f' * 128
|
||||||
|
i_info = {'image_checksum': value}
|
||||||
|
csum, algo = checksum_utils.get_checksum_and_algo(i_info)
|
||||||
|
self.assertEqual(value, csum)
|
||||||
|
self.assertEqual('sha512', algo)
|
||||||
|
|
||||||
|
@mock.patch.object(checksum_utils, 'get_checksum_from_url', autospec=True)
|
||||||
|
def test_get_checksum_and_algo_image_checksum_http_url(self, mock_get):
|
||||||
|
value = 'http://checksum-url'
|
||||||
|
i_info = {
|
||||||
|
'image_checksum': value,
|
||||||
|
'image_source': 'image-ref'
|
||||||
|
}
|
||||||
|
mock_get.return_value = 'f' * 64
|
||||||
|
csum, algo = checksum_utils.get_checksum_and_algo(i_info)
|
||||||
|
mock_get.assert_called_once_with(value, 'image-ref')
|
||||||
|
self.assertEqual('f' * 64, csum)
|
||||||
|
self.assertEqual('sha256', algo)
|
||||||
|
|
||||||
|
@mock.patch.object(checksum_utils, 'get_checksum_from_url', autospec=True)
|
||||||
|
def test_get_checksum_and_algo_image_checksum_https_url(self, mock_get):
|
||||||
|
value = 'https://checksum-url'
|
||||||
|
i_info = {
|
||||||
|
'image_checksum': value,
|
||||||
|
'image_source': 'image-ref'
|
||||||
|
}
|
||||||
|
mock_get.return_value = 'f' * 128
|
||||||
|
csum, algo = checksum_utils.get_checksum_and_algo(i_info)
|
||||||
|
mock_get.assert_called_once_with(value, 'image-ref')
|
||||||
|
self.assertEqual('f' * 128, csum)
|
||||||
|
self.assertEqual('sha512', algo)
|
||||||
|
|
||||||
|
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'get',
|
||||||
|
autospec=True)
|
||||||
|
class IronicChecksumUtilsGetChecksumTestCase(base.TestCase):
|
||||||
|
|
||||||
|
def test_get_checksum_from_url_empty_response(self, mock_get):
|
||||||
|
mock_get.return_value = ''
|
||||||
|
error = ('Failed to download image https://checksum-url, '
|
||||||
|
'reason: Checksum file empty.')
|
||||||
|
self.assertRaisesRegex(exception.ImageDownloadFailed,
|
||||||
|
error,
|
||||||
|
checksum_utils.get_checksum_from_url,
|
||||||
|
'https://checksum-url',
|
||||||
|
'https://image-url/file')
|
||||||
|
mock_get.assert_called_once_with('https://checksum-url')
|
||||||
|
|
||||||
|
def test_get_checksum_from_url_one_line(self, mock_get):
|
||||||
|
mock_get.return_value = 'a' * 32
|
||||||
|
csum = checksum_utils.get_checksum_from_url(
|
||||||
|
'https://checksum-url', 'https://image-url/file')
|
||||||
|
mock_get.assert_called_once_with('https://checksum-url')
|
||||||
|
self.assertEqual('a' * 32, csum)
|
||||||
|
|
||||||
|
def test_get_checksum_from_url_nomatch_line(self, mock_get):
|
||||||
|
mock_get.return_value = 'foobar'
|
||||||
|
# For some reason assertRaisesRegex really doesn't like
|
||||||
|
# the error. Easiest path is just to assertTrue the compare.
|
||||||
|
exc = self.assertRaises(exception.ImageDownloadFailed,
|
||||||
|
checksum_utils.get_checksum_from_url,
|
||||||
|
'https://checksum-url',
|
||||||
|
'https://image-url/file')
|
||||||
|
self.assertTrue(
|
||||||
|
'Invalid checksum file (No valid checksum found' in str(exc))
|
||||||
|
mock_get.assert_called_once_with('https://checksum-url')
|
||||||
|
|
||||||
|
def test_get_checksum_from_url_multiline(self, mock_get):
|
||||||
|
test_csum = ('f2ca1bb6c7e907d06dafe4687e579fce76b37e4e9'
|
||||||
|
'3b7605022da52e6ccc26fd2')
|
||||||
|
mock_get.return_value = ('fee f00\n%s file\nbar fee\nf00' % test_csum)
|
||||||
|
# For some reason assertRaisesRegex really doesn't like
|
||||||
|
# the error. Easiest path is just to assertTrue the compare.
|
||||||
|
checksum = checksum_utils.get_checksum_from_url(
|
||||||
|
'https://checksum-url',
|
||||||
|
'https://image-url/file')
|
||||||
|
self.assertEqual(test_csum, checksum)
|
||||||
|
mock_get.assert_called_once_with('https://checksum-url')
|
||||||
|
|
||||||
|
def test_get_checksum_from_url_multiline_no_file(self, mock_get):
|
||||||
|
test_csum = 'a' * 64
|
||||||
|
error = ("Failed to download image https://checksum-url, reason: "
|
||||||
|
"Checksum file does not contain name file")
|
||||||
|
mock_get.return_value = ('f00\n%s\nbar\nf00' % test_csum)
|
||||||
|
# For some reason assertRaisesRegex really doesn't like
|
||||||
|
# the error. Easiest path is just to assertTrue the compare.
|
||||||
|
self.assertRaisesRegex(exception.ImageDownloadFailed,
|
||||||
|
error,
|
||||||
|
checksum_utils.get_checksum_from_url,
|
||||||
|
'https://checksum-url',
|
||||||
|
'https://image-url/file')
|
||||||
|
mock_get.assert_called_once_with('https://checksum-url')
|
@ -574,6 +574,40 @@ class HttpImageServiceTestCase(base.TestCase):
|
|||||||
verify=True,
|
verify=True,
|
||||||
timeout=15, auth=None)
|
timeout=15, auth=None)
|
||||||
|
|
||||||
|
@mock.patch.object(requests, 'get', autospec=True)
|
||||||
|
def test_get_success(self, req_get_mock):
|
||||||
|
response_mock = req_get_mock.return_value
|
||||||
|
response_mock.status_code = http_client.OK
|
||||||
|
response_mock.text = 'value'
|
||||||
|
self.assertEqual('value', self.service.get('http://url'))
|
||||||
|
req_get_mock.assert_called_once_with('http://url', stream=False,
|
||||||
|
verify=True, timeout=60,
|
||||||
|
auth=None)
|
||||||
|
|
||||||
|
@mock.patch.object(requests, 'get', autospec=True)
|
||||||
|
def test_get_handles_exceptions(self, req_get_mock):
|
||||||
|
for exc in [OSError, requests.ConnectionError,
|
||||||
|
requests.RequestException, IOError]:
|
||||||
|
req_get_mock.reset_mock()
|
||||||
|
req_get_mock.side_effect = exc
|
||||||
|
self.assertRaises(exception.ImageDownloadFailed,
|
||||||
|
self.service.get,
|
||||||
|
'http://url')
|
||||||
|
req_get_mock.assert_called_once_with('http://url', stream=False,
|
||||||
|
verify=True, timeout=60,
|
||||||
|
auth=None)
|
||||||
|
|
||||||
|
@mock.patch.object(requests, 'get', autospec=True)
|
||||||
|
def test_get_success_verify_false(self, req_get_mock):
|
||||||
|
cfg.CONF.set_override('webserver_verify_ca', False)
|
||||||
|
response_mock = req_get_mock.return_value
|
||||||
|
response_mock.status_code = http_client.OK
|
||||||
|
response_mock.text = 'value'
|
||||||
|
self.assertEqual('value', self.service.get('http://url'))
|
||||||
|
req_get_mock.assert_called_once_with('http://url', stream=False,
|
||||||
|
verify=False, timeout=60,
|
||||||
|
auth=None)
|
||||||
|
|
||||||
|
|
||||||
class FileImageServiceTestCase(base.TestCase):
|
class FileImageServiceTestCase(base.TestCase):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
|
@ -23,6 +23,7 @@ from unittest import mock
|
|||||||
|
|
||||||
from oslo_concurrency import processutils
|
from oslo_concurrency import processutils
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
|
from oslo_utils import fileutils
|
||||||
|
|
||||||
from ironic.common import exception
|
from ironic.common import exception
|
||||||
from ironic.common.glance_service import service_utils as glance_utils
|
from ironic.common.glance_service import service_utils as glance_utils
|
||||||
@ -73,6 +74,100 @@ class IronicImagesTestCase(base.TestCase):
|
|||||||
image_to_raw_mock.assert_called_once_with(
|
image_to_raw_mock.assert_called_once_with(
|
||||||
'image_href', 'path', 'path.part')
|
'image_href', 'path', 'path.part')
|
||||||
|
|
||||||
|
@mock.patch.object(fileutils, 'compute_file_checksum',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service, 'get_image_service', autospec=True)
|
||||||
|
@mock.patch.object(images, 'image_to_raw', autospec=True)
|
||||||
|
@mock.patch.object(builtins, 'open', autospec=True)
|
||||||
|
def test_fetch_image_service_force_raw_with_checksum(
|
||||||
|
self, open_mock, image_to_raw_mock,
|
||||||
|
image_service_mock, mock_checksum):
|
||||||
|
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
|
||||||
|
mock_file_handle.__enter__.return_value = 'file'
|
||||||
|
open_mock.return_value = mock_file_handle
|
||||||
|
mock_checksum.return_value = 'f00'
|
||||||
|
|
||||||
|
images.fetch('context', 'image_href', 'path', force_raw=True,
|
||||||
|
checksum='f00', checksum_algo='sha256')
|
||||||
|
|
||||||
|
mock_checksum.assert_called_once_with('path', algorithm='sha256')
|
||||||
|
open_mock.assert_called_once_with('path', 'wb')
|
||||||
|
image_service_mock.return_value.download.assert_called_once_with(
|
||||||
|
'image_href', 'file')
|
||||||
|
image_to_raw_mock.assert_called_once_with(
|
||||||
|
'image_href', 'path', 'path.part')
|
||||||
|
|
||||||
|
@mock.patch.object(fileutils, 'compute_file_checksum',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service, 'get_image_service', autospec=True)
|
||||||
|
@mock.patch.object(images, 'image_to_raw', autospec=True)
|
||||||
|
@mock.patch.object(builtins, 'open', autospec=True)
|
||||||
|
def test_fetch_image_service_with_checksum_mismatch(
|
||||||
|
self, open_mock, image_to_raw_mock,
|
||||||
|
image_service_mock, mock_checksum):
|
||||||
|
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
|
||||||
|
mock_file_handle.__enter__.return_value = 'file'
|
||||||
|
open_mock.return_value = mock_file_handle
|
||||||
|
mock_checksum.return_value = 'a00'
|
||||||
|
|
||||||
|
self.assertRaises(exception.ImageChecksumError,
|
||||||
|
images.fetch, 'context', 'image_href',
|
||||||
|
'path', force_raw=True,
|
||||||
|
checksum='f00', checksum_algo='sha256')
|
||||||
|
|
||||||
|
mock_checksum.assert_called_once_with('path', algorithm='sha256')
|
||||||
|
open_mock.assert_called_once_with('path', 'wb')
|
||||||
|
image_service_mock.return_value.download.assert_called_once_with(
|
||||||
|
'image_href', 'file')
|
||||||
|
# If the checksum fails, then we don't attempt to convert the image.
|
||||||
|
image_to_raw_mock.assert_not_called()
|
||||||
|
|
||||||
|
@mock.patch.object(fileutils, 'compute_file_checksum',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service, 'get_image_service', autospec=True)
|
||||||
|
@mock.patch.object(images, 'image_to_raw', autospec=True)
|
||||||
|
@mock.patch.object(builtins, 'open', autospec=True)
|
||||||
|
def test_fetch_image_service_force_raw_no_checksum_algo(
|
||||||
|
self, open_mock, image_to_raw_mock,
|
||||||
|
image_service_mock, mock_checksum):
|
||||||
|
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
|
||||||
|
mock_file_handle.__enter__.return_value = 'file'
|
||||||
|
open_mock.return_value = mock_file_handle
|
||||||
|
mock_checksum.return_value = 'f00'
|
||||||
|
|
||||||
|
images.fetch('context', 'image_href', 'path', force_raw=True,
|
||||||
|
checksum='f00')
|
||||||
|
|
||||||
|
mock_checksum.assert_called_once_with('path', algorithm='md5')
|
||||||
|
open_mock.assert_called_once_with('path', 'wb')
|
||||||
|
image_service_mock.return_value.download.assert_called_once_with(
|
||||||
|
'image_href', 'file')
|
||||||
|
image_to_raw_mock.assert_called_once_with(
|
||||||
|
'image_href', 'path', 'path.part')
|
||||||
|
|
||||||
|
@mock.patch.object(fileutils, 'compute_file_checksum',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service, 'get_image_service', autospec=True)
|
||||||
|
@mock.patch.object(images, 'image_to_raw', autospec=True)
|
||||||
|
@mock.patch.object(builtins, 'open', autospec=True)
|
||||||
|
def test_fetch_image_service_force_raw_combined_algo(
|
||||||
|
self, open_mock, image_to_raw_mock,
|
||||||
|
image_service_mock, mock_checksum):
|
||||||
|
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
|
||||||
|
mock_file_handle.__enter__.return_value = 'file'
|
||||||
|
open_mock.return_value = mock_file_handle
|
||||||
|
mock_checksum.return_value = 'f00'
|
||||||
|
|
||||||
|
images.fetch('context', 'image_href', 'path', force_raw=True,
|
||||||
|
checksum='sha512:f00')
|
||||||
|
|
||||||
|
mock_checksum.assert_called_once_with('path', algorithm='sha512')
|
||||||
|
open_mock.assert_called_once_with('path', 'wb')
|
||||||
|
image_service_mock.return_value.download.assert_called_once_with(
|
||||||
|
'image_href', 'file')
|
||||||
|
image_to_raw_mock.assert_called_once_with(
|
||||||
|
'image_href', 'path', 'path.part')
|
||||||
|
|
||||||
@mock.patch.object(image_format_inspector, 'detect_file_format',
|
@mock.patch.object(image_format_inspector, 'detect_file_format',
|
||||||
autospec=True)
|
autospec=True)
|
||||||
def test_image_to_raw_not_permitted_format(self, detect_format_mock):
|
def test_image_to_raw_not_permitted_format(self, detect_format_mock):
|
||||||
|
@ -24,6 +24,7 @@ from oslo_utils import fileutils
|
|||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
|
||||||
from ironic.common import boot_devices
|
from ironic.common import boot_devices
|
||||||
|
from ironic.common import checksum_utils
|
||||||
from ironic.common import exception
|
from ironic.common import exception
|
||||||
from ironic.common import faults
|
from ironic.common import faults
|
||||||
from ironic.common import image_service
|
from ironic.common import image_service
|
||||||
@ -604,10 +605,33 @@ class OtherFunctionTestCase(db_base.DbTestCase):
|
|||||||
utils.fetch_images(None, mock_cache, [('uuid', 'path')])
|
utils.fetch_images(None, mock_cache, [('uuid', 'path')])
|
||||||
mock_clean_up_caches.assert_called_once_with(None, 'master_dir',
|
mock_clean_up_caches.assert_called_once_with(None, 'master_dir',
|
||||||
[('uuid', 'path')])
|
[('uuid', 'path')])
|
||||||
mock_cache.fetch_image.assert_called_once_with('uuid', 'path',
|
mock_cache.fetch_image.assert_called_once_with(
|
||||||
|
'uuid', 'path',
|
||||||
ctx=None,
|
ctx=None,
|
||||||
force_raw=True,
|
force_raw=True,
|
||||||
expected_format=None)
|
expected_format=None,
|
||||||
|
expected_checksum=None,
|
||||||
|
expected_checksum_algo=None)
|
||||||
|
|
||||||
|
@mock.patch.object(image_cache, 'clean_up_caches', autospec=True)
|
||||||
|
def test_fetch_images_checksum(self, mock_clean_up_caches):
|
||||||
|
|
||||||
|
mock_cache = mock.MagicMock(
|
||||||
|
spec_set=['fetch_image', 'master_dir'], master_dir='master_dir')
|
||||||
|
utils.fetch_images(None, mock_cache, [('uuid', 'path')],
|
||||||
|
force_raw=True,
|
||||||
|
expected_format='qcow2',
|
||||||
|
expected_checksum='f00',
|
||||||
|
expected_checksum_algo='sha256')
|
||||||
|
mock_clean_up_caches.assert_called_once_with(None, 'master_dir',
|
||||||
|
[('uuid', 'path')])
|
||||||
|
mock_cache.fetch_image.assert_called_once_with(
|
||||||
|
'uuid', 'path',
|
||||||
|
ctx=None,
|
||||||
|
force_raw=True,
|
||||||
|
expected_format='qcow2',
|
||||||
|
expected_checksum='f00',
|
||||||
|
expected_checksum_algo='sha256')
|
||||||
|
|
||||||
@mock.patch.object(image_cache, 'clean_up_caches', autospec=True)
|
@mock.patch.object(image_cache, 'clean_up_caches', autospec=True)
|
||||||
def test_fetch_images_fail(self, mock_clean_up_caches):
|
def test_fetch_images_fail(self, mock_clean_up_caches):
|
||||||
@ -2464,7 +2488,9 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
@mock.patch.object(image_service, 'GlanceImageService', autospec=True)
|
@mock.patch.object(image_service, 'GlanceImageService', autospec=True)
|
||||||
def _test_build_instance_info(self, glance_mock, validate_mock,
|
def _test_build_instance_info(self, glance_mock, validate_mock,
|
||||||
image_info={}, expect_raw=False,
|
image_info={}, expect_raw=False,
|
||||||
expect_format='qcow2'):
|
expect_format='qcow2',
|
||||||
|
expect_checksum='fake-sha512',
|
||||||
|
expect_checksum_algo='sha512'):
|
||||||
glance_mock.return_value.show = mock.MagicMock(spec_set=[],
|
glance_mock.return_value.show = mock.MagicMock(spec_set=[],
|
||||||
return_value=image_info)
|
return_value=image_info)
|
||||||
with task_manager.acquire(
|
with task_manager.acquire(
|
||||||
@ -2477,7 +2503,9 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
task.context,
|
task.context,
|
||||||
task.node,
|
task.node,
|
||||||
force_raw=expect_raw,
|
force_raw=expect_raw,
|
||||||
expected_format=expect_format)
|
expected_format=expect_format,
|
||||||
|
expected_checksum=expect_checksum,
|
||||||
|
expected_checksum_algo=expect_checksum_algo)
|
||||||
symlink_dir = utils._get_http_image_symlink_dir_path()
|
symlink_dir = utils._get_http_image_symlink_dir_path()
|
||||||
symlink_file = utils._get_http_image_symlink_file_path(
|
symlink_file = utils._get_http_image_symlink_file_path(
|
||||||
self.node.uuid)
|
self.node.uuid)
|
||||||
@ -2531,7 +2559,8 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
cfg.CONF.set_override('force_raw_images', True)
|
cfg.CONF.set_override('force_raw_images', True)
|
||||||
self.image_info['os_hash_algo'] = 'md5'
|
self.image_info['os_hash_algo'] = 'md5'
|
||||||
image_path, instance_info = self._test_build_instance_info(
|
image_path, instance_info = self._test_build_instance_info(
|
||||||
image_info=self.image_info, expect_raw=True)
|
image_info=self.image_info, expect_raw=True,
|
||||||
|
expect_checksum_algo='md5')
|
||||||
|
|
||||||
self.assertIsNone(instance_info['image_checksum'])
|
self.assertIsNone(instance_info['image_checksum'])
|
||||||
self.assertEqual(instance_info['image_disk_format'], 'raw')
|
self.assertEqual(instance_info['image_disk_format'], 'raw')
|
||||||
@ -2543,7 +2572,8 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.image_info['os_hash_algo'] = 'md5'
|
self.image_info['os_hash_algo'] = 'md5'
|
||||||
self.image_info['disk_format'] = 'raw'
|
self.image_info['disk_format'] = 'raw'
|
||||||
image_path, instance_info = self._test_build_instance_info(
|
image_path, instance_info = self._test_build_instance_info(
|
||||||
image_info=self.image_info, expect_raw=True, expect_format='raw')
|
image_info=self.image_info, expect_raw=True, expect_format='raw',
|
||||||
|
expect_checksum_algo='md5')
|
||||||
|
|
||||||
self.assertEqual(instance_info['image_checksum'], 'aa')
|
self.assertEqual(instance_info['image_checksum'], 'aa')
|
||||||
self.assertEqual(instance_info['image_disk_format'], 'raw')
|
self.assertEqual(instance_info['image_disk_format'], 'raw')
|
||||||
@ -2576,7 +2606,9 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.assertEqual('raw', info['image_disk_format'])
|
self.assertEqual('raw', info['image_disk_format'])
|
||||||
self.cache_image_mock.assert_called_once_with(
|
self.cache_image_mock.assert_called_once_with(
|
||||||
task.context, task.node, force_raw=True,
|
task.context, task.node, force_raw=True,
|
||||||
expected_format=None)
|
expected_format=None,
|
||||||
|
expected_checksum='aa',
|
||||||
|
expected_checksum_algo=None)
|
||||||
self.checksum_mock.assert_called_once_with(
|
self.checksum_mock.assert_called_once_with(
|
||||||
self.fake_path, algorithm='sha256')
|
self.fake_path, algorithm='sha256')
|
||||||
validate_href_mock.assert_called_once_with(
|
validate_href_mock.assert_called_once_with(
|
||||||
@ -2609,7 +2641,9 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.assertEqual('fake-checksum', info['image_os_hash_value'])
|
self.assertEqual('fake-checksum', info['image_os_hash_value'])
|
||||||
self.cache_image_mock.assert_called_once_with(
|
self.cache_image_mock.assert_called_once_with(
|
||||||
task.context, task.node, force_raw=True,
|
task.context, task.node, force_raw=True,
|
||||||
expected_format=None)
|
expected_format=None,
|
||||||
|
expected_checksum='aa',
|
||||||
|
expected_checksum_algo=None)
|
||||||
self.checksum_mock.assert_called_once_with(
|
self.checksum_mock.assert_called_once_with(
|
||||||
self.fake_path, algorithm='sha256')
|
self.fake_path, algorithm='sha256')
|
||||||
validate_href_mock.assert_called_once_with(
|
validate_href_mock.assert_called_once_with(
|
||||||
@ -2645,7 +2679,9 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.assertEqual('fake-checksum', info['image_os_hash_value'])
|
self.assertEqual('fake-checksum', info['image_os_hash_value'])
|
||||||
self.cache_image_mock.assert_called_once_with(
|
self.cache_image_mock.assert_called_once_with(
|
||||||
task.context, task.node, force_raw=True,
|
task.context, task.node, force_raw=True,
|
||||||
expected_format='qcow2')
|
expected_format='qcow2',
|
||||||
|
expected_checksum='aa',
|
||||||
|
expected_checksum_algo=None)
|
||||||
self.checksum_mock.assert_called_once_with(
|
self.checksum_mock.assert_called_once_with(
|
||||||
self.fake_path, algorithm='sha256')
|
self.fake_path, algorithm='sha256')
|
||||||
validate_href_mock.assert_called_once_with(
|
validate_href_mock.assert_called_once_with(
|
||||||
@ -2726,7 +2762,9 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.assertEqual('fake-checksum', info['image_os_hash_value'])
|
self.assertEqual('fake-checksum', info['image_os_hash_value'])
|
||||||
self.cache_image_mock.assert_called_once_with(
|
self.cache_image_mock.assert_called_once_with(
|
||||||
task.context, task.node, force_raw=True,
|
task.context, task.node, force_raw=True,
|
||||||
expected_format=None)
|
expected_format=None,
|
||||||
|
expected_checksum='aa',
|
||||||
|
expected_checksum_algo=None)
|
||||||
self.checksum_mock.assert_called_once_with(
|
self.checksum_mock.assert_called_once_with(
|
||||||
self.fake_path, algorithm='sha256')
|
self.fake_path, algorithm='sha256')
|
||||||
validate_href_mock.assert_called_once_with(
|
validate_href_mock.assert_called_once_with(
|
||||||
@ -2755,7 +2793,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.context, self.node.uuid, shared=False) as task:
|
self.context, self.node.uuid, shared=False) as task:
|
||||||
|
|
||||||
info = utils.build_instance_info_for_deploy(task)
|
info = utils.build_instance_info_for_deploy(task)
|
||||||
|
|
||||||
self.assertEqual(expected_url, info['image_url'])
|
self.assertEqual(expected_url, info['image_url'])
|
||||||
self.assertEqual('aa', info['image_checksum'])
|
self.assertEqual('aa', info['image_checksum'])
|
||||||
self.assertEqual('raw', info['image_disk_format'])
|
self.assertEqual('raw', info['image_disk_format'])
|
||||||
@ -2763,11 +2800,175 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
|
|||||||
self.assertIsNone(info['image_os_hash_value'])
|
self.assertIsNone(info['image_os_hash_value'])
|
||||||
self.cache_image_mock.assert_called_once_with(
|
self.cache_image_mock.assert_called_once_with(
|
||||||
task.context, task.node, force_raw=True,
|
task.context, task.node, force_raw=True,
|
||||||
expected_format='raw')
|
expected_format='raw', expected_checksum='aa',
|
||||||
|
expected_checksum_algo=None)
|
||||||
self.checksum_mock.assert_not_called()
|
self.checksum_mock.assert_not_called()
|
||||||
validate_href_mock.assert_called_once_with(
|
validate_href_mock.assert_called_once_with(
|
||||||
mock.ANY, expected_url, False)
|
mock.ANY, expected_url, False)
|
||||||
|
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'get',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'validate_href',
|
||||||
|
autospec=True)
|
||||||
|
def test_build_instance_info_remote_checksum_image(self,
|
||||||
|
validate_href_mock,
|
||||||
|
get_mock):
|
||||||
|
# Test case where we would download both the image and the checksum
|
||||||
|
# and ultimately convert the image.
|
||||||
|
get_mock.return_value = 'd8e8fca2dc0f896fd7cb4cb0031ba249'
|
||||||
|
cfg.CONF.set_override('image_download_source', 'local', group='agent')
|
||||||
|
i_info = self.node.instance_info
|
||||||
|
driver_internal_info = self.node.driver_internal_info
|
||||||
|
i_info['image_source'] = 'http://image-ref'
|
||||||
|
i_info['image_checksum'] = 'http://image-checksum'
|
||||||
|
i_info['root_gb'] = 10
|
||||||
|
i_info['image_disk_format'] = 'qcow2'
|
||||||
|
driver_internal_info['is_whole_disk_image'] = True
|
||||||
|
self.node.instance_info = i_info
|
||||||
|
self.node.driver_internal_info = driver_internal_info
|
||||||
|
self.node.save()
|
||||||
|
|
||||||
|
expected_url = (
|
||||||
|
'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid)
|
||||||
|
|
||||||
|
with task_manager.acquire(
|
||||||
|
self.context, self.node.uuid, shared=False) as task:
|
||||||
|
|
||||||
|
info = utils.build_instance_info_for_deploy(task)
|
||||||
|
self.assertEqual(expected_url, info['image_url'])
|
||||||
|
self.assertEqual('raw', info['image_disk_format'])
|
||||||
|
self.cache_image_mock.assert_called_once_with(
|
||||||
|
task.context, task.node, force_raw=True,
|
||||||
|
expected_format='qcow2',
|
||||||
|
expected_checksum='d8e8fca2dc0f896fd7cb4cb0031ba249',
|
||||||
|
expected_checksum_algo=None)
|
||||||
|
self.checksum_mock.assert_called_once_with(
|
||||||
|
self.fake_path, algorithm='sha256')
|
||||||
|
validate_href_mock.assert_called_once_with(
|
||||||
|
mock.ANY, expected_url, False)
|
||||||
|
get_mock.assert_called_once_with('http://image-checksum')
|
||||||
|
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'get',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'validate_href',
|
||||||
|
autospec=True)
|
||||||
|
def test_build_instance_info_remote_checksum_sha256(self,
|
||||||
|
validate_href_mock,
|
||||||
|
get_mock):
|
||||||
|
# Test case where we would download both the image and the checksum
|
||||||
|
# and ultimately convert the image.
|
||||||
|
get_mock.return_value = 'a' * 64 + '\n'
|
||||||
|
cfg.CONF.set_override('image_download_source', 'local', group='agent')
|
||||||
|
i_info = self.node.instance_info
|
||||||
|
driver_internal_info = self.node.driver_internal_info
|
||||||
|
i_info['image_source'] = 'https://image-ref'
|
||||||
|
i_info['image_checksum'] = 'https://image-checksum'
|
||||||
|
i_info['root_gb'] = 10
|
||||||
|
i_info['image_disk_format'] = 'qcow2'
|
||||||
|
driver_internal_info['is_whole_disk_image'] = True
|
||||||
|
self.node.instance_info = i_info
|
||||||
|
self.node.driver_internal_info = driver_internal_info
|
||||||
|
self.node.save()
|
||||||
|
|
||||||
|
expected_url = (
|
||||||
|
'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid)
|
||||||
|
|
||||||
|
with task_manager.acquire(
|
||||||
|
self.context, self.node.uuid, shared=False) as task:
|
||||||
|
|
||||||
|
info = utils.build_instance_info_for_deploy(task)
|
||||||
|
self.assertEqual(expected_url, info['image_url'])
|
||||||
|
self.assertEqual('raw', info['image_disk_format'])
|
||||||
|
self.cache_image_mock.assert_called_once_with(
|
||||||
|
task.context, task.node, force_raw=True,
|
||||||
|
expected_format='qcow2',
|
||||||
|
expected_checksum='a' * 64,
|
||||||
|
expected_checksum_algo='sha256')
|
||||||
|
self.checksum_mock.assert_called_once_with(
|
||||||
|
self.fake_path, algorithm='sha256')
|
||||||
|
validate_href_mock.assert_called_once_with(
|
||||||
|
mock.ANY, expected_url, False)
|
||||||
|
get_mock.assert_called_once_with('https://image-checksum')
|
||||||
|
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'get',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'validate_href',
|
||||||
|
autospec=True)
|
||||||
|
def test_build_instance_info_remote_checksum_sha512(self,
|
||||||
|
validate_href_mock,
|
||||||
|
get_mock):
|
||||||
|
# Test case where we would download both the image and the checksum
|
||||||
|
# and ultimately convert the image.
|
||||||
|
get_mock.return_value = 'a' * 128 + '\n'
|
||||||
|
cfg.CONF.set_override('image_download_source', 'local', group='agent')
|
||||||
|
i_info = self.node.instance_info
|
||||||
|
driver_internal_info = self.node.driver_internal_info
|
||||||
|
i_info['image_source'] = 'https://image-ref'
|
||||||
|
i_info['image_checksum'] = 'https://image-checksum'
|
||||||
|
i_info['root_gb'] = 10
|
||||||
|
i_info['image_disk_format'] = 'qcow2'
|
||||||
|
driver_internal_info['is_whole_disk_image'] = True
|
||||||
|
self.node.instance_info = i_info
|
||||||
|
self.node.driver_internal_info = driver_internal_info
|
||||||
|
self.node.save()
|
||||||
|
|
||||||
|
expected_url = (
|
||||||
|
'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid)
|
||||||
|
|
||||||
|
with task_manager.acquire(
|
||||||
|
self.context, self.node.uuid, shared=False) as task:
|
||||||
|
|
||||||
|
info = utils.build_instance_info_for_deploy(task)
|
||||||
|
self.assertEqual(expected_url, info['image_url'])
|
||||||
|
self.assertEqual('raw', info['image_disk_format'])
|
||||||
|
self.cache_image_mock.assert_called_once_with(
|
||||||
|
task.context, task.node, force_raw=True,
|
||||||
|
expected_format='qcow2',
|
||||||
|
expected_checksum='a' * 128,
|
||||||
|
expected_checksum_algo='sha512')
|
||||||
|
self.checksum_mock.assert_called_once_with(
|
||||||
|
self.fake_path, algorithm='sha256')
|
||||||
|
validate_href_mock.assert_called_once_with(
|
||||||
|
mock.ANY, expected_url, False)
|
||||||
|
get_mock.assert_called_once_with('https://image-checksum')
|
||||||
|
|
||||||
|
@mock.patch.object(checksum_utils, 'get_checksum_from_url',
|
||||||
|
autospec=True)
|
||||||
|
@mock.patch.object(image_service.HttpImageService, 'validate_href',
|
||||||
|
autospec=True)
|
||||||
|
def test_build_instance_info_md5_not_permitted(
|
||||||
|
self,
|
||||||
|
validate_href_mock,
|
||||||
|
get_from_url_mock):
|
||||||
|
cfg.CONF.set_override('allow_md5_checksum', False, group='agent')
|
||||||
|
# Test case where we would download both the image and the checksum
|
||||||
|
# and ultimately convert the image.
|
||||||
|
cfg.CONF.set_override('image_download_source', 'local', group='agent')
|
||||||
|
i_info = self.node.instance_info
|
||||||
|
driver_internal_info = self.node.driver_internal_info
|
||||||
|
i_info['image_source'] = 'image-ref'
|
||||||
|
i_info['image_checksum'] = 'ad606d6a24a2dec982bc2993aaaf9160'
|
||||||
|
i_info['root_gb'] = 10
|
||||||
|
i_info['image_disk_format'] = 'qcow2'
|
||||||
|
driver_internal_info['is_whole_disk_image'] = True
|
||||||
|
self.node.instance_info = i_info
|
||||||
|
self.node.driver_internal_info = driver_internal_info
|
||||||
|
self.node.save()
|
||||||
|
|
||||||
|
with task_manager.acquire(
|
||||||
|
self.context, self.node.uuid, shared=False) as task:
|
||||||
|
|
||||||
|
self.assertRaisesRegex(exception.ImageChecksumAlgorithmFailure,
|
||||||
|
'The requested image checksum algorithm '
|
||||||
|
'cannot be loaded',
|
||||||
|
utils.build_instance_info_for_deploy,
|
||||||
|
task)
|
||||||
|
|
||||||
|
self.cache_image_mock.assert_not_called()
|
||||||
|
self.checksum_mock.assert_not_called()
|
||||||
|
validate_href_mock.assert_not_called()
|
||||||
|
get_from_url_mock.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
class TestStorageInterfaceUtils(db_base.DbTestCase):
|
class TestStorageInterfaceUtils(db_base.DbTestCase):
|
||||||
def setUp(self):
|
def setUp(self):
|
||||||
|
@ -66,7 +66,8 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
self.cache.fetch_image(self.uuid, self.dest_path)
|
self.cache.fetch_image(self.uuid, self.dest_path)
|
||||||
self.assertFalse(mock_download.called)
|
self.assertFalse(mock_download.called)
|
||||||
mock_fetch.assert_called_once_with(
|
mock_fetch.assert_called_once_with(
|
||||||
None, self.uuid, self.dest_path, True)
|
None, self.uuid, self.dest_path, True,
|
||||||
|
None, None, None)
|
||||||
self.assertFalse(mock_clean_up.called)
|
self.assertFalse(mock_clean_up.called)
|
||||||
mock_image_service.assert_not_called()
|
mock_image_service.assert_not_called()
|
||||||
|
|
||||||
@ -83,7 +84,8 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
self.uuid, self.dest_path)
|
self.uuid, self.dest_path)
|
||||||
self.assertFalse(mock_download.called)
|
self.assertFalse(mock_download.called)
|
||||||
mock_fetch.assert_called_once_with(
|
mock_fetch.assert_called_once_with(
|
||||||
None, self.uuid, self.dest_path, True)
|
None, self.uuid, self.dest_path, True,
|
||||||
|
None, None, None)
|
||||||
self.assertFalse(mock_clean_up.called)
|
self.assertFalse(mock_clean_up.called)
|
||||||
mock_image_service.assert_not_called()
|
mock_image_service.assert_not_called()
|
||||||
|
|
||||||
@ -158,7 +160,8 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
mock_download.assert_called_once_with(
|
mock_download.assert_called_once_with(
|
||||||
self.cache, self.uuid, self.master_path, self.dest_path,
|
self.cache, self.uuid, self.master_path, self.dest_path,
|
||||||
mock_image_service.return_value.show.return_value,
|
mock_image_service.return_value.show.return_value,
|
||||||
ctx=None, force_raw=True, expected_format=None)
|
ctx=None, force_raw=True, expected_format=None,
|
||||||
|
expected_checksum=None, expected_checksum_algo=None)
|
||||||
mock_clean_up.assert_called_once_with(self.cache)
|
mock_clean_up.assert_called_once_with(self.cache)
|
||||||
mock_image_service.assert_called_once_with(self.uuid, context=None)
|
mock_image_service.assert_called_once_with(self.uuid, context=None)
|
||||||
mock_image_service.return_value.show.assert_called_once_with(self.uuid)
|
mock_image_service.return_value.show.assert_called_once_with(self.uuid)
|
||||||
@ -180,7 +183,8 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
mock_download.assert_called_once_with(
|
mock_download.assert_called_once_with(
|
||||||
self.cache, self.uuid, self.master_path, self.dest_path,
|
self.cache, self.uuid, self.master_path, self.dest_path,
|
||||||
mock_image_service.return_value.show.return_value,
|
mock_image_service.return_value.show.return_value,
|
||||||
ctx=None, force_raw=True, expected_format=None)
|
ctx=None, force_raw=True, expected_format=None,
|
||||||
|
expected_checksum=None, expected_checksum_algo=None)
|
||||||
mock_clean_up.assert_called_once_with(self.cache)
|
mock_clean_up.assert_called_once_with(self.cache)
|
||||||
|
|
||||||
def test_fetch_image_not_uuid(self, mock_download, mock_clean_up,
|
def test_fetch_image_not_uuid(self, mock_download, mock_clean_up,
|
||||||
@ -193,7 +197,8 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
mock_download.assert_called_once_with(
|
mock_download.assert_called_once_with(
|
||||||
self.cache, href, master_path, self.dest_path,
|
self.cache, href, master_path, self.dest_path,
|
||||||
mock_image_service.return_value.show.return_value,
|
mock_image_service.return_value.show.return_value,
|
||||||
ctx=None, force_raw=True, expected_format=None)
|
ctx=None, force_raw=True, expected_format=None,
|
||||||
|
expected_checksum=None, expected_checksum_algo=None)
|
||||||
self.assertTrue(mock_clean_up.called)
|
self.assertTrue(mock_clean_up.called)
|
||||||
|
|
||||||
def test_fetch_image_not_uuid_no_force_raw(self, mock_download,
|
def test_fetch_image_not_uuid_no_force_raw(self, mock_download,
|
||||||
@ -202,11 +207,14 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
href = u'http://abc.com/ubuntu.qcow2'
|
href = u'http://abc.com/ubuntu.qcow2'
|
||||||
href_converted = str(uuid.uuid5(uuid.NAMESPACE_URL, href))
|
href_converted = str(uuid.uuid5(uuid.NAMESPACE_URL, href))
|
||||||
master_path = os.path.join(self.master_dir, href_converted)
|
master_path = os.path.join(self.master_dir, href_converted)
|
||||||
self.cache.fetch_image(href, self.dest_path, force_raw=False)
|
self.cache.fetch_image(href, self.dest_path, force_raw=False,
|
||||||
|
expected_checksum='f00',
|
||||||
|
expected_checksum_algo='sha256')
|
||||||
mock_download.assert_called_once_with(
|
mock_download.assert_called_once_with(
|
||||||
self.cache, href, master_path, self.dest_path,
|
self.cache, href, master_path, self.dest_path,
|
||||||
mock_image_service.return_value.show.return_value,
|
mock_image_service.return_value.show.return_value,
|
||||||
ctx=None, force_raw=False, expected_format=None)
|
ctx=None, force_raw=False, expected_format=None,
|
||||||
|
expected_checksum='f00', expected_checksum_algo='sha256')
|
||||||
self.assertTrue(mock_clean_up.called)
|
self.assertTrue(mock_clean_up.called)
|
||||||
|
|
||||||
|
|
||||||
@ -214,7 +222,8 @@ class TestImageCacheFetch(BaseTest):
|
|||||||
class TestImageCacheDownload(BaseTest):
|
class TestImageCacheDownload(BaseTest):
|
||||||
|
|
||||||
def test__download_image(self, mock_fetch):
|
def test__download_image(self, mock_fetch):
|
||||||
def _fake_fetch(ctx, uuid, tmp_path, *args):
|
def _fake_fetch(ctx, uuid, tmp_path, force_raw, expected_format,
|
||||||
|
expected_checksum, expected_checksum_algo):
|
||||||
self.assertEqual(self.uuid, uuid)
|
self.assertEqual(self.uuid, uuid)
|
||||||
self.assertNotEqual(self.dest_path, tmp_path)
|
self.assertNotEqual(self.dest_path, tmp_path)
|
||||||
self.assertNotEqual(os.path.dirname(tmp_path), self.master_dir)
|
self.assertNotEqual(os.path.dirname(tmp_path), self.master_dir)
|
||||||
@ -236,7 +245,8 @@ class TestImageCacheDownload(BaseTest):
|
|||||||
# Make sure we don't use any parts of the URL anywhere.
|
# Make sure we don't use any parts of the URL anywhere.
|
||||||
url = "http://example.com/image.iso?secret=%s" % ("x" * 1000)
|
url = "http://example.com/image.iso?secret=%s" % ("x" * 1000)
|
||||||
|
|
||||||
def _fake_fetch(ctx, href, tmp_path, *args):
|
def _fake_fetch(ctx, href, tmp_path, force_raw, expected_format,
|
||||||
|
expected_checksum, expected_checksum_algo):
|
||||||
self.assertEqual(url, href)
|
self.assertEqual(url, href)
|
||||||
self.assertNotEqual(self.dest_path, tmp_path)
|
self.assertNotEqual(self.dest_path, tmp_path)
|
||||||
self.assertNotEqual(os.path.dirname(tmp_path), self.master_dir)
|
self.assertNotEqual(os.path.dirname(tmp_path), self.master_dir)
|
||||||
@ -520,7 +530,8 @@ class TestImageCacheCleanUp(base.TestCase):
|
|||||||
@mock.patch.object(utils, 'rmtree_without_raise', autospec=True)
|
@mock.patch.object(utils, 'rmtree_without_raise', autospec=True)
|
||||||
@mock.patch.object(image_cache, '_fetch', autospec=True)
|
@mock.patch.object(image_cache, '_fetch', autospec=True)
|
||||||
def test_temp_images_not_cleaned(self, mock_fetch, mock_rmtree):
|
def test_temp_images_not_cleaned(self, mock_fetch, mock_rmtree):
|
||||||
def _fake_fetch(ctx, uuid, tmp_path, *args):
|
def _fake_fetch(ctx, uuid, tmp_path, force_raw, expected_format,
|
||||||
|
expected_checksum, expected_checksum_algo):
|
||||||
with open(tmp_path, 'w') as fp:
|
with open(tmp_path, 'w') as fp:
|
||||||
fp.write("TEST" * 10)
|
fp.write("TEST" * 10)
|
||||||
|
|
||||||
@ -774,9 +785,13 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
mock_format_inspector.return_value = image_check
|
mock_format_inspector.return_value = image_check
|
||||||
mock_show.return_value = {}
|
mock_show.return_value = {}
|
||||||
mock_size.return_value = 100
|
mock_size.return_value = 100
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True,
|
||||||
|
expected_checksum='1234',
|
||||||
|
expected_checksum_algo='md5')
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum='1234',
|
||||||
|
checksum_algo='md5')
|
||||||
mock_clean.assert_called_once_with('/foo', 100)
|
mock_clean.assert_called_once_with('/foo', 100)
|
||||||
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
|
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
|
||||||
'/foo/bar.part')
|
'/foo/bar.part')
|
||||||
@ -808,7 +823,8 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
mock_size.return_value = 100
|
mock_size.return_value = 100
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum=None, checksum_algo=None)
|
||||||
mock_clean.assert_called_once_with('/foo', 100)
|
mock_clean.assert_called_once_with('/foo', 100)
|
||||||
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
|
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
|
||||||
'/foo/bar.part')
|
'/foo/bar.part')
|
||||||
@ -838,9 +854,13 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
mock_exists.return_value = True
|
mock_exists.return_value = True
|
||||||
mock_size.return_value = 100
|
mock_size.return_value = 100
|
||||||
mock_image_show.return_value = {}
|
mock_image_show.return_value = {}
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True,
|
||||||
|
expected_format=None, expected_checksum='f00',
|
||||||
|
expected_checksum_algo='sha256')
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum='f00',
|
||||||
|
checksum_algo='sha256')
|
||||||
mock_clean.assert_called_once_with('/foo', 100)
|
mock_clean.assert_called_once_with('/foo', 100)
|
||||||
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
|
mock_raw.assert_called_once_with('fake-uuid', '/foo/bar',
|
||||||
'/foo/bar.part')
|
'/foo/bar.part')
|
||||||
@ -868,9 +888,13 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
image_check.__str__.return_value = 'raw'
|
image_check.__str__.return_value = 'raw'
|
||||||
image_check.safety_check.return_value = True
|
image_check.safety_check.return_value = True
|
||||||
mock_format_inspector.return_value = image_check
|
mock_format_inspector.return_value = image_check
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True,
|
||||||
|
expected_checksum='e00',
|
||||||
|
expected_checksum_algo='sha256')
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum='e00',
|
||||||
|
checksum_algo='sha256')
|
||||||
mock_clean.assert_not_called()
|
mock_clean.assert_not_called()
|
||||||
mock_size.assert_not_called()
|
mock_size.assert_not_called()
|
||||||
mock_raw.assert_not_called()
|
mock_raw.assert_not_called()
|
||||||
@ -898,9 +922,14 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
self.assertRaises(exception.InvalidImage,
|
self.assertRaises(exception.InvalidImage,
|
||||||
image_cache._fetch,
|
image_cache._fetch,
|
||||||
'fake', 'fake-uuid',
|
'fake', 'fake-uuid',
|
||||||
'/foo/bar', force_raw=True)
|
'/foo/bar', force_raw=True,
|
||||||
|
expected_format=None,
|
||||||
|
expected_checksum='a00',
|
||||||
|
expected_checksum_algo='sha512')
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum='a00',
|
||||||
|
checksum_algo='sha512')
|
||||||
mock_clean.assert_not_called()
|
mock_clean.assert_not_called()
|
||||||
mock_size.assert_not_called()
|
mock_size.assert_not_called()
|
||||||
mock_raw.assert_not_called()
|
mock_raw.assert_not_called()
|
||||||
@ -929,7 +958,8 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
'fake', 'fake-uuid',
|
'fake', 'fake-uuid',
|
||||||
'/foo/bar', force_raw=True)
|
'/foo/bar', force_raw=True)
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum=None, checksum_algo=None)
|
||||||
mock_clean.assert_not_called()
|
mock_clean.assert_not_called()
|
||||||
mock_size.assert_not_called()
|
mock_size.assert_not_called()
|
||||||
mock_raw.assert_not_called()
|
mock_raw.assert_not_called()
|
||||||
@ -958,7 +988,8 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
|
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum=None, checksum_algo=None)
|
||||||
mock_size.assert_has_calls([
|
mock_size.assert_has_calls([
|
||||||
mock.call('/foo/bar.part', estimate=False),
|
mock.call('/foo/bar.part', estimate=False),
|
||||||
mock.call('/foo/bar.part', estimate=True),
|
mock.call('/foo/bar.part', estimate=True),
|
||||||
@ -995,7 +1026,8 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
mock_size.return_value = 100
|
mock_size.return_value = 100
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum=None, checksum_algo=None)
|
||||||
mock_clean.assert_not_called()
|
mock_clean.assert_not_called()
|
||||||
mock_raw.assert_not_called()
|
mock_raw.assert_not_called()
|
||||||
mock_remove.assert_not_called()
|
mock_remove.assert_not_called()
|
||||||
@ -1026,7 +1058,8 @@ class TestFetchCleanup(base.TestCase):
|
|||||||
mock_size.return_value = 100
|
mock_size.return_value = 100
|
||||||
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
image_cache._fetch('fake', 'fake-uuid', '/foo/bar', force_raw=True)
|
||||||
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
mock_fetch.assert_called_once_with('fake', 'fake-uuid',
|
||||||
'/foo/bar.part', force_raw=False)
|
'/foo/bar.part', force_raw=False,
|
||||||
|
checksum=None, checksum_algo=None)
|
||||||
mock_clean.assert_not_called()
|
mock_clean.assert_not_called()
|
||||||
mock_raw.assert_not_called()
|
mock_raw.assert_not_called()
|
||||||
mock_remove.assert_not_called()
|
mock_remove.assert_not_called()
|
||||||
|
@ -0,0 +1,44 @@
|
|||||||
|
---
|
||||||
|
security:
|
||||||
|
- |
|
||||||
|
An issue in Ironic has been resolved where image checksums would not be
|
||||||
|
checked prior to the conversion of an image to a ``raw`` format image from
|
||||||
|
another image format.
|
||||||
|
|
||||||
|
With default settings, this normally would not take place, however the
|
||||||
|
``image_download_source`` option, which is available to be set at a
|
||||||
|
``node`` level for a single deployment, by default for that baremetal node
|
||||||
|
in all cases, or via the ``[agent]image_download_source`` configuration
|
||||||
|
option when set to ``local``. By default, this setting is ``http``.
|
||||||
|
|
||||||
|
This was in concert with the ``[DEFAULT]force_raw_images`` when set to
|
||||||
|
``True``, which caused Ironic to download and convert the file.
|
||||||
|
|
||||||
|
In a fully integrated context of Ironic's use in a larger OpenStack
|
||||||
|
deployment, where images are coming from the Glance image service, the
|
||||||
|
previous pattern was not problematic. The overall issue was introduced as
|
||||||
|
a result of the capability to supply, cache, and convert a disk image
|
||||||
|
provided as a URL by an authenticated user.
|
||||||
|
|
||||||
|
Ironic will now validate the user supplied checksum prior to image
|
||||||
|
conversion on the conductor. This can be disabled using the
|
||||||
|
``[conductor]disable_file_checksum`` configuration option.
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixes a security issue where Ironic would fail to checksum disk image
|
||||||
|
files it downloads when Ironic had been requested to download and convert
|
||||||
|
the image to a raw image format. This required the
|
||||||
|
``image_download_source`` to be explicitly set to ``local``, which is not
|
||||||
|
the default.
|
||||||
|
|
||||||
|
This fix can be disabled by setting
|
||||||
|
``[conductor]disable_file_checksum`` to ``True``, however this
|
||||||
|
option will be removed in new major Ironic releases.
|
||||||
|
|
||||||
|
As a result of this, parity has been introduced to align Ironic to
|
||||||
|
Ironic-Python-Agent's support for checksums used by ``standalone``
|
||||||
|
users of Ironic. This includes support for remote checksum files to be
|
||||||
|
supplied by URL, in order to prevent breaking existing users which may
|
||||||
|
have inadvertently been leveraging the prior code path. This support can
|
||||||
|
be disabled by setting
|
||||||
|
``[conductor]disable_support_for_checksum_files`` to ``True``.
|
Loading…
Reference in New Issue
Block a user