Merge "Support Automatic Lessee from instance metadata"

doc/source/admin/secure-rbac.rst

@@ -257,6 +257,25 @@ How do I assign a lessee?
 
    # baremetal node set --lessee <project_id> <node>
 
+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?
 ---------------------------------------------------
 

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\']"

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:
+    else:
-        # This should only be on deploy...
+        # NOTE(JayF): Don't apply lessee when rebuilding
-        project = utils.get_token_project_from_request(task.context)
+        auto_lessee = apply_automatic_lessee(task)
-        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})
 
     # 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:

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 '
+                help=_('Deprecated. If Ironic should set the node.lessee '
-                       'indicated by Keystone for a requested deployment. '
+                       'field at deployment. Use '
-                       'Allows rights to be granted to directly access the '
+                       '[\'conductor\']/automatic_lessee_source instead.'),
-                       'deployed node as a lessee within the RBAC security '
+                deprecated_for_removal=True),
-                       'model. The conductor does *not* record this value '
+    cfg.StrOpt('automatic_lessee_source',
-                       'otherwise, and this information is not backfilled '
+               help=_('Source for Project ID the Ironic should '
-                       'for prior instances which have been deployed.')),
+                      '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,

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:
+        if iinfo is None:
-            self.config(automatic_lessee=True, group='conductor')
+            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:
+        expected_project = iinfo.get('project_id', 'user-project')
-            self.assertEqual('user-project', node.lessee)
+        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):
+    def test_start_deploy_lessee_source_none(self):
-        self._test_start_deploy(automatic_lessee=True)
+        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',

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.