Use the security context of the playbook when checking out roles
When the executor checks out and runs content, the security context
(trusted or untrusted) comes in to play in two ways: whether
speculative merging should be used when checking out the content
and the level of access to ansible.
This is straightforward for playbooks: when running an untrusted
playbook, use the speculatively merged repo and the untrusted
ansible environment. When running a trusted playbook, only use
the branch tip of the playbook's repo, and the trusted ansible
environment.
When we consider roles, we also need to consider whether to use
the speculatively merged role repo, or the branch tip. The current
code uses the security context of the role repo to decide which to
do (untrusted role repo uses the speculatively merged repo,
trusted role repo uses branch tip). However this presents a problem.
Consider a job defined in a trusted repo which uses a role defined
in an untrusted repo. The playbook will be run in the trusted
execution context, but the role it depends on will come from the
speculatively-merged role repo. This means a user could propose a
change which Depends-On a change to the role repo and cause mischief.
The author of the job in the trusted repo should be able to rely on
the fact that when that job runs, both the playbook and the roles
used by that playbook only contain code that is actually in the
respective repositories. Likewise, should a job in an untrusted
repo inherit from that job, when its playbook runs, it should be
able to use a speculative change to the role repo.
In short, when we are running a trusted playbook, we should use the
branch tip of all role repos used by that playbook. And when we run
an untrusted playbook, we should use any speculatively merged changes
to those roles.
Since we can run both kinds of playbooks in a single job, this change
prepares roles in both manners, if necessary. If any playbook run by
the job is untrusted, we will prepare the speculatively-merged repo
as a role. If any playbook is trusted (or the role does not appear in
the dependency chain for the change), we prepare the branch tip of the
role repo. When we run the playbooks, we use the appropriate version
of each role based on the security context of the playbook.
Change-Id: I06dd3851a8f805dba9afe1b4a0eaa1b2fdd4efa2
diff --git a/zuul/configloader.py b/zuul/configloader.py
index d7cef94..d981d8c 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -408,7 +408,7 @@
return model.ZuulRole(role.get('name', name),
project.connection_name,
- project.name, trusted)
+ project.name)
class ProjectTemplateParser(object):
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index fa0f4d5..4801de2 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -108,7 +108,8 @@
self.pre_playbooks = []
self.post_playbooks = []
self.roles = []
- self.roles_path = []
+ self.trusted_roles_path = []
+ self.untrusted_roles_path = []
self.untrusted_config = os.path.join(
self.ansible_root, 'untrusted.cfg')
self.trusted_config = os.path.join(self.ansible_root, 'trusted.cfg')
@@ -142,6 +143,10 @@
count = len(self.roles)
root = os.path.join(self.ansible_root, 'role_%i' % (count,))
os.makedirs(root)
+ trusted = os.path.join(root, 'trusted')
+ os.makedirs(trusted)
+ untrusted = os.path.join(root, 'untrusted')
+ os.makedirs(untrusted)
self.roles.append(root)
return root
@@ -601,9 +606,9 @@
repo.delete_remote(repo.remotes.origin)
# is the playbook in a repo that we have already prepared?
- self.preparePlaybookRepos(args)
+ trusted, untrusted = self.preparePlaybookRepos(args)
- self.prepareRoles(args)
+ self.prepareRoles(args, trusted, untrusted)
# TODOv3: Ansible the ansible thing here.
self.prepareAnsibleFiles(args)
@@ -737,15 +742,24 @@
return None
def preparePlaybookRepos(self, args):
+ trusted = untrusted = False
for playbook in args['pre_playbooks']:
jobdir_playbook = self.jobdir.addPrePlaybook()
self.preparePlaybookRepo(jobdir_playbook, playbook,
args, required=True)
+ if playbook['trusted']:
+ trusted = True
+ else:
+ untrusted = True
for playbook in args['playbooks']:
jobdir_playbook = self.jobdir.addPlaybook()
self.preparePlaybookRepo(jobdir_playbook, playbook,
args, required=False)
+ if playbook['trusted']:
+ trusted = True
+ else:
+ untrusted = True
if jobdir_playbook.path is not None:
self.jobdir.playbook = jobdir_playbook
break
@@ -756,6 +770,11 @@
jobdir_playbook = self.jobdir.addPostPlaybook()
self.preparePlaybookRepo(jobdir_playbook, playbook,
args, required=True)
+ if playbook['trusted']:
+ trusted = True
+ else:
+ untrusted = True
+ return (trusted, untrusted)
def preparePlaybookRepo(self, jobdir_playbook, playbook, args, required):
self.log.debug("Prepare playbook repo for %s" % (playbook,))
@@ -799,11 +818,11 @@
required=required,
trusted=playbook['trusted'])
- def prepareRoles(self, args):
+ def prepareRoles(self, args, trusted, untrusted):
for role in args['roles']:
if role['type'] == 'zuul':
root = self.jobdir.addRole()
- self.prepareZuulRole(args, role, root)
+ self.prepareZuulRole(args, role, root, trusted, untrusted)
def findRole(self, path, trusted=False):
d = os.path.join(path, 'tasks')
@@ -826,17 +845,22 @@
self._blockPluginDirs(os.path.join(path, entry))
return path
- def prepareZuulRole(self, args, role, root):
+ def prepareZuulRole(self, args, role, root, trusted, untrusted):
self.log.debug("Prepare zuul role for %s" % (role,))
# Check out the role repo if needed
source = self.executor_server.connections.getSource(
role['connection'])
project = source.getProject(role['project'])
- role_repo = None
- if not role['trusted']:
- # This is a project repo, so it is safe to use the already
- # checked out version (from speculative merging) of the
- # role
+ untrusted_role_repo = None
+ trusted_role_repo = None
+ trusted_root = os.path.join(root, 'trusted')
+ untrusted_root = os.path.join(root, 'untrusted')
+ name = role['target_name']
+
+ if untrusted:
+ # There is at least one untrusted playbook. For that
+ # case, use the already checked out version (from
+ # speculative merging) of the role.
for i in args['items']:
if (i['connection'] == role['connection'] and
@@ -847,27 +871,70 @@
path = os.path.join(self.jobdir.src_root,
project.canonical_hostname,
project.name)
- link = os.path.join(root, role['name'])
+ # The name of the symlink is the requested name of
+ # the role (which may be the repo name or may be
+ # something else; this can come into play if this
+ # is a bare role).
+ link = os.path.join(untrusted_root, name)
+ link = os.path.realpath(link)
+ if not link.startswith(os.path.realpath(untrusted_root)):
+ raise Exception("Invalid role name %s", name)
os.symlink(path, link)
- role_repo = link
+ untrusted_role_repo = link
break
- # The role repo is either a config repo, or it isn't in
- # the stack of changes we are testing, so check out the branch
- # tip into a dedicated space.
-
- if not role_repo:
- merger = self.executor_server._getMerger(root)
+ if trusted or not untrusted_role_repo:
+ # There is at least one trusted playbook which will need a
+ # trusted checkout of the role, or the role did not appear
+ # in the dependency chain for the change (in which case,
+ # there is no existing untrusted checkout of it). Check
+ # out the branch tip into a dedicated space.
+ merger = self.executor_server._getMerger(trusted_root)
merger.checkoutBranch(role['connection'], project.name,
'master')
- role_repo = os.path.join(root, project.canonical_hostname,
- project.name)
+ orig_repo_path = os.path.join(trusted_root,
+ project.canonical_hostname,
+ project.name)
+ if name != project.name:
+ # The requested name of the role is not the same as
+ # the project name, so rename the git repo as the
+ # requested name. It is the only item in this
+ # directory, so we don't need to worry about
+ # collisions.
+ target = os.path.join(trusted_root,
+ project.canonical_hostname,
+ name)
+ target = os.path.realpath(target)
+ if not target.startswith(os.path.realpath(trusted_root)):
+ raise Exception("Invalid role name %s", name)
+ os.rename(orig_repo_path, target)
+ trusted_role_repo = target
+ else:
+ trusted_role_repo = orig_repo_path
- role_path = self.findRole(role_repo, trusted=role['trusted'])
- if role_path is None:
- # In the case of a bare role, add the containing directory
- role_path = os.path.join(root, project.canonical_hostname)
- self.jobdir.roles_path.append(role_path)
+ if not untrusted_role_repo:
+ # In the case that there was no untrusted checkout,
+ # use the trusted checkout.
+ untrusted_role_repo = trusted_role_repo
+ untrusted_root = trusted_root
+
+ if untrusted:
+ untrusted_role_path = self.findRole(untrusted_role_repo,
+ trusted=False)
+ if untrusted_role_path is None:
+ # In the case of a bare role, add the containing directory
+ untrusted_role_path = os.path.join(untrusted_root,
+ project.canonical_hostname)
+ self.jobdir.untrusted_roles_path.append(untrusted_role_path)
+
+ if trusted:
+ trusted_role_path = self.findRole(trusted_role_repo,
+ trusted=True)
+ if trusted_role_path is None:
+ # In the case of a bare role, add the containing directory
+ trusted_role_path = os.path.join(trusted_root,
+ project.canonical_hostname)
+ self.jobdir.trusted_roles_path.append(trusted_role_path)
def prepareAnsibleFiles(self, args):
keys = []
@@ -909,9 +976,6 @@
config.write('gathering = explicit\n')
config.write('library = %s\n'
% self.executor_server.library_dir)
- if self.jobdir.roles_path:
- config.write('roles_path = %s\n' %
- ':'.join(self.jobdir.roles_path))
config.write('command_warnings = False\n')
config.write('callback_plugins = %s\n'
% self.executor_server.callback_dir)
@@ -924,6 +988,12 @@
% self.executor_server.action_dir)
config.write('lookup_plugins = %s\n'
% self.executor_server.lookup_dir)
+ roles_path = self.jobdir.untrusted_roles_path
+ else:
+ roles_path = self.jobdir.trusted_roles_path
+
+ if roles_path:
+ config.write('roles_path = %s\n' % ':'.join(roles_path))
# On trusted jobs, we want to prevent the printing of args,
# since trusted jobs might have access to secrets that they may
diff --git a/zuul/model.py b/zuul/model.py
index af80028..73dec39 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -673,11 +673,10 @@
class ZuulRole(Role):
"""A reference to an ansible role in a Zuul project."""
- def __init__(self, target_name, connection_name, project_name, trusted):
+ def __init__(self, target_name, connection_name, project_name):
super(ZuulRole, self).__init__(target_name)
self.connection_name = connection_name
self.project_name = project_name
- self.trusted = trusted
def __repr__(self):
return '<ZuulRole %s %s>' % (self.project_name, self.target_name)
@@ -687,8 +686,7 @@
return False
return (super(ZuulRole, self).__eq__(other) and
self.connection_name == other.connection_name,
- self.project_name == other.project_name,
- self.trusted == other.trusted)
+ self.project_name == other.project_name)
def toDict(self):
# Render to a dict to use in passing json to the executor
@@ -696,7 +694,6 @@
d['type'] = 'zuul'
d['connection'] = self.connection_name
d['project'] = self.project_name
- d['trusted'] = self.trusted
return d