Merge "Retrieve filtered list of hosts for a task, not all hosts" into feature/zuulv3
diff --git a/doc/source/admin/components.rst b/doc/source/admin/components.rst
index aa6d8c8..2c70d47 100644
--- a/doc/source/admin/components.rst
+++ b/doc/source/admin/components.rst
@@ -311,10 +311,10 @@
*trusted* execution context, otherwise, it is run in the *untrusted*
execution context.
-Both execution contexts use `bubblewrap`_ to create a namespace to
-ensure that playbook executions are isolated and are unable to access
-files outside of a restricted environment. The administrator may
-configure additional local directories on the executor to be made
+Both execution contexts use `bubblewrap`_ [#nullwrap]_ to create a
+namespace to ensure that playbook executions are isolated and are unable
+to access files outside of a restricted environment. The administrator
+may configure additional local directories on the executor to be made
available to the restricted environment.
The trusted execution context has access to all Ansible features,
@@ -335,6 +335,8 @@
protections are made as part of a defense-in-depth strategy.
.. _bubblewrap: https://github.com/projectatomic/bubblewrap
+.. [#nullwrap] Unless one has set execution_wrapper to nullwrap in the
+ executor configuration.
Configuration
~~~~~~~~~~~~~
@@ -437,6 +439,25 @@
List of paths, separated by ``:`` to read-write bind mount into
untrusted bubblewrap contexts.
+ .. attr:: execution_wrapper
+ :default: bubblewrap
+
+ Name of the execution wrapper to use when executing
+ `ansible-playbook`. The default, `bubblewrap` is recommended for
+ all installations.
+
+ There is also a `nullwrap` driver for situations where one wants
+ to run Zuul without access to bubblewrap or in such a way that
+ bubblewrap may interfere with the jobs themselves. However,
+ `nullwrap` is considered unsafe, as `bubblewrap` provides
+ significant protections against malicious users and accidental
+ breakage in playbooks. As such, `nullwrap` is not recommended
+ for use in production.
+
+ This option, and thus, `nullwrap`, may be removed in the future.
+ `bubblewrap` has become integral to securely operating Zuul. If you
+ have a valid use case for it, we encourage you to let us know.
+
.. attr:: merger
.. attr:: git_user_email
diff --git a/tests/unit/test_bubblewrap.py b/tests/unit/test_bubblewrap.py
index d94b3f2..661d868 100644
--- a/tests/unit/test_bubblewrap.py
+++ b/tests/unit/test_bubblewrap.py
@@ -32,14 +32,15 @@
def test_bubblewrap_wraps(self):
bwrap = bubblewrap.BubblewrapDriver()
+ context = bwrap.getExecutionContext()
work_dir = tempfile.mkdtemp()
ssh_agent = SshAgent()
self.addCleanup(ssh_agent.stop)
ssh_agent.start()
- po = bwrap.getPopen(work_dir=work_dir,
- ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
- self.assertTrue(po.passwd_r > 2)
- self.assertTrue(po.group_r > 2)
+ po = context.getPopen(work_dir=work_dir,
+ ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
+ self.assertTrue(po.fds[0] > 2)
+ self.assertTrue(po.fds[1] > 2)
self.assertTrue(work_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,
@@ -50,19 +51,19 @@
# And that it did not print things on stderr
self.assertEqual(0, len(errs.strip()))
# Make sure the _r's are closed
- self.assertIsNone(po.passwd_r)
- self.assertIsNone(po.group_r)
+ self.assertEqual([], po.fds)
def test_bubblewrap_leak(self):
bwrap = bubblewrap.BubblewrapDriver()
+ context = bwrap.getExecutionContext()
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'])
+ po = context.getPopen(work_dir=work_dir,
+ ansible_dir=ansible_dir,
+ ssh_auth_sock=ssh_agent.env['SSH_AUTH_SOCK'])
leak_time = 60
# Use hexadecimal notation to avoid false-positive
true_proc = po(['bash', '-c', 'sleep 0x%X & disown' % leak_time])
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index e4e71d9..98cffcc 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -1292,8 +1292,7 @@
], ordered=False)
matches = self.searchForContent(self.history[0].jobdir.root,
b'test-password')
- self.assertEqual(set(['/ansible/playbook_0/secrets.yaml',
- '/work/secret-file.txt']),
+ self.assertEqual(set(['/work/secret-file.txt']),
set(matches))
def test_secret_file(self):
@@ -1328,8 +1327,7 @@
], ordered=False)
matches = self.searchForContent(self.history[0].jobdir.root,
b'test-password')
- self.assertEqual(set(['/ansible/playbook_0/secrets.yaml',
- '/work/failure-file.txt']),
+ self.assertEqual(set(['/work/failure-file.txt']),
set(matches))
def test_secret_file_fail(self):
diff --git a/zuul/driver/__init__.py b/zuul/driver/__init__.py
index 6ac9197..682b251 100644
--- a/zuul/driver/__init__.py
+++ b/zuul/driver/__init__.py
@@ -258,25 +258,22 @@
"""
@abc.abstractmethod
- def getPopen(self, **kwargs):
- """Create and return a subprocess.Popen factory wrapped however the
- driver sees fit.
+ def getExecutionContext(self, ro_paths=None, rw_paths=None, secrets=None):
+ """Create and return an execution context.
+
+ The execution context is meant to be used for a single
+ invocation of a command.
This method is required by the interface
- :arg dict kwargs: key/values for use by driver as needed
-
- :returns: a callable that takes the same args as subprocess.Popen
- :rtype: Callable
- """
- pass
-
- @abc.abstractmethod
- def setMountsMap(self, state_dir, ro_paths=None, rw_paths=None):
- """Add additional mount point to the execution environment.
-
- :arg str state_dir: the state directory to be read write
:arg list ro_paths: read only files or directories to bind mount
:arg list rw_paths: read write files or directories to bind mount
+ :arg dict secrets: a dictionary where the key is a file path,
+ and the value is the content which should be written to
+ that path in a secure manner.
+
+ :returns: a new ExecutionContext object.
+ :rtype: BaseExecutionContext
+
"""
pass
diff --git a/zuul/driver/bubblewrap/__init__.py b/zuul/driver/bubblewrap/__init__.py
index cbaa609..a8969cc 100644
--- a/zuul/driver/bubblewrap/__init__.py
+++ b/zuul/driver/bubblewrap/__init__.py
@@ -22,18 +22,19 @@
import shlex
import subprocess
import sys
+import threading
import re
from typing import Dict, List # flake8: noqa
from zuul.driver import (Driver, WrapperInterface)
+from zuul.execution_context import BaseExecutionContext
class WrappedPopen(object):
- def __init__(self, command, passwd_r, group_r):
+ def __init__(self, command, fds):
self.command = command
- self.passwd_r = passwd_r
- self.group_r = group_r
+ self.fds = fds
def __call__(self, args, *sub_args, **kwargs):
try:
@@ -45,7 +46,7 @@
# versions. So until we are py3 only we can only bwrap
# things that are close_fds=False
pass_fds = list(kwargs.get('pass_fds', []))
- for fd in (self.passwd_r, self.group_r):
+ for fd in self.fds:
if fd not in pass_fds:
pass_fds.append(fd)
kwargs['pass_fds'] = pass_fds
@@ -55,42 +56,32 @@
return proc
def __del__(self):
- if self.passwd_r:
+ for fd in self.fds:
try:
- os.close(self.passwd_r)
+ os.close(fd)
except OSError:
pass
- self.passwd_r = None
- if self.group_r:
- try:
- os.close(self.group_r)
- except OSError:
- pass
- self.group_r = None
+ self.fds = []
-class BubblewrapDriver(Driver, WrapperInterface):
- name = 'bubblewrap'
- log = logging.getLogger("zuul.BubblewrapDriver")
+class BubblewrapExecutionContext(BaseExecutionContext):
+ log = logging.getLogger("zuul.BubblewrapExecutionContext")
- mounts_map = {'rw': [], 'ro': []} # type: Dict[str, List]
- release_file_re = re.compile('^\W+-release$')
-
- def __init__(self):
- self.bwrap_command = self._bwrap_command()
-
- def reconfigure(self, tenant):
- pass
-
- def stop(self):
- pass
-
- def setMountsMap(self, ro_paths=None, rw_paths=None):
- if not ro_paths:
- ro_paths = []
- if not rw_paths:
- rw_paths = []
+ def __init__(self, bwrap_command, ro_paths, rw_paths, secrets):
+ self.bwrap_command = bwrap_command
self.mounts_map = {'ro': ro_paths, 'rw': rw_paths}
+ self.secrets = secrets
+
+ def startPipeWriter(self, pipe, data):
+ # In case we have a large amount of data to write through a
+ # pipe, spawn a thread to handle the writes.
+ t = threading.Thread(target=self._writer, args=(pipe, data))
+ t.daemon = True
+ t.start()
+
+ def _writer(self, pipe, data):
+ os.write(pipe, data)
+ os.close(pipe)
def getPopen(self, **kwargs):
# Set zuul_dir if it was not passed in
@@ -110,6 +101,9 @@
for bind in self.mounts_map[mount_type]:
bwrap_command.extend([bind_arg, bind, bind])
+ # A list of file descriptors which must be held open so that
+ # bwrap may read from them.
+ read_fds = []
# Need users and groups
uid = os.getuid()
passwd = list(pwd.getpwuid(uid))
@@ -121,6 +115,7 @@
os.write(passwd_w, passwd_bytes)
os.write(passwd_w, b'\n')
os.close(passwd_w)
+ read_fds.append(passwd_r)
gid = os.getgid()
group = grp.getgrgid(gid)
@@ -130,6 +125,20 @@
os.write(group_w, group_bytes)
os.write(group_w, b'\n')
os.close(group_w)
+ read_fds.append(group_r)
+
+ # Create a tmpfs for each directory which holds secrets, and
+ # tell bubblewrap to write the contents to a file therein.
+ secret_dirs = set()
+ for fn, content in self.secrets.items():
+ secret_dir = os.path.dirname(fn)
+ if secret_dir not in secret_dirs:
+ bwrap_command.extend(['--tmpfs', secret_dir])
+ secret_dirs.add(secret_dir)
+ secret_r, secret_w = os.pipe()
+ self.startPipeWriter(secret_w, content.encode('utf8'))
+ bwrap_command.extend(['--file', str(secret_r), fn])
+ read_fds.append(secret_r)
kwargs = dict(kwargs) # Don't update passed in dict
kwargs['uid'] = uid
@@ -141,10 +150,26 @@
self.log.debug("Bubblewrap command: %s",
" ".join(shlex.quote(c) for c in command))
- wrapped_popen = WrappedPopen(command, passwd_r, group_r)
+ wrapped_popen = WrappedPopen(command, read_fds)
return wrapped_popen
+
+class BubblewrapDriver(Driver, WrapperInterface):
+ log = logging.getLogger("zuul.BubblewrapDriver")
+ name = 'bubblewrap'
+
+ release_file_re = re.compile('^\W+-release$')
+
+ def __init__(self):
+ self.bwrap_command = self._bwrap_command()
+
+ def reconfigure(self, tenant):
+ pass
+
+ def stop(self):
+ pass
+
def _bwrap_command(self):
bwrap_command = [
'bwrap',
@@ -185,6 +210,18 @@
return bwrap_command
+ def getExecutionContext(self, ro_paths=None, rw_paths=None, secrets=None):
+ if not ro_paths:
+ ro_paths = []
+ if not rw_paths:
+ rw_paths = []
+ if not secrets:
+ secrets = {}
+ return BubblewrapExecutionContext(
+ self.bwrap_command,
+ ro_paths, rw_paths,
+ secrets)
+
def main(args=None):
logging.basicConfig(level=logging.DEBUG)
@@ -192,18 +229,27 @@
driver = BubblewrapDriver()
parser = argparse.ArgumentParser()
- parser.add_argument('--ro-bind', nargs='+')
- parser.add_argument('--rw-bind', nargs='+')
+ parser.add_argument('--ro-paths', nargs='+')
+ parser.add_argument('--rw-paths', nargs='+')
+ parser.add_argument('--secret', nargs='+')
parser.add_argument('work_dir')
parser.add_argument('run_args', nargs='+')
cli_args = parser.parse_args()
ssh_auth_sock = os.environ.get('SSH_AUTH_SOCK')
- driver.setMountsMap(cli_args.ro_bind, cli_args.rw_bind)
+ secrets = {}
+ if cli_args.secret:
+ for secret in cli_args.secret:
+ fn, content = secret.split('=', 1)
+ secrets[fn]=content
- popen = driver.getPopen(work_dir=cli_args.work_dir,
- ssh_auth_sock=ssh_auth_sock)
+ context = driver.getExecutionContext(
+ cli_args.ro_paths, cli_args.rw_paths,
+ secrets)
+
+ popen = context.getPopen(work_dir=cli_args.work_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 50fea27..178e4c7 100644
--- a/zuul/driver/nullwrap/__init__.py
+++ b/zuul/driver/nullwrap/__init__.py
@@ -18,14 +18,30 @@
import subprocess
from zuul.driver import (Driver, WrapperInterface)
+from zuul.execution_context import BaseExecutionContext
+
+
+class NullExecutionContext(BaseExecutionContext):
+ log = logging.getLogger("zuul.NullExecutionContext")
+
+ def getPopen(self, **kwargs):
+ return subprocess.Popen
class NullwrapDriver(Driver, WrapperInterface):
name = 'nullwrap'
log = logging.getLogger("zuul.NullwrapDriver")
- def getPopen(self, **kwargs):
- return subprocess.Popen
-
- def setMountsMap(self, **kwargs):
- pass
+ def getExecutionContext(self, ro_paths=None, rw_paths=None, secrets=None):
+ # The bubblewrap driver writes secrets to a tmpfs so that they
+ # don't hit the disk (unless the kernel swaps the memory to
+ # disk, which can be mitigated with encrypted swap). We
+ # haven't implemented similar functionality in nullwrap, so
+ # for safety, raise an exception in that case. If you are
+ # interested in implementing this functionality, please
+ # contact us on the mailing list.
+ if secrets:
+ raise NotImplementedError(
+ "The nullwrap driver does not support the use of secrets. "
+ "Consider using the bubblewrap driver instead.")
+ return NullExecutionContext()
diff --git a/zuul/execution_context/__init__.py b/zuul/execution_context/__init__.py
new file mode 100644
index 0000000..29b2912
--- /dev/null
+++ b/zuul/execution_context/__init__.py
@@ -0,0 +1,40 @@
+# Copyright 2016 Red Hat, Inc.
+#
+# Licensed 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.
+
+import abc
+
+
+class BaseExecutionContext(object, metaclass=abc.ABCMeta):
+ """The execution interface returned by a wrapper.
+
+ Wrapper drivers return instances which implement this interface.
+
+ It is used to hold information and aid in the execution of a
+ single command.
+
+ """
+
+ @abc.abstractmethod
+ def getPopen(self, **kwargs):
+ """Create and return a subprocess.Popen factory wrapped however the
+ driver sees fit.
+
+ This method is required by the interface
+
+ :arg dict kwargs: key/values for use by driver as needed
+
+ :returns: a callable that takes the same args as subprocess.Popen
+ :rtype: Callable
+ """
+ pass
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 9340f09..1a445f1 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -255,8 +255,9 @@
self.roles_path = []
self.ansible_config = os.path.join(self.root, 'ansible.cfg')
self.project_link = os.path.join(self.root, 'project')
- self.secrets = os.path.join(self.root, 'secrets.yaml')
- self.has_secrets = False
+ self.secrets_root = os.path.join(self.root, 'secrets')
+ self.secrets = os.path.join(self.secrets_root, 'secrets.yaml')
+ self.secrets_content = None
def addRole(self):
count = len(self.roles)
@@ -1266,6 +1267,13 @@
for role in playbook['roles']:
self.prepareRole(jobdir_playbook, role, args)
+ secrets = playbook['secrets']
+ if secrets:
+ if 'zuul' in secrets:
+ raise Exception("Defining secrets named 'zuul' is not allowed")
+ jobdir_playbook.secrets_content = yaml.safe_dump(
+ secrets, default_flow_style=False)
+
self.writeAnsibleConfig(jobdir_playbook, playbook)
def checkoutTrustedProject(self, project, branch):
@@ -1390,15 +1398,6 @@
def writeAnsibleConfig(self, jobdir_playbook, playbook):
trusted = jobdir_playbook.trusted
- secrets = playbook['secrets'].copy()
- if secrets:
- if 'zuul' in secrets:
- raise Exception("Defining secrets named 'zuul' is not allowed")
- with open(jobdir_playbook.secrets, 'w') as secrets_yaml:
- secrets_yaml.write(
- yaml.safe_dump(secrets, default_flow_style=False))
- jobdir_playbook.has_secrets = True
-
# TODO(mordred) This should likely be extracted into a more generalized
# mechanism for deployers being able to add callback
# plugins.
@@ -1441,7 +1440,7 @@
# role. Otherwise, printing the args could be useful for
# debugging.
config.write('display_args_to_stdout = %s\n' %
- str(not secrets))
+ str(not playbook['secrets']))
config.write('[ssh_connection]\n')
# NB: when setting pipelining = True, keep_remote_files
@@ -1509,10 +1508,14 @@
if self.executor_variables_file:
ro_paths.append(self.executor_variables_file)
- self.executor_server.execution_wrapper.setMountsMap(ro_paths,
- rw_paths)
+ secrets = {}
+ if playbook.secrets_content:
+ secrets[playbook.secrets] = playbook.secrets_content
- popen = self.executor_server.execution_wrapper.getPopen(
+ context = self.executor_server.execution_wrapper.getExecutionContext(
+ ro_paths, rw_paths, secrets)
+
+ popen = context.getPopen(
work_dir=self.jobdir.work_root,
ssh_auth_sock=env_copy.get('SSH_AUTH_SOCK'))
@@ -1593,7 +1596,7 @@
verbose = '-v'
cmd = ['ansible-playbook', verbose, playbook.path]
- if playbook.has_secrets:
+ if playbook.secrets_content:
cmd.extend(['-e', '@' + playbook.secrets])
if success is not None: