From 44aef15d6ef65c2296b13330d230308075f8df37 Mon Sep 17 00:00:00 2001 From: Tristan Cacqueray Date: Thu, 15 Jun 2017 06:00:12 +0000 Subject: [PATCH] executor: run trusted playbook in a bubblewrap This change renames untrusted_wrapper to execution_wrapper and uses bubblewrap for both trusted and untrusted playbooks by default. This change adds new options to the zuul.conf executor section to let operators define what directories to mount ro or rw for both context: * trusted_ro_dirs/trusted_rw_dirs, and * untrusted_ro_dirs/untrusted_rw_dirs Change-Id: I9a8a74a338a8a837913db5e2effeef1bd949a49c Story: 2001070 Task: 4687 --- etc/zuul.conf-sample | 2 ++ tests/base.py | 2 ++ tests/unit/test_bubblewrap.py | 3 --- zuul/driver/__init__.py | 10 ++++++++++ zuul/driver/bubblewrap/__init__.py | 12 +++++++++--- zuul/driver/nullwrap/__init__.py | 3 +++ zuul/executor/server.py | 28 +++++++++++++++++++--------- 7 files changed, 45 insertions(+), 15 deletions(-) diff --git a/etc/zuul.conf-sample b/etc/zuul.conf-sample index 1aee1fa18d..2909ea6df2 100644 --- a/etc/zuul.conf-sample +++ b/etc/zuul.conf-sample @@ -26,6 +26,8 @@ zuul_url=http://zuul.example.com/p [executor] default_username=zuul +trusted_ro_dirs=/opt/zuul-scripts:/var/cache +trusted_rw_dirs=/opt/zuul-logs [webapp] listen_address=0.0.0.0 diff --git a/tests/base.py b/tests/base.py index 93b5785de9..bf6ad26957 100755 --- a/tests/base.py +++ b/tests/base.py @@ -2024,6 +2024,8 @@ class ZuulTestCase(BaseTestCase): project = reponame.replace('_', '/') self.copyDirToRepo(project, os.path.join(git_path, reponame)) + # Make test_root persist after ansible run for .flag test + self.config.set('executor', 'trusted_rw_dirs', self.test_root) self.setupAllProjectKeys() def setupSimpleLayout(self): diff --git a/tests/unit/test_bubblewrap.py b/tests/unit/test_bubblewrap.py index 1928222d90..675221ee8b 100644 --- a/tests/unit/test_bubblewrap.py +++ b/tests/unit/test_bubblewrap.py @@ -31,17 +31,14 @@ class TestBubblewrap(testtools.TestCase): def test_bubblewrap_wraps(self): bwrap = bubblewrap.BubblewrapDriver() work_dir = tempfile.mkdtemp() - ansible_dir = tempfile.mkdtemp() ssh_agent = SshAgent() self.addCleanup(ssh_agent.stop) ssh_agent.start() po = bwrap.getPopen(work_dir=work_dir, - ansible_dir=ansible_dir, ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK']) self.assertTrue(po.passwd_r > 2) self.assertTrue(po.group_r > 2) self.assertTrue(work_dir in po.command) - self.assertTrue(ansible_dir in po.command) # Now run /usr/bin/id to verify passwd/group entries made it in true_proc = po(['/usr/bin/id'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) diff --git a/zuul/driver/__init__.py b/zuul/driver/__init__.py index 0c3105d1be..bb28d1fd93 100644 --- a/zuul/driver/__init__.py +++ b/zuul/driver/__init__.py @@ -278,3 +278,13 @@ class WrapperInterface(object): :rtype: Callable """ pass + + @abc.abstractmethod + def setMountsMap(self, state_dir, ro_dirs=[], rw_dirs=[]): + """Add additional mount point to the execution environment. + + :arg str state_dir: the state directory to be read write + :arg list ro_dirs: read only directories paths + :arg list rw_dirs: read write directories paths + """ + pass diff --git a/zuul/driver/bubblewrap/__init__.py b/zuul/driver/bubblewrap/__init__.py index 580aaf9499..b92b365e8e 100644 --- a/zuul/driver/bubblewrap/__init__.py +++ b/zuul/driver/bubblewrap/__init__.py @@ -84,7 +84,6 @@ class BubblewrapDriver(Driver, WrapperInterface): '--ro-bind', '/bin', '/bin', '--ro-bind', '/sbin', '/sbin', '--ro-bind', '/etc/resolv.conf', '/etc/resolv.conf', - '--ro-bind', '{ansible_dir}', '{ansible_dir}', '--ro-bind', '{ssh_auth_sock}', '{ssh_auth_sock}', '--dir', '{work_dir}', '--bind', '{work_dir}', '{work_dir}', @@ -99,6 +98,7 @@ class BubblewrapDriver(Driver, WrapperInterface): '--file', '{uid_fd}', '/etc/passwd', '--file', '{gid_fd}', '/etc/group', ] + mounts_map = {'rw': [], 'ro': []} def reconfigure(self, tenant): pass @@ -106,6 +106,9 @@ class BubblewrapDriver(Driver, WrapperInterface): def stop(self): pass + def setMountsMap(self, state_dir, ro_dirs=[], rw_dirs=[]): + self.mounts_map = {'ro': ro_dirs, 'rw': [state_dir] + rw_dirs} + def getPopen(self, **kwargs): # Set zuul_dir if it was not passed in if 'zuul_dir' in kwargs: @@ -119,6 +122,11 @@ class BubblewrapDriver(Driver, WrapperInterface): if not zuul_dir.startswith('/usr'): bwrap_command.extend(['--ro-bind', zuul_dir, zuul_dir]) + for mount_type in ('ro', 'rw'): + bind_arg = '--ro-bind' if mount_type == 'ro' else '--bind' + for bind in self.mounts_map[mount_type]: + bwrap_command.extend([bind_arg, bind, bind]) + # Need users and groups uid = os.getuid() passwd = pwd.getpwuid(uid) @@ -159,14 +167,12 @@ def main(args=None): parser = argparse.ArgumentParser() parser.add_argument('work_dir') - parser.add_argument('ansible_dir') parser.add_argument('run_args', nargs='+') cli_args = parser.parse_args() ssh_auth_sock = os.environ.get('SSH_AUTH_SOCK') popen = driver.getPopen(work_dir=cli_args.work_dir, - ansible_dir=cli_args.ansible_dir, ssh_auth_sock=ssh_auth_sock) x = popen(cli_args.run_args) x.wait() diff --git a/zuul/driver/nullwrap/__init__.py b/zuul/driver/nullwrap/__init__.py index ebcd1da390..50fea2759a 100644 --- a/zuul/driver/nullwrap/__init__.py +++ b/zuul/driver/nullwrap/__init__.py @@ -26,3 +26,6 @@ class NullwrapDriver(Driver, WrapperInterface): def getPopen(self, **kwargs): return subprocess.Popen + + def setMountsMap(self, **kwargs): + pass diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 7d7299e9e1..7256aafa7b 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -382,9 +382,9 @@ class ExecutorServer(object): 'default_username', 'zuul') self.merge_email = get_default(self.config, 'merger', 'git_user_email') self.merge_name = get_default(self.config, 'merger', 'git_user_name') - untrusted_wrapper_name = get_default(self.config, 'executor', - 'untrusted_wrapper', 'bubblewrap') - self.untrusted_wrapper = connections.drivers[untrusted_wrapper_name] + execution_wrapper_name = get_default(self.config, 'executor', + 'execution_wrapper', 'bubblewrap') + self.execution_wrapper = connections.drivers[execution_wrapper_name] self.connections = connections # This merger and its git repos are used to maintain @@ -1238,14 +1238,24 @@ class AnsibleJob(object): if trusted: config_file = self.jobdir.trusted_config - popen = subprocess.Popen + opt_prefix = 'trusted' else: config_file = self.jobdir.untrusted_config - driver = self.executor_server.untrusted_wrapper - popen = driver.getPopen( - work_dir=self.jobdir.root, - ansible_dir=self.executor_server.ansible_dir, - ssh_auth_sock=env_copy.get('SSH_AUTH_SOCK')) + opt_prefix = 'untrusted' + ro_dirs = get_default(self.executor_server.config, 'executor', + '%s_ro_dirs' % opt_prefix) + rw_dirs = get_default(self.executor_server.config, 'executor', + '%s_rw_dirs' % opt_prefix) + state_dir = get_default(self.executor_server.config, 'zuul', + 'state_dir', '/var/lib/zuul', expand_user=True) + ro_dirs = ro_dirs.split(":") if ro_dirs else [] + rw_dirs = rw_dirs.split(":") if rw_dirs else [] + self.executor_server.execution_wrapper.setMountsMap(state_dir, ro_dirs, + rw_dirs) + + popen = self.executor_server.execution_wrapper.getPopen( + work_dir=self.jobdir.root, + ssh_auth_sock=env_copy.get('SSH_AUTH_SOCK')) env_copy['ANSIBLE_CONFIG'] = config_file