PR#166: Modify DistGitAuth.is_supported_branch to work with EPEL 10 - pagure-dist-git

pagure-dist-git

#166 Modify DistGitAuth.is_supported_branch to work with EPEL 10

Merged 2 months ago by humaton. Opened 2 months ago by carlwgeorge.

carlwgeorge/pagure-dist-git epel10 into master

Modify DistGitAuth.is_supported_branch to work with EPEL 10

Carl George • 2 months ago

7fd616c

Add two EPEL branch tests

Carl George • 2 months ago

58b66f7

Adjust several tests to align assertions with mocked responses

Carl George • 2 months ago

5100717

dist_git_auth.py

file modified

+36 -10

		`@@ -96,18 +96,44 @@`
		`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`

pagure_distgit_tests/test_dist_git_auth.py

file modified

+222 -77

		`@@ -453,28 +453,34 @@`
		`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")`

		`@@ -483,7 +489,7 @@`
		`self.session,`
		`project=project,`
		`username="pingou",`
		`- refname="refs/heads/f26",`
		`+ refname="refs/heads/f34",`
		`pull_request=None,`
		`repodir=None,`
		`repotype="main",`
		`@@ -494,7 +500,7 @@`
		`)`

		`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")`
		`@@ -503,26 +509,31 @@`
		`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`

		`@@ -531,7 +542,7 @@`
		`self.session,`
		`project=project,`
		`username="pingou",`
		`- refname="refs/heads/f27",`
		`+ refname="refs/heads/f39",`
		`pull_request=None,`
		`repodir=None,`
		`repotype="main",`
		`@@ -541,7 +552,7 @@`
		`)`
		`)`

		`- 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):`
		`@@ -549,26 +560,31 @@`
		`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`

		`@@ -577,7 +593,109 @@`
		`self.session,`
		`project=project,`
		`username="foo",`
		`- refname="refs/heads/f27",`
		`+ refname="refs/heads/f39",`
		`+ pull_request=None,`
		`+ repodir=None,`
		`+ repotype="main",`
		`+ revfrom=None,`
		`+ revto=None,`
		`+ is_internal=False,`
		`+ )`
		`+ )`
		`+`
		`+ 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 = {`
		`+ "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`
		`+`
		`+ 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()`
		`+ res.ok = True`
		`+ res.json.return_value = {`
		`+ "pages": 1,`
		`+ "releases": [`
		`+ {`
		`+ "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,`
		`+ },`
		`+ ],`
		`+ }`
		`+ 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",`
		`@@ -587,14 +705,17 @@`
		`)`
		`)`

		`- self.expect_info_msg("Branch refs/heads/f27 is supported")`
		`+ 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()`
		`res.ok = True`
		`- res.json.return_value = {}`
		`+ res.json.return_value = {`
		`+ "pages": 1,`
		`+ "releases": [],`
		`+ }`
		`mock_requests.get.return_value = res`

		`self.assertFalse(`
		`@@ -630,7 +751,10 @@`
		`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(`
		`@@ -655,7 +779,10 @@`
		`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(`
		`@@ -712,7 +839,10 @@`
		`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(`
		`@@ -738,7 +868,10 @@`
		`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(`
		`@@ -880,7 +1013,10 @@`
		`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(`
		`@@ -905,7 +1041,10 @@`
		`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(`
		`@@ -964,7 +1103,10 @@`
		`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(`
		`@@ -1020,7 +1162,10 @@`
		`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(`

carlwgeorge commented 2 months ago

The important parts of this pull request are in the last commit. The commits before that are mainly for getting the tests in order so that the real changes in the last commit are more trustworthy. Please see the individual commit messages for more details.

I do want to emphasize that without this we are blocked from pushing to any epel10 branches.

https://pagure.io/releng/issue/12236

humaton commented 2 months ago

Is it really necessary to change all the formatting? This is making your code changes lost in the PR...

carlwgeorge commented 2 months ago

Viewing just the last commit is the best way to see the exact code changes. Before I started making those changes I wanted to ensure all the tests were passing, including TestStyle.test_code_with_black. I can certainly rework this without the formatting fixes and ignore that test failure if you like.

rebased onto 9a2fd08

2 months ago

carlwgeorge commented 2 months ago

Rebased without the formatting fixes.

humaton commented 2 months ago

Looks good.