diff --git a/doc/source/admin/secure-rbac.rst b/doc/source/admin/secure-rbac.rst index 3e431e4eb5..ae67724826 100644 --- a/doc/source/admin/secure-rbac.rst +++ b/doc/source/admin/secure-rbac.rst @@ -257,6 +257,25 @@ How do I assign a lessee? # baremetal node set --lessee +Ironic will, by default, automatically manage lessee at deployment time, +setting the lessee field on deploy of a node and unset it before the node +begins cleaning. + +Operators can customize or disable this behavior via +:oslo.config:option:`conductor.automatic_lessee_source` configuration. + +If :oslo.config:option:`conductor.automatic_lessee_source` is set to +``instance`` (the default), this uses ``node.instance_info['project_id']``, +which is set when OpenStack Nova deploys an instance. + +If :oslo.config:option:`conductor.automatic_lessee_source` is set to +``request``, the lessee is set to the project_id in the request context -- +ideal for standalone Ironic deployments still utilizing OpenStack Keystone. + +If :oslo.config:option:`conductor.automatic_lessee_source` is set to to +``none``, Ironic not will set a lessee on deploy. + + What is the difference between an owner and lessee? --------------------------------------------------- diff --git a/ironic/common/lessee_sources.py b/ironic/common/lessee_sources.py new file mode 100644 index 0000000000..785200adba --- /dev/null +++ b/ironic/common/lessee_sources.py @@ -0,0 +1,28 @@ +# All Rights Reserved. +# +# 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. + + +""" +Mapping of values for CONF.conductor.automatic_lessee_source, representing +different possible sources for lessee data. +""" + +NONE = 'none' +"Do not set lessee" + +REQUEST = 'request' +"Use metadata in request context" + +INSTANCE = 'instance' +"Use instance_info[\'project_id\']" diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index a14898ede4..6ac51c7b37 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -23,6 +23,7 @@ from ironic.common import async_steps from ironic.common import exception from ironic.common.glance_service import service_utils as glance_utils from ironic.common.i18n import _ +from ironic.common import lessee_sources from ironic.common import states from ironic.common import swift from ironic.conductor import notification_utils as notify_utils @@ -64,6 +65,75 @@ def validate_node(task, event='deploy'): op=_('provisioning')) +def apply_automatic_lessee(task): + """Apply a automatic lessee to the node, if applicable + + First of all, until removed next cycle, we check to see if + CONF.automatic_lessee was explicitly set "False" by an operator -- if so, + we do not apply a lessee. + + When CONF.conductor.automatic_lessee_source is instance: + - Take the lessee from instance_info[project_id] (e.g. as set by nova) + + When CONF.conductor.automatic_lessee_source is request: + - Take the lessee from request context (e.g. from keystone) + + When CONF.conductor.automatic_lessee_source is none: + OR the legacy CONF.automatic_lessee is explicitly set by an operator to + False (regardless of lessee_source) + - Don't apply a lessee to the node + + :param task: a TaskManager instance. + :returns: True if node had a lessee applied + """ + node = task.node + applied = False + # TODO(JayF): During 2025.1 cycle, remove automatic_lessee boolean config. + if CONF.conductor.automatic_lessee: + project = None + if CONF.conductor.automatic_lessee_source == lessee_sources.REQUEST: + project = utils.get_token_project_from_request(task.context) + if project is None: + LOG.debug('Could not automatically save lessee: No project ' + 'found in request context for node %(uuid)s.', + {'uuid': node.uuid}) + + elif CONF.conductor.automatic_lessee_source == lessee_sources.INSTANCE: + # NOTE(JayF): If we have a project_id explicitly set (typical nova + # case), use it. Otherwise, try to derive it from the context of + # the request (typical standalone+keystone) case. + project = node.instance_info.get('project_id') + if project is None: + LOG.debug('Could not automatically save lessee: node[' + '\'instance_info\'][\'project_id\'] is unset for ' + 'node %(uuid)s.', + {'uuid': node.uuid}) + + # NOTE(JayF): the CONF.conductor.automatic_lessee_source == 'none' + # falls through since project will never be set. + if project: + if node.lessee is None: + LOG.debug('Adding lessee %(project)s to node %(uuid)s.', + {'project': project, + 'uuid': node.uuid}) + node.set_driver_internal_info('automatic_lessee', True) + node.lessee = project + applied = True + else: + # Since the model is a bit of a matrix and we're largely + # just empowering operators, lets at least log a warning + # since they may need to remedy something here. Or maybe + # not. + LOG.warning('Could not automatically save lessee ' + '%(project)s to node %(uuid)s. Node already ' + 'has a defined lessee of %(lessee)s.', + {'project': project, + 'uuid': node.uuid, + 'lessee': node.lessee}) + + return applied + + @METRICS.timer('start_deploy') @task_manager.require_exclusive_lock def start_deploy(task, manager, configdrive=None, event='deploy', @@ -92,31 +162,14 @@ def start_deploy(task, manager, configdrive=None, event='deploy', instance_info.pop('kernel', None) instance_info.pop('ramdisk', None) node.instance_info = instance_info - elif CONF.conductor.automatic_lessee: - # This should only be on deploy... - project = utils.get_token_project_from_request(task.context) - if (project and node.lessee is None): - LOG.debug('Adding lessee $(project)s to node %(uuid)s.', - {'project': project, - 'uuid': node.uuid}) - node.set_driver_internal_info('automatic_lessee', True) - node.lessee = project - elif project and node.lessee is not None: - # Since the model is a bit of a matrix and we're largely - # just empowering operators, lets at least log a warning - # since they may need to remedy something here. Or maybe - # not. - LOG.warning('Could not automatically save lessee ' - '$(project)s to node %(uuid)s. Node already ' - 'has a defined lessee of %(lessee)s.', - {'project': project, - 'uuid': node.uuid, - 'lessee': node.lessee}) + else: + # NOTE(JayF): Don't apply lessee when rebuilding + auto_lessee = apply_automatic_lessee(task) # Infer the image type to make sure the deploy driver # validates only the necessary variables for different # image types. - if utils.update_image_type(task.context, task.node): + if utils.update_image_type(task.context, task.node) or auto_lessee: node.save() try: diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index bdff2e172d..809c48cf79 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -20,6 +20,7 @@ from oslo_config import types from ironic.common import boot_modes from ironic.common.i18n import _ +from ironic.common import lessee_sources opts = [ @@ -345,15 +346,36 @@ opts = [ 'for multiple steps. If set to 0, this specific step ' 'will not run during verification. ')), cfg.BoolOpt('automatic_lessee', - default=False, + default=True, mutable=True, - help=_('If the conductor should record the Project ID ' - 'indicated by Keystone for a requested deployment. ' - 'Allows rights to be granted to directly access the ' - 'deployed node as a lessee within the RBAC security ' - 'model. The conductor does *not* record this value ' - 'otherwise, and this information is not backfilled ' - 'for prior instances which have been deployed.')), + help=_('Deprecated. If Ironic should set the node.lessee ' + 'field at deployment. Use ' + '[\'conductor\']/automatic_lessee_source instead.'), + deprecated_for_removal=True), + cfg.StrOpt('automatic_lessee_source', + help=_('Source for Project ID the Ironic should ' + 'record at deployment time in node.lessee field. If set ' + 'to none, Ironic will not set a lessee field. ' + 'If set to instance (default), uses Project ID ' + 'indicated in instance metadata set by Nova or ' + 'another external deployment service. ' + 'If set to keystone, Ironic uses Project ID indicated ' + 'by Keystone context. '), + choices=[ + (lessee_sources.INSTANCE, _( # 'instance' + 'Populates node.lessee field using metadata from ' + 'node.instance_info[\'project_id\'] at deployment ' + 'time. Useful for Nova-fronted deployments.')), + (lessee_sources.REQUEST, _( # 'request' + 'Populates node.lessee field using metadata ' + 'from request context. Only useful for direct ' + 'deployment requests to Ironic; not those proxied ' + 'via an external service like Nova.')), + (lessee_sources.NONE, _( # 'none' + 'Ironic will not populate the node.lessee field.')) + ], + default='instance', + mutable=True), cfg.IntOpt('max_concurrent_deploy', default=250, min=1, diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index f073460853..c96a6fae1a 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -21,6 +21,7 @@ from oslo_utils import uuidutils from ironic.common import exception from ironic.common import images +from ironic.common import lessee_sources from ironic.common import states from ironic.common import swift from ironic.conductor import deployments @@ -391,19 +392,29 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_validate_deploy_user_steps_and_templates, mock_deploy_validate, mock_power_validate, mock_process_event, - automatic_lessee=False): + automatic_lessee=None, iinfo=None, + automatic_lessee_source=None): self.context.auth_token_info = { 'token': {'project': {'id': 'user-project'}} } - if automatic_lessee: - self.config(automatic_lessee=True, group='conductor') + if iinfo is None: + iinfo = {} + + if automatic_lessee is not None: + self.config(automatic_lessee=automatic_lessee, group='conductor') + + if automatic_lessee_source is not None: + self.config(automatic_lessee_source=automatic_lessee_source, + group='conductor') + self._start_service() mock_iwdi.return_value = False deploy_steps = [{"interface": "bios", "step": "factory_reset", "priority": 95}] node = obj_utils.create_test_node(self.context, driver='fake-hardware', provision_state=states.AVAILABLE, - target_provision_state=states.ACTIVE) + target_provision_state=states.ACTIVE, + instance_info=iinfo) task = task_manager.TaskManager(self.context, node.uuid) deployments.start_deploy(task, self.service, configdrive=None, @@ -422,18 +433,87 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock.ANY, 'deploy', call_args=( deployments.do_node_deploy, task, 1, None, deploy_steps), callback=mock.ANY, err_handler=mock.ANY) - if automatic_lessee: - self.assertEqual('user-project', node.lessee) + expected_project = iinfo.get('project_id', 'user-project') + if (automatic_lessee_source not in [None, lessee_sources.NONE] + and automatic_lessee in [None, True]): + self.assertEqual(expected_project, node.lessee) self.assertIn('automatic_lessee', node.driver_internal_info) else: self.assertIsNone(node.lessee) self.assertNotIn('automatic_lessee', node.driver_internal_info) - def test_start_deploy(self): + def test_start_deploy_lessee_legacy_false(self): self._test_start_deploy(automatic_lessee=False) - def test_start_deploy_records_lessee(self): - self._test_start_deploy(automatic_lessee=True) + def test_start_deploy_lessee_source_none(self): + self._test_start_deploy(automatic_lessee_source=lessee_sources.NONE) + + def test_start_deploy_lessee_source_request(self): + """Validates that project_id from request context is the lessee.""" + self._test_start_deploy(automatic_lessee_source=lessee_sources.REQUEST) + + def test_start_deploy_lessee_source_instance(self): + """Validates that project_id from instance info is the lessee.""" + self._test_start_deploy( + automatic_lessee_source=lessee_sources.INSTANCE, + iinfo={'project_id': 'user-project-iinfo'}) + + @mock.patch.object(task_manager.TaskManager, 'process_event', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate', + autospec=True) + @mock.patch('ironic.drivers.modules.fake.FakeDeploy.validate', + autospec=True) + @mock.patch.object(conductor_steps, + 'validate_user_deploy_steps_and_templates', + autospec=True) + @mock.patch.object(conductor_utils, 'validate_instance_info_traits', + autospec=True) + @mock.patch.object(images, 'is_whole_disk_image', autospec=True) + def test_start_deploy_lessee_legacy_false_even_if_src_set( + self, mock_iwdi, mock_validate_traits, + mock_validate_deploy_user_steps_and_templates, + mock_deploy_validate, mock_power_validate, mock_process_event): + self.context.auth_token_info = { + 'token': {'project': {'id': 'user-project'}} + } + iinfo = {'project_id': 'user-project-iinfo'} + + # Legacy lessee is disabled + self.config(automatic_lessee=False, group='conductor') + # but source is also set -- we should respect the disablement above all + self.config(automatic_lessee_source=lessee_sources.INSTANCE, + group='conductor') + + self._start_service() + mock_iwdi.return_value = False + deploy_steps = [{"interface": "bios", "step": "factory_reset", + "priority": 95}] + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + provision_state=states.AVAILABLE, + target_provision_state=states.ACTIVE, + instance_info=iinfo) + task = task_manager.TaskManager(self.context, node.uuid) + + deployments.start_deploy(task, self.service, configdrive=None, + event='deploy', deploy_steps=deploy_steps) + node.refresh() + mock_iwdi.assert_called_once_with(task.context, + task.node.instance_info) + self.assertFalse(node.driver_internal_info['is_whole_disk_image']) + self.assertEqual('partition', node.instance_info['image_type']) + mock_power_validate.assert_called_once_with(task.driver.power, task) + mock_deploy_validate.assert_called_once_with(task.driver.deploy, task) + mock_validate_traits.assert_called_once_with(task.node) + mock_validate_deploy_user_steps_and_templates.assert_called_once_with( + task, deploy_steps, skip_missing=True) + mock_process_event.assert_called_with( + mock.ANY, 'deploy', call_args=( + deployments.do_node_deploy, task, 1, None, deploy_steps), + callback=mock.ANY, err_handler=mock.ANY) + + self.assertIsNone(node.lessee) + self.assertNotIn('automatic_lessee', node.driver_internal_info) @mock.patch.object(images, 'is_source_a_path', autospec=True) @mock.patch.object(task_manager.TaskManager, 'process_event', diff --git a/releasenotes/notes/automatic-lessee-source-37abe917b8cb5c36.yaml b/releasenotes/notes/automatic-lessee-source-37abe917b8cb5c36.yaml new file mode 100644 index 0000000000..69b9937c11 --- /dev/null +++ b/releasenotes/notes/automatic-lessee-source-37abe917b8cb5c36.yaml @@ -0,0 +1,24 @@ +features: + - | + Ironic now supports automatically setting node.lessee at deployment time + using metadata provided at deploy time, typically by OpenStack Nova. When + ``[conductor]/automatic_lessee_source`` is set to ``instance``, + Ironic will set the lessee field on the node and remove it before cleaning. +upgrade: + - | + ``[conductor]/automatic_lessee`` has been deprecated in favor of + ``[conductor]/automatic_lessee_source``. + + Standalone Ironic deployments previously setting ``automatic_lessee`` to + ``True`` now may want to set ``automatic_lessee_source`` to ``request`` to + retain existing behavior. + + Deployers explicitly setting ``automatic_lessee`` to false may want to set + ``automatic_lessee_source`` to ``none`` to retain existing behavior. The + old configuration option, when explicitly set, will be honored until + fully removed. + - | + Ironic will now automatically set the node.lessee field for all + deployments by default when provided in node instance_info at deployment + time. Deployers are encouraged to review their security settings and + Ironic Secure RBAC documentation to ensure no unexpected access is granted. \ No newline at end of file