On Thu, 2017-08-03 at 14:13 +0100, Radostin Stoyanov wrote:
> Add and update unit tests.
> ---
> src/virtBootstrap/utils.py | 41 +++++++++++++++++++++++++++
> tests/test_build_qcow2_image.py | 63 +++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 102 insertions(+), 2 deletions(-)
>
> diff --git a/src/virtBootstrap/utils.py b/src/virtBootstrap/utils.py
> index b6c20f3..8f56d7a 100644
> --- a/src/virtBootstrap/utils.py
> +++ b/src/virtBootstrap/utils.py
> @@ -66,6 +66,7 @@ class Build_QCOW2_Image(object):
> @param tar_files: List of paths to tar files from which to the rootfs
> @param uid_map: Mappings for UID of files in rootfs
> @param gid_map: Mappings for GID of files in rootfs
> + @param root_password: Root password to be set
> @param dest: Destination directory where qcow2 images will be stored
> @param progress: Instance of the progress module
>
> @@ -80,6 +81,7 @@ class Build_QCOW2_Image(object):
> self.progress = kwargs['progress']
> self.uid_map = kwargs.get('uid_map', None)
> self.gid_map = kwargs.get('gid_map', None)
> + self.root_password = kwargs.get('root_password', None)
> self.fmt = 'qcow2'
> self.qcow2_files = [os.path.join(kwargs['dest'], 'layer-%s.qcow2' % i)
> for i in range(len(self.tar_files))]
> @@ -88,6 +90,7 @@ class Build_QCOW2_Image(object):
> self.create_base_qcow2_layer(self.tar_files[0], self.qcow2_files[0])
> if len(self.tar_files) > 1:
> self.create_backing_chains()
> +
> self.g.shutdown()
>
> def create_disk(self, qcow2_file, backingfile=None, readonly=False):
> @@ -140,6 +143,8 @@ class Build_QCOW2_Image(object):
> self.g.mkfs("ext3", dev)
> self.tar_in(tar_file, dev)
> self.progress("Extracting content of base layer", logger=logger)
> + if len(self.tar_files) == 1:
> + self.set_root_password()
Even though it's rather likely to work, you can't be sure the latest shadow file
will be this one. There are two solutions, either
1/ Loop over the layers from leave to base and change the shadow in the first
found one.
2/ Add an overlay layer that changes the shadow file
> self.g.shutdown()
>
> def create_backing_chains(self):
> @@ -162,6 +167,8 @@ class Build_QCOW2_Image(object):
> logger=logger)
> self.tar_in(tar_file, devices[i])
>
> + self.set_root_password()
> +
> def map_id(self, tar_members, map_uid, map_gid):
> """
> Remapping ownership of all files inside image.
> @@ -183,6 +190,40 @@ class Build_QCOW2_Image(object):
> if new_uid != -1 or new_gid != -1:
> self.g.lchown(new_uid, new_gid, os.path.join('/', member.name))
>
> + def set_root_password(self):
> + """
> + Set root password in the shadow file of image.
> +
> + Mount the last the layer to update the shadow file with the
> + hash for the root password.
> + """
> + if self.root_password is None:
> + return
> +
> + self.progress("Setting root password", logger=logger)
> +
> + last_layer_dev = self.g.list_devices()[-1]
> + self.g.mount(last_layer_dev, '/')
> +
> + if not self.g.is_file('/etc/shadow'):
> + logger.error('showfile was not found in this image')
'showfile'? did you mean 'shadow file'?
> + return
> +
> + shadow_content = self.g.read_file('/etc/shadow').split('\n')
> +
> + if not shadow_content:
> + logger.error('showfile was empty')
same here
> + return
> +
> + # Note: 'shadow_content' is of type list, thus pass-by-reference
> + # is used here.
> + set_password_in_shadow_content(
> + shadow_content,
> + self.root_password
> + )
> + self.g.write('/etc/shadow', '\n'.join(shadow_content))
> + self.g.umount('/')
> +
>
> def get_compression_type(tar_file):
> """
> diff --git a/tests/test_build_qcow2_image.py b/tests/test_build_qcow2_image.py
> index 74d7883..9cd60b3 100644
> --- a/tests/test_build_qcow2_image.py
> +++ b/tests/test_build_qcow2_image.py
> @@ -46,7 +46,7 @@ class TestBuild_QCOW2_Image(unittest.TestCase):
> 'dest': 'dest',
> 'uid_map': [[0, 1000, 10], [500, 500, 10]],
> 'gid_map': [[0, 1000, 10], [500, 500, 10]],
> -
> + 'root_password': 'secret'
> }
>
> m_guestfs = mock.Mock()
> @@ -71,7 +71,7 @@ class TestBuild_QCOW2_Image(unittest.TestCase):
> self.assertIs(src_instance.tar_files, kwargs['tar_files'])
> self.assertIs(src_instance.uid_map, kwargs['uid_map'])
> self.assertIs(src_instance.gid_map, kwargs['gid_map'])
> -
> + self.assertIs(src_instance.root_password, kwargs['root_password'])
> self.assertIs(src_instance.progress, kwargs['progress'])
> self.assertIs(src_instance.g, m_guestfs)
>
> @@ -233,6 +233,7 @@ class TestBuild_QCOW2_Image(unittest.TestCase):
> dev = '/dev/sda'
>
> m_self = mock.Mock(spec=utils.Build_QCOW2_Image)
> + m_self.tar_files = [tar_file]
> m_self.g = mock.Mock()
> m_self.progress = mock.Mock()
> m_self.g.list_devices.return_value = [dev]
> @@ -263,6 +264,7 @@ class TestBuild_QCOW2_Image(unittest.TestCase):
> 'Extracting content of base layer',
> logger=utils.logger
> ),
> + mock.call.set_root_password(),
> mock.call.g.shutdown()
> ]
> )
> @@ -320,6 +322,7 @@ class TestBuild_QCOW2_Image(unittest.TestCase):
> ),
> mock.call.tar_in(tar_file, devices[i])
> ))
> + expected_calls.append(mock.call.set_root_password())
>
> self.assertEqual(m_self.method_calls, expected_calls)
>
> @@ -365,3 +368,59 @@ class TestBuild_QCOW2_Image(unittest.TestCase):
> )
>
> self.assertEqual(m_self.g.lchown.mock_calls, expected_calls)
> +
> + ###################################
> + # Tests for: set_root_password()
> + ###################################
> + def test_set_root_password_does_not_make_any_calls(self):
> + """
> + Ensures that set_root_password() does not make any calls
> + when self.root_password is None.
> + """
> + m_self = mock.Mock(spec=utils.Build_QCOW2_Image)
> + m_self.root_password = None
> + utils.Build_QCOW2_Image.set_root_password(m_self)
> + self.assertEqual(m_self.mock_calls, [])
> +
> + def test_set_root_password(self):
> + """
> + Ensures that set_root_password():
> + - Mounts the last device (assumed to be the last layer)
> + - Read the content of /etc/shadow
> + - Calls set_password_in_shadow_content()
> + - Write the content back to /etc/shadow
> + """
> + m_self = mock.Mock(spec=utils.Build_QCOW2_Image)
> + m_self.progress = mock.Mock()
> + m_self.g = mock.Mock()
> +
> + m_self.root_password = 'secret'
> + devices = ['/dev/sda', '/dev/sdb', '/dev/sdc', '/dev/sde']
> + m_self.g.list_devices.return_value = devices
> + m_self.g.read_file.return_value = "old shadow file content"
> +
> + def get_new_content(shadow_content, _ignore):
> + """
> + Dummy function to update the shadow content.
> + """
> + shadow_content[0] = "new shadow file content"
> +
> + with mock.patch(
> + 'virtBootstrap.utils.set_password_in_shadow_content',
> + side_effect=get_new_content
> + ) as m_set_password_in_shadow_content:
> + utils.Build_QCOW2_Image.set_root_password(m_self)
> +
> + m_set_password_in_shadow_content.assert_called_once()
> +
> + expected_calls = [
> + mock.call.progress('Setting root password', logger=utils.logger),
> + mock.call.g.list_devices(),
> + mock.call.g.mount(devices[-1], '/'),
> + mock.call.g.is_file('/etc/shadow'),
> + mock.call.g.read_file('/etc/shadow'),
> + mock.call.g.write('/etc/shadow', "new shadow file content"),
> + mock.call.g.umount('/')
> + ]
> +
> + self.assertEqual(m_self.mock_calls, expected_calls)
You should have some black box tests here:
* Prepare a few tar files
* Convert to QCOW2 images
* Check that the shadow file is the expected one in the leaf image.
Obviously there should be variations of these tests:
* shadow file in the base layer
* shadow file in arbitrary layers
Such tests would surely be much more helpful to check against regressions.
--
Cedric
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
