From 349e5074e33a44668533feca14cc415f766f53cd Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 24 May 2019 15:07:51 +0200 Subject: [PATCH] Split InstanceConfig into GenericConfig and CloudInitConfig As a side effect allows providing custom user data, not only users. Change-Id: Ia4c305ea996b3bb58c5898777cc7a3c8b93a413d --- metalsmith/__init__.py | 2 +- metalsmith/_cmd.py | 6 +- metalsmith/_provisioner.py | 8 +- metalsmith/{_config.py => instance_config.py} | 124 +++++++++++++----- metalsmith/test/test_cmd.py | 8 +- ...test_config.py => test_instance_config.py} | 65 +++++++-- metalsmith/test/test_provisioner.py | 6 +- .../instance-config-c4ea6a169f782138.yaml | 14 ++ 8 files changed, 171 insertions(+), 62 deletions(-) rename metalsmith/{_config.py => instance_config.py} (62%) rename metalsmith/test/{test_config.py => test_instance_config.py} (64%) create mode 100644 releasenotes/notes/instance-config-c4ea6a169f782138.yaml diff --git a/metalsmith/__init__.py b/metalsmith/__init__.py index 3e9da1c..24aca6a 100644 --- a/metalsmith/__init__.py +++ b/metalsmith/__init__.py @@ -13,9 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from metalsmith._config import InstanceConfig from metalsmith._instance import Instance from metalsmith._instance import InstanceState from metalsmith._provisioner import Provisioner +from metalsmith.instance_config import InstanceConfig __all__ = ['Instance', 'InstanceConfig', 'InstanceState', 'Provisioner'] diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py index da46454..2e85ab0 100644 --- a/metalsmith/_cmd.py +++ b/metalsmith/_cmd.py @@ -19,10 +19,10 @@ import sys from openstack import config as os_config -from metalsmith import _config from metalsmith import _format from metalsmith import _provisioner from metalsmith import _utils +from metalsmith import instance_config from metalsmith import sources @@ -63,9 +63,11 @@ def _do_deploy(api, args, formatter): ramdisk=args.image_ramdisk, checksum=args.image_checksum) - config = _config.InstanceConfig(ssh_keys=ssh_keys) if args.user_name: + config = instance_config.CloudInitConfig(ssh_keys=ssh_keys) config.add_user(args.user_name, sudo=args.passwordless_sudo) + else: + config = instance_config.GenericConfig(ssh_keys=ssh_keys) node = api.reserve_node(resource_class=args.resource_class, conductor_group=args.conductor_group, diff --git a/metalsmith/_provisioner.py b/metalsmith/_provisioner.py index 5adda64..92cae91 100644 --- a/metalsmith/_provisioner.py +++ b/metalsmith/_provisioner.py @@ -21,12 +21,12 @@ from openstack import connection from openstack import exceptions as os_exc import six -from metalsmith import _config from metalsmith import _instance from metalsmith import _nics from metalsmith import _scheduler from metalsmith import _utils from metalsmith import exceptions +from metalsmith import instance_config from metalsmith import sources @@ -304,8 +304,8 @@ class Provisioner(_utils.GetNodeMixin): the value of the local_gb property is used. :param swap_size_mb: The size of the swap partition. It's an error to specify it for a whole disk image. - :param config: :py:class:`metalsmith.InstanceConfig` object with - the configuration to pass to the instance. + :param config: configuration to pass to the instance, one of + objects from :py:mod:`metalsmith.instance_config`. :param hostname: Hostname to assign to the instance. If provided, overrides the ``hostname`` passed to ``reserve_node``. :param netboot: Whether to use networking boot for final instances. @@ -328,7 +328,7 @@ class Provisioner(_utils.GetNodeMixin): :raises: :py:class:`metalsmith.exceptions.Error` """ if config is None: - config = _config.InstanceConfig() + config = instance_config.GenericConfig() if isinstance(image, six.string_types): image = sources.GlanceImage(image) diff --git a/metalsmith/_config.py b/metalsmith/instance_config.py similarity index 62% rename from metalsmith/_config.py rename to metalsmith/instance_config.py index 71c2406..6efbf6b 100644 --- a/metalsmith/_config.py +++ b/metalsmith/instance_config.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy import json import logging import warnings @@ -25,41 +26,23 @@ from metalsmith import _utils LOG = logging.getLogger(__name__) -class InstanceConfig(object): +class GenericConfig(object): """Configuration of the target instance. The information attached to this object will be passed via a configdrive to the instance's first boot script (e.g. cloud-init). + This class represents generic configuration compatible with most first-boot + implementations. Use :py:class:`CloudInitConfig` for features specific to + `cloud-init `_. + :ivar ssh_keys: List of SSH public keys. - :ivar users: Users to add on first boot. + :ivar user_data: User data as a string. """ - def __init__(self, ssh_keys=None): + def __init__(self, ssh_keys=None, user_data=None): self.ssh_keys = ssh_keys or [] - self.users = [] - - def add_user(self, name, admin=True, password_hash=None, sudo=False, - **kwargs): - """Add a user to be created on first boot. - - :param name: user name. - :param admin: whether to add the user to the admin group (wheel). - :param password_hash: user password hash, if password authentication - is expected. - :param sudo: whether to allow the user sudo without password. - :param kwargs: other arguments to pass. - """ - kwargs['name'] = name - if admin: - kwargs.setdefault('groups', []).append('wheel') - if password_hash: - kwargs['passwd'] = password_hash - if sudo: - kwargs['sudo'] = 'ALL=(ALL) NOPASSWD:ALL' - if self.ssh_keys: - kwargs.setdefault('ssh_authorized_keys', self.ssh_keys) - self.users.append(kwargs) + self.user_data = user_data def generate(self, node): """Generate the config drive information. @@ -88,17 +71,19 @@ class InstanceConfig(object): 'availability_zone': '', 'files': [], 'meta': {}} - user_data = {} - user_data_str = None - - if self.users: - user_data['users'] = self.users - - if user_data: - user_data_str = "#cloud-config\n" + json.dumps(user_data) + user_data = self.populate_user_data() return {'meta_data': metadata, - 'user_data': user_data_str} + 'user_data': user_data} + + def populate_user_data(self): + """Get user data for this configuration. + + Can be overridden to provide additional features. + + :return: user data as a string. + """ + return self.user_data def build_configdrive(self, node): """Make the config drive ISO. @@ -122,3 +107,72 @@ class InstanceConfig(object): 'metadata %(meta)s', {'node': _utils.log_res(node), 'meta': metadata}) return configdrive.build(metadata, user_data=user_data, **cd) + + +class CloudInitConfig(GenericConfig): + """Configuration of the target instance using cloud-init. + + Compared to :class:`GenericConfig`, this adds support for managing users. + + :ivar ssh_keys: List of SSH public keys. + :ivar user_data: Cloud-init script as a dictionary. + :ivar users: Users to add on first boot. + """ + + def __init__(self, ssh_keys=None, user_data=None): + if user_data is not None and not isinstance(user_data, dict): + raise TypeError('Custom user data must be a dictionary for ' + 'CloudInitConfig, got %r' % user_data) + super(CloudInitConfig, self).__init__(ssh_keys, user_data or {}) + self.users = [] + + def add_user(self, name, admin=True, password_hash=None, sudo=False, + **kwargs): + """Add a user to be created on first boot. + + :param name: user name. + :param admin: whether to add the user to the admin group (wheel). + :param password_hash: user password hash, if password authentication + is expected. + :param sudo: whether to allow the user sudo without password. + :param kwargs: other arguments to pass. + """ + kwargs['name'] = name + if admin: + kwargs.setdefault('groups', []).append('wheel') + if password_hash: + kwargs['passwd'] = password_hash + if sudo: + kwargs['sudo'] = 'ALL=(ALL) NOPASSWD:ALL' + if self.ssh_keys: + kwargs.setdefault('ssh_authorized_keys', self.ssh_keys) + self.users.append(kwargs) + + def populate_user_data(self): + """Get user data for this configuration. + + Takes the custom user data and appends requested users to it. + + :return: user data as a string. + """ + if not isinstance(self.user_data, dict): + raise TypeError('Custom user data must be a dictionary for ' + 'CloudInitConfig, got %r' % self.user_data) + + if self.users: + user_data = copy.deepcopy(self.user_data) + user_data.setdefault('users', []).extend(self.users) + else: + user_data = self.user_data + + if user_data: + return "#cloud-config\n" + json.dumps(user_data) + + +class InstanceConfig(CloudInitConfig): + """DEPRECATED, use :class:`.GenericConfig` or :class:`.CloudInitConfig`.""" + + def __init__(self, *args, **kwargs): + warnings.warn('InstanceConfig is deprecated, use GenericConfig or ' + 'CloudInitConfig instead', DeprecationWarning) + super(InstanceConfig, self).__init__(*args, **kwargs) diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py index 2b4fc38..425fbc9 100644 --- a/metalsmith/test/test_cmd.py +++ b/metalsmith/test/test_cmd.py @@ -22,9 +22,9 @@ import six import testtools from metalsmith import _cmd -from metalsmith import _config from metalsmith import _instance from metalsmith import _provisioner +from metalsmith import instance_config from metalsmith import sources @@ -301,7 +301,8 @@ class TestDeploy(testtools.TestCase): config = mock_pr.return_value.provision_node.call_args[1]['config'] self.assertEqual(['foo'], config.ssh_keys) - @mock.patch.object(_config.InstanceConfig, 'add_user', autospec=True) + @mock.patch.object(instance_config.CloudInitConfig, 'add_user', + autospec=True) def test_args_user_name(self, mock_add_user, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', '--user-name', 'banana', '--resource-class', 'compute'] @@ -311,7 +312,8 @@ class TestDeploy(testtools.TestCase): self.assertEqual([], config.ssh_keys) mock_add_user.assert_called_once_with(config, 'banana', sudo=False) - @mock.patch.object(_config.InstanceConfig, 'add_user', autospec=True) + @mock.patch.object(instance_config.CloudInitConfig, 'add_user', + autospec=True) def test_args_user_name_with_sudo(self, mock_add_user, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', '--user-name', 'banana', '--resource-class', 'compute', diff --git a/metalsmith/test/test_config.py b/metalsmith/test/test_instance_config.py similarity index 64% rename from metalsmith/test/test_config.py rename to metalsmith/test/test_instance_config.py index 634736a..ffa819a 100644 --- a/metalsmith/test/test_config.py +++ b/metalsmith/test/test_instance_config.py @@ -19,17 +19,21 @@ import mock from openstack.baremetal import configdrive import testtools -from metalsmith import _config +import metalsmith from metalsmith import _utils +from metalsmith import instance_config -class TestInstanceConfig(testtools.TestCase): +class TestGenericConfig(testtools.TestCase): + CLASS = instance_config.GenericConfig + def setUp(self): - super(TestInstanceConfig, self).setUp() + super(TestGenericConfig, self).setUp() self.node = mock.Mock(id='1234') self.node.name = 'node name' - def _check(self, config, expected_metadata, expected_userdata=None): + def _check(self, config, expected_metadata, expected_userdata=None, + cloud_init=True): expected_m = {'public_keys': {}, 'uuid': '1234', 'name': 'node name', @@ -51,38 +55,47 @@ class TestInstanceConfig(testtools.TestCase): if expected_userdata: self.assertIsNotNone(user_data) user_data = user_data.decode('utf-8') - header, user_data = user_data.split('\n', 1) - self.assertEqual('#cloud-config', header) + if cloud_init: + header, user_data = user_data.split('\n', 1) + self.assertEqual('#cloud-config', header) user_data = json.loads(user_data) self.assertEqual(expected_userdata, user_data) def test_default(self): - config = _config.InstanceConfig() + config = self.CLASS() self._check(config, {}) def test_ssh_keys(self): - config = _config.InstanceConfig(ssh_keys=['abc', 'def']) + config = self.CLASS(ssh_keys=['abc', 'def']) self._check(config, {'public_keys': {'0': 'abc', '1': 'def'}}) def test_ssh_keys_as_dict(self): - config = _config.InstanceConfig(ssh_keys={'default': 'abc'}) + config = self.CLASS(ssh_keys={'default': 'abc'}) self._check(config, {'public_keys': {'default': 'abc'}}) + def test_custom_user_data(self): + config = self.CLASS(user_data='{"answer": 42}') + self._check(config, {}, {"answer": 42}, cloud_init=False) + + +class TestCloudInitConfig(TestGenericConfig): + CLASS = instance_config.CloudInitConfig + def test_add_user(self): - config = _config.InstanceConfig() + config = self.CLASS() config.add_user('admin') self._check(config, {}, {'users': [{'name': 'admin', 'groups': ['wheel']}]}) def test_add_user_admin(self): - config = _config.InstanceConfig() + config = self.CLASS() config.add_user('admin', admin=False) self._check(config, {}, {'users': [{'name': 'admin'}]}) def test_add_user_sudo(self): - config = _config.InstanceConfig() + config = self.CLASS() config.add_user('admin', sudo=True) self._check(config, {}, {'users': [{'name': 'admin', @@ -90,7 +103,7 @@ class TestInstanceConfig(testtools.TestCase): 'sudo': 'ALL=(ALL) NOPASSWD:ALL'}]}) def test_add_user_passwd(self): - config = _config.InstanceConfig() + config = self.CLASS() config.add_user('admin', password_hash='123') self._check(config, {}, {'users': [{'name': 'admin', @@ -98,9 +111,33 @@ class TestInstanceConfig(testtools.TestCase): 'passwd': '123'}]}) def test_add_user_with_keys(self): - config = _config.InstanceConfig(ssh_keys=['abc', 'def']) + config = self.CLASS(ssh_keys=['abc', 'def']) config.add_user('admin') self._check(config, {'public_keys': {'0': 'abc', '1': 'def'}}, {'users': [{'name': 'admin', 'groups': ['wheel'], 'ssh_authorized_keys': ['abc', 'def']}]}) + + # Overriding tests since CloudInitConfig does not support plain strings + # for user_data, only dictionaries. + def test_custom_user_data(self): + config = self.CLASS(user_data={'answer': 42}) + self._check(config, {}, {'answer': 42}) + + def test_custom_user_data_with_users(self): + config = self.CLASS(user_data={'answer': 42}) + config.add_user('admin') + self._check(config, {}, + {'users': [{'name': 'admin', + 'groups': ['wheel']}], + 'answer': 42}) + + def test_user_data_not_dict(self): + self.assertRaises(TypeError, self.CLASS, user_data="string") + config = self.CLASS() + config.user_data = "string" + self.assertRaises(TypeError, config.populate_user_data) + + +class TestDeprecatedInstanceConfig(TestCloudInitConfig): + CLASS = metalsmith.InstanceConfig diff --git a/metalsmith/test/test_provisioner.py b/metalsmith/test/test_provisioner.py index e8d9ca4..3adf0ea 100644 --- a/metalsmith/test/test_provisioner.py +++ b/metalsmith/test/test_provisioner.py @@ -19,11 +19,11 @@ from openstack import exceptions as os_exc import requests import testtools -from metalsmith import _config from metalsmith import _instance from metalsmith import _provisioner from metalsmith import _utils from metalsmith import exceptions +from metalsmith import instance_config from metalsmith import sources @@ -395,7 +395,7 @@ class TestProvisionNode(Base): ], } self.configdrive_mock = self.useFixture( - fixtures.MockPatchObject(_config.InstanceConfig, + fixtures.MockPatchObject(instance_config.GenericConfig, 'generate', autospec=True) ).mock @@ -459,7 +459,7 @@ class TestProvisionNode(Base): self.assertFalse(self.api.network.delete_port.called) def test_with_config(self): - config = mock.Mock(spec=_config.InstanceConfig) + config = mock.Mock(spec=instance_config.GenericConfig) inst = self.pr.provision_node(self.node, 'image', [{'network': 'network'}], config=config) diff --git a/releasenotes/notes/instance-config-c4ea6a169f782138.yaml b/releasenotes/notes/instance-config-c4ea6a169f782138.yaml new file mode 100644 index 0000000..c6c02b0 --- /dev/null +++ b/releasenotes/notes/instance-config-c4ea6a169f782138.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + It is now possible to provide custom ``user_data`` into instance + configuration. +upgrade: + - | + The ``InstanceConfig`` class has been split into ``GenericConfig`` and + ``CloudInitConfig`` for clarity on which features come from what. +deprecations: + - | + The ``metalsmith.InstanceConfig`` class is deprecated, use + ``GenericConfig`` or ``CloudInitConfig`` from the new module + ``metalsmith.instance_config``.