Merge "Reload tenant in case of new project branches"

tests/fixtures/config/unprotected-branches/git/org_project2/playbooks/used-job.yaml

@@ -0,0 +1,2 @@
+- hosts: all
+  tasks: []

tests/fixtures/config/unprotected-branches/git/org_project2/zuul.yaml

@@ -1,2 +1,8 @@
 - job:
-    name: unused-job
+    name: used-job
+    run: playbooks/used-job.yaml
+
+- project:
+    check:
+      jobs:
+        - used-job

tests/unit/test_github_driver.py

@@ -1004,6 +1004,7 @@ class TestGithubUnprotectedBranches(ZuulTestCase):
         time.sleep(1)
 
         # Delete the branch
+        repo._delete_branch('master')
         pevent = self.fake_github.getPushEvent(project='org/project2',
                                                old_rev=A.head_sha,
                                                new_rev='0' * 40,
@@ -1018,6 +1019,45 @@ class TestGithubUnprotectedBranches(ZuulTestCase):
         # branch
         self.assertLess(old, new)
 
+    # This test verifies that a PR is considered in case it was created for
+    # a branch just has been set to protected before a tenant reconfiguration
+    # took place.
+    def test_reconfigure_on_pr_to_new_protected_branch(self):
+        self.create_branch('org/project2', 'release')
+
+        github = self.fake_github.getGithubClient()
+        repo = github.repo_from_project('org/project2')
+        repo._set_branch_protection('master', True)
+        repo._create_branch('release')
+        repo._create_branch('feature')
+
+        self.sched.reconfigure(self.config)
+        self.waitUntilSettled()
+
+        repo._set_branch_protection('release', True)
+
+        self.executor_server.hold_jobs_in_build = True
+
+        A = self.fake_github.openFakePullRequest(
+            'org/project2', 'release', 'A')
+        self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
+        self.waitUntilSettled()
+
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        self.assertEqual('SUCCESS',
+                         self.getJobFromHistory('used-job').result)
+
+        job = self.getJobFromHistory('used-job')
+        zuulvars = job.parameters['zuul']
+        self.assertEqual(str(A.number), zuulvars['change'])
+        self.assertEqual(str(A.head_sha), zuulvars['patchset'])
+        self.assertEqual('release', zuulvars['branch'])
+
+        self.assertEqual(1, len(self.history))
+
 
 class TestGithubWebhook(ZuulTestCase):
     config_file = 'zuul-github-driver.conf'

zuul/driver/github/githubconnection.py

@@ -220,12 +220,30 @@ class GithubEventConnector(threading.Thread):
 
         if event:
             event.delivery = delivery
+            project = self.connection.source.getProject(event.project_name)
             if event.change_number:
-                project = self.connection.source.getProject(event.project_name)
                 self.connection._getChange(project,
                                            event.change_number,
                                            event.patch_number,
                                            refresh=True)
+
+            # If this event references a branch and we're excluding unprotected
+            # branches, we might need to check whether the branch is now
+            # protected.
+            if event.branch:
+                b = self.connection.getBranch(project.name, event.branch)
+                if b is not None:
+                    branch_protected = b.get('protected')
+                    self.connection.checkBranchCache(project, event.branch,
+                                                     branch_protected)
+                    event.branch_protected = branch_protected
+                else:
+                    # This can happen if the branch was deleted in GitHub. In
+                    # this case we assume that the branch COULD have been
+                    # protected before. The cache update is handled by the
+                    # push event, so we don't touch the cache here again.
+                    event.branch_protected = True
+
             event.project_hostname = self.connection.canonical_hostname
             self.connection.logEvent(event)
             self.connection.sched.addEvent(event)
@@ -268,20 +286,11 @@ class GithubEventConnector(threading.Thread):
                 # know if branch protection has been disabled before deletion
                 # of the branch.
                 # FIXME(tobiash): Find a way to handle that case
-                event.branch_protected = True
                 self.connection._clearBranchCache(project)
             elif event.branch_created:
                 # A new branch never can be protected because that needs to be
                 # configured after it has been created.
-                event.branch_protected = False
                 self.connection._clearBranchCache(project)
-            else:
-                # An updated branch can be protected or not so we have to ask
-                # GitHub whether it is.
-                b = self.connection.getBranch(event.project_name, event.branch)
-                event.branch_protected = b.get('protected')
-                self.connection.checkBranchCache(project, event.branch,
-                                                 event.branch_protected)
 
         return event
 

zuul/scheduler.py

@@ -1031,7 +1031,15 @@ class Scheduler(threading.Thread):
                     (event.branch_deleted and
                      self.abide.getUnparsedConfig(event.project_name,
                                                   event.branch) is not None)):
+                    reconfigure_tenant = True
 
+                # If the driver knows the branch but we don't have a config, we
+                # also need to reconfigure. This happens if a GitHub branch
+                # was just configured as protected without a push in between.
+                if (event.branch in project.source.getProjectBranches(
+                        project, tenant)
+                    and self.abide.getUnparsedConfig(
+                        project.canonical_name, event.branch) is None):
                     reconfigure_tenant = True
 
                 # If the branch is unprotected and unprotected branches