Drop rootwrap support

After removing the iSCSI deploy and changing ISO parsing code to use
a corresponding library, Ironic no longer executes any commands as root
and it should stay this way.

Change-Id: I47d2bab9b94345fbcf89a2a80028853050a041ea

devstack/lib/ironic

@@ -74,7 +74,6 @@ IRONIC_STATE_PATH=/var/lib/ironic
 IRONIC_AUTH_CACHE_DIR=${IRONIC_AUTH_CACHE_DIR:-/var/cache/ironic}
 IRONIC_CONF_DIR=${IRONIC_CONF_DIR:-/etc/ironic}
 IRONIC_CONF_FILE=$IRONIC_CONF_DIR/ironic.conf
-IRONIC_ROOTWRAP_CONF=$IRONIC_CONF_DIR/rootwrap.conf
 # Deploy Ironic API under uwsgi (NOT mod_wsgi) server.
 # Devstack aims to remove mod_wsgi support, so ironic shouldn't use it too.
 # If set to False that will fall back to use the eventlet server that
@@ -1678,24 +1677,6 @@ function configure_ironic_conductor {
         configure_client_for $conf_section
     done
 
-    configure_rootwrap ironic
-
-    # additional rootwrap config from ironic-lib
-    local ironic_lib_prefix
-    if use_library_from_git "ironic-lib"; then
-        ironic_lib_prefix=${GITDIR["ironic-lib"]}
-    else
-        # pip uses default python 'data' path
-        ironic_lib_prefix=$(python3 -c "import sysconfig; \
-            print(sysconfig.get_path('data'))")
-
-        # on Centos7 the data is installed to /usr/local
-        if [ ! -d $ironic_lib_prefix/etc/ironic/rootwrap.d ]; then
-            ironic_lib_prefix=/usr/local
-        fi
-    fi
-    sudo install -o root -g root -m 644 $ironic_lib_prefix/etc/ironic/rootwrap.d/*.filters /etc/ironic/rootwrap.d
-
     # set up drivers / hardware types
     iniset $IRONIC_CONF_FILE DEFAULT enabled_hardware_types $IRONIC_ENABLED_HARDWARE_TYPES
 
@@ -1756,7 +1737,6 @@ function configure_ironic_conductor {
         iniset $IRONIC_CONF_FILE pxe loader_file_paths $IRONIC_LOADER_PATHS
     fi
 
-    iniset $IRONIC_CONF_FILE DEFAULT rootwrap_config $IRONIC_ROOTWRAP_CONF
     iniset $IRONIC_CONF_FILE service_catalog endpoint_override "$IRONIC_SERVICE_PROTOCOL://$([[ $IRONIC_HTTP_SERVER =~ : ]] && echo "[$IRONIC_HTTP_SERVER]" || echo $IRONIC_HTTP_SERVER)/baremetal"
     if [[ -n "$IRONIC_CALLBACK_TIMEOUT" ]]; then
         iniset $IRONIC_CONF_FILE conductor deploy_callback_timeout $IRONIC_CALLBACK_TIMEOUT

etc/ironic/rootwrap.conf

@@ -1,5 +1,6 @@
 # Configuration for ironic-rootwrap
 # This file should be owned by (and only writable by) the root user
+# DEPRECATED for removal: Ironic no longer needs root.
 
 [DEFAULT]
 # List of directories to load filter definitions from (separated by ',').

etc/ironic/rootwrap.d/ironic-utils.filters

@@ -1,7 +1,3 @@
 # ironic-rootwrap command filters for disk manipulation
 # This file should be owned by (and only-writable by) the root user
-
+# DEPRECATED for removal: Ironic no longer needs root.
-[Filters]
-# ironic/common/utils.py
-mount: CommandFilter, mount, root
-umount: CommandFilter, umount, root

ironic/common/images.py

@@ -715,7 +715,7 @@ def _get_deploy_iso_files(deploy_iso, mountdir):
 
     :param deploy_iso: path to the deploy iso where its
                        contents are fetched to.
-    :raises: ImageCreationFailed if mount fails.
+    :raises: ImageCreationFailed if extraction fails.
     :returns: a tuple consisting of - 1. a dictionary containing
                                          the values as required
                                          by create_isolinux_image,

ironic/common/utils.py

@@ -60,13 +60,6 @@ DATETIME_RE = re.compile(
 USING_SQLITE = None
 
 
-def _get_root_helper():
-    # NOTE(jlvillal): This function has been moved to ironic-lib. And is
-    # planned to be deleted here. If need to modify this function, please
-    # also do the same modification in ironic-lib
-    return 'sudo ironic-rootwrap %s' % CONF.rootwrap_config
-
-
 def execute(*cmd, **kwargs):
     """Convenience wrapper around oslo's execute() method.
 
@@ -84,8 +77,6 @@ def execute(*cmd, **kwargs):
         env = kwargs.pop('env_variables', os.environ.copy())
         env['LC_ALL'] = 'C'
         kwargs['env_variables'] = env
-    if kwargs.get('run_as_root') and 'root_helper' not in kwargs:
-        kwargs['root_helper'] = _get_root_helper()
     result = processutils.execute(*cmd, **kwargs)
     LOG.debug('Execution completed, command line is "%s"',
               ' '.join(map(str, cmd)))
@@ -318,33 +309,6 @@ def safe_rstrip(value, chars=None):
     return value.rstrip(chars) or value
 
 
-def mount(src, dest, *args):
-    """Mounts a device/image file on specified location.
-
-    :param src: the path to the source file for mounting
-    :param dest: the path where it needs to be mounted.
-    :param args: a tuple containing the arguments to be
-        passed to mount command.
-    :raises: processutils.ProcessExecutionError if it failed
-        to run the process.
-    """
-    args = ('mount', ) + args + (src, dest)
-    execute(*args, run_as_root=True)
-
-
-def umount(loc, *args):
-    """Umounts a mounted location.
-
-    :param loc: the path to be unmounted.
-    :param args: a tuple containing the arguments to be
-        passed to the umount command.
-    :raises: processutils.ProcessExecutionError if it failed
-        to run the process.
-    """
-    args = ('umount', ) + args + (loc, )
-    execute(*args, run_as_root=True)
-
-
 def check_dir(directory_to_check=None, required_space=1):
     """Check a directory is usable.
 

ironic/conf/default.py

@@ -408,10 +408,6 @@ service_opts = [
 ]
 
 utils_opts = [
-    cfg.StrOpt('rootwrap_config',
-               default="/etc/ironic/rootwrap.conf",
-               help=_('Path to the rootwrap configuration file to use for '
-                      'running commands as root.')),
     cfg.StrOpt('tempdir',
                default=tempfile.gettempdir(),
                sample_default=tempfile.gettempdir(),

ironic/drivers/modules/deploy_utils.py

@@ -41,14 +41,6 @@ from ironic.drivers.modules import image_cache
 from ironic.drivers import utils as driver_utils
 from ironic import objects
 
-# TODO(Faizan): Move this logic to common/utils.py and deprecate
-# rootwrap_config.
-# This is required to set the default value of ironic_lib option
-# only if rootwrap_config does not contain the default value.
-
-if CONF.rootwrap_config != '/etc/ironic/rootwrap.conf':
-    root_helper = 'sudo ironic-rootwrap %s' % CONF.rootwrap_config
-    CONF.set_default('root_helper', root_helper, 'ironic_lib')
 
 LOG = logging.getLogger(__name__)
 

ironic/tests/unit/common/test_images.py

@@ -519,13 +519,11 @@ class FsImageTestCase(base.TestCase):
         walk_mock.assert_called_once_with('tmpdir1')
         rmtree_mock.assert_called_once_with('tmpdir1')
 
-    @mock.patch.object(utils, 'mount', autospec=True)
+    def test__get_deploy_iso_files_fail_with_ExecutionError(self):
-    def test__get_deploy_iso_files_fail_with_ExecutionError(
+        self.assertRaisesRegex(exception.ImageCreationFailed,
-            self, get_iso_files_mock):
+                               'No such file or directory',
-        get_iso_files_mock.side_effect = processutils.ProcessExecutionError
+                               images._get_deploy_iso_files,
-        self.assertRaises(exception.ImageCreationFailed,
+                               'path/to/deployiso', 'tmpdir1')
-                          images._get_deploy_iso_files,
-                          'path/to/deployiso', 'tmpdir1')
 
     @mock.patch.object(shutil, 'rmtree', autospec=True)
     @mock.patch.object(images, '_create_root_fs', autospec=True)

ironic/tests/unit/common/test_utils.py

@@ -76,20 +76,6 @@ class ExecuteTestCase(base.TestCase):
         execute_mock.assert_called_once_with('foo',
                                              env_variables={'foo': 'bar'})
 
-    def test_execute_get_root_helper(self):
-        with mock.patch.object(
-                processutils, 'execute', autospec=True) as execute_mock:
-            helper = utils._get_root_helper()
-            utils.execute('foo', run_as_root=True)
-            execute_mock.assert_called_once_with('foo', run_as_root=True,
-                                                 root_helper=helper)
-
-    def test_execute_without_root_helper(self):
-        with mock.patch.object(
-                processutils, 'execute', autospec=True) as execute_mock:
-            utils.execute('foo', run_as_root=False)
-            execute_mock.assert_called_once_with('foo', run_as_root=False)
-
 
 class GenericUtilsTestCase(base.TestCase):
     @mock.patch.object(utils, 'hashlib', autospec=True)

releasenotes/notes/no-root-8127c35b4702d242.yaml

@@ -0,0 +1,6 @@
+---
+deprecations:
+  - |
+    Rootwrap support is deprecated since Ironic no longer runs any commands as
+    root. Files ``/etc/ironic/rootwrap.conf``, ``/etc/ironic/rootwrap.d``
+    and the ``ironic-rootwrap`` command will be removed in a future release.

requirements.txt

@@ -21,6 +21,7 @@ oslo.concurrency>=4.2.0 # Apache-2.0
 oslo.config>=6.8.0 # Apache-2.0
 oslo.context>=2.22.0 # Apache-2.0
 oslo.db>=9.1.0 # Apache-2.0
+# TODO(dtantsur): remove rootwrap when we no longer provide ironic-rootwrap CLI
 oslo.rootwrap>=5.8.0 # Apache-2.0
 oslo.log>=4.3.0 # Apache-2.0
 oslo.middleware>=3.31.0 # Apache-2.0

setup.cfg

@@ -22,6 +22,7 @@ classifier =
     Programming Language :: Python :: 3.11
 
 [files]
+# TODO(dtantsur): remove rootwrap files after the packagers drop them.
 data_files =
     etc/ironic =
         etc/ironic/rootwrap.conf