From 82249141b5d869a357aa08cdab753c1521ec87b3 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Tue, 30 Jul 2019 11:23:12 +0200 Subject: [PATCH] Accumulated changes required for Yomi (#165) * cmdmod: fix runas and group in run_chroot The parameters runas and group for cmdmod.run() will change the efective user and group before executing the command. But in a chroot environment is expected that the change happends inside the chroot, not outside, as the user and groups are refering to objects that can only exist inside the environment. This patch add the userspec parameter to the chroot command, to change the user in the correct place. (cherry picked from commit f0434aaeeee3ace4e3fc65c04e69984f08b2541e) * chroot: add missing sys directory (cherry picked from commit cdf74426bcad4e8bf329bf604c77ea83bfca8b2c) * chroot: change variable name to root (cherry picked from commit 7f68b65b1b0f9eec2a6b07b02714ead0121f0e4b) * chroot: fix bug in safe_kwargs iteration (cherry picked from commit 39da1c69ea2781bed6e9d8e6879b70d65fa5a5b0) * test_cmdmod: fix test_run_cwd_in_combination_with_runas (cherry picked from commit 42640ecf161caf64c61e9b02927882f92c850092) * test_cmdmod: add test_run_chroot_runas test (cherry picked from commit d900035089a22f6741d2095fd1f6694597041a88) * freezer: do not fail in cache dir is present (cherry picked from commit 25137c51e6d6e53e3099b6cddbf51d4cb2c53d8d) * freezer: clean freeze YAML profile on restore (cherry picked from commit 56b97c997257f12038399549dc987b7723ab225f) * zypperpkg: fix pkg.list_pkgs cache The cache from pkg.list_pkgs for the zypper installer is too aggresive. Some parameters will deliver different package lists, like root and includes. The current cache do not take those parameters into consideration, so the next time that this function is called, the last list of packages will be returned, without checking if the current parameters match the old one. This patch create a different cache key for each parameter combination, so the cached data will be separated too. (cherry picked from commit 9c54bb3e8c93ba21fc583bdefbcadbe53cbcd7b5) --- salt/modules/cmdmod.py | 12 +++++++++--- salt/modules/zypperpkg.py | 13 ++++++++++--- tests/unit/modules/test_cmdmod.py | 16 ++++++++++++++++ tests/unit/modules/test_zypperpkg.py | 21 +++++++++++++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index eed7656a6dd..0d2f720bbbc 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -3094,13 +3094,19 @@ def run_chroot(root, if isinstance(cmd, (list, tuple)): cmd = ' '.join([six.text_type(i) for i in cmd]) - cmd = 'chroot {0} {1} -c {2}'.format(root, sh_, _cmd_quote(cmd)) + + # If runas and group are provided, we expect that the user lives + # inside the chroot, not outside. + if runas: + userspec = '--userspec {}:{}'.format(runas, group if group else '') + else: + userspec = '' + + cmd = 'chroot {} {} {} -c {}'.format(userspec, root, sh_, _cmd_quote(cmd)) run_func = __context__.pop('cmd.run_chroot.func', run_all) ret = run_func(cmd, - runas=runas, - group=group, cwd=cwd, stdin=stdin, shell=shell, diff --git a/salt/modules/zypperpkg.py b/salt/modules/zypperpkg.py index 3760b525e77..8179cd8c1dd 100644 --- a/salt/modules/zypperpkg.py +++ b/salt/modules/zypperpkg.py @@ -449,8 +449,14 @@ def _clean_cache(): ''' Clean cached results ''' + keys = [] for cache_name in ['pkg.list_pkgs', 'pkg.list_provides']: - __context__.pop(cache_name, None) + for contextkey in __context__: + if contextkey.startswith(cache_name): + keys.append(contextkey) + + for key in keys: + __context__.pop(key, None) def list_upgrades(refresh=True, root=None, **kwargs): @@ -811,9 +817,10 @@ def list_pkgs(versions_as_list=False, root=None, includes=None, **kwargs): includes = includes if includes else [] - contextkey = 'pkg.list_pkgs' + # Results can be different if a different root or a different + # inclusion types are passed + contextkey = 'pkg.list_pkgs_{}_{}'.format(root, includes) - # TODO(aplanas): this cached value depends on the parameters if contextkey not in __context__: ret = {} cmd = ['rpm'] diff --git a/tests/unit/modules/test_cmdmod.py b/tests/unit/modules/test_cmdmod.py index f8fba59294a..8d763435f8b 100644 --- a/tests/unit/modules/test_cmdmod.py +++ b/tests/unit/modules/test_cmdmod.py @@ -371,6 +371,22 @@ def test_run_cwd_doesnt_exist_issue_7154(self): else: raise RuntimeError + @skipIf(salt.utils.platform.is_windows(), 'Do not run on Windows') + @skipIf(salt.utils.platform.is_darwin(), 'Do not run on MacOS') + def test_run_cwd_in_combination_with_runas(self): + ''' + cmd.run executes command in the cwd directory + when the runas parameter is specified + ''' + cmd = 'pwd' + cwd = '/tmp' + runas = os.getlogin() + + with patch.dict(cmdmod.__grains__, {'os': 'Darwin', + 'os_family': 'Solaris'}): + stdout = cmdmod._run(cmd, cwd=cwd, runas=runas).get('stdout') + self.assertEqual(stdout, cwd) + def test_run_all_binary_replace(self): ''' Test for failed decoding of binary data, for instance when doing diff --git a/tests/unit/modules/test_zypperpkg.py b/tests/unit/modules/test_zypperpkg.py index 12c22bfcb2f..61020433842 100644 --- a/tests/unit/modules/test_zypperpkg.py +++ b/tests/unit/modules/test_zypperpkg.py @@ -571,6 +571,7 @@ def _add_data(data, key, value): patch.dict(zypper.__salt__, {'pkg_resource.stringify': MagicMock()}): pkgs = zypper.list_pkgs(versions_as_list=True) self.assertFalse(pkgs.get('gpg-pubkey', False)) + self.assertTrue('pkg.list_pkgs_None_[]' in zypper.__context__) for pkg_name, pkg_version in { 'jakarta-commons-discovery': ['0.4-129.686'], 'yast2-ftp-server': ['3.1.8-8.1'], @@ -613,6 +614,7 @@ def _add_data(data, key, value): patch.dict(pkg_resource.__salt__, {'pkg.parse_arch_from_name': zypper.parse_arch_from_name}): pkgs = zypper.list_pkgs(attr=['epoch', 'release', 'arch', 'install_date_time_t']) self.assertFalse(pkgs.get('gpg-pubkey', False)) + self.assertTrue('pkg.list_pkgs_None_[]' in zypper.__context__) for pkg_name, pkg_attr in { 'jakarta-commons-discovery': [{ 'version': '0.4', @@ -1456,3 +1458,22 @@ def test_list_patterns(self, get_visible_patterns): 'summary': 'description b', }, } + + def test__clean_cache_empty(self): + '''Test that an empty cached can be cleaned''' + context = {} + with patch.dict(zypper.__context__, context): + zypper._clean_cache() + assert context == {} + + def test__clean_cache_filled(self): + '''Test that a filled cached can be cleaned''' + context = { + 'pkg.list_pkgs_/mnt_[]': None, + 'pkg.list_pkgs_/mnt_[patterns]': None, + 'pkg.list_provides': None, + 'pkg.other_data': None, + } + with patch.dict(zypper.__context__, context): + zypper._clean_cache() + self.assertEqual(zypper.__context__, {'pkg.other_data': None})