Support Automatic Lessee from instance metadata

Ironic already has support for automatically setting a lessee on
deployment, but it is only supported for direct deployments with Ironic,
as it uses request context which is not preserved in the Nova driver.

Now, when combined with the related Nova change, Ironic can support this
behavior for fully integrated installations. On deploy time, Nova will
set several fields -- including project_id -- in instance info. If
enabled, Ironic will then use that project_id as the automatic lessee.
The previous behavior of using the project_id from the request context
is still supported as a fallback.

This is being tracked in nova as blueprint ironic-guest-metadata.

Closes-Bug: #2063352
Change-Id: Id381a3d201c2f1b137279decc0e32096d4d95012

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:
-        # 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:

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,

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',

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.