Merge "Drop rootwrap support"

2024-01-23 20:14:36 +00:00 · 2024-01-23 20:14:36 +00:00 · cd17f5e61a
commit cd17f5e61a
parent 13d6b02716 be09717be2
12 changed files with 16 additions and 95 deletions
--- a/devstack/lib/ironic
+++ b/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
@ -1686,24 +1685,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

@ -1764,7 +1745,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
--- a/etc/ironic/rootwrap.conf
+++ b/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 ',').
--- a/etc/ironic/rootwrap.d/ironic-utils.filters
+++ b/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
-
-[Filters]
-# ironic/common/utils.py
-mount: CommandFilter, mount, root
-umount: CommandFilter, umount, root
+# DEPRECATED for removal: Ironic no longer needs root.
--- a/ironic/common/images.py
+++ b/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,
--- a/ironic/common/utils.py
+++ b/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.

--- a/ironic/conf/default.py
+++ b/ironic/conf/default.py
@ -413,10 +413,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(),
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/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__)

--- a/ironic/tests/unit/common/test_images.py
+++ b/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, get_iso_files_mock):
-        get_iso_files_mock.side_effect = processutils.ProcessExecutionError
-        self.assertRaises(exception.ImageCreationFailed,
-                          images._get_deploy_iso_files,
-                          'path/to/deployiso', 'tmpdir1')
+    def test__get_deploy_iso_files_fail_with_ExecutionError(self):
+        self.assertRaisesRegex(exception.ImageCreationFailed,
+                               'No such file or directory',
+                               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)
--- a/ironic/tests/unit/common/test_utils.py
+++ b/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)
--- a/releasenotes/notes/no-root-8127c35b4702d242.yaml
+++ b/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.
--- a/requirements.txt
+++ b/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
--- a/setup.cfg
+++ b/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