Merge "Filter out unprotected branches from builds if excluded"

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

@@ -2831,7 +2831,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
@@ -2843,6 +2844,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

@@ -637,7 +637,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():
@@ -647,6 +647,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
@@ -709,7 +711,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'],
@@ -737,7 +740,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,))
 
@@ -757,7 +760,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 = {}
@@ -778,7 +781,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:
@@ -809,7 +812,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
@@ -837,7 +840,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

@@ -2183,12 +2183,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):