Filter out unprotected branches from builds if excluded

When working with GitHub Enterprise the recommended working model is
branch&pull within the same repo. This is especially necessary for
workflows that combine multiple repos in a single workspace. This has
the side effect that those repos can contain a large number of
branches that never will be part of a job. Having many branches in a
repo can have a large impact on the executor performance so exclude
them from the repo state if we exclude them in the tenant config. This
change only affects branches, not tags or other references.

Change-Id: Ic8e75fa8bf76d2e5a0b1779fa3538ee9a5c43411

releasenotes/notes/builds-branches-4ca6d1e038979d1d.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    If unprotected branches are excluded on a project they now also get
+    filtered out from jobs.

tests/fixtures/config/unprotected-branches/git/org_project3/README

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

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

@@ -8,3 +8,5 @@
           - org/project1
           - org/project2:
               exclude-unprotected-branches: true
+          - org/project3:
+              exclude-unprotected-branches: true

tests/unit/test_github_driver.py

@@ -1132,6 +1132,79 @@ class TestGithubUnprotectedBranches(ZuulTestCase):
         self.assertIn('master', tpc1.parsed_branch_config.keys())
         self.assertIn('master', tpc2.parsed_branch_config.keys())
 
+    def test_filtered_branches_in_build(self):
+        """
+        Tests unprotected branches are filtered in builds if excluded
+        """
+        self.executor_server.keep_jobdir = True
+
+        # Enable branch protection on org/project2@master
+        github = self.fake_github.getGithubClient()
+        repo = github.repo_from_project('org/project2')
+        self.create_branch('org/project2', 'feat-x')
+        repo._set_branch_protection('master', True)
+
+        # Enable branch protection on org/project3@stable. We'll use a PR on
+        # this branch as a depends-on to validate that the stable branch
+        # which is not protected in org/project3 is not filtered out.
+        repo = github.repo_from_project('org/project3')
+        self.create_branch('org/project3', 'stable')
+        repo._set_branch_protection('stable', True)
+
+        self.sched.reconfigure(self.config)
+        self.waitUntilSettled()
+
+        A = self.fake_github.openFakePullRequest('org/project3', 'stable', 'A')
+        msg = "Depends-On: https://github.com/org/project1/pull/%s" % A.number
+        B = self.fake_github.openFakePullRequest('org/project2', 'master', 'B',
+                                                 body=msg)
+
+        self.fake_github.emitEvent(B.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+
+        build = self.history[0]
+        path = os.path.join(
+            build.jobdir.src_root, 'github.com', 'org/project2')
+        build_repo = git.Repo(path)
+        branches = [x.name for x in build_repo.branches]
+        self.assertNotIn('feat-x', branches)
+
+        self.assertHistory([
+            dict(name='used-job', result='SUCCESS',
+                 changes="%s,%s %s,%s" % (A.number, A.head_sha,
+                                          B.number, B.head_sha)),
+        ])
+
+    def test_unfiltered_branches_in_build(self):
+        """
+        Tests unprotected branches are not filtered in builds if not excluded
+        """
+        self.executor_server.keep_jobdir = True
+
+        # Enable branch protection on org/project1@master
+        github = self.fake_github.getGithubClient()
+        repo = github.repo_from_project('org/project1')
+        self.create_branch('org/project1', 'feat-x')
+        repo._set_branch_protection('master', True)
+        self.sched.reconfigure(self.config)
+        self.waitUntilSettled()
+
+        A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A')
+        self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+
+        build = self.history[0]
+        path = os.path.join(
+            build.jobdir.src_root, 'github.com', 'org/project1')
+        build_repo = git.Repo(path)
+        branches = [x.name for x in build_repo.branches]
+        self.assertIn('feat-x', branches)
+
+        self.assertHistory([
+            dict(name='project-test', result='SUCCESS',
+                 changes="%s,%s" % (A.number, A.head_sha)),
+        ])
+
     def test_unprotected_push(self):
         """Test that unprotected pushes don't cause tenant reconfigurations"""
 

zuul/executor/server.py

@@ -2799,7 +2799,8 @@ class ExecutorServer(object):
         args = json.loads(job.arguments)
         zuul_event_id = args.get('zuul_event_id')
         success, repo_state = self.merger.getRepoState(
-            args['items'], repo_locks=self.repo_locks)
+            args['items'], branches=args.get('branches'),
+            repo_locks=self.repo_locks)
         result = dict(updated=success,
                       repo_state=repo_state)
         result['zuul_event_id'] = zuul_event_id
@@ -2811,6 +2812,7 @@ class ExecutorServer(object):
         ret = self.merger.mergeChanges(args['items'], args.get('files'),
                                        args.get('dirs', []),
                                        args.get('repo_state'),
+                                       branches=args.get('branches'),
                                        repo_locks=self.repo_locks,
                                        zuul_event_id=zuul_event_id)
         result = dict(merged=(ret is not None))

zuul/manager/__init__.py

@@ -629,16 +629,43 @@ class PipelineManager(object):
                   (item, files, dirs))
         build_set = item.current_build_set
         build_set.merge_state = build_set.PENDING
+
+        # If the involved projects exclude unprotected branches we should also
+        # exclude them from the merge and repo state except the branch of the
+        # change that is tested.
+        tenant = item.pipeline.tenant
+        items = list(item.items_ahead) + [item]
+        projects = [
+            item.change.project for item in items
+            if tenant.getProject(item.change.project.canonical_name)[1]
+        ]
+        if all(tenant.getExcludeUnprotectedBranches(project)
+               for project in projects):
+            branches = set()
+
+            # Add all protected branches of all involved projects
+            for project in projects:
+                branches.update(tenant.getProjectBranches(project))
+
+            # Additionally add all target branches of all involved items.
+            branches.update(item.change.branch for item in items
+                            if hasattr(item.change, 'branch'))
+            branches = list(branches)
+        else:
+            branches = None
+
         if isinstance(item.change, model.Change):
             self.sched.merger.mergeChanges(build_set.merger_items,
                                            item.current_build_set, files, dirs,
                                            precedence=self.pipeline.precedence,
-                                           event=item.event)
+                                           event=item.event,
+                                           branches=branches)
         else:
             self.sched.merger.getRepoState(build_set.merger_items,
                                            item.current_build_set,
                                            precedence=self.pipeline.precedence,
-                                           event=item.event)
+                                           event=item.event,
+                                           branches=branches)
         return False
 
     def scheduleFilesChanges(self, item):

zuul/merger/client.py

@@ -116,7 +116,7 @@ class MergeClient(object):
 
     def mergeChanges(self, items, build_set, files=None, dirs=None,
                      repo_state=None, precedence=zuul.model.PRECEDENCE_NORMAL,
-                     event=None):
+                     branches=None, event=None):
         if event is not None:
             zuul_event_id = event.zuul_event_id
         else:
@@ -125,19 +125,21 @@ class MergeClient(object):
                     files=files,
                     dirs=dirs,
                     repo_state=repo_state,
+                    branches=branches,
                     zuul_event_id=zuul_event_id)
         self.submitJob('merger:merge', data, build_set, precedence,
                        event=event)
 
     def getRepoState(self, items, build_set,
                      precedence=zuul.model.PRECEDENCE_NORMAL,
-                     event=None):
+                     branches=None, event=None):
         if event is not None:
             zuul_event_id = event.zuul_event_id
         else:
             zuul_event_id = None
 
-        data = dict(items=items, zuul_event_id=zuul_event_id)
+        data = dict(items=items, branches=branches,
+                    zuul_event_id=zuul_event_id)
         self.submitJob('merger:refstate', data, build_set, precedence,
                        event=event)
 

zuul/merger/merger.py

@@ -618,7 +618,7 @@ class Merger(object):
         repo.checkout(branch, zuul_event_id=zuul_event_id)
 
     def _saveRepoState(self, connection_name, project_name, repo,
-                       repo_state, recent):
+                       repo_state, recent, branches):
         projects = repo_state.setdefault(connection_name, {})
         project = projects.setdefault(project_name, {})
         for ref in repo.getRefs():
@@ -628,6 +628,8 @@ class Merger(object):
                 continue
             if ref.path.startswith('refs/heads/'):
                 branch = ref.path[len('refs/heads/'):]
+                if branches is not None and branch not in branches:
+                    continue
                 key = (connection_name, project_name, branch)
                 if key not in recent:
                     recent[key] = ref.object
@@ -690,7 +692,8 @@ class Merger(object):
         orig_commit = repo.revParse('FETCH_HEAD')
         return orig_commit, commit
 
-    def _mergeItem(self, item, recent, repo_state, zuul_event_id):
+    def _mergeItem(self, item, recent, repo_state, zuul_event_id,
+                   branches=None):
         log = get_annotated_logger(self.log, zuul_event_id)
         log.debug("Processing ref %s for project %s/%s / %s uuid %s" %
                   (item['ref'], item['connection'],
@@ -718,7 +721,7 @@ class Merger(object):
             # Save the repo state so that later mergers can repeat
             # this process.
             self._saveRepoState(item['connection'], item['project'], repo,
-                                repo_state, recent)
+                                repo_state, recent, branches)
         else:
             log.debug("Found base commit %s for %s" % (base, key,))
 
@@ -738,7 +741,7 @@ class Merger(object):
         return orig_commit, commit
 
     def mergeChanges(self, items, files=None, dirs=None, repo_state=None,
-                     repo_locks=None, zuul_event_id=None):
+                     repo_locks=None, branches=None, zuul_event_id=None):
         log = get_annotated_logger(self.log, zuul_event_id)
         # connection+project+branch -> commit
         recent = {}
@@ -759,7 +762,7 @@ class Merger(object):
                 log.debug("Merging for change %s,%s" %
                           (item["number"], item["patchset"]))
                 orig_commit, commit = self._mergeItem(
-                    item, recent, repo_state, zuul_event_id)
+                    item, recent, repo_state, zuul_event_id, branches=branches)
                 if not commit:
                     return None
                 if files or dirs:
@@ -790,7 +793,7 @@ class Merger(object):
             self._restoreRepoState(item['connection'], item['project'], repo,
                                    repo_state, zuul_event_id)
 
-    def getRepoState(self, items, repo_locks=None):
+    def getRepoState(self, items, branches=None, repo_locks=None):
         # Gets the repo state for items.  Generally this will be
         # called in any non-change pipeline.  We will return the repo
         # state for each item, but manipulated with any information in
@@ -818,7 +821,7 @@ class Merger(object):
                         return (False, {})
 
                     self._saveRepoState(item['connection'], item['project'],
-                                        repo, repo_state, recent)
+                                        repo, repo_state, recent, branches)
 
                 if item.get('newrev'):
                     # This is a ref update rather than a branch tip, so make

zuul/merger/server.py

@@ -141,6 +141,7 @@ class MergeServer(object):
         ret = self.merger.mergeChanges(
             args['items'], args.get('files'),
             args.get('dirs'), args.get('repo_state'),
+            branches=args.get('branches'),
             zuul_event_id=zuul_event_id)
         result = dict(merged=(ret is not None))
         if ret is None:
@@ -154,7 +155,8 @@ class MergeServer(object):
     def refstate(self, job):
         args = json.loads(job.arguments)
         zuul_event_id = args.get('zuul_event_id')
-        success, repo_state = self.merger.getRepoState(args['items'])
+        success, repo_state = self.merger.getRepoState(
+            args['items'], branches=args.get('branches'))
         result = dict(updated=success,
                       repo_state=repo_state)
         result['zuul_event_id'] = zuul_event_id

zuul/model.py

@@ -2180,12 +2180,15 @@ class QueueItem(object):
             return None
         return self.job_graph.jobs.get(name)
 
-    def getNonLiveItemsAhead(self):
-        items = []
+    @property
+    def items_ahead(self):
         item_ahead = self.item_ahead
-        while item_ahead and not item_ahead.live:
-            items.append(item_ahead)
+        while item_ahead:
+            yield item_ahead
             item_ahead = item_ahead.item_ahead
+
+    def getNonLiveItemsAhead(self):
+        items = [item for item in self.items_ahead if not item.live]
         return reversed(items)
 
     def haveAllJobsStarted(self):