diff options
| -rw-r--r-- | doc/source/cli/index.rst | 1 | ||||
| -rw-r--r-- | doc/source/cli/nova-idmapshift.rst | 92 | ||||
| -rw-r--r-- | doc/source/conf.py | 1 | ||||
| -rw-r--r-- | etc/nova/rootwrap.d/compute.filters | 3 | ||||
| -rw-r--r-- | nova/privsep/idmapshift.py (renamed from nova/cmd/idmapshift.py) | 121 | ||||
| -rw-r--r-- | nova/tests/unit/privsep/test_idmapshift.py (renamed from nova/tests/unit/cmd/test_idmapshift.py) | 251 | ||||
| -rw-r--r-- | nova/tests/unit/virt/libvirt/test_utils.py | 24 | ||||
| -rw-r--r-- | nova/virt/libvirt/utils.py | 18 | ||||
| -rw-r--r-- | releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml | 7 | ||||
| -rw-r--r-- | setup.cfg | 1 |
10 files changed, 34 insertions, 485 deletions
diff --git a/doc/source/cli/index.rst b/doc/source/cli/index.rst index 70df266db9..adddfa9e4b 100644 --- a/doc/source/cli/index.rst +++ b/doc/source/cli/index.rst @@ -78,7 +78,6 @@ are documented for completeness and debugging if something goes wrong. :maxdepth: 1 nova-rootwrap - nova-idmapshift Deprecated Services ------------------- diff --git a/doc/source/cli/nova-idmapshift.rst b/doc/source/cli/nova-idmapshift.rst deleted file mode 100644 index b747e2de25..0000000000 --- a/doc/source/cli/nova-idmapshift.rst +++ /dev/null @@ -1,92 +0,0 @@ -=============== -nova-idmapshift -=============== - ------------------------------------------ -Tool used by Nova libvirt-lxc virt driver ------------------------------------------ - -:Author: openstack@lists.openstack.org -:Date: 2012-09-27 -:Copyright: OpenStack Foundation -:Version: 2012.1 -:Manual section: 1 -:Manual group: cloud computing - -SYNOPSIS -======== - -nova-idmapshift [options] path - -DESCRIPTION -=========== - -nova-idmapshift is a tool that properly sets the ownership of a filesystem for use -with linux user namespaces. This tool can only be used with linux lxc containers. - -When using user namespaces with linux lxc containers, the filesystem of the -container must be owned by the targeted user and group ids being applied -to that container. Otherwise, processes inside the container won't be able -to access the filesystem. - -For example: -nova-idmapshift -i -u 0:10000:2000 -g 0:10000:2000 path - -This command will idempotently shift `path` to proper ownership using -the provided uid and gid mappings. - -When using the uid map string '0:10000:2000', this means that -user ids inside the container between 0 and 1999 will map to user ids on -the host between 10000 and 11999. Root (0) becomes 10000, user 1 becomes -10001, user 50 becomes 10050 and user 1999 becomes 11999. This means that -files that are owned by root need to actually be owned by user 10000, and -files owned by 50 need to be owned by 10050, and so on. - -nova-idmapshift will take the uid and gid strings used for user namespaces and -properly set up the filesystem for use by those users. Uids and gids outside -of provided ranges will be mapped to nobody-id (default is max uid/gid) -so that they are inaccessible inside the container. - -OPTIONS -======= - -Positional arguments -~~~~~~~~~~~~~~~~~~~~ - - path Root path of the filesystem to be shifted - -Optional arguments -~~~~~~~~~~~~~~~~~~ - -h, --help Show this help message and exit. - -u USER_MAPS, --uid=USER_MAPS - User ID mappings, in the form: - [[guest-uid:host-uid:count],...] - -g GROUP_MAPS, --gid=GROUP_MAPS - Group ID mappings, in the form: - [[guest-gid:host-gid:count],...] - -n nobody-id, --nobody nobody-id - ID to map all unmapped uid and gids to. - Defaults to 65534. - -i, --idempotent Shift operation will only be performed if filesystem appears unshifted. - Defaults to false. - -c, --confirm Will perform check on the filesystem: - - Returns 0 when filesystem appears shifted. - - Returns 1 when filesystem appears unshifted. - - Defaults to false. - -d, --dry-run Print chown operations, but won't perform them. - Defaults to false. - -v, --verbose Print chown operations while performing them. - Defaults to false. - -SEE ALSO -======== - -* `OpenStack Nova <https://docs.openstack.org/nova/latest/>`__ - -BUGS -==== - -* Nova bugs are managed at Launchpad `Bugs : Nova <https://bugs.launchpad.net/nova>`__ diff --git a/doc/source/conf.py b/doc/source/conf.py index 365ead270e..37cbe24e3b 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -123,7 +123,6 @@ _man_pages = [ ('nova-console', u'Cloud controller fabric'), ('nova-consoleauth', u'Cloud controller fabric'), ('nova-dhcpbridge', u'Cloud controller fabric'), - ('nova-idmapshift', u'Cloud controller fabric'), ('nova-manage', u'Cloud controller fabric'), ('nova-network', u'Cloud controller fabric'), ('nova-novncproxy', u'Cloud controller fabric'), diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index ca540fb272..77026083c4 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -137,9 +137,6 @@ brctl: CommandFilter, brctl, root # nova/virt/xenapi/vm_utils.py: 'mkswap' mkswap: CommandFilter, mkswap, root -# nova/virt/libvirt/utils.py: 'nova-idmapshift' -nova-idmapshift: CommandFilter, nova-idmapshift, root - # nova/virt/xenapi/vm_utils.py: 'mkfs' # nova/utils.py: 'mkfs', fs, path, label mkfs: CommandFilter, mkfs, root diff --git a/nova/cmd/idmapshift.py b/nova/privsep/idmapshift.py index 93b6d2193e..585146d86f 100644 --- a/nova/cmd/idmapshift.py +++ b/nova/privsep/idmapshift.py @@ -11,56 +11,12 @@ # 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. + """ -########## -IDMapShift -########## IDMapShift is a tool that properly sets the ownership of a filesystem for use with linux user namespaces. -===== -Usage -===== - - nova-idmapshift -i -u 0:10000:2000 -g 0:10000:2000 path - -This command will idempotently shift `path` to proper ownership using -the provided uid and gid mappings. - -========= -Arguments -========= - - nova-idmapshift -i -c -d -v - -u [[guest-uid:host-uid:count],...] - -g [[guest-gid:host-gid:count],...] - -n [nobody-id] - path - -path: Root path of the filesystem to be shifted - --i, --idempotent: Shift operation will only be performed if filesystem -appears unshifted - --c, --confirm: Will perform check on filesystem -Returns 0 when filesystem appears shifted -Returns 1 when filesystem appears unshifted - --d, --dry-run: Print chown operations, but won't perform them - --v, --verbose: Print chown operations while performing them - --u, --uid: User ID mappings, maximum of 3 ranges - --g, --gid: Group ID mappings, maximum of 3 ranges - --n, --nobody: ID to map all unmapped uid and gids to. - -======= -Purpose -======= - When using user namespaces with linux containers, the filesystem of the container must be owned by the targeted user and group ids being applied to that container. Otherwise, processes inside the container won't be able @@ -80,12 +36,13 @@ inaccessible inside the container. """ -import argparse import os -import sys -from nova.i18n import _ +from oslo_log import log as logging + +import nova.privsep +LOG = logging.getLogger(__name__) NOBODY_ID = 65534 @@ -102,30 +59,25 @@ def find_target_id(fsid, mappings, nobody, memo): def print_chown(path, uid, gid, target_uid, target_gid): - print('%s %s:%s -> %s:%s' % (path, uid, gid, target_uid, target_gid)) + LOG.debug('%s %s:%s -> %s:%s', path, uid, gid, target_uid, target_gid) -def shift_path(path, uid_mappings, gid_mappings, nobody, uid_memo, gid_memo, - dry_run=False, verbose=False): +def shift_path(path, uid_mappings, gid_mappings, nobody, uid_memo, gid_memo): stat = os.lstat(path) uid = stat.st_uid gid = stat.st_gid target_uid = find_target_id(uid, uid_mappings, nobody, uid_memo) target_gid = find_target_id(gid, gid_mappings, nobody, gid_memo) - if verbose: - print_chown(path, uid, gid, target_uid, target_gid) - if not dry_run: - os.lchown(path, target_uid, target_gid) + print_chown(path, uid, gid, target_uid, target_gid) + os.lchown(path, target_uid, target_gid) -def shift_dir(fsdir, uid_mappings, gid_mappings, nobody, - dry_run=False, verbose=False): +def shift_dir(fsdir, uid_mappings, gid_mappings, nobody): uid_memo = dict() gid_memo = dict() def shift_path_short(p): shift_path(p, uid_mappings, gid_mappings, nobody, - dry_run=dry_run, verbose=verbose, uid_memo=uid_memo, gid_memo=gid_memo) shift_path_short(fsdir) @@ -182,51 +134,8 @@ def confirm_dir(fsdir, uid_mappings, gid_mappings, nobody): return True -def id_map_type(val): - maps = val.split(',') - id_maps = [] - for m in maps: - map_vals = m.split(':') - - if len(map_vals) != 3: - msg = ('Invalid id map %s, correct syntax is ' - 'guest-id:host-id:count.') - raise argparse.ArgumentTypeError(msg % val) - - try: - vals = [int(i) for i in map_vals] - except ValueError: - msg = 'Invalid id map %s, values must be integers' % val - raise argparse.ArgumentTypeError(msg) - - id_maps.append(tuple(vals)) - return id_maps - - -def main(): - parser = argparse.ArgumentParser( - description=_('nova-idmapshift is a tool that properly ' - 'sets the ownership of a filesystem for ' - 'use with linux user namespaces. ' - 'This tool can only be used with linux ' - 'lxc containers. See the man page for ' - 'details.')) - parser.add_argument('path') - parser.add_argument('-u', '--uid', type=id_map_type, default=[]) - parser.add_argument('-g', '--gid', type=id_map_type, default=[]) - parser.add_argument('-n', '--nobody', default=NOBODY_ID, type=int) - parser.add_argument('-i', '--idempotent', action='store_true') - parser.add_argument('-c', '--confirm', action='store_true') - parser.add_argument('-d', '--dry-run', action='store_true') - parser.add_argument('-v', '--verbose', action='store_true') - args = parser.parse_args() - - if args.idempotent or args.confirm: - if confirm_dir(args.path, args.uid, args.gid, args.nobody): - sys.exit(0) - else: - if args.confirm: - sys.exit(1) - - shift_dir(args.path, args.uid, args.gid, args.nobody, - dry_run=args.dry_run, verbose=args.verbose) +@nova.privsep.sys_admin_pctxt.entrypoint +def shift(path, uid_map, gid_map): + if confirm_dir(uid_map, gid_map, path, NOBODY_ID): + return + shift_dir(path, uid_map, gid_map, NOBODY_ID) diff --git a/nova/tests/unit/cmd/test_idmapshift.py b/nova/tests/unit/privsep/test_idmapshift.py index f2ddb7c56b..a8b3695c3d 100644 --- a/nova/tests/unit/cmd/test_idmapshift.py +++ b/nova/tests/unit/privsep/test_idmapshift.py @@ -12,13 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -import argparse - import fixtures import mock from six.moves import StringIO -from nova.cmd import idmapshift +from nova.privsep import idmapshift from nova import test @@ -111,32 +109,9 @@ class ShiftPathTestCase(BaseTestCase): mock_lstat.assert_has_calls([mock.call('/test/path')]) mock_lchown.assert_has_calls([mock.call('/test/path', 10000, 10000)]) - @mock.patch('os.lchown') - @mock.patch('os.lstat') - def test_shift_path_dry_run(self, mock_lstat, mock_lchown): - mock_lstat.return_value = FakeStat(0, 0) - idmapshift.shift_path('/test/path', self.uid_maps, self.gid_maps, - idmapshift.NOBODY_ID, dict(), dict(), - dry_run=True) - mock_lstat.assert_has_calls([mock.call('/test/path')]) - self.assertEqual(0, len(mock_lchown.mock_calls)) - - @mock.patch('os.lchown') - @mock.patch('nova.cmd.idmapshift.print_chown') - @mock.patch('os.lstat') - def test_shift_path_verbose(self, mock_lstat, mock_print, mock_lchown): - mock_lstat.return_value = FakeStat(0, 0) - idmapshift.shift_path('/test/path', self.uid_maps, self.gid_maps, - idmapshift.NOBODY_ID, dict(), dict(), - verbose=True) - mock_lstat.assert_has_calls([mock.call('/test/path')]) - mock_print_call = mock.call('/test/path', 0, 0, 10000, 10000) - mock_print.assert_has_calls([mock_print_call]) - mock_lchown.assert_has_calls([mock.call('/test/path', 10000, 10000)]) - class ShiftDirTestCase(BaseTestCase): - @mock.patch('nova.cmd.idmapshift.shift_path') + @mock.patch('nova.privsep.idmapshift.shift_path') @mock.patch('os.path.join') @mock.patch('os.walk') def test_shift_dir(self, mock_walk, mock_join, mock_shift_path): @@ -152,32 +127,7 @@ class ShiftDirTestCase(BaseTestCase): mock_join.assert_has_calls(mock_join_calls) args = (self.uid_maps, self.gid_maps, idmapshift.NOBODY_ID) - kwargs = dict(dry_run=False, verbose=False, - uid_memo=dict(), gid_memo=dict()) - shift_path_calls = [mock.call('/', *args, **kwargs)] - shift_path_calls += [mock.call('/' + x, *args, **kwargs) - for x in files] - mock_shift_path.assert_has_calls(shift_path_calls) - - @mock.patch('nova.cmd.idmapshift.shift_path') - @mock.patch('os.path.join') - @mock.patch('os.walk') - def test_shift_dir_dry_run(self, mock_walk, mock_join, mock_shift_path): - mock_walk.return_value = [('/', ['a', 'b'], ['c', 'd'])] - mock_join.side_effect = join_side_effect - - idmapshift.shift_dir('/', self.uid_maps, self.gid_maps, - idmapshift.NOBODY_ID, dry_run=True) - - mock_walk.assert_has_calls([mock.call('/')]) - - files = ['a', 'b', 'c', 'd'] - mock_join_calls = [mock.call('/', x) for x in files] - mock_join.assert_has_calls(mock_join_calls) - - args = (self.uid_maps, self.gid_maps, idmapshift.NOBODY_ID) - kwargs = dict(dry_run=True, verbose=False, - uid_memo=dict(), gid_memo=dict()) + kwargs = dict(uid_memo=dict(), gid_memo=dict()) shift_path_calls = [mock.call('/', *args, **kwargs)] shift_path_calls += [mock.call('/' + x, *args, **kwargs) for x in files] @@ -264,7 +214,7 @@ class ConfirmDirTestCase(BaseTestCase): self.uid_map_ranges = idmapshift.get_ranges(self.uid_maps) self.gid_map_ranges = idmapshift.get_ranges(self.gid_maps) - @mock.patch('nova.cmd.idmapshift.confirm_path') + @mock.patch('nova.privsep.idmapshift.confirm_path') @mock.patch('os.path.join') @mock.patch('os.walk') def test_confirm_dir(self, mock_walk, mock_join, mock_confirm_path): @@ -286,7 +236,7 @@ class ConfirmDirTestCase(BaseTestCase): for x in files] mock_confirm_path.assert_has_calls(confirm_path_calls) - @mock.patch('nova.cmd.idmapshift.confirm_path') + @mock.patch('nova.privsep.idmapshift.confirm_path') @mock.patch('os.path.join') @mock.patch('os.walk') def test_confirm_dir_short_circuit_root(self, mock_walk, mock_join, @@ -302,7 +252,7 @@ class ConfirmDirTestCase(BaseTestCase): confirm_path_calls = [mock.call('/', *args)] mock_confirm_path.assert_has_calls(confirm_path_calls) - @mock.patch('nova.cmd.idmapshift.confirm_path') + @mock.patch('nova.privsep.idmapshift.confirm_path') @mock.patch('os.path.join') @mock.patch('os.walk') def test_confirm_dir_short_circuit_file(self, mock_walk, mock_join, @@ -328,7 +278,7 @@ class ConfirmDirTestCase(BaseTestCase): mock.call('/' + 'a', *args)] mock_confirm_path.assert_has_calls(confirm_path_calls) - @mock.patch('nova.cmd.idmapshift.confirm_path') + @mock.patch('nova.privsep.idmapshift.confirm_path') @mock.patch('os.path.join') @mock.patch('os.walk') def test_confirm_dir_short_circuit_dir(self, mock_walk, mock_join, @@ -358,162 +308,6 @@ class ConfirmDirTestCase(BaseTestCase): mock_confirm_path.assert_has_calls(confirm_path_calls) -class IDMapTypeTestCase(test.NoDBTestCase): - def test_id_map_type(self): - result = idmapshift.id_map_type("1:1:1,2:2:2") - self.assertEqual([(1, 1, 1), (2, 2, 2)], result) - - def test_id_map_type_not_int(self): - self.assertRaises(argparse.ArgumentTypeError, idmapshift.id_map_type, - "a:1:1") - - def test_id_map_type_not_proper_format(self): - self.assertRaises(argparse.ArgumentTypeError, idmapshift.id_map_type, - "1:1") - - -class MainTestCase(BaseTestCase): - @mock.patch('nova.cmd.idmapshift.shift_dir') - @mock.patch('argparse.ArgumentParser') - def test_main(self, mock_parser_class, mock_shift_dir): - mock_parser = mock.MagicMock() - mock_parser.parse_args.return_value = mock_parser - mock_parser.idempotent = False - mock_parser.confirm = False - mock_parser.path = '/test/path' - mock_parser.uid = self.uid_maps - mock_parser.gid = self.gid_maps - mock_parser.nobody = idmapshift.NOBODY_ID - mock_parser.dry_run = False - mock_parser.verbose = False - mock_parser_class.return_value = mock_parser - - idmapshift.main() - - mock_shift_dir_call = mock.call('/test/path', self.uid_maps, - self.gid_maps, idmapshift.NOBODY_ID, - dry_run=False, verbose=False) - mock_shift_dir.assert_has_calls([mock_shift_dir_call]) - - @mock.patch('nova.cmd.idmapshift.shift_dir') - @mock.patch('nova.cmd.idmapshift.confirm_dir') - @mock.patch('argparse.ArgumentParser') - def test_main_confirm_dir_idempotent_unshifted(self, mock_parser_class, - mock_confirm_dir, - mock_shift_dir): - mock_parser = mock.MagicMock() - mock_parser.parse_args.return_value = mock_parser - mock_parser.idempotent = True - mock_parser.confirm = False - mock_parser.path = '/test/path' - mock_parser.uid = self.uid_maps - mock_parser.gid = self.gid_maps - mock_parser.nobody = idmapshift.NOBODY_ID - mock_parser.dry_run = False - mock_parser.verbose = False - mock_parser_class.return_value = mock_parser - mock_confirm_dir.return_value = False - - idmapshift.main() - - mock_confirm_dir_call = mock.call('/test/path', self.uid_maps, - self.gid_maps, idmapshift.NOBODY_ID) - mock_confirm_dir.assert_has_calls([mock_confirm_dir_call]) - mock_shift_dir_call = mock.call('/test/path', self.uid_maps, - self.gid_maps, idmapshift.NOBODY_ID, - dry_run=False, verbose=False) - mock_shift_dir.assert_has_calls([mock_shift_dir_call]) - - @mock.patch('nova.cmd.idmapshift.shift_dir') - @mock.patch('nova.cmd.idmapshift.confirm_dir') - @mock.patch('argparse.ArgumentParser') - def test_main_confirm_dir_idempotent_shifted(self, mock_parser_class, - mock_confirm_dir, - mock_shift_dir): - mock_parser = mock.MagicMock() - mock_parser.parse_args.return_value = mock_parser - mock_parser.idempotent = True - mock_parser.confirm = False - mock_parser.path = '/test/path' - mock_parser.uid = self.uid_maps - mock_parser.gid = self.gid_maps - mock_parser.nobody = idmapshift.NOBODY_ID - mock_parser.dry_run = False - mock_parser.verbose = False - mock_parser_class.return_value = mock_parser - mock_confirm_dir.return_value = True - - try: - idmapshift.main() - except SystemExit as sys_exit: - self.assertEqual(sys_exit.code, 0) - - mock_confirm_dir_call = mock.call('/test/path', self.uid_maps, - self.gid_maps, idmapshift.NOBODY_ID) - mock_confirm_dir.assert_has_calls([mock_confirm_dir_call]) - mock_shift_dir.assert_has_calls([]) - - @mock.patch('nova.cmd.idmapshift.shift_dir') - @mock.patch('nova.cmd.idmapshift.confirm_dir') - @mock.patch('argparse.ArgumentParser') - def test_main_confirm_dir_confirm_unshifted(self, mock_parser_class, - mock_confirm_dir, - mock_shift_dir): - mock_parser = mock.MagicMock() - mock_parser.parse_args.return_value = mock_parser - mock_parser.idempotent = False - mock_parser.confirm = True - mock_parser.exit_on_fail = True - mock_parser.path = '/test/path' - mock_parser.uid = self.uid_maps - mock_parser.gid = self.gid_maps - mock_parser.nobody = idmapshift.NOBODY_ID - mock_parser.dry_run = False - mock_parser.verbose = False - mock_parser_class.return_value = mock_parser - mock_confirm_dir.return_value = False - - try: - idmapshift.main() - except SystemExit as sys_exit: - self.assertEqual(sys_exit.code, 1) - - mock_confirm_dir_call = mock.call('/test/path', self.uid_maps, - self.gid_maps, idmapshift.NOBODY_ID) - mock_confirm_dir.assert_has_calls([mock_confirm_dir_call]) - mock_shift_dir.assert_has_calls([]) - - @mock.patch('nova.cmd.idmapshift.shift_dir') - @mock.patch('nova.cmd.idmapshift.confirm_dir') - @mock.patch('argparse.ArgumentParser') - def test_main_confirm_dir_confirm_shifted(self, mock_parser_class, - mock_confirm_dir, - mock_shift_dir): - mock_parser = mock.MagicMock() - mock_parser.parse_args.return_value = mock_parser - mock_parser.idempotent = False - mock_parser.confirm = True - mock_parser.exit_on_fail = True - mock_parser.path = '/test/path' - mock_parser.uid = self.uid_maps - mock_parser.gid = self.gid_maps - mock_parser.nobody = idmapshift.NOBODY_ID - mock_parser.dry_run = False - mock_parser.verbose = False - mock_parser_class.return_value = mock_parser - mock_confirm_dir.return_value = True - - try: - idmapshift.main() - except SystemExit as sys_exit: - self.assertEqual(sys_exit.code, 0) - - mock_confirm_dir_call = mock.call('/test/path', self.uid_maps, - self.gid_maps, idmapshift.NOBODY_ID) - mock_confirm_dir.assert_has_calls([mock_confirm_dir_call]) - mock_shift_dir.assert_has_calls([]) - - class IntegrationTestCase(BaseTestCase): @mock.patch('os.lchown') @mock.patch('os.lstat') @@ -540,7 +334,7 @@ class IntegrationTestCase(BaseTestCase): mock_lstat.side_effect = lstat idmapshift.shift_dir('/tmp/test', self.uid_maps, self.gid_maps, - idmapshift.NOBODY_ID, verbose=True) + idmapshift.NOBODY_ID) lchown_calls = [ mock.call('/tmp/test', 10000, 10000), @@ -554,35 +348,6 @@ class IntegrationTestCase(BaseTestCase): ] mock_lchown.assert_has_calls(lchown_calls) - @mock.patch('os.lchown') - @mock.patch('os.lstat') - @mock.patch('os.path.join') - @mock.patch('os.walk') - def test_integrated_shift_dir_dry_run(self, mock_walk, mock_join, - mock_lstat, mock_lchown): - mock_walk.return_value = [('/tmp/test', ['a', 'b', 'c'], ['d']), - ('/tmp/test/d', ['1', '2'], [])] - mock_join.side_effect = join_side_effect - - def lstat(path): - stats = { - 't': FakeStat(0, 0), - 'a': FakeStat(0, 0), - 'b': FakeStat(0, 2), - 'c': FakeStat(30000, 30000), - 'd': FakeStat(100, 100), - '1': FakeStat(0, 100), - '2': FakeStat(100, 100), - } - return stats[path[-1]] - - mock_lstat.side_effect = lstat - - idmapshift.shift_dir('/tmp/test', self.uid_maps, self.gid_maps, - idmapshift.NOBODY_ID, dry_run=True, verbose=True) - - self.assertEqual(0, len(mock_lchown.mock_calls)) - @mock.patch('os.lstat') @mock.patch('os.path.join') @mock.patch('os.walk') diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 6005f560d3..631e83b657 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -35,7 +35,6 @@ from nova.tests import uuidsentinel as uuids from nova import utils from nova.virt.disk import api as disk from nova.virt import images -from nova.virt.libvirt import config as vconfig from nova.virt.libvirt import guest as libvirt_guest from nova.virt.libvirt import utils as libvirt_utils @@ -518,29 +517,6 @@ disk size: 4.4M finally: os.unlink(dst_path) - @mock.patch.object(utils, 'execute') - def test_chown_for_id_maps(self, mock_execute): - id_maps = [vconfig.LibvirtConfigGuestUIDMap(), - vconfig.LibvirtConfigGuestUIDMap(), - vconfig.LibvirtConfigGuestGIDMap(), - vconfig.LibvirtConfigGuestGIDMap()] - id_maps[0].target = 10000 - id_maps[0].count = 2000 - id_maps[1].start = 2000 - id_maps[1].target = 40000 - id_maps[1].count = 2000 - id_maps[2].target = 10000 - id_maps[2].count = 2000 - id_maps[3].start = 2000 - id_maps[3].target = 40000 - id_maps[3].count = 2000 - libvirt_utils.chown_for_id_maps('/some/path', id_maps) - execute_args = ('nova-idmapshift', '-i', - '-u', '0:10000:2000,2000:40000:2000', - '-g', '0:10000:2000,2000:40000:2000', - '/some/path') - mock_execute.assert_called_once_with(*execute_args, run_as_root=True) - def _do_test_extract_snapshot(self, mock_execute, src_format='qcow2', dest_format='raw', out_format='raw'): libvirt_utils.extract_snapshot('/path/to/disk/image', src_format, diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index b0571e8b33..a3fde1a7bf 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -29,6 +29,7 @@ from oslo_utils import fileutils import nova.conf from nova.i18n import _ from nova.objects import fields as obj_fields +import nova.privsep.idmapshift import nova.privsep.libvirt from nova import utils from nova.virt.disk import api as disk @@ -243,10 +244,6 @@ def write_to_file(path, contents, umask=None): os.umask(saved_umask) -def _id_map_to_config(id_map): - return "%s:%s:%s" % (id_map.start, id_map.target, id_map.count) - - def chown_for_id_maps(path, id_maps): """Change ownership of file or directory for an id mapped environment @@ -254,14 +251,11 @@ def chown_for_id_maps(path, id_maps): :param path: File or directory whose ownership to change :param id_maps: List of type LibvirtConfigGuestIDMap """ - uid_maps_str = ','.join([_id_map_to_config(id_map) for id_map in id_maps if - isinstance(id_map, - vconfig.LibvirtConfigGuestUIDMap)]) - gid_maps_str = ','.join([_id_map_to_config(id_map) for id_map in id_maps if - isinstance(id_map, - vconfig.LibvirtConfigGuestGIDMap)]) - utils.execute('nova-idmapshift', '-i', '-u', uid_maps_str, - '-g', gid_maps_str, path, run_as_root=True) + uid_maps = [id_map for id_map in id_maps if + isinstance(id_map, vconfig.LibvirtConfigGuestUIDMap)] + gid_maps = [id_map for id_map in id_maps if + isinstance(id_map, vconfig.LibvirtConfigGuestGIDMap)] + nova.privsep.idmapshift.shift(path, uid_maps, gid_maps) def extract_snapshot(disk_path, source_fmt, out_path, dest_fmt): diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index a0fa43f197..d0f527702d 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml @@ -6,7 +6,10 @@ upgrade: - | Calls to mount in the virt disk api no longer ignore the value of stderr. - | + The nova-idmapshift binary has been removed. This has been replaced by + internal functionality using privsep. + - | The following commands are no longer required to be listed in your rootwrap configuration: cat; chown; cryptsetup; dd; lvcreate; lvremove; lvs; mkdir; - mount; ploop; prl_disk_tool; readlink; shred; tee; touch; umount; vgs; - and xend. + mount; nova-idmapshift; ploop; prl_disk_tool; readlink; shred; tee; touch; + umount; vgs; and xend. @@ -61,7 +61,6 @@ console_scripts = nova-console = nova.cmd.console:main nova-consoleauth = nova.cmd.consoleauth:main nova-dhcpbridge = nova.cmd.dhcpbridge:main - nova-idmapshift = nova.cmd.idmapshift:main nova-manage = nova.cmd.manage:main nova-network = nova.cmd.network:main nova-novncproxy = nova.cmd.novncproxy:main |