Reload tenant in case of new project branches
Since GitHub doesn't report branch protection changes, it is possible that a pull request to a branch is not considered since the configuration for that branch isn't loaded yet. This is, however, surprising for the user. This change updates the branch protection cache if a pull request was received and when the trigger event is then processed in the scheduler, it checks if the target branch is a protected branch whose config is not yet in the tenant's configuration cache. If that's the case, a tenant reload is issued. Change-Id: I0d851a8773b73de54b0b777691a6a0d9f788f096
This commit is contained in:
parent
85f6598722
commit
2a6d129ea1
2
tests/fixtures/config/unprotected-branches/git/org_project2/playbooks/used-job.yaml
vendored
Normal file
2
tests/fixtures/config/unprotected-branches/git/org_project2/playbooks/used-job.yaml
vendored
Normal file
|
@ -0,0 +1,2 @@
|
|||
- hosts: all
|
||||
tasks: []
|
|
@ -1,2 +1,8 @@
|
|||
- job:
|
||||
name: unused-job
|
||||
name: used-job
|
||||
run: playbooks/used-job.yaml
|
||||
|
||||
- project:
|
||||
check:
|
||||
jobs:
|
||||
- used-job
|
||||
|
|
|
@ -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'
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -1032,7 +1032,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
|
||||
|
|
Loading…
Reference in New Issue