diff options
author | Simon Westphahl <simon.westphahl@bmw.de> | 2023-03-01 18:07:30 +0100 |
---|---|---|
committer | Simon Westphahl <simon.westphahl@bmw.de> | 2023-03-02 12:25:42 +0100 |
commit | 59857a14b5aad73186ceec162b6052bf2058a9ba (patch) | |
tree | e12711b9ce09047bfe60690d75a26af621b448df /tests/unit | |
parent | bb2e04065cfada51eb32fe955d3fdc5b2c0bc6c7 (diff) | |
download | zuul-59857a14b5aad73186ceec162b6052bf2058a9ba.tar.gz |
Fix race related to PR with changed base branch
Some people use a workflow that's known as "stacked pull requests" in
order to split a change into more reviewable chunks.
In this workflow the first PR in the stack targets a protected branch
(e.g. master) and all other PRs target the unprotected branch of the
next item in the stack.
E.g. master <- feature-A (PR#1) <- feature-B (PR#2) <- ...
Now, when the first PR in the stack is merged Github will automatically
change the target branch of dependent PRs. For the above example this
would look like the following after PR#1 is merged:
master <- feature-B (PR#2) <- ...
The problem here is that we might still process events for a PR before
the base branch change, but the Github API already returns the info
about the updated target branch.
The problem with this is, that we used the target branch name from the
event (outdated branch name) and and the information from the change
object (new target branch) whether or not the target branch is protected
to determine if a branch was configured as protected.
In the above example Zuul might wrongly conclude that the "feature-A"
branch (taken from the event) is now protected.
In the related incident we also observed that this triggered a
reconfiguration with the wrong state of now two protected branches
(masters + feature-A). Since the project in question previously had only
one branch this lead to a change in branch matching behavior for jobs
defined in that repository.
Change-Id: Ia037e3070aaecb05c062865a6bb0479b86e4dcde
Diffstat (limited to 'tests/unit')
-rw-r--r-- | tests/unit/test_github_driver.py | 35 |
1 files changed, 35 insertions, 0 deletions
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index fb46aa7d1..47e84ca7f 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1741,6 +1741,41 @@ class TestGithubUnprotectedBranches(ZuulTestCase): # branch self.assertLess(old, new) + def test_base_branch_updated(self): + self.create_branch('org/project2', 'feature') + github = self.fake_github.getGithubClient() + repo = github.repo_from_project('org/project2') + repo._set_branch_protection('master', True) + + # Make sure Zuul picked up and cached the configured branches + self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + self.waitUntilSettled() + + github_connection = self.scheds.first.connections.connections['github'] + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + project = github_connection.source.getProject('org/project2') + + # Verify that only the master branch is considered protected + branches = github_connection.getProjectBranches(project, tenant) + self.assertEqual(branches, ["master"]) + + A = self.fake_github.openFakePullRequest('org/project2', 'master', + 'A') + # Fake an event from a pull-request that changed the base + # branch from "feature" to "master". The PR is already + # using "master" as base, but the event still references + # the old "feature" branch. + event = A.getPullRequestOpenedEvent() + event[1]["pull_request"]["base"]["ref"] = "feature" + + self.fake_github.emitEvent(event) + self.waitUntilSettled() + + # Make sure we are still only considering "master" to be + # protected. + branches = github_connection.getProjectBranches(project, tenant) + self.assertEqual(branches, ["master"]) + # 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. |