zuul/zuul
Code Issues Proposed changes

Merge "Don't update protected flag for missing project"

Browse Source
This commit is contained in:
Zuul
2026-06-16 22:33:18 +00:00
committed by Gerrit Code Review
parent f00d0faae7 6e1780fb6e
commit 653c9c18a1
6 changed files with 111 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/unit/test_github_driver.py
+63
View File
@@ -1716,6 +1716,24 @@ class TestGithubDriver(ZuulTestCase):
self.assertIn("can not be merged due to: draft state",
B.comments[0])
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_pr_event_branch_protection(self):
# Test that we do not try to update the protected status of a
# project when we don't have exclude_unprotected_branches set.
github = self.fake_github.getGithubClient()
github.addProjectByName('org/project')
repo = github.repo_from_project('org/project')
repo._set_branch_protection('master', True)
with (mock.patch(
"zuul.zk.branch_cache_new.BranchCacheNew.setProtected")
as cache_mock):
A = self.fake_github.openFakePullRequest(
'org/project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
cache_mock.assert_not_called()
class TestMultiGithubDriver(ZuulTestCase):
config_file = 'zuul-multi-github.conf'
@@ -2029,6 +2047,28 @@ class TestGithubUnprotectedBranches(ZuulTestCase):
self.assertEqual(1, len(self.history))
def test_pr_event_foreign_project(self):
# Test that we do not try to update the protected status of a
# project that is not in any tenant config.
self.init_repo("org/new-project")
files = {'README': ''}
self.addCommitToRepo("org/new-project", 'Initial commit',
files=files, tag='init')
github = self.fake_github.getGithubClient()
github.addProjectByName('org/new-project')
repo = github.repo_from_project('org/new-project')
repo._set_branch_protection('master', True)
with (mock.patch(
"zuul.zk.branch_cache_new.BranchCacheNew.setProtected")
as cache_mock):
A = self.fake_github.openFakePullRequest(
'org/new-project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
cache_mock.assert_not_called()
def _test_push_event_reconfigure(self, project, branch,
expect_reconfigure=False,
old_sha=None, new_sha=None,
@@ -2159,6 +2199,29 @@ class TestGithubUnprotectedBranches(ZuulTestCase):
self.assertNotEqual(new_layout, prev_layout)
prev_layout = new_layout
@okay_tracebacks('No branches for project')
def test_branch_protection_rule_update_foreign_project(self):
# Test the branch_protection_rule event from a project not in
# any tenant config
self.init_repo("org/new-project")
files = {'README': ''}
self.addCommitToRepo("org/new-project", 'Initial commit',
files=files, tag='init')
github = self.fake_github.getGithubClient()
github.addProjectByName('org/new-project')
repo = github.repo_from_project('org/new-project')
repo._set_branch_protection('master', True)
with (mock.patch(
"zuul.zk.branch_cache_new.BranchCacheNew.setProtected")
as cache_mock):
self.fake_github.emitEvent(
self.fake_github.getBranchProtectionRuleEvent(
'org/new-project', 'created'))
self.waitUntilSettled()
cache_mock.assert_not_called()
class TestGithubLockedBranches(ZuulTestCase):
config_file = 'zuul-github-driver.conf'
tests/unit/test_zk.py
+23 -11
View File
@@ -2442,12 +2442,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', protected_flags)
.values()
if bi.protected is True]),
[bi.name for bi in test_data['project1']['protected']]
)
self.assertRaises(
LookupError,
lambda: cache.getProjectBranches('project1', all_flags),
lambda: cache.getProjectBranches('project1', all_flags).values(),
)
cache.setProjectBranches('project1', all_flags,
@@ -2455,12 +2456,15 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', protected_flags)
.values()
if bi.protected is True]),
[bi.name for bi in test_data['project1']['protected']]
)
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', all_flags)]),
cache.getProjectBranches('project1', all_flags)
.values()
]),
[bi.name for bi in test_data['project1']['all']]
)
@@ -2494,11 +2498,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertRaises(
LookupError,
lambda: cache.getProjectBranches('project1', protected_flags)
lambda: cache.getProjectBranches(
'project1', protected_flags).values()
)
self.assertRaises(
LookupError,
lambda: cache.getProjectBranches('project1', all_flags)
lambda: cache.getProjectBranches(
'project1', all_flags).values()
)
# Test the other order; all followed by protected-only
@@ -2506,11 +2512,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
test_data['project1']['all'])
self.assertRaises(
LookupError,
lambda: cache.getProjectBranches('project1', protected_flags)
lambda: cache.getProjectBranches(
'project1', protected_flags).values()
)
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', all_flags)]),
cache.getProjectBranches(
'project1', all_flags).values()]),
[bi.name for bi in test_data['project1']['all']]
)
@@ -2519,12 +2527,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', protected_flags)
.values()
if bi.protected is True]),
[bi.name for bi in test_data['project1']['protected']]
)
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', all_flags)]),
cache.getProjectBranches('project1', all_flags).values()]),
[bi.name for bi in test_data['project1']['all']]
)
@@ -2573,12 +2582,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', protected_flags)
.values()
if bi.protected is True]),
[bi.name for bi in data1['project1']['protected']]
)
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', all_flags)]),
cache.getProjectBranches('project1', all_flags).values()]),
[bi.name for bi in data1['project1']['all']]
)
@@ -2587,12 +2597,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', protected_flags)
.values()
if bi.protected is True]),
[bi.name for bi in data2['project1']['protected']]
)
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', all_flags)]),
cache.getProjectBranches('project1', all_flags).values()]),
[bi.name for bi in data2['project1']['all']]
)
@@ -2601,12 +2612,13 @@ class TestBranchCache(ZooKeeperBaseTestCase):
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', protected_flags)
.values()
if bi.protected is True]),
[bi.name for bi in data1['project1']['protected']]
)
self.assertEqual(
sorted([bi.name for bi in
cache.getProjectBranches('project1', all_flags)]),
cache.getProjectBranches('project1', all_flags).values()]),
[bi.name for bi in data1['project1']['all']]
)
@@ -2616,7 +2628,7 @@ class TestBranchCache(ZooKeeperBaseTestCase):
cache = BranchCache(self.zk_client, conn, self.component_registry)
self.assertRaises(
LookupError,
lambda: cache.getProjectBranches('project1', True)
lambda: cache.getProjectBranches('project1', True).values()
)
self.assertIsNone(
cache.getProjectBranches('project1', True, default=None)
zuul/connection/__init__.py
+14 -19
View File
@@ -335,8 +335,10 @@ class ZKBranchCacheMixin:
branches = self._branch_cache.getProjectBranches(
project.name, required_flags, min_ltime)
if branches is not None:
branches = [b.name for b in self._filterProjectBranches(
branches, exclude_unprotected, exclude_locked)]
branches = [b.name for b in
self._filterProjectBranches(
branches.values(), exclude_unprotected,
exclude_locked)]
except LookupError:
if self.read_only:
# A scheduler hasn't attempted to fetch them yet
@@ -523,23 +525,16 @@ class ZKBranchCacheMixin:
branches = self._branch_cache.getProjectBranches(
project_name, required_flags, default=None)
if not branches:
branches = []
branches = [b.name for b in branches]
update = False
if (event.branch in branches) and (not protected):
update = True
if (event.branch not in branches) and (protected):
update = True
if update:
self.log.info("Project %s branch %s protected state "
"changed to %s",
project_name, event.branch, protected)
self._branch_cache.setProtected(
project_name, event.branch, protected)
event.branch_cache_ltime = self._branch_cache.ltime
if branches is not None:
if ((branch_info := branches.get(event.branch)) is None or
branch_info.protected != protected):
log = get_annotated_logger(self.log, event)
log.info("Project %s branch %s protected state "
"changed to %s",
project_name, event.branch, protected)
self._branch_cache.setProtected(
project_name, event.branch, protected)
event.branch_cache_ltime = self._branch_cache.ltime
event.branch_protected = protected
else:
zuul/driver/github/githubconnection.py
+3 -2
View File
@@ -680,7 +680,7 @@ class GithubEventProcessor(object):
if cached_branches is None:
raise RuntimeError(f"No branches for project {project_name}")
else:
cached_branches = [b.name for b in cached_branches
cached_branches = [b.name for b in cached_branches.values()
if b.protected is True]
old_protected_branches = set(cached_branches)
@@ -697,7 +697,8 @@ class GithubEventProcessor(object):
self.connection._branch_cache.getProjectBranches(
project_name, BranchFlag.PROTECTED)
new_protected_branches = set(
[b.name for b in new_protected_branches if b.protected is True])
[b.name for b in new_protected_branches.values()
if b.protected is True])
newly_protected = new_protected_branches - old_protected_branches
newly_unprotected = old_protected_branches - new_protected_branches
zuul/zk/branch_cache_new.py
+4 -3
View File
@@ -239,8 +239,9 @@ class BranchCacheNew:
:param any default:
Optional default value to return if no cache entry exits.
:returns: The list of branch names, or None if there was
an error when fetching the branches.
:returns: A dictionary of {branch_name: BranchInfo} objects,
or None if there was an error when fetching the branches.
"""
project_info = self._getProjectInfoForRead(project_name, min_ltime)
if project_info is None:
@@ -258,7 +259,7 @@ class BranchCacheNew:
return return_default(default, project_name)
# We have the necessary info for this filtering.
return list(project_info.branches.values())
return project_info.branches
def setProjectBranches(self, project_name,
valid_flags, branch_infos):
zuul/zk/branch_cache_old.py
+4 -3
View File
@@ -202,8 +202,9 @@ class BranchCacheOld:
:param any default:
Optional default value to return if no cache entry exits.
:returns: The list of branch names, or None if there was
an error when fetching the branches.
:returns: A dictionary of {branch_name: BranchInfo} objects,
or None if there was an error when fetching the branches.
"""
if self.ltime < min_ltime:
with (zk_locked(self.rlock),
@@ -232,7 +233,7 @@ class BranchCacheOld:
return return_default(default, project_name)
# We have the necessary info for this filtering.
return list(project_info.branches.values())
return project_info.branches
def setProjectBranches(self, project_name,
valid_flags, branch_infos):