Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix kwargs iteration for chroot.call #53996

Closed
wants to merge 4 commits into from
Closed

Fix kwargs iteration for chroot.call #53996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b5ca46
chroot: add missing sys directory
aplanas Jul 24, 2019
19967cf
chroot: change variable name to root
aplanas Jul 24, 2019
b87c46c
chroot: fix bug in safe_kwargs iteration
aplanas Jul 24, 2019
4c82038
test_chroot: assert over cmd.run_chroot params
aplanas Aug 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions salt/modules/chroot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,25 @@ def __virtual__():
return (False, 'Module chroot requires the command chroot')


def exist(name):
def exist(root):
'''
Return True if the chroot environment is present.
'''
dev = os.path.join(name, 'dev')
proc = os.path.join(name, 'proc')
return all(os.path.isdir(i) for i in (name, dev, proc))
dev = os.path.join(root, 'dev')
proc = os.path.join(root, 'proc')
sys = os.path.join(root, 'sys')
return all(os.path.isdir(i) for i in (root, dev, proc, sys))


def create(name):
def create(root):
'''
Create a basic chroot environment.

Note that this environment is not functional. The caller needs to
install the minimal required binaries, including Python if
chroot.call is called.

name
root
Path to the chroot environment

CLI Example:
Expand All @@ -77,26 +78,28 @@ def create(name):
salt myminion chroot.create /chroot

'''
if not exist(name):
dev = os.path.join(name, 'dev')
proc = os.path.join(name, 'proc')
if not exist(root):
dev = os.path.join(root, 'dev')
proc = os.path.join(root, 'proc')
sys = os.path.join(root, 'sys')
try:
os.makedirs(dev, mode=0o755)
os.makedirs(proc, mode=0o555)
os.makedirs(sys, mode=0o555)
except OSError as e:
log.error('Error when trying to create chroot directories: %s', e)
return False
return True


def call(name, function, *args, **kwargs):
def call(root, function, *args, **kwargs):
'''
Executes a Salt function inside a chroot environment.

The chroot does not need to have Salt installed, but Python is
required.

name
root
Path to the chroot environment

function
Expand All @@ -107,18 +110,19 @@ def call(name, function, *args, **kwargs):
.. code-block:: bash

salt myminion chroot.call /chroot test.ping
salt myminion chroot.call /chroot ssh.set_auth_key user key=mykey

'''

if not function:
raise CommandExecutionError('Missing function parameter')

if not exist(name):
if not exist(root):
raise CommandExecutionError('Chroot environment not found')

# Create a temporary directory inside the chroot where we can
# untar salt-thin
thin_dest_path = tempfile.mkdtemp(dir=name)
thin_dest_path = tempfile.mkdtemp(dir=root)
thin_path = __utils__['thin.gen_thin'](
__opts__['cachedir'],
extra_mods=__salt__['config.option']('thin_extra_mods', ''),
Expand All @@ -130,7 +134,7 @@ def call(name, function, *args, **kwargs):
return {'result': False, 'comment': stdout}

chroot_path = os.path.join(os.path.sep,
os.path.relpath(thin_dest_path, name))
os.path.relpath(thin_dest_path, root))
try:
safe_kwargs = clean_kwargs(**kwargs)
salt_argv = [
Expand All @@ -144,8 +148,8 @@ def call(name, function, *args, **kwargs):
'-l', 'quiet',
'--',
function
] + list(args) + ['{}={}'.format(k, v) for (k, v) in safe_kwargs]
ret = __salt__['cmd.run_chroot'](name, [str(x) for x in salt_argv])
] + list(args) + ['{}={}'.format(k, v) for (k, v) in safe_kwargs.items()]
ret = __salt__['cmd.run_chroot'](root, [str(x) for x in salt_argv])
if ret['retcode'] != EX_OK:
raise CommandExecutionError(ret['stderr'])

Expand Down
57 changes: 53 additions & 4 deletions tests/unit/modules/test_chroot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

# Import Python Libs
from __future__ import absolute_import, print_function, unicode_literals
import sys

# Import Salt Testing Libs
from tests.support.mixins import LoaderModuleMockMixin
Expand Down Expand Up @@ -63,10 +64,10 @@ def test_exist(self, isdir):
'''
Test if the chroot environment exist.
'''
isdir.side_effect = (True, True, True)
isdir.side_effect = (True, True, True, True)
self.assertTrue(chroot.exist('/chroot'))

isdir.side_effect = (True, True, False)
isdir.side_effect = (True, True, True, False)
self.assertFalse(chroot.exist('/chroot'))

@patch('os.makedirs')
Expand Down Expand Up @@ -149,7 +150,12 @@ def test_call_fails_salt_thin(self, mkdtemp, exist):
utils_mock['thin.gen_thin'].assert_called_once()
salt_mock['config.option'].assert_called()
salt_mock['archive.tar'].assert_called_once()
salt_mock['cmd.run_chroot'].assert_called_once()
salt_mock['cmd.run_chroot'].assert_called_with(
'/chroot',
['python{}'.format(sys.version_info[0]), '/tmp01/salt-call',
'--metadata', '--local',
'--log-file', '/tmp01/log', '--cachedir', '/tmp01/cache',
'--out', 'json', '-l', 'quiet', '--', 'test.ping'])
utils_mock['files.rm_rf'].assert_called_once()

@patch('salt.modules.chroot.exist')
Expand Down Expand Up @@ -180,5 +186,48 @@ def test_call_success(self, mkdtemp, exist):
utils_mock['thin.gen_thin'].assert_called_once()
salt_mock['config.option'].assert_called()
salt_mock['archive.tar'].assert_called_once()
salt_mock['cmd.run_chroot'].assert_called_once()
salt_mock['cmd.run_chroot'].assert_called_with(
'/chroot',
['python{}'.format(sys.version_info[0]), '/tmp01/salt-call',
'--metadata', '--local',
'--log-file', '/tmp01/log', '--cachedir', '/tmp01/cache',
'--out', 'json', '-l', 'quiet', '--', 'test.ping'])
utils_mock['files.rm_rf'].assert_called_once()

@patch('salt.modules.chroot.exist')
@patch('tempfile.mkdtemp')
def test_call_success_parameters(self, mkdtemp, exist):
'''
Test execution of Salt functions in chroot with parameters.
'''
# Success test
exist.return_value = True
mkdtemp.return_value = '/chroot/tmp01'
utils_mock = {
'thin.gen_thin': MagicMock(return_value='/salt-thin.tgz'),
'files.rm_rf': MagicMock(),
'json.find_json': MagicMock(return_value={'return': 'result'})
}
salt_mock = {
'archive.tar': MagicMock(return_value=''),
'config.option': MagicMock(),
'cmd.run_chroot': MagicMock(return_value={
'retcode': 0,
'stdout': '',
}),
}
with patch.dict(chroot.__utils__, utils_mock), \
patch.dict(chroot.__salt__, salt_mock):
self.assertEqual(chroot.call('/chroot', 'ssh.set_auth_key',
user='user', key='key'), 'result')
utils_mock['thin.gen_thin'].assert_called_once()
salt_mock['config.option'].assert_called()
salt_mock['archive.tar'].assert_called_once()
salt_mock['cmd.run_chroot'].assert_called_with(
'/chroot',
['python{}'.format(sys.version_info[0]), '/tmp01/salt-call',
'--metadata', '--local',
'--log-file', '/tmp01/log', '--cachedir', '/tmp01/cache',
'--out', 'json', '-l', 'quiet',
'--', 'ssh.set_auth_key', 'user=user', 'key=key'])
utils_mock['files.rm_rf'].assert_called_once()
aplanas marked this conversation as resolved.
Show resolved Hide resolved