diff --git a/doc/source/user/config.rst b/doc/source/user/config.rst index 0b2b5d4531..64dc393fec 100644 --- a/doc/source/user/config.rst +++ b/doc/source/user/config.rst @@ -697,8 +697,19 @@ unless otherwise specified: roles: a Galaxy role, which is simply a role that is installed from Ansible Galaxy, or a Zuul role, which is a role provided by a project managed by Zuul. Zuul roles are able to benefit from - speculative merging and cross-project dependencies when used by jobs - in untrusted projects. + speculative merging and cross-project dependencies when used by + playbooks in untrusted projects. + + In the case of job inheritance or variance, the roles used for each + of the playbooks run by the job will be only those which were + defined along with that playbook. If a child job inherits from a + parent which defines a pre and post playbook, then the pre and post + playbooks it inherits from the parent job will run only with the + roles that were defined on the parent. If the child adds its own + pre and post playbooks, then any roles added by the child will be + available to the child's playbooks. This is so that a job which + inherits from a parent does not inadvertantly alter the behavior of + the parent's playbooks by the addition of conflicting roles. A project which supplies a role may be structured in one of two configurations: a bare role (in which the role exists at the root of diff --git a/tests/base.py b/tests/base.py index 9709bf783a..1cc9999182 100755 --- a/tests/base.py +++ b/tests/base.py @@ -1359,12 +1359,12 @@ class RecordingAnsibleJob(zuul.executor.server.AnsibleJob): self.recordResult(result) return result - def runAnsible(self, cmd, timeout, trusted=False): + def runAnsible(self, cmd, timeout, config_file, trusted): build = self.executor_server.job_builds[self.job.unique] if self.executor_server._run_ansible: result = super(RecordingAnsibleJob, self).runAnsible( - cmd, timeout, trusted=trusted) + cmd, timeout, config_file, trusted) else: result = build.run() return result diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 7fe101e903..23a6f26755 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -95,9 +95,9 @@ class TestJob(BaseTestCase): def test_job_inheritance(self): # This is standard job inheritance. - base_pre = model.PlaybookContext(self.context, 'base-pre') - base_run = model.PlaybookContext(self.context, 'base-run') - base_post = model.PlaybookContext(self.context, 'base-post') + base_pre = model.PlaybookContext(self.context, 'base-pre', []) + base_run = model.PlaybookContext(self.context, 'base-run', []) + base_post = model.PlaybookContext(self.context, 'base-post', []) base = model.Job('base') base.timeout = 30 @@ -121,9 +121,9 @@ class TestJob(BaseTestCase): def test_job_variants(self): # This simulates freezing a job. - py27_pre = model.PlaybookContext(self.context, 'py27-pre') - py27_run = model.PlaybookContext(self.context, 'py27-run') - py27_post = model.PlaybookContext(self.context, 'py27-post') + py27_pre = model.PlaybookContext(self.context, 'py27-pre', []) + py27_run = model.PlaybookContext(self.context, 'py27-run', []) + py27_post = model.PlaybookContext(self.context, 'py27-post', []) py27 = model.Job('py27') py27.timeout = 30 diff --git a/zuul/configloader.py b/zuul/configloader.py index 254527adf4..023d01efae 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -400,21 +400,34 @@ class JobParser(object): parent = layout.getJob(conf['parent']) job.inheritFrom(parent) + # Roles are part of the playbook context so we must establish + # them earlier than playbooks. + if 'roles' in conf: + roles = [] + for role in conf.get('roles', []): + if 'zuul' in role: + r = JobParser._makeZuulRole(tenant, job, role) + if r: + roles.append(r) + job.addRoles(roles) + for pre_run_name in as_list(conf.get('pre-run')): pre_run = model.PlaybookContext(job.source_context, - pre_run_name) + pre_run_name, job.roles) job.pre_run = job.pre_run + (pre_run,) for post_run_name in as_list(conf.get('post-run')): post_run = model.PlaybookContext(job.source_context, - post_run_name) + post_run_name, job.roles) job.post_run = (post_run,) + job.post_run if 'run' in conf: - run = model.PlaybookContext(job.source_context, conf['run']) + run = model.PlaybookContext(job.source_context, conf['run'], + job.roles) job.run = (run,) else: if not project_pipeline: run_name = os.path.join('playbooks', job.name) - run = model.PlaybookContext(job.source_context, run_name) + run = model.PlaybookContext(job.source_context, run_name, + job.roles) job.implied_run = (run,) + job.implied_run for k in JobParser.simple_attributes: @@ -460,15 +473,6 @@ class JobParser(object): job.dependencies = frozenset(as_list(conf.get('dependencies'))) - if 'roles' in conf: - roles = [] - for role in conf.get('roles', []): - if 'zuul' in role: - r = JobParser._makeZuulRole(tenant, job, role) - if r: - roles.append(r) - job.addRoles(roles) - variables = conf.get('vars', None) if variables: job.updateVariables(variables) diff --git a/zuul/executor/client.py b/zuul/executor/client.py index c36d5694d3..2a205bf335 100644 --- a/zuul/executor/client.py +++ b/zuul/executor/client.py @@ -237,7 +237,6 @@ class ExecutorClient(object): params['playbooks'] = [x.toDict() for x in job.run] params['pre_playbooks'] = [x.toDict() for x in job.pre_run] params['post_playbooks'] = [x.toDict() for x in job.post_run] - params['roles'] = [x.toDict() for x in job.roles] nodeset = item.current_build_set.getJobNodeSet(job.name) nodes = [] diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 818c7e2ded..c6b819eb8a 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -72,13 +72,6 @@ class Watchdog(object): self._running = False -class JobDirPlaybook(object): - def __init__(self, root): - self.root = root - self.trusted = None - self.path = None - - class SshAgent(object): log = logging.getLogger("zuul.ExecutorServer") @@ -151,6 +144,24 @@ class SshAgent(object): return result +class JobDirPlaybook(object): + def __init__(self, root): + self.root = root + self.trusted = None + self.path = None + self.roles = [] + self.roles_path = [] + self.ansible_config = os.path.join(self.root, 'ansible.cfg') + self.project_link = os.path.join(self.root, 'project') + + def addRole(self): + count = len(self.roles) + root = os.path.join(self.root, 'role_%i' % (count,)) + os.makedirs(root) + self.roles.append(root) + return root + + class JobDir(object): def __init__(self, root, keep, build_uuid): ''' @@ -163,11 +174,23 @@ class JobDir(object): ''' # root # ansible - # trusted.cfg - # untrusted.cfg + # inventory.yaml + # playbook_0 + # project -> ../trusted/project_0/... + # role_0 -> ../trusted/project_0/... + # trusted + # project_0 + # + # # work + # .ssh + # known_hosts # src + # + # # logs + # job-output.txt + # results.json self.keep = keep if root: tmpdir = root @@ -175,16 +198,16 @@ class JobDir(object): tmpdir = tempfile.gettempdir() self.root = os.path.join(tmpdir, build_uuid) os.mkdir(self.root, 0o700) - # Work self.work_root = os.path.join(self.root, 'work') os.makedirs(self.work_root) self.src_root = os.path.join(self.work_root, 'src') os.makedirs(self.src_root) self.log_root = os.path.join(self.work_root, 'logs') os.makedirs(self.log_root) - # Ansible self.ansible_root = os.path.join(self.root, 'ansible') os.makedirs(self.ansible_root) + self.trusted_root = os.path.join(self.root, 'trusted') + os.makedirs(self.trusted_root) ssh_dir = os.path.join(self.work_root, '.ssh') os.mkdir(ssh_dir, 0o700) self.result_data_file = os.path.join(self.work_root, 'results.json') @@ -196,13 +219,23 @@ class JobDir(object): self.playbook = None # A pointer to the candidate we have chosen self.pre_playbooks = [] self.post_playbooks = [] - self.roles = [] - 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') self.job_output_file = os.path.join(self.log_root, 'job-output.txt') + self.trusted_projects = [] + self.trusted_project_index = {} + + def addTrustedProject(self, canonical_name, branch): + # Trusted projects are placed in their own directories so that + # we can support using different branches of the same project + # in different playbooks. + count = len(self.trusted_projects) + root = os.path.join(self.trusted_root, 'project_%i' % (count,)) + os.makedirs(root) + self.trusted_projects.append(root) + self.trusted_project_index[(canonical_name, branch)] = root + return root + + def getTrustedProject(self, canonical_name, branch): + return self.trusted_project_index.get((canonical_name, branch)) def addPrePlaybook(self): count = len(self.pre_playbooks) @@ -228,17 +261,6 @@ class JobDir(object): self.playbooks.append(playbook) return playbook - def addRole(self): - 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 - def cleanup(self): if not self.keep: shutil.rmtree(self.root) @@ -760,8 +782,13 @@ class AnsibleJob(object): projects.add((project['connection'], project['name'])) # ...as well as all playbook and role projects. - repos = (args['pre_playbooks'] + args['playbooks'] + - args['post_playbooks'] + args['roles']) + repos = [] + playbooks = (args['pre_playbooks'] + args['playbooks'] + + args['post_playbooks']) + for playbook in playbooks: + repos.append(playbook) + repos += playbook['roles'] + for repo in repos: self.log.debug("Job %s: updating playbook or role %s" % (self.job.unique, repo)) @@ -806,12 +833,9 @@ class AnsibleJob(object): for repo in repos.values(): repo.deleteRemote('origin') - # is the playbook in a repo that we have already prepared? - trusted, untrusted = self.preparePlaybookRepos(args) + # This prepares each playbook and the roles needed for each. + self.preparePlaybooks(args) - self.prepareRoles(args, trusted, untrusted) - - # TODOv3: Ansible the ansible thing here. self.prepareAnsibleFiles(args) data = { @@ -997,42 +1021,29 @@ class AnsibleJob(object): raise Exception("Unable to find playbook %s" % path) return None - def preparePlaybookRepos(self, args): - trusted = untrusted = False + def preparePlaybooks(self, args): 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 + self.preparePlaybook(jobdir_playbook, playbook, + args, required=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 + self.preparePlaybook(jobdir_playbook, playbook, + args, required=False) if jobdir_playbook.path is not None: self.jobdir.playbook = jobdir_playbook break + if self.jobdir.playbook is None: raise Exception("No valid playbook found") for playbook in args['post_playbooks']: jobdir_playbook = self.jobdir.addPostPlaybook() - self.preparePlaybookRepo(jobdir_playbook, playbook, - args, required=True) - if playbook['trusted']: - trusted = True - else: - untrusted = True - return (trusted, untrusted) + self.preparePlaybook(jobdir_playbook, playbook, + args, required=True) - def preparePlaybookRepo(self, jobdir_playbook, playbook, args, required): + def preparePlaybook(self, jobdir_playbook, playbook, args, required): self.log.debug("Prepare playbook repo for %s" % (playbook,)) # Check out the playbook repo if needed and set the path to # the playbook that should be run. @@ -1040,6 +1051,7 @@ class AnsibleJob(object): source = self.executor_server.connections.getSource( playbook['connection']) project = source.getProject(playbook['project']) + path = None if not playbook['trusted']: # This is a project repo, so it is safe to use the already # checked out version (from speculative merging) of the @@ -1052,34 +1064,48 @@ class AnsibleJob(object): project.canonical_hostname, project.name, playbook['path']) - jobdir_playbook.path = self.findPlaybook( - path, - required=required, - trusted=playbook['trusted']) - return - # The playbook 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. + break + if not path: + # The playbook 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. + path = self.checkoutTrustedProject(project, playbook['branch']) + path = os.path.join(path, playbook['path']) - merger = self.executor_server._getMerger(jobdir_playbook.root, - self.log) - merger.checkoutBranch(playbook['connection'], project.name, - playbook['branch']) - - path = os.path.join(jobdir_playbook.root, - project.canonical_hostname, - project.name, - playbook['path']) jobdir_playbook.path = self.findPlaybook( path, required=required, trusted=playbook['trusted']) - def prepareRoles(self, args, trusted, untrusted): - for role in args['roles']: - if role['type'] == 'zuul': - root = self.jobdir.addRole() - self.prepareZuulRole(args, role, root, trusted, untrusted) + # If this playbook doesn't exist, don't bother preparing + # roles. + if not jobdir_playbook.path: + return + + for role in playbook['roles']: + self.prepareRole(jobdir_playbook, role, args) + + self.writeAnsibleConfig(jobdir_playbook) + + def checkoutTrustedProject(self, project, branch): + root = self.jobdir.getTrustedProject(project.canonical_name, + branch) + if not root: + root = self.jobdir.addTrustedProject(project.canonical_name, + branch) + merger = self.executor_server._getMerger(root, self.log) + merger.checkoutBranch(project.connection_name, project.name, + branch) + + path = os.path.join(root, + project.canonical_hostname, + project.name) + return path + + def prepareRole(self, jobdir_playbook, role, args): + if role['type'] == 'zuul': + root = jobdir_playbook.addRole() + self.prepareZuulRole(jobdir_playbook, role, args, root) def findRole(self, path, trusted=False): d = os.path.join(path, 'tasks') @@ -1099,97 +1125,50 @@ class AnsibleJob(object): # It is neither a bare role, nor a collection of roles raise Exception("Unable to find role in %s" % (path,)) - def prepareZuulRole(self, args, role, root, trusted, untrusted): + def prepareZuulRole(self, jobdir_playbook, role, args, root): 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']) - 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'] + path = None - if untrusted: - # There is at least one untrusted playbook. For that - # case, use the already checked out version (from - # speculative merging) of the role. + if not jobdir_playbook.trusted: + # This playbook is untrested. Use the already checked out + # version (from speculative merging) of the role if it + # exists. for i in args['items']: if (i['connection'] == role['connection'] and i['project'] == role['project']): - # We already have this repo prepared; - # copy it into location. - + # We already have this repo prepared; use it. path = os.path.join(self.jobdir.src_root, project.canonical_hostname, project.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) - untrusted_role_repo = link break - 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 + if not path: + # This is a trusted playbook 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, - self.log) - merger.checkoutBranch(role['connection'], project.name, - 'master') - 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 + path = self.checkoutTrustedProject(project, 'master') - 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 + # 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(root, name) + link = os.path.realpath(link) + if not link.startswith(os.path.realpath(root)): + raise Exception("Invalid role name %s", name) + os.symlink(path, link) - 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) + role_path = self.findRole(link, trusted=jobdir_playbook.trusted) + if role_path is None: + # In the case of a bare role, add the containing directory + role_path = root + jobdir_playbook.roles_path.append(role_path) def prepareAnsibleFiles(self, args): all_vars = dict(args['vars']) @@ -1213,11 +1192,10 @@ class AnsibleJob(object): for key in node['host_keys']: known_hosts.write('%s\n' % key) - self.writeAnsibleConfig(self.jobdir.untrusted_config) - self.writeAnsibleConfig(self.jobdir.trusted_config, trusted=True) + def writeAnsibleConfig(self, jobdir_playbook): + trusted = jobdir_playbook.trusted - def writeAnsibleConfig(self, config_path, trusted=False): - with open(config_path, 'w') as config: + with open(jobdir_playbook.ansible_config, 'w') as config: config.write('[defaults]\n') config.write('hostfile = %s\n' % self.jobdir.inventory) config.write('local_tmp = %s/.ansible/local_tmp\n' % @@ -1240,12 +1218,10 @@ class AnsibleJob(object): % 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)) + if jobdir_playbook.roles_path: + config.write('roles_path = %s\n' % ':'.join( + jobdir_playbook.roles_path)) # On trusted jobs, we want to prevent the printing of args, # since trusted jobs might have access to secrets that they may @@ -1286,7 +1262,7 @@ class AnsibleJob(object): except Exception: self.log.exception("Exception while killing ansible process:") - def runAnsible(self, cmd, timeout, trusted=False): + def runAnsible(self, cmd, timeout, config_file, trusted): env_copy = os.environ.copy() env_copy.update(self.ssh_agent.env) env_copy['LOGNAME'] = 'zuul' @@ -1301,10 +1277,8 @@ class AnsibleJob(object): env_copy['PYTHONPATH'] = os.path.pathsep.join(pythonpath) if trusted: - config_file = self.jobdir.trusted_config opt_prefix = 'trusted' else: - config_file = self.jobdir.untrusted_config opt_prefix = 'untrusted' ro_dirs = get_default(self.executor_server.config, 'executor', '%s_ro_dirs' % opt_prefix) @@ -1409,7 +1383,9 @@ class AnsibleJob(object): cmd.extend(['-e', 'zuul_execution_phase_count=%s' % count]) result, code = self.runAnsible( - cmd=cmd, timeout=timeout, trusted=playbook.trusted) + cmd=cmd, timeout=timeout, + config_file=playbook.ansible_config, + trusted=playbook.trusted) self.log.debug("Ansible complete, result %s code %s" % ( self.RESULT_MAP[result], code)) return result, code diff --git a/zuul/model.py b/zuul/model.py index de733d7720..66c043d5f8 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -632,11 +632,17 @@ class PlaybookContext(object): Jobs refer to objects of this class for their main, pre, and post playbooks so that we can keep track of which repos and security - contexts are needed in order to run them.""" + contexts are needed in order to run them. - def __init__(self, source_context, path): + We also keep a list of roles so that playbooks only run with the + roles which were defined at the point the playbook was defined. + + """ + + def __init__(self, source_context, path, roles): self.source_context = source_context self.path = path + self.roles = roles def __repr__(self): return '' % (self.source_context, @@ -649,7 +655,8 @@ class PlaybookContext(object): if not isinstance(other, PlaybookContext): return False return (self.source_context == other.source_context and - self.path == other.path) + self.path == other.path and + self.roles == other.roles) def toDict(self): # Render to a dict to use in passing json to the executor @@ -658,6 +665,7 @@ class PlaybookContext(object): project=self.source_context.project.name, branch=self.source_context.branch, trusted=self.source_context.trusted, + roles=[r.toDict() for r in self.roles], path=self.path)