Check out more appropriate branches of role and playbook repos

Currently when a job adds a zuul role repo to a playbook, we only
use the master branch of the role repo, unless the role repo
appears in the dependency chain for the change under test.

That means that if the role repo appears in required-projects,
but not as a dependency, then we use the master branch instead of
what was specified in required-projects.  That doesn't seem to make
much sense and is likely an oversight.  We attempt to use the
prepared repos where possible (ie, the requested branches match
and the playbook is not trusted).  However, the current check for
that only looks at 'items', that is, the dependency chain.  Instead,
we should look at 'projects', which includes not only the projects
which appear in 'items', but also those that appear in
required-projects.

The same check is performed for playbooks, and therefore is also
updated.

Also, in the case where a role repo doesn't appear in either the
dependency chain or in required-projects, we were hard-coded to
check out the 'master' branch.  Instead, re-use some of the logic
used when preparing required-projects to attempt to find the best
branch to check out.  We will try the job override branch first,
then the zuul branch, then the project default branch.

All playbook project repos are now prepared outside of the work dir,
even in cases where their projects also appear in the work dir.  If
the playbook is untrusted, then the repo is cloned into the "untrusted/"
jobdir directory (with speculative changes applied).  To account for
this, the "allow_trusted" flag in the ansible safe path checker is
updated to allow access to both "trusted/" and "untrusted/" paths.

Change-Id: If95a9b0aaff982040cd4e6e957f9588b26ef7935

doc/source/user/config.rst

@@ -794,6 +794,15 @@ Here is an example of two job definitions:
       the addition of conflicting roles.  Roles added by a child will
       appear before those it inherits from its parent.
 
+      If a project used for a Zuul role has branches, the usual
+      process of selecting which branch should be checked out applies.
+      See :attr:`job.override-checkout` for a description of that
+      process and how to override it.  As a special case, if the role
+      project is the project in which this job definition appears,
+      then the branch in which this definition appears will be used.
+      In other words, a playbook may not use a role from a different
+      branch of the same project.
+
       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 the project), or a contained role (in which the role

releasenotes/notes/role-checkouts-89632d2ff5eb8b78.yaml

@@ -0,0 +1,15 @@
+---
+fixes:
+  - |
+    Zuul role repository checkouts now honor :attr:`job.override-checkout`.
+
+    Previously, when a Zuul role was specified for a job, Zuul would
+    usually checkout the master branch, unless that repository
+    appeared in the dependency chain for a patch.  It will now follow
+    the usual procedure for determining the branch to check out,
+    including honoring :attr:`job.override-checkout` options.
+
+    This may alter the behavior of currently existing jobs.  Depending
+    on circumstances, you may need to set
+    :attr:`job.override-checkout` or copy roles to other branches of
+    projects.

tests/fixtures/config/role-branches/git/common-config/playbooks/pre-base.yaml

@@ -0,0 +1,3 @@
+- hosts: all
+  roles:
+    - pre-base

tests/fixtures/config/role-branches/git/common-config/roles/pre-base/tasks/main.yaml

@@ -0,0 +1,3 @@
+- name: base pre
+  debug:
+    msg: base pre

tests/fixtures/config/role-branches/git/common-config/zuul.yaml

@@ -0,0 +1,62 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- pipeline:
+    name: gate
+    manager: dependent
+    post-review: True
+    trigger:
+      gerrit:
+        - event: comment-added
+          approval:
+            - Approved: 1
+    success:
+      gerrit:
+        Verified: 2
+        submit: true
+    failure:
+      gerrit:
+        Verified: -2
+    start:
+      gerrit:
+        Verified: 0
+    precedence: high
+
+- job:
+    name: base
+    parent: null
+    pre-run: playbooks/pre-base.yaml
+
+- project:
+    name: common-config
+    check:
+      jobs: []
+    gate:
+      jobs:
+        - noop
+
+- project:
+    name: project1
+    check:
+      jobs: []
+    gate:
+      jobs:
+        - noop
+
+- project:
+    name: project2
+    check:
+      jobs: []
+    gate:
+      jobs:
+        - noop

tests/fixtures/config/role-branches/git/project1/README

@@ -0,0 +1 @@
+test

tests/fixtures/config/role-branches/git/project1/playbooks/parent-job.yaml

@@ -0,0 +1,3 @@
+- hosts: all
+  roles:
+    - run-parent-job

tests/fixtures/config/role-branches/git/project1/roles/run-parent-job/tasks/main.yaml

@@ -0,0 +1,3 @@
+- name: run parent job
+  debug:
+    msg: run parent job

tests/fixtures/config/role-branches/git/project1/zuul.yaml

@@ -0,0 +1,6 @@
+- job:
+    name: parent-job
+    required-projects:
+      - project1
+    pre-run: playbooks/parent-job-pre.yaml
+    run: playbooks/parent-job.yaml

tests/fixtures/config/role-branches/git/project2/README

@@ -0,0 +1 @@
+test

tests/fixtures/config/role-branches/git/project2/playbooks/child-job.yaml

@@ -0,0 +1,3 @@
+- hosts: all
+  roles:
+    - run-parent-job

tests/fixtures/config/role-branches/git/project2/zuul.yaml

@@ -0,0 +1,23 @@
+- job:
+    name: child-job
+    parent: parent-job
+    run: playbooks/child-job.yaml
+
+- job:
+    name: child-job-override
+    parent: child-job
+    override-checkout: stable
+
+- job:
+    name: child-job-project-override
+    parent: child-job
+    required-projects:
+      - name: project1
+        override-checkout: stable
+
+- project:
+    check:
+      jobs:
+        - child-job
+        - child-job-override
+        - child-job-project-override

tests/fixtures/config/role-branches/main.yaml

@@ -0,0 +1,9 @@
+- tenant:
+    name: tenant-one
+    source:
+      gerrit:
+        config-projects:
+          - common-config
+        untrusted-projects:
+          - project1
+          - project2

tests/unit/test_v3.py

@@ -2333,7 +2333,7 @@ class TestProjectKeys(ZuulTestCase):
 
 
 class RoleTestCase(ZuulTestCase):
-    def _assertRolePath(self, build, playbook, content):
+    def _getRolesPaths(self, build, playbook):
         path = os.path.join(self.test_root, build.uuid,
                             'ansible', playbook, 'ansible.cfg')
         roles_paths = []
@@ -2341,7 +2341,10 @@ class RoleTestCase(ZuulTestCase):
             for line in f:
                 if line.startswith('roles_path'):
                     roles_paths.append(line)
-        print(roles_paths)
+        return roles_paths
+
+    def _assertRolePath(self, build, playbook, content):
+        roles_paths = self._getRolesPaths(build, playbook)
         if content:
             self.assertEqual(len(roles_paths), 1,
                              "Should have one roles_path line in %s" %
@@ -2352,6 +2355,121 @@ class RoleTestCase(ZuulTestCase):
                              "Should have no roles_path line in %s" %
                              (playbook,))
 
+    def _assertInRolePath(self, build, playbook, files):
+        roles_paths = self._getRolesPaths(build, playbook)[0]
+        roles_paths = roles_paths.split('=')[-1].strip()
+        roles_paths = roles_paths.split(':')
+
+        files = set(files)
+        matches = set()
+        for rpath in roles_paths:
+            for rolename in os.listdir(rpath):
+                if rolename in files:
+                    matches.add(rolename)
+        self.assertEqual(files, matches)
+
+
+class TestRoleBranches(RoleTestCase):
+    tenant_config_file = 'config/role-branches/main.yaml'
+
+    def _addRole(self, project, branch, role, parent=None):
+        data = textwrap.dedent("""
+            - name: %s
+              debug:
+                msg: %s
+            """ % (role, role))
+        file_dict = {'roles/%s/tasks/main.yaml' % role: data}
+        A = self.fake_gerrit.addFakeChange(project, branch,
+                                           'add %s' % role,
+                                           files=file_dict,
+                                           parent=parent)
+        A.addApproval('Code-Review', 2)
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.waitUntilSettled()
+        self.fake_gerrit.addEvent(A.getChangeMergedEvent())
+        self.waitUntilSettled()
+        return A.patchsets[-1]['ref']
+
+    def _addPlaybook(self, project, branch, playbook, role, parent=None):
+        data = textwrap.dedent("""
+            - hosts: all
+              roles:
+                - %s
+            """ % role)
+        file_dict = {'playbooks/%s.yaml' % playbook: data}
+        A = self.fake_gerrit.addFakeChange(project, branch,
+                                           'add %s' % playbook,
+                                           files=file_dict,
+                                           parent=parent)
+        A.addApproval('Code-Review', 2)
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.waitUntilSettled()
+        self.fake_gerrit.addEvent(A.getChangeMergedEvent())
+        self.waitUntilSettled()
+        return A.patchsets[-1]['ref']
+
+    def _assertInFile(self, path, content):
+        with open(path) as f:
+            self.assertIn(content, f.read())
+
+    def test_playbook_role_branches(self):
+        # This tests that the correct branch of a repo which contains
+        # a playbook or a role is checked out.  Most of the action
+        # happens on project1, which holds a parent job, so that we
+        # can test the behavior of a project which is not in the
+        # dependency chain.
+        # First we create some branch-specific content in project1:
+        self.create_branch('project1', 'stable')
+
+        # A pre-playbook with unique stable branch content.
+        p = self._addPlaybook('project1', 'stable',
+                              'parent-job-pre', 'parent-stable-role')
+        # A role that only exists on the stable branch.
+        self._addRole('project1', 'stable', 'stable-role', parent=p)
+
+        # The same for the master branch.
+        p = self._addPlaybook('project1', 'master',
+                              'parent-job-pre', 'parent-master-role')
+        self._addRole('project1', 'master', 'master-role', parent=p)
+
+        self.sched.reconfigure(self.config)
+        # Push a change to project2 which will run 3 jobs which
+        # inherit from project1.
+        self.executor_server.hold_jobs_in_build = True
+        A = self.fake_gerrit.addFakeChange('project2', 'master', 'A')
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        self.assertEqual(len(self.builds), 3)
+
+        # This job should use the master branch since that's the
+        # zuul.branch for this change.
+        build = self.getBuildByName('child-job')
+        self._assertInRolePath(build, 'playbook_0', ['master-role'])
+        self._assertInFile(build.jobdir.pre_playbooks[1].path,
+                           'parent-master-role')
+
+        # The main playbook is on the master branch of project2, but
+        # there is a job-level branch override, so the project1 role
+        # should be from the stable branch.  The job-level override
+        # will cause Zuul to select the project1 pre-playbook from the
+        # stable branch as well, so we should see it using the stable
+        # role.
+        build = self.getBuildByName('child-job-override')
+        self._assertInRolePath(build, 'playbook_0', ['stable-role'])
+        self._assertInFile(build.jobdir.pre_playbooks[1].path,
+                           'parent-stable-role')
+
+        # The same, but using a required-projects override.
+        build = self.getBuildByName('child-job-project-override')
+        self._assertInRolePath(build, 'playbook_0', ['stable-role'])
+        self._assertInFile(build.jobdir.pre_playbooks[1].path,
+                           'parent-stable-role')
+
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
 
 class TestRoles(RoleTestCase):
     tenant_config_file = 'config/roles/main.yaml'

zuul/ansible/paths.py

@@ -44,6 +44,8 @@ def _is_safe_path(path, allow_trusted=False):
     if allow_trusted:
         allowed_paths.append(
             os.path.abspath(os.path.join(home_path, '../trusted')))
+        allowed_paths.append(
+            os.path.abspath(os.path.join(home_path, '../untrusted')))
 
     def _is_safe(path_to_check):
         for allowed_path in allowed_paths:

zuul/configloader.py

@@ -801,16 +801,14 @@ class JobParser(object):
             return None
 
         return model.ZuulRole(role.get('name', name),
-                              project.connection_name,
+                              project.canonical_name)
-                              project.name)
 
     def _makeImplicitRole(self, job):
         project = job.source_context.project
         name = project.name.split('/')[-1]
         name = JobParser.ANSIBLE_ROLE_RE.sub('', name)
         return model.ZuulRole(name,
-                              project.connection_name,
+                              project.canonical_name,
-                              project.name,
                               implicit=True)
 
 

zuul/executor/client.py

@@ -196,10 +196,29 @@ class ExecutorClient(object):
         params['override_checkout'] = job.override_checkout
         params['repo_state'] = item.current_build_set.repo_state
 
+        def make_playbook(playbook):
+            d = playbook.toDict()
+            for role in d['roles']:
+                if role['type'] != 'zuul':
+                    continue
+                project_config = item.layout.project_configs.get(
+                    role['project_canonical_name'], None)
+                if project_config:
+                    role['project_default_branch'] = \
+                        project_config.default_branch
+                else:
+                    role['project_default_branch'] = 'master'
+                role_trusted, role_project = item.layout.tenant.getProject(
+                    role['project_canonical_name'])
+                role_connection = role_project.source.connection
+                role['connection'] = role_connection.connection_name
+                role['project'] = role_project.name
+            return d
+
         if job.name != 'noop':
-            params['playbooks'] = [x.toDict() for x in job.run]
+            params['playbooks'] = [make_playbook(x) for x in job.run]
-            params['pre_playbooks'] = [x.toDict() for x in job.pre_run]
+            params['pre_playbooks'] = [make_playbook(x) for x in job.pre_run]
-            params['post_playbooks'] = [x.toDict() for x in job.post_run]
+            params['post_playbooks'] = [make_playbook(x) for x in job.post_run]
 
         nodes = []
         for node in nodeset.getNodes():

zuul/executor/server.py

@@ -257,6 +257,7 @@ class JobDirPlaybook(object):
     def __init__(self, root):
         self.root = root
         self.trusted = None
+        self.project_canonical_name = None
         self.branch = None
         self.canonical_name_and_path = None
         self.path = None
@@ -302,6 +303,10 @@ class JobDir(object):
         #     project_0
         #       <git.example.com>
         #         <project>
+        #   untrusted (mounted in bwrap read-only)
+        #     project_0
+        #       <git.example.com>
+        #         <project>
         #   work (mounted in bwrap read-write)
         #     .ssh
         #       known_hosts
@@ -328,6 +333,8 @@ class JobDir(object):
         os.makedirs(self.ansible_root)
         self.trusted_root = os.path.join(self.root, 'trusted')
         os.makedirs(self.trusted_root)
+        self.untrusted_root = os.path.join(self.root, 'untrusted')
+        os.makedirs(self.untrusted_root)
         ssh_dir = os.path.join(self.work_root, '.ssh')
         os.mkdir(ssh_dir, 0o700)
         # Create ansible cache directory
@@ -368,6 +375,8 @@ class JobDir(object):
             ))
         self.trusted_projects = []
         self.trusted_project_index = {}
+        self.untrusted_projects = []
+        self.untrusted_project_index = {}
 
         # Create a JobDirPlaybook for the Ansible setup run.  This
         # doesn't use an actual playbook, but it lets us use the same
@@ -392,6 +401,23 @@ class JobDir(object):
     def getTrustedProject(self, canonical_name, branch):
         return self.trusted_project_index.get((canonical_name, branch))
 
+    def addUntrustedProject(self, canonical_name, branch):
+        # Similar to trusted projects, but these hold checkouts of
+        # projects which are allowed to have speculative changes
+        # applied.  They might, however, be different branches than
+        # what is used in the working dir, so they need their own
+        # location.  Moreover, we might avoid mischief if a job alters
+        # the contents of the working dir.
+        count = len(self.untrusted_projects)
+        root = os.path.join(self.untrusted_root, 'project_%i' % (count,))
+        os.makedirs(root)
+        self.untrusted_projects.append(root)
+        self.untrusted_project_index[(canonical_name, branch)] = root
+        return root
+
+    def getUntrustedProject(self, canonical_name, branch):
+        return self.untrusted_project_index.get((canonical_name, branch))
+
     def addPrePlaybook(self):
         count = len(self.pre_playbooks)
         root = os.path.join(self.ansible_root, 'pre_playbook_%i' % (count,))
@@ -431,6 +457,9 @@ class UpdateTask(object):
     def __init__(self, connection_name, project_name):
         self.connection_name = connection_name
         self.project_name = project_name
+        self.canonical_name = None
+        self.branches = None
+        self.refs = None
         self.event = threading.Event()
 
     def __eq__(self, other):
@@ -584,6 +613,7 @@ class AnsibleJob(object):
         self.aborted = False
         self.aborted_reason = None
         self.thread = None
+        self.project_info = {}
         self.private_key_file = get_default(self.executor_server.config,
                                             'executor', 'private_key_file',
                                             '~/.ssh/id_rsa')
@@ -675,10 +705,16 @@ class AnsibleJob(object):
 
         for task in tasks:
             task.wait()
+            self.project_info[task.canonical_name] = {
+                'refs': task.refs,
+                'branches': task.branches,
+            }
 
         self.log.debug("Git updates complete")
-        merger = self.executor_server._getMerger(self.jobdir.src_root,
+        merger = self.executor_server._getMerger(
-                                                 self.log)
+            self.jobdir.src_root,
+            self.executor_server.merge_root,
+            self.log)
         repos = {}
         for project in args['projects']:
             self.log.debug("Cloning %s/%s" % (project['connection'],
@@ -710,19 +746,24 @@ class AnsibleJob(object):
                 ref = args['zuul']['ref']
             else:
                 ref = None
-            selected = self.checkoutBranch(repo,
+            selected_ref, selected_desc = self.resolveBranch(
-                                           project['name'],
+                project['canonical_name'],
-                                           ref,
+                ref,
-                                           args['branch'],
+                args['branch'],
-                                           args['override_branch'],
+                args['override_branch'],
-                                           args['override_checkout'],
+                args['override_checkout'],
-                                           project['override_branch'],
+                project['override_branch'],
-                                           project['override_checkout'],
+                project['override_checkout'],
-                                           project['default_branch'])
+                project['default_branch'])
+            self.log.info("Checking out %s %s %s",
+                          project['canonical_name'], selected_desc,
+                          selected_ref)
+            repo.checkout(selected_ref)
+
             # Update the inventory variables to indicate the ref we
             # checked out
             p = args['zuul']['projects'][project['canonical_name']]
-            p['checkout'] = selected
+            p['checkout'] = selected_ref
         # Delete the origin remote from each repo we set up since
         # it will not be valid within the jobs.
         for repo in repos.values():
@@ -811,51 +852,44 @@ class AnsibleJob(object):
             repo.setRef('refs/heads/' + branch, commit)
         return True
 
-    def checkoutBranch(self, repo, project_name, ref, zuul_branch,
+    def resolveBranch(self, project_canonical_name, ref, zuul_branch,
-                       job_override_branch, job_override_checkout,
+                      job_override_branch, job_override_checkout,
-                       project_override_branch, project_override_checkout,
+                      project_override_branch, project_override_checkout,
-                       project_default_branch):
+                      project_default_branch):
-        branches = repo.getBranches()
+        branches = self.project_info[project_canonical_name]['branches']
-        refs = [r.name for r in repo.getRefs()]
+        refs = self.project_info[project_canonical_name]['refs']
         selected_ref = None
+        selected_desc = None
         if project_override_checkout in refs:
             selected_ref = project_override_checkout
-            self.log.info("Checking out %s project override ref %s",
+            selected_desc = 'project override ref'
-                          project_name, selected_ref)
         elif project_override_branch in branches:
             selected_ref = project_override_branch
-            self.log.info("Checking out %s project override branch %s",
+            selected_desc = 'project override branch'
-                          project_name, selected_ref)
         elif job_override_checkout in refs:
             selected_ref = job_override_checkout
-            self.log.info("Checking out %s job override ref %s",
+            selected_desc = 'job override ref'
-                          project_name, selected_ref)
         elif job_override_branch in branches:
             selected_ref = job_override_branch
-            self.log.info("Checking out %s job override branch %s",
+            selected_desc = 'job override branch'
-                          project_name, selected_ref)
         elif ref and ref.startswith('refs/heads/'):
             selected_ref = ref[len('refs/heads/'):]
-            self.log.info("Checking out %s branch ref %s",
+            selected_desc = 'branch ref'
-                          project_name, selected_ref)
         elif ref and ref.startswith('refs/tags/'):
             selected_ref = ref[len('refs/tags/'):]
-            self.log.info("Checking out %s tag ref %s",
+            selected_desc = 'tag ref'
-                          project_name, selected_ref)
         elif zuul_branch and zuul_branch in branches:
             selected_ref = zuul_branch
-            self.log.info("Checking out %s zuul branch %s",
+            selected_desc = 'zuul branch'
-                          project_name, selected_ref)
         elif project_default_branch in branches:
             selected_ref = project_default_branch
-            self.log.info("Checking out %s project default branch %s",
+            selected_desc = 'project default branch'
-                          project_name, selected_ref)
         else:
             raise ExecutorError("Project %s does not have the "
                                 "default branch %s" %
-                                (project_name, project_default_branch))
+                                (project_canonical_name,
-        repo.checkout(selected_ref)
+                                 project_default_branch))
-        return selected_ref
+        return (selected_ref, selected_desc)
 
     def getAnsibleTimeout(self, start, timeout):
         if timeout is not None:
@@ -1096,41 +1130,31 @@ class AnsibleJob(object):
             self.preparePlaybook(jobdir_playbook, playbook, args)
 
     def preparePlaybook(self, jobdir_playbook, playbook, args):
-        self.log.debug("Prepare playbook repo for %s" %
-                       (playbook['project'],))
         # Check out the playbook repo if needed and set the path to
         # the playbook that should be run.
+        self.log.debug("Prepare playbook repo for %s: %s@%s" %
+                       (playbook['trusted'] and 'trusted' or 'untrusted',
+                        playbook['project'], playbook['branch']))
         source = self.executor_server.connections.getSource(
             playbook['connection'])
         project = source.getProject(playbook['project'])
+        branch = playbook['branch']
         jobdir_playbook.trusted = playbook['trusted']
-        jobdir_playbook.branch = playbook['branch']
+        jobdir_playbook.branch = branch
+        jobdir_playbook.project_canonical_name = project.canonical_name
         jobdir_playbook.canonical_name_and_path = os.path.join(
             project.canonical_name, playbook['path'])
         path = None
-        if not playbook['trusted']:
+
-            # This is a project repo, so it is safe to use the already
+        if not jobdir_playbook.trusted:
-            # checked out version (from speculative merging) of the
+            path = self.checkoutUntrustedProject(project, branch, args)
-            # playbook
+        else:
-            for i in args['items']:
+            path = self.checkoutTrustedProject(project, branch)
-                if (i['connection'] == playbook['connection'] and
+        path = os.path.join(path, playbook['path'])
-                    i['project'] == playbook['project']):
-                    # We already have this repo prepared
-                    path = os.path.join(self.jobdir.src_root,
-                                        project.canonical_hostname,
-                                        project.name,
-                                        playbook['path'])
-                    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'])
 
         jobdir_playbook.path = self.findPlaybook(
             path,
-            trusted=playbook['trusted'])
+            trusted=jobdir_playbook.trusted)
 
         # If this playbook doesn't exist, don't bother preparing
         # roles.
@@ -1154,9 +1178,57 @@ class AnsibleJob(object):
         if not root:
             root = self.jobdir.addTrustedProject(project.canonical_name,
                                                  branch)
-            merger = self.executor_server._getMerger(root, self.log)
+            self.log.debug("Cloning %s@%s into new trusted space %s",
+                           project, branch, root)
+            merger = self.executor_server._getMerger(
+                root,
+                self.executor_server.merge_root,
+                self.log)
             merger.checkoutBranch(project.connection_name, project.name,
                                   branch)
+        else:
+            self.log.debug("Using existing repo %s@%s in trusted space %s",
+                           project, branch, root)
+
+        path = os.path.join(root,
+                            project.canonical_hostname,
+                            project.name)
+        return path
+
+    def checkoutUntrustedProject(self, project, branch, args):
+        root = self.jobdir.getUntrustedProject(project.canonical_name,
+                                               branch)
+        if not root:
+            root = self.jobdir.addUntrustedProject(project.canonical_name,
+                                                   branch)
+            # If the project is in the dependency chain, clone from
+            # there so we pick up any speculative changes, otherwise,
+            # clone from the cache.
+            merger = None
+            for p in args['projects']:
+                if (p['connection'] == project.connection_name and
+                    p['name'] == project.name):
+                    # We already have this repo prepared
+                    self.log.debug("Found workdir repo for untrusted project")
+                    merger = self.executor_server._getMerger(
+                        root,
+                        self.jobdir.src_root,
+                        self.log)
+                    break
+
+            if merger is None:
+                merger = self.executor_server._getMerger(
+                    root,
+                    self.executor_server.merge_root,
+                    self.log)
+
+            self.log.debug("Cloning %s@%s into new untrusted space %s",
+                           project, branch, root)
+            merger.checkoutBranch(project.connection_name, project.name,
+                                  branch)
+        else:
+            self.log.debug("Using existing repo %s@%s in trusted space %s",
+                           project, branch, root)
 
         path = os.path.join(root,
                             project.canonical_hostname,
@@ -1198,26 +1270,38 @@ class AnsibleJob(object):
         name = role['target_name']
         path = None
 
-        if not jobdir_playbook.trusted:
+        # Find the branch to use for this role.  We should generally
-            # This playbook is untrested.  Use the already checked out
+        # follow the normal fallback procedure, unless this role's
-            # version (from speculative merging) of the role if it
+        # project is the playbook's project, in which case we should
-            # exists.
+        # use the playbook branch.
-
+        if jobdir_playbook.project_canonical_name == project.canonical_name:
-            for i in args['items']:
+            branch = jobdir_playbook.branch
-                if (i['connection'] == role['connection'] and
+            self.log.debug("Role project is playbook project, "
-                    i['project'] == role['project']):
+                           "using playbook branch %s", branch)
-                    # We already have this repo prepared; use it.
+        else:
-                    path = os.path.join(self.jobdir.src_root,
+            # Find if the project is one of the job-specified projects.
-                                        project.canonical_hostname,
+            # If it is, we can honor the project checkout-override options.
-                                        project.name)
+            args_project = {}
+            for p in args['projects']:
+                if (p['canonical_name'] == project.canonical_name):
+                    args_project = p
                     break
 
-        if not path:
+            branch, selected_desc = self.resolveBranch(
-            # This is a trusted playbook or the role did not appear
+                project.canonical_name,
-            # in the dependency chain for the change (in which case,
+                None,
-            # there is no existing untrusted checkout of it).  Check
+                args['branch'],
-            # out the branch tip into a dedicated space.
+                args['override_branch'],
-            path = self.checkoutTrustedProject(project, 'master')
+                args['override_checkout'],
+                args_project.get('override_branch'),
+                args_project.get('override_checkout'),
+                role['project_default_branch'])
+            self.log.debug("Role using %s %s", selected_desc, branch)
+
+        if not jobdir_playbook.trusted:
+            path = self.checkoutUntrustedProject(project, branch, args)
+        else:
+            path = self.checkoutTrustedProject(project, branch)
 
         # The name of the symlink is the requested name of the role
         # (which may be the repo name or may be something else; this
@@ -1400,6 +1484,7 @@ class AnsibleJob(object):
         ro_paths.append(self.executor_server.ansible_dir)
         ro_paths.append(self.jobdir.ansible_root)
         ro_paths.append(self.jobdir.trusted_root)
+        ro_paths.append(self.jobdir.untrusted_root)
         ro_paths.append(playbook.root)
 
         rw_paths.append(self.jobdir.ansible_cache_root)
@@ -1735,7 +1820,7 @@ class ExecutorServer(object):
         # up-to-date copies of all the repos that are used by jobs, as
         # well as to support the merger:cat functon to supply
         # configuration information to Zuul when it starts.
-        self.merger = self._getMerger(self.merge_root)
+        self.merger = self._getMerger(self.merge_root, None)
         self.update_queue = DeduplicateQueue()
 
         command_socket = get_default(
@@ -1776,11 +1861,7 @@ class ExecutorServer(object):
                                               self.stopJobDiskFull,
                                               self.merge_root)
 
-    def _getMerger(self, root, logger=None):
+    def _getMerger(self, root, cache_root, logger=None):
-        if root != self.merge_root:
-            cache_root = self.merge_root
-        else:
-            cache_root = None
         return zuul.merger.merger.Merger(
             root, self.connections, self.merge_email, self.merge_name,
             self.merge_speed_limit, self.merge_speed_time, cache_root, logger)
@@ -1960,6 +2041,12 @@ class ExecutorServer(object):
             self.log.info("Updating repo %s/%s" % (
                 task.connection_name, task.project_name))
             self.merger.updateRepo(task.connection_name, task.project_name)
+            repo = self.merger.getRepo(task.connection_name, task.project_name)
+            source = self.connections.getSource(task.connection_name)
+            project = source.getProject(task.project_name)
+            task.canonical_name = project.canonical_name
+            task.branches = repo.getBranches()
+            task.refs = [r.name for r in repo.getRefs()]
             self.log.debug("Finished updating repo %s/%s" %
                            (task.connection_name, task.project_name))
         task.setComplete()

zuul/model.py

@@ -766,15 +766,14 @@ class Role(object, metaclass=abc.ABCMeta):
 class ZuulRole(Role):
     """A reference to an ansible role in a Zuul project."""
 
-    def __init__(self, target_name, connection_name, project_name,
+    def __init__(self, target_name, project_canonical_name, implicit=False):
-                 implicit=False):
         super(ZuulRole, self).__init__(target_name)
-        self.connection_name = connection_name
+        self.project_canonical_name = project_canonical_name
-        self.project_name = project_name
         self.implicit = implicit
 
     def __repr__(self):
-        return '<ZuulRole %s %s>' % (self.project_name, self.target_name)
+        return '<ZuulRole %s %s>' % (self.project_canonical_name,
+                                     self.target_name)
 
     __hash__ = object.__hash__
 
@@ -784,15 +783,13 @@ class ZuulRole(Role):
         # Implicit is not consulted for equality so that we can handle
         # implicit to explicit conversions.
         return (super(ZuulRole, self).__eq__(other) and
-                self.connection_name == other.connection_name and
+                self.project_canonical_name == other.project_canonical_name)
-                self.project_name == other.project_name)
 
     def toDict(self):
         # Render to a dict to use in passing json to the executor
         d = super(ZuulRole, self).toDict()
         d['type'] = 'zuul'
-        d['connection'] = self.connection_name
+        d['project_canonical_name'] = self.project_canonical_name
-        d['project'] = self.project_name
         d['implicit'] = self.implicit
         return d