From 5100717b142ee66d5d8d70e4019833f89f439cf7 Mon Sep 17 00:00:00 2001 From: Carl George Date: Aug 05 2024 17:23:35 +0000 Subject: [PATCH 1/3] Adjust several tests to align assertions with mocked responses Previously these tests had their mocked responses updated to newer release versions, but the input refname and info_msg still used the old release versions. The tests did still pass, but that was seemingly by accident. Signed-off-by: Carl George --- diff --git a/pagure_distgit_tests/test_dist_git_auth.py b/pagure_distgit_tests/test_dist_git_auth.py index 33fbfc3..17ee7f3 100644 --- a/pagure_distgit_tests/test_dist_git_auth.py +++ b/pagure_distgit_tests/test_dist_git_auth.py @@ -483,7 +483,7 @@ class DistGitAuthTestsFedora(DistGitAuthTests): self.session, project=project, username="pingou", - refname="refs/heads/f26", + refname="refs/heads/f34", pull_request=None, repodir=None, repotype="main", @@ -494,7 +494,7 @@ class DistGitAuthTestsFedora(DistGitAuthTests): ) self.expect_info_msg( - "Branch refs/heads/f26 is unsupported. Cannot push to a disabled branch (maybe eol?)." + "Branch refs/heads/f34 is unsupported. Cannot push to a disabled branch (maybe eol?)." ) @patch("dist_git_auth.requests") @@ -531,7 +531,7 @@ class DistGitAuthTestsFedora(DistGitAuthTests): self.session, project=project, username="pingou", - refname="refs/heads/f27", + refname="refs/heads/f39", pull_request=None, repodir=None, repotype="main", @@ -541,7 +541,7 @@ class DistGitAuthTestsFedora(DistGitAuthTests): ) ) - self.expect_info_msg("Branch refs/heads/f27 is supported") + self.expect_info_msg("Branch refs/heads/f39 is supported") @patch("dist_git_auth.requests") def test_protected_supported_branch_non_committer(self, mock_requests): @@ -577,7 +577,7 @@ class DistGitAuthTestsFedora(DistGitAuthTests): self.session, project=project, username="foo", - refname="refs/heads/f27", + refname="refs/heads/f39", pull_request=None, repodir=None, repotype="main", @@ -587,7 +587,7 @@ class DistGitAuthTestsFedora(DistGitAuthTests): ) ) - self.expect_info_msg("Branch refs/heads/f27 is supported") + self.expect_info_msg("Branch refs/heads/f39 is supported") @patch("dist_git_auth.requests") def test_protected_unspecified_branch_blacklisted(self, mock_requests): From 58b66f75f04bcc24060be263b73861aeeadc010d Mon Sep 17 00:00:00 2001 From: Carl George Date: Aug 05 2024 17:23:35 +0000 Subject: [PATCH 2/3] Add two EPEL branch tests Previous expectations about using bodhi to validate branches are going to be disrupted with EPEL 10. Let's add tests for both a minor version branch (epel10.1) and the leading branch (epel10). The latter will fail for now because the mocked respose matches what bodhi actually returns with the current approach, which of course needs adjustment. Signed-off-by: Carl George --- diff --git a/pagure_distgit_tests/test_dist_git_auth.py b/pagure_distgit_tests/test_dist_git_auth.py index 17ee7f3..94410da 100644 --- a/pagure_distgit_tests/test_dist_git_auth.py +++ b/pagure_distgit_tests/test_dist_git_auth.py @@ -590,6 +590,89 @@ class DistGitAuthTestsFedora(DistGitAuthTests): self.expect_info_msg("Branch refs/heads/f39 is supported") @patch("dist_git_auth.requests") + def test_protected_supported_branch_epel_minor(self, mock_requests): + project = self.create_namespaced_project("rpms", "test") + res = Mock() + res.ok = True + res.json.return_value = { + "name": "EPEL-10.1", + "long_name": "Fedora EPEL 10.1", + "version": "10.1", + "id_prefix": "FEDORA-EPEL", + "branch": "epel10.1", + "dist_tag": "epel10.1", + "stable_tag": "epel10.1", + "testing_tag": "epel10.1-testing", + "candidate_tag": "epel10.1-testing-candidate", + "pending_signing_tag": "epel10.1-signing-pending", + "pending_testing_tag": "epel10.1-testing-pending", + "pending_stable_tag": "epel10.1-pending", + "override_tag": "epel10.1-override", + "mail_template": "fedora_epel_legacy_errata_template", + "state": "current", + "composed_by_bodhi": True, + "create_automatic_updates": False, + "package_manager": "unspecified", + "testing_repository": None, + "eol": None, + } + mock_requests.get.return_value = res + + self.assertFalse( + self.dga.check_acl( + self.session, + project=project, + username="foo", + refname="refs/heads/epel10.1", + pull_request=None, + repodir=None, + repotype="main", + revfrom=None, + revto=None, + is_internal=False, + ) + ) + + self.expect_info_msg("Branch refs/heads/epel10.1 is supported") + + @patch("dist_git_auth.requests") + def test_protected_supported_branch_epel(self, mock_requests): + project = self.create_namespaced_project("rpms", "test") + res = Mock() + # This is of course a bad response, but it is what is currently + # returned from bodhi for epel10. This test is expected to fail for + # now until we change how we query bodhi for supported releases. + res.ok = False + res.json.return_value = { + "errors": [ + { + "description": "No such release", + "location": "body", + "name": "name", + }, + ], + "status": "error", + } + mock_requests.get.return_value = res + + self.assertFalse( + self.dga.check_acl( + self.session, + project=project, + username="foo", + refname="refs/heads/epel10", + pull_request=None, + repodir=None, + repotype="main", + revfrom=None, + revto=None, + is_internal=False, + ) + ) + + self.expect_info_msg("Branch refs/heads/epel10 is supported") + + @patch("dist_git_auth.requests") def test_protected_unspecified_branch_blacklisted(self, mock_requests): project = self.create_namespaced_project("rpms", "test") res = Mock() From 7fd616c53ac86d29d47b5a28f65527361c0741e8 Mon Sep 17 00:00:00 2001 From: Carl George Date: Aug 05 2024 17:23:35 +0000 Subject: [PATCH 3/3] Modify DistGitAuth.is_supported_branch to work with EPEL 10 The current implementation of this method assumes that the branch name matches the dist_tag of the corresponding bodhi release. Historically this has usually been correct, but it will not be anymore with EPEL 10. We are implementing minor versions, but there will still be a main epel10 branch in dist-git. Initially this will look like this: epel10 branch -> epel10.0 in koji/bodhi Later on it will be switched to this: epel10 branch -> epel10.1 in koji/bodhi epel10.0 branch -> epel10.0 in koji/bodhi This pattern will continue throughout the remaining minor versions. Bodhi allows you to query a release by the name, long_name, or dist_tag, but not the actual branch. To work around this, we can change this method to first query bodhi for all active releases, collect the branches of those releases, then use those branches for an initial check of whether a specific branch is active or not. If a branch is not active then we will fall back to the old logic of querying for it directly. This is because other code that calls this method distinguishes between a return of None (no release found, or a release found but with an empty state) or False (release was found, but the state is not current, pending, or frozen). Without this change, no commits can be pushed to epel10 branches, so it is a blocker to getting EPEL 10 started. https://pagure.io/releng/issue/12236 Signed-off-by: Carl George --- diff --git a/dist_git_auth.py b/dist_git_auth.py index 71f35bd..546ee7f 100644 --- a/dist_git_auth.py +++ b/dist_git_auth.py @@ -96,18 +96,44 @@ class DistGitAuth(GitAuthHelper): return None refname = refname[len("refs/heads/") :] - # Check if the branch is active on Bodhi if refname not in ["main", "rawhide"]: - resp = requests.get(f"{self.bodhi_url}releases/{refname}") - - if resp.ok: - resp = resp.json().get("state") - if not resp: # case when response is empty - return None - if resp not in ["current", "pending", "frozen"]: - return False - else: + # Get active bodhi branches + active_states = ("current", "pending", "frozen") + resp = requests.get( + f"{self.bodhi_url}releases/", + params={"state": active_states}, + ) + if not resp.ok: return None + resp_json = resp.json() + releases = resp_json["releases"] + pages = resp_json["pages"] + if pages > 1: + for page in range(2, pages + 1): + resp = requests.get( + f"{self.bodhi_url}releases/", + params={"state": active_states, "page": page}, + ) + if not resp.ok: + return None + resp_json = resp.json() + releases.extend(resp_json["releases"]) + + active_branches = {release["branch"] for release in releases} + + if refname not in active_branches: + # branch is either non-active or not used by any bodhi + # releases, ask for it by name to find out for sure + resp = requests.get(f"{self.bodhi_url}releases/{refname}") + + if resp.ok: + resp = resp.json().get("state") + if not resp: # case when response is empty + return None + if resp not in active_states: + return False + else: + return None # Branch can be supported, but package can be retired, # in that case don't push diff --git a/pagure_distgit_tests/test_dist_git_auth.py b/pagure_distgit_tests/test_dist_git_auth.py index 94410da..1fd5381 100644 --- a/pagure_distgit_tests/test_dist_git_auth.py +++ b/pagure_distgit_tests/test_dist_git_auth.py @@ -453,28 +453,34 @@ class DistGitAuthTestsFedora(DistGitAuthTests): def test_protected_unsupported_branch(self, mock_requests): res = Mock() res.ok = True - res.json.return_value = { - "name": "F34", - "long_name": "Fedora 34", - "version": "34", - "id_prefix": "FEDORA", - "branch": "f34", - "dist_tag": "f34", - "stable_tag": "f34-updates", - "testing_tag": "f34-updates-testing", - "candidate_tag": "f34-updates-candidate", - "pending_signing_tag": "f34-signing-pending", - "pending_testing_tag": "f34-updates-testing-pending", - "pending_stable_tag": "f34-updates-pending", - "override_tag": "f34-override", - "mail_template": "fedora_errata_template", - "state": "archived", - "composed_by_bodhi": True, - "create_automatic_updates": False, - "package_manager": "dnf", - "testing_repository": "updates-testing", - "eol": None, - } + res.json.side_effect = ( + { + "pages": 1, + "releases": [], + }, + { + "name": "F34", + "long_name": "Fedora 34", + "version": "34", + "id_prefix": "FEDORA", + "branch": "f34", + "dist_tag": "f34", + "stable_tag": "f34-updates", + "testing_tag": "f34-updates-testing", + "candidate_tag": "f34-updates-candidate", + "pending_signing_tag": "f34-signing-pending", + "pending_testing_tag": "f34-updates-testing-pending", + "pending_stable_tag": "f34-updates-pending", + "override_tag": "f34-override", + "mail_template": "fedora_errata_template", + "state": "archived", + "composed_by_bodhi": True, + "create_automatic_updates": False, + "package_manager": "dnf", + "testing_repository": "updates-testing", + "eol": None, + }, + ) mock_requests.get.return_value = res project = self.create_namespaced_project("rpms", "test") @@ -503,26 +509,31 @@ class DistGitAuthTestsFedora(DistGitAuthTests): res = Mock() res.ok = True res.json.return_value = { - "name": "F39", - "long_name": "Fedora 39", - "version": "39", - "id_prefix": "FEDORA", - "branch": "f39", - "dist_tag": "f39", - "stable_tag": "f39-updates", - "testing_tag": "f39-updates-testing", - "candidate_tag": "f39-updates-candidate", - "pending_signing_tag": "f39-signing-pending", - "pending_testing_tag": "f39-updates-testing-pending", - "pending_stable_tag": "f39-updates-pending", - "override_tag": "f39-override", - "mail_template": "fedora_errata_template", - "state": "current", - "composed_by_bodhi": True, - "create_automatic_updates": False, - "package_manager": "dnf", - "testing_repository": "updates-testing", - "eol": "2024-11-12", + "pages": 1, + "releases": [ + { + "name": "F39", + "long_name": "Fedora 39", + "version": "39", + "id_prefix": "FEDORA", + "branch": "f39", + "dist_tag": "f39", + "stable_tag": "f39-updates", + "testing_tag": "f39-updates-testing", + "candidate_tag": "f39-updates-candidate", + "pending_signing_tag": "f39-signing-pending", + "pending_testing_tag": "f39-updates-testing-pending", + "pending_stable_tag": "f39-updates-pending", + "override_tag": "f39-override", + "mail_template": "fedora_errata_template", + "state": "current", + "composed_by_bodhi": True, + "create_automatic_updates": False, + "package_manager": "dnf", + "testing_repository": "updates-testing", + "eol": "2024-11-12", + }, + ], } mock_requests.get.return_value = res @@ -549,26 +560,31 @@ class DistGitAuthTestsFedora(DistGitAuthTests): res = Mock() res.ok = True res.json.return_value = { - "name": "F39", - "long_name": "Fedora 39", - "version": "39", - "id_prefix": "FEDORA", - "branch": "f39", - "dist_tag": "f39", - "stable_tag": "f39-updates", - "testing_tag": "f39-updates-testing", - "candidate_tag": "f39-updates-candidate", - "pending_signing_tag": "f39-signing-pending", - "pending_testing_tag": "f39-updates-testing-pending", - "pending_stable_tag": "f39-updates-pending", - "override_tag": "f39-override", - "mail_template": "fedora_errata_template", - "state": "current", - "composed_by_bodhi": True, - "create_automatic_updates": False, - "package_manager": "dnf", - "testing_repository": "updates-testing", - "eol": "2024-11-12", + "pages": 1, + "releases": [ + { + "name": "F39", + "long_name": "Fedora 39", + "version": "39", + "id_prefix": "FEDORA", + "branch": "f39", + "dist_tag": "f39", + "stable_tag": "f39-updates", + "testing_tag": "f39-updates-testing", + "candidate_tag": "f39-updates-candidate", + "pending_signing_tag": "f39-signing-pending", + "pending_testing_tag": "f39-updates-testing-pending", + "pending_stable_tag": "f39-updates-pending", + "override_tag": "f39-override", + "mail_template": "fedora_errata_template", + "state": "current", + "composed_by_bodhi": True, + "create_automatic_updates": False, + "package_manager": "dnf", + "testing_repository": "updates-testing", + "eol": "2024-11-12", + }, + ], } mock_requests.get.return_value = res @@ -595,26 +611,31 @@ class DistGitAuthTestsFedora(DistGitAuthTests): res = Mock() res.ok = True res.json.return_value = { - "name": "EPEL-10.1", - "long_name": "Fedora EPEL 10.1", - "version": "10.1", - "id_prefix": "FEDORA-EPEL", - "branch": "epel10.1", - "dist_tag": "epel10.1", - "stable_tag": "epel10.1", - "testing_tag": "epel10.1-testing", - "candidate_tag": "epel10.1-testing-candidate", - "pending_signing_tag": "epel10.1-signing-pending", - "pending_testing_tag": "epel10.1-testing-pending", - "pending_stable_tag": "epel10.1-pending", - "override_tag": "epel10.1-override", - "mail_template": "fedora_epel_legacy_errata_template", - "state": "current", - "composed_by_bodhi": True, - "create_automatic_updates": False, - "package_manager": "unspecified", - "testing_repository": None, - "eol": None, + "pages": 1, + "releases": [ + { + "name": "EPEL-10.1", + "long_name": "Fedora EPEL 10.1", + "version": "10.1", + "id_prefix": "FEDORA-EPEL", + "branch": "epel10.1", + "dist_tag": "epel10.1", + "stable_tag": "epel10.1", + "testing_tag": "epel10.1-testing", + "candidate_tag": "epel10.1-testing-candidate", + "pending_signing_tag": "epel10.1-signing-pending", + "pending_testing_tag": "epel10.1-testing-pending", + "pending_stable_tag": "epel10.1-pending", + "override_tag": "epel10.1-override", + "mail_template": "fedora_epel_legacy_errata_template", + "state": "current", + "composed_by_bodhi": True, + "create_automatic_updates": False, + "package_manager": "unspecified", + "testing_repository": None, + "eol": None, + }, + ], } mock_requests.get.return_value = res @@ -639,19 +660,33 @@ class DistGitAuthTestsFedora(DistGitAuthTests): def test_protected_supported_branch_epel(self, mock_requests): project = self.create_namespaced_project("rpms", "test") res = Mock() - # This is of course a bad response, but it is what is currently - # returned from bodhi for epel10. This test is expected to fail for - # now until we change how we query bodhi for supported releases. - res.ok = False + res.ok = True res.json.return_value = { - "errors": [ + "pages": 1, + "releases": [ { - "description": "No such release", - "location": "body", - "name": "name", + "name": "EPEL-10.0", + "long_name": "Fedora EPEL 10.0", + "version": "10.0", + "id_prefix": "FEDORA-EPEL", + "branch": "epel10", + "dist_tag": "epel10.0", + "stable_tag": "epel10.0", + "testing_tag": "epel10.0-testing", + "candidate_tag": "epel10.0-testing-candidate", + "pending_signing_tag": "epel10.0-signing-pending", + "pending_testing_tag": "epel10.0-testing-pending", + "pending_stable_tag": "epel10.0-pending", + "override_tag": "epel10.0-override", + "mail_template": "fedora_epel_legacy_errata_template", + "state": "current", + "composed_by_bodhi": True, + "create_automatic_updates": False, + "package_manager": "unspecified", + "testing_repository": None, + "eol": None, }, ], - "status": "error", } mock_requests.get.return_value = res @@ -677,7 +712,10 @@ class DistGitAuthTestsFedora(DistGitAuthTests): project = self.create_namespaced_project("rpms", "test") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertFalse( @@ -713,7 +751,10 @@ class DistGitAuthTestsFedora(DistGitAuthTests): project = self.create_namespaced_project("rpms", "test") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertTrue( @@ -738,7 +779,10 @@ class DistGitAuthTestsFedora(DistGitAuthTests): project = self.create_namespaced_project("rpms", "test") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertFalse( @@ -795,7 +839,10 @@ class DistGitAuthTestsFedoraCommitterAccess(DistGitAuthTests): project = get_project(self.session, name="test", namespace="rpms") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertFalse( @@ -821,7 +868,10 @@ class DistGitAuthTestsFedoraCommitterAccess(DistGitAuthTests): project = get_project(self.session, name="test", namespace="rpms") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertTrue( @@ -963,7 +1013,10 @@ class DistGitAuthTestsCentOS(DistGitAuthTests): project = self.create_namespaced_project("rpms", "test") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertFalse( @@ -988,7 +1041,10 @@ class DistGitAuthTestsCentOS(DistGitAuthTests): project = self.create_namespaced_project("rpms", "test") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertFalse( @@ -1047,7 +1103,10 @@ class DistGitAuthTestsFedoraBranchOverride(DistGitAuthTests): project = get_project(self.session, name="test", namespace="rpms") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertFalse( @@ -1103,7 +1162,10 @@ class DistGitAuthTestsFedoraBranchOverride(DistGitAuthTests): project = get_project(self.session, name="cockpit", namespace="container") res = Mock() res.ok = True - res.json.return_value = {} + res.json.return_value = { + "pages": 1, + "releases": [], + } mock_requests.get.return_value = res self.assertTrue(