From 82fd71daa6054076899da57c7f45d1074e591e8e Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Wed, 7 Nov 2018 17:59:09 +0100 Subject: [PATCH 1/6] chroot: add new chroot module (cherry picked from commit 984fb62f5df60b5d764788b2833387f04cf7b2cb) --- salt/modules/chroot.py | 165 +++++++++++++++++++++++++++ tests/unit/modules/test_chroot.py | 184 ++++++++++++++++++++++++++++++ 2 files changed, 349 insertions(+) create mode 100644 salt/modules/chroot.py create mode 100644 tests/unit/modules/test_chroot.py diff --git a/salt/modules/chroot.py b/salt/modules/chroot.py new file mode 100644 index 000000000000..6e4705b67ee7 --- /dev/null +++ b/salt/modules/chroot.py @@ -0,0 +1,165 @@ +# -*- coding: utf-8 -*- +# +# Author: Alberto Planas +# +# Copyright 2018 SUSE LINUX GmbH, Nuernberg, Germany. +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, 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. + +''' +:maintainer: Alberto Planas +:maturity: new +:depends: None +:platform: Linux +''' +from __future__ import absolute_import, print_function, unicode_literals +import logging +import os +import sys +import tempfile + +from salt.defaults.exitcodes import EX_OK +from salt.exceptions import CommandExecutionError +from salt.utils.args import clean_kwargs + +log = logging.getLogger(__name__) + + +def __virtual__(): + ''' + Chroot command is required. + ''' + if __utils__['path.which']('chroot') is not None: + return True + else: + return (False, 'Module chroot requires the command chroot') + + +def exist(name): + ''' + 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)) + + +def create(name): + ''' + 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 + Path to the chroot environment + + CLI Example: + + .. code-block:: bash + + salt myminion chroot.create /chroot + + ''' + if not exist(name): + dev = os.path.join(name, 'dev') + proc = os.path.join(name, 'proc') + try: + os.makedirs(dev, mode=0o755) + os.makedirs(proc, 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): + ''' + Executes a Salt function inside a chroot environment. + + The chroot does not need to have Salt installed, but Python is + required. + + name + Path to the chroot environment + + function + Salt execution module function + + CLI Example: + + .. code-block:: bash + + salt myminion chroot.call /chroot test.ping + + ''' + + if not function: + raise CommandExecutionError('Missing function parameter') + + if not exist(name): + 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_path = __utils__['thin.gen_thin']( + __opts__['cachedir'], + extra_mods=__salt__['config.option']('thin_extra_mods', ''), + so_mods=__salt__['config.option']('thin_so_mods', '') + ) + stdout = __salt__['archive.tar']('xzf', thin_path, dest=thin_dest_path) + if stdout: + __utils__['files.rm_rf'](thin_dest_path) + return {'result': False, 'comment': stdout} + + chroot_path = os.path.join(os.path.sep, + os.path.relpath(thin_dest_path, name)) + try: + safe_kwargs = clean_kwargs(**kwargs) + salt_argv = [ + 'python{}'.format(sys.version_info[0]), + os.path.join(chroot_path, 'salt-call'), + '--metadata', + '--local', + '--log-file', os.path.join(chroot_path, 'log'), + '--cachedir', os.path.join(chroot_path, 'cache'), + '--out', 'json', + '-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]) + if ret['retcode'] != EX_OK: + raise CommandExecutionError(ret['stderr']) + + # Process "real" result in stdout + try: + data = __utils__['json.find_json'](ret['stdout']) + local = data.get('local', data) + if isinstance(local, dict) and 'retcode' in local: + __context__['retcode'] = local['retcode'] + return local.get('return', data) + except ValueError: + return { + 'result': False, + 'comment': "Can't parse container command output" + } + finally: + __utils__['files.rm_rf'](thin_dest_path) diff --git a/tests/unit/modules/test_chroot.py b/tests/unit/modules/test_chroot.py new file mode 100644 index 000000000000..7181dd7e5096 --- /dev/null +++ b/tests/unit/modules/test_chroot.py @@ -0,0 +1,184 @@ +# -*- coding: utf-8 -*- +# +# Author: Alberto Planas +# +# Copyright 2018 SUSE LINUX GmbH, Nuernberg, Germany. +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, 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. + +''' +:maintainer: Alberto Planas +:platform: Linux +''' + +# Import Python Libs +from __future__ import absolute_import, print_function, unicode_literals + +# Import Salt Testing Libs +from tests.support.mixins import LoaderModuleMockMixin +from tests.support.unit import skipIf, TestCase +from tests.support.mock import ( + MagicMock, + NO_MOCK, + NO_MOCK_REASON, + patch, +) + +from salt.exceptions import CommandExecutionError +import salt.modules.chroot as chroot + + +@skipIf(NO_MOCK, NO_MOCK_REASON) +class ChrootTestCase(TestCase, LoaderModuleMockMixin): + ''' + Test cases for salt.modules.chroot + ''' + + def setup_loader_modules(self): + return { + chroot: { + '__salt__': {}, + '__utils__': {}, + '__opts__': {'cachedir': ''}, + } + } + + @patch('os.path.isdir') + def test_exist(self, isdir): + ''' + Test if the chroot environment exist. + ''' + isdir.side_effect = (True, True, True) + self.assertTrue(chroot.exist('/chroot')) + + isdir.side_effect = (True, True, False) + self.assertFalse(chroot.exist('/chroot')) + + @patch('os.makedirs') + @patch('salt.modules.chroot.exist') + def test_create(self, exist, makedirs): + ''' + Test the creation of an empty chroot environment. + ''' + exist.return_value = True + self.assertTrue(chroot.create('/chroot')) + makedirs.assert_not_called() + + exist.return_value = False + self.assertTrue(chroot.create('/chroot')) + makedirs.assert_called() + + @patch('salt.modules.chroot.exist') + def test_call_fails_input_validation(self, exist): + ''' + Test execution of Salt functions in chroot. + ''' + # Basic input validation + exist.return_value = False + self.assertRaises(CommandExecutionError, chroot.call, '/chroot', '') + self.assertRaises(CommandExecutionError, chroot.call, '/chroot', 'test.ping') + + @patch('salt.modules.chroot.exist') + @patch('tempfile.mkdtemp') + def test_call_fails_untar(self, mkdtemp, exist): + ''' + Test execution of Salt functions in chroot. + ''' + # Fail the tar command + exist.return_value = True + mkdtemp.return_value = '/chroot/tmp01' + utils_mock = { + 'thin.gen_thin': MagicMock(return_value='/salt-thin.tgz'), + 'files.rm_rf': MagicMock(), + } + salt_mock = { + 'archive.tar': MagicMock(return_value='Error'), + 'config.option': MagicMock(), + } + with patch.dict(chroot.__utils__, utils_mock), \ + patch.dict(chroot.__salt__, salt_mock): + self.assertEqual(chroot.call('/chroot', 'test.ping'), { + 'result': False, + 'comment': 'Error' + }) + utils_mock['thin.gen_thin'].assert_called_once() + salt_mock['config.option'].assert_called() + salt_mock['archive.tar'].assert_called_once() + utils_mock['files.rm_rf'].assert_called_once() + + @patch('salt.modules.chroot.exist') + @patch('tempfile.mkdtemp') + def test_call_fails_salt_thin(self, mkdtemp, exist): + ''' + Test execution of Salt functions in chroot. + ''' + # Fail the inner command + exist.return_value = True + mkdtemp.return_value = '/chroot/tmp01' + utils_mock = { + 'thin.gen_thin': MagicMock(return_value='/salt-thin.tgz'), + 'files.rm_rf': MagicMock(), + } + salt_mock = { + 'archive.tar': MagicMock(return_value=''), + 'config.option': MagicMock(), + 'cmd.run_chroot': MagicMock(return_value={ + 'retcode': 1, + 'stderr': 'Error', + }), + } + with patch.dict(chroot.__utils__, utils_mock), \ + patch.dict(chroot.__salt__, salt_mock): + self.assertRaises(CommandExecutionError, chroot.call, '/chroot', + 'test.ping') + 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() + utils_mock['files.rm_rf'].assert_called_once() + + @patch('salt.modules.chroot.exist') + @patch('tempfile.mkdtemp') + def test_call_success(self, mkdtemp, exist): + ''' + Test execution of Salt functions in chroot. + ''' + # 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', 'test.ping'), '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_once() + utils_mock['files.rm_rf'].assert_called_once() From a21471a6bc2b91577f4772f585a156e5a8c03734 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Wed, 24 Jul 2019 13:50:48 +0200 Subject: [PATCH 2/6] chroot: add missing sys directory --- salt/modules/chroot.py | 5 ++++- tests/unit/modules/test_chroot.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/salt/modules/chroot.py b/salt/modules/chroot.py index 6e4705b67ee7..bac13f45c06d 100644 --- a/salt/modules/chroot.py +++ b/salt/modules/chroot.py @@ -56,7 +56,8 @@ def exist(name): ''' dev = os.path.join(name, 'dev') proc = os.path.join(name, 'proc') - return all(os.path.isdir(i) for i in (name, dev, proc)) + sys = os.path.join(name, 'sys') + return all(os.path.isdir(i) for i in (name, dev, proc, sys)) def create(name): @@ -80,9 +81,11 @@ def create(name): if not exist(name): dev = os.path.join(name, 'dev') proc = os.path.join(name, 'proc') + sys = os.path.join(name, '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 diff --git a/tests/unit/modules/test_chroot.py b/tests/unit/modules/test_chroot.py index 7181dd7e5096..6204abf03bef 100644 --- a/tests/unit/modules/test_chroot.py +++ b/tests/unit/modules/test_chroot.py @@ -63,10 +63,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') From 0643179efd6244cb89995816f24b2c1bb0cb21a1 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Wed, 24 Jul 2019 14:07:05 +0200 Subject: [PATCH 3/6] chroot: change variable name to root --- salt/modules/chroot.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/salt/modules/chroot.py b/salt/modules/chroot.py index bac13f45c06d..f6013b74860b 100644 --- a/salt/modules/chroot.py +++ b/salt/modules/chroot.py @@ -50,17 +50,17 @@ 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') - sys = os.path.join(name, 'sys') - return all(os.path.isdir(i) for i in (name, dev, proc, sys)) + 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. @@ -68,7 +68,7 @@ def create(name): install the minimal required binaries, including Python if chroot.call is called. - name + root Path to the chroot environment CLI Example: @@ -78,10 +78,10 @@ def create(name): salt myminion chroot.create /chroot ''' - if not exist(name): - dev = os.path.join(name, 'dev') - proc = os.path.join(name, 'proc') - sys = os.path.join(name, 'sys') + 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) @@ -92,14 +92,14 @@ def create(name): 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 @@ -116,12 +116,12 @@ def call(name, function, *args, **kwargs): 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', ''), @@ -133,7 +133,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 = [ @@ -148,7 +148,7 @@ def call(name, function, *args, **kwargs): '--', 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]) + ret = __salt__['cmd.run_chroot'](root, [str(x) for x in salt_argv]) if ret['retcode'] != EX_OK: raise CommandExecutionError(ret['stderr']) From 3c4d06cbe85f530504fb534d69a23ddd985eb729 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Wed, 24 Jul 2019 14:13:47 +0200 Subject: [PATCH 4/6] chroot: fix bug in safe_kwargs iteration --- salt/modules/chroot.py | 3 ++- tests/unit/modules/test_chroot.py | 39 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/salt/modules/chroot.py b/salt/modules/chroot.py index f6013b74860b..17b5890d8c7a 100644 --- a/salt/modules/chroot.py +++ b/salt/modules/chroot.py @@ -110,6 +110,7 @@ def call(root, 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 ''' @@ -147,7 +148,7 @@ def call(root, function, *args, **kwargs): '-l', 'quiet', '--', function - ] + list(args) + ['{}={}'.format(k, v) for (k, v) in safe_kwargs] + ] + 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']) diff --git a/tests/unit/modules/test_chroot.py b/tests/unit/modules/test_chroot.py index 6204abf03bef..c6e4787a8273 100644 --- a/tests/unit/modules/test_chroot.py +++ b/tests/unit/modules/test_chroot.py @@ -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 @@ -182,3 +183,41 @@ def test_call_success(self, mkdtemp, exist): salt_mock['archive.tar'].assert_called_once() salt_mock['cmd.run_chroot'].assert_called_once() 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() From e1fbec7fd5246177bb4d3ef2f3aaf086ee0213fa Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Thu, 29 Aug 2019 11:47:02 +0200 Subject: [PATCH 5/6] test_chroot: assert over cmd.run_chroot params --- tests/unit/modules/test_chroot.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/unit/modules/test_chroot.py b/tests/unit/modules/test_chroot.py index c6e4787a8273..761d6ebf0aa4 100644 --- a/tests/unit/modules/test_chroot.py +++ b/tests/unit/modules/test_chroot.py @@ -150,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') @@ -181,7 +186,12 @@ 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') From ae1d548ebce386523e5e3424f475bb27c61ded1d Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Thu, 21 Nov 2019 13:28:43 +0100 Subject: [PATCH 6/6] test_chroot: skip Windows for testing Also reformulate one test to avoid random ordering of kwargs parameters. --- tests/unit/modules/test_chroot.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/modules/test_chroot.py b/tests/unit/modules/test_chroot.py index 761d6ebf0aa4..0219a967e797 100644 --- a/tests/unit/modules/test_chroot.py +++ b/tests/unit/modules/test_chroot.py @@ -41,9 +41,11 @@ ) from salt.exceptions import CommandExecutionError +import salt.utils.platform import salt.modules.chroot as chroot +@skipIf(salt.utils.platform.is_windows(), 'This test cannot work on Windows') @skipIf(NO_MOCK, NO_MOCK_REASON) class ChrootTestCase(TestCase, LoaderModuleMockMixin): ''' @@ -218,8 +220,8 @@ def test_call_success_parameters(self, mkdtemp, exist): } 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') + self.assertEqual(chroot.call('/chroot', 'module.function', + key='value'), 'result') utils_mock['thin.gen_thin'].assert_called_once() salt_mock['config.option'].assert_called() salt_mock['archive.tar'].assert_called_once() @@ -229,5 +231,5 @@ def test_call_success_parameters(self, mkdtemp, exist): '--metadata', '--local', '--log-file', '/tmp01/log', '--cachedir', '/tmp01/cache', '--out', 'json', '-l', 'quiet', - '--', 'ssh.set_auth_key', 'user=user', 'key=key']) + '--', 'module.function', 'key=value']) utils_mock['files.rm_rf'].assert_called_once()