openstack/metalsmith
Code Issues Proposed changes

Stop requiring checksums with file images

Browse Source 
Neither deploy method requires checksums with file images, they're
simply ignored. Deprecate providing them.

Change-Id: Ia123c1d3c57cc2814e3f971209cbee3ab336f7bd
This commit is contained in:
Dmitry Tantsur 2020-09-03 14:23:52 +02:00
parent 97099bc0ff
commit 3621535d67
5 changed files with 32 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
metalsmith/sources.py
View File

@ -19,6 +19,7 @@ import abc
import logging
import os
from urllib import parse as urlparse
import warnings
import openstack.exceptions
import requests
@ -196,24 +197,26 @@ class FileWholeDiskImage(_Source):
available at the same location to all conductors in the same group.
"""
def __init__(self, location, checksum):
def __init__(self, location, checksum=None):
"""Create a local file source.
:param location: Location of the image, optionally starting with
``file://``.
:param checksum: MD5 checksum of the image.
:param checksum: MD5 checksum of the image. DEPRECATED: checksums do
not actually work with file images.
"""
if not location.startswith('file://'):
location = 'file://' + location
self.location = location
self.checksum = checksum
if self.checksum:
warnings.warn("Checksums cannot be used with file images",
DeprecationWarning)
def _node_updates(self, connection):
LOG.debug('Image: %(image)s, checksum %(checksum)s',
{'image': self.location, 'checksum': self.checksum})
LOG.debug('Image: %s', self.location)
return {
'image_source': self.location,
'image_checksum': self.checksum,
}
@ -227,7 +230,8 @@ class FilePartitionImage(FileWholeDiskImage):
available at the same location to all conductors in the same group.
"""
def __init__(self, location, kernel_location, ramdisk_location, checksum):
def __init__(self, location, kernel_location, ramdisk_location,
checksum=None):
"""Create a local file source.
:param location: Location of the image, optionally starting with
@ -236,7 +240,8 @@ class FilePartitionImage(FileWholeDiskImage):
optionally starting with ``file://``.
:param ramdisk_location: Location of the ramdisk of the image,
optionally starting with ``file://``.
:param checksum: MD5 checksum of the image.
:param checksum: MD5 checksum of the image. DEPRECATED: checksums do
not actually work with file images.
"""
super(FilePartitionImage, self).__init__(location, checksum)
if not kernel_location.startswith('file://'):
@ -289,14 +294,13 @@ def detect(image, kernel=None, ramdisk=None, checksum=None):
kernel_type = _link_type(kernel)
ramdisk_type = _link_type(ramdisk)
if not checksum:
raise ValueError('checksum is required for HTTP and file images')
if image_type == 'http' and not checksum:
raise ValueError('checksum is required for HTTP images')
if image_type == 'file':
if (kernel_type not in (None, 'file')
or ramdisk_type not in (None, 'file')
or checksum_type == 'http'):
raise ValueError('kernal, ramdisk and checksum can only be files '
or ramdisk_type not in (None, 'file')):
raise ValueError('kernel and ramdisk can only be files '
'for file images')
if kernel or ramdisk:

13
metalsmith/test/test_cmd.py
View File

@ -463,20 +463,17 @@ class TestDeploy(Base):
def test_args_file_whole_disk_image(self, mock_pr):
args = ['deploy', '--image', 'file:///var/lib/ironic/image.img',
'--image-checksum', '95e750180c7921ea0d545c7165db66b8',
'--network', 'mynet', '--resource-class', 'compute']
self._check(mock_pr, args, {}, {'image': mock.ANY})
source = mock_pr.return_value.provision_node.call_args[1]['image']
self.assertIsInstance(source, sources.FileWholeDiskImage)
self.assertEqual('file:///var/lib/ironic/image.img', source.location)
self.assertEqual('95e750180c7921ea0d545c7165db66b8', source.checksum)
def test_args_file_partition_disk_image(self, mock_pr):
args = ['deploy', '--image', 'file:///var/lib/ironic/image.img',
'--image-kernel', 'file:///var/lib/ironic/image.vmlinuz',
'--image-ramdisk', 'file:///var/lib/ironic/image.initrd',
'--image-checksum', '95e750180c7921ea0d545c7165db66b8',
'--network', 'mynet', '--resource-class', 'compute']
self._check(mock_pr, args, {}, {'image': mock.ANY})
@ -487,16 +484,6 @@ class TestDeploy(Base):
source.kernel_location)
self.assertEqual('file:///var/lib/ironic/image.initrd',
source.ramdisk_location)
self.assertEqual('95e750180c7921ea0d545c7165db66b8', source.checksum)
@mock.patch.object(_cmd.LOG, 'critical', autospec=True)
def test_args_file_image_without_checksum(self, mock_log, mock_pr):
args = ['deploy', '--image', 'file:///var/lib/ironic/image.img',
'--resource-class', 'compute']
self.assertRaises(SystemExit, _cmd.main, args)
self.assertTrue(mock_log.called)
self.assertFalse(mock_pr.return_value.reserve_node.called)
self.assertFalse(mock_pr.return_value.provision_node.called)
@mock.patch.object(_cmd.LOG, 'critical', autospec=True)
def test_args_file_image_with_incorrect_kernel(self, mock_log, mock_pr):

7
metalsmith/test/test_provisioner.py
View File

@ -1102,13 +1102,12 @@ abcd image
def test_with_file_whole_disk(self):
self.instance_info['image_source'] = 'file:///foo/img'
self.instance_info['image_checksum'] = 'abcd'
del self.instance_info['kernel']
del self.instance_info['ramdisk']
inst = self.pr.provision_node(
self.node,
sources.FileWholeDiskImage('file:///foo/img', checksum='abcd'),
sources.FileWholeDiskImage('file:///foo/img'),
[{'network': 'network'}])
self.assertEqual(inst.uuid, self.node.id)
@ -1132,7 +1131,6 @@ abcd image
def test_with_file_partition(self):
self.instance_info['image_source'] = 'file:///foo/img'
self.instance_info['image_checksum'] = 'abcd'
self.instance_info['kernel'] = 'file:///foo/vmlinuz'
self.instance_info['ramdisk'] = 'file:///foo/initrd'
@ -1140,8 +1138,7 @@ abcd image
self.node,
sources.FilePartitionImage('/foo/img',
'/foo/vmlinuz',
'/foo/initrd',
checksum='abcd'),
'/foo/initrd'),
[{'network': 'network'}])
self.assertEqual(inst.uuid, self.node.id)

13
metalsmith/test/test_sources.py
View File

@ -64,25 +64,25 @@ class TestDetect(unittest.TestCase):
sources.detect, 'foobar', **kwargs)
def test_checksum_required(self):
for tp in ('file', 'http', 'https'):
for tp in ('http', 'https'):
self.assertRaisesRegex(ValueError, 'checksum is required',
sources.detect, '%s://foo' % tp)
def test_file_whole_disk(self):
source = sources.detect('file:///image', checksum='abcd')
source = sources.detect('file:///image')
self.assertIs(source.__class__, sources.FileWholeDiskImage)
self.assertEqual(source.location, 'file:///image')
self.assertEqual(source.checksum, 'abcd')
self.assertIsNone(source.checksum)
source._validate(mock.Mock(), None)
def test_file_partition_disk(self):
source = sources.detect('file:///image', checksum='abcd',
source = sources.detect('file:///image',
kernel='file:///kernel',
ramdisk='file:///ramdisk')
self.assertIs(source.__class__, sources.FilePartitionImage)
self.assertEqual(source.location, 'file:///image')
self.assertEqual(source.checksum, 'abcd')
self.assertIsNone(source.checksum)
self.assertEqual(source.kernel_location, 'file:///kernel')
self.assertEqual(source.ramdisk_location, 'file:///ramdisk')
@ -99,8 +99,7 @@ class TestDetect(unittest.TestCase):
for kwargs in [{'kernel': 'foo'},
{'ramdisk': 'foo'},
{'kernel': 'http://foo'},
{'ramdisk': 'http://foo'},
{'checksum': 'http://foo'}]:
{'ramdisk': 'http://foo'}]:
kwargs.setdefault('checksum', 'abcd')
self.assertRaisesRegex(ValueError, 'can only be files',
sources.detect, 'file:///image', **kwargs)

8
releasenotes/notes/file-checksum-d19370a8fde00a81.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Checksums are no longer required (nor used) with file images.
deprecations:
- |
Providing checksums for file images is deprecated. No deploy implementation
actually supports them.