diff --git a/devstack/lib/ironic b/devstack/lib/ironic index aa0b4fcdb4..25cb2a8868 100644 --- 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 @@ -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 diff --git a/etc/ironic/rootwrap.conf b/etc/ironic/rootwrap.conf index 8065c57571..607d59b4ae 100644 --- 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 ','). diff --git a/etc/ironic/rootwrap.d/ironic-utils.filters b/etc/ironic/rootwrap.d/ironic-utils.filters index e1ac730d8b..3166b22b45 100644 --- 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. diff --git a/ironic/common/images.py b/ironic/common/images.py index aa883ada3b..34018633f9 100644 --- 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, diff --git a/ironic/common/utils.py b/ironic/common/utils.py index efd3049a95..6500b6f685 100644 --- 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. diff --git a/ironic/conf/default.py b/ironic/conf/default.py index e5e9af35b8..b92f76cd64 100644 --- a/ironic/conf/default.py +++ b/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(), diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 18a506869b..d5e4f773bd 100644 --- 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__) diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index 5530feb38c..5c8e106336 100644 --- 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) diff --git a/ironic/tests/unit/common/test_utils.py b/ironic/tests/unit/common/test_utils.py index 96c6612d61..7c270e9767 100644 --- 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) diff --git a/releasenotes/notes/no-root-8127c35b4702d242.yaml b/releasenotes/notes/no-root-8127c35b4702d242.yaml new file mode 100644 index 0000000000..2234237859 --- /dev/null +++ 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. diff --git a/requirements.txt b/requirements.txt index 7df883dc07..2545ce1948 100644 --- 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 diff --git a/setup.cfg b/setup.cfg index 7abf7b3e23..611eb7a710 100644 --- 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