From 8f7bef79f866581b932121da01c8f4c434e3acae Mon Sep 17 00:00:00 2001 From: Karsten Hopp Date: Dec 19 2019 11:07:28 +0000 Subject: Add bugzillla override functions and tests This allows repo admins and pagure admins to set overrides for the default bugzilla assignees for Fedora and for EPEL. This information used to be in https://pagure.io/releng/fedora-scm-requests and will be migrated to dist-git. Adds endpoints to the dist-git plugin: - /bzoverrides// (GET) returns { "epel_assignee": "karsten", "fedora_assignee": " karsten" } - /bzoverrides// (POST) sending a POST request with a correct token and data { "epel_assignee": "someuser", "fedora_assignee": " karsten" } will set the default EPEL bugzilla assignee to 'someuser'. An empty user string will reset the assignee to the repo admin. Signed-off-by: Karsten Hopp --- diff --git a/README.rst b/README.rst index edb3259..799c039 100644 --- a/README.rst +++ b/README.rst @@ -28,7 +28,6 @@ This plugin reuses the Pagure configuration, and adds several keys to it. refs. - ``SIG_PREFIXES``: List of prefixes for SIG refs. - Example configurations ====================== @@ -73,8 +72,10 @@ The tests here require the *test suite* of pagure itself to work. You have to modify your PYTHONPATH to find them. Run with:: $ PYTHONPATH=.:/path/to/pagure/checkout nosetests dist_git_auth_tests.py +or + $ PYTHONPATH=.:/path/to/pagure/checkout nosetests bugzilla-override-tests.py You can use our requirements-testing.txt to install testing dependencies with pip: $ pip install -r /path/to/pagure/checkout/requirements.txt $ pip install -r /path/to/pagure/checkout/requirements-testing.txt - $ pip install -r requirements-testing.txt \ No newline at end of file + $ pip install -r requirements-testing.txt diff --git a/bugzilla-override-tests.py b/bugzilla-override-tests.py new file mode 100644 index 0000000..138ebcb --- /dev/null +++ b/bugzilla-override-tests.py @@ -0,0 +1,198 @@ +from __future__ import print_function + +import os +import json + +# These are the tests from the pagure/ git repo. +# Run with: +# PYTHONPATH=.:/path/to/pagure/checkout nosetests dist_git_auth_tests.py +import tests + +from pagure_distgit import plugin + + +def setUp(): + tests.setUp() + + +def tearDown(): + tests.tearDown() + + +class PagureFlaskApiProjectBZOverrideTests(tests.Modeltests): + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskApiProjectBZOverrideTests, self).setUp() + self.session.flush() + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, "repos"), bare=True) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + self._app.register_blueprint(plugin.DISTGIT_NS) + + def test_override_endpoint(self): + """Test bugzilla overrides. """ + + output = self.app.get("/api/0/somenamespace/test3") + self.assertEqual(output.status_code, 200) + output = self.app.get("/_dg/bzoverrides/somenamespace/test3") + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + expected_result = {"epel_assignee": "pingou", "fedora_assignee": "pingou"} + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + headers = {"Authorization": "token foo_token"} + datainput = { + "epel_assignee": "foo", + "fedora_assignee": "pingou", + } + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + # invalid token + self.assertEqual(output.status_code, 401) + + headers = {"Authorization": "token BBBZZZOOO"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + self.assertEqual(output.status_code, 401) + + output = self.app.get("/_dg/bzoverrides/somenamespace/test3") + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # Change assignees with a valid token + headers = {"Authorization": "token aaabbbcccddd"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + self.assertEqual(output.status_code, 200) + expected_result = {"epel_assignee": "foo", "fedora_assignee": "pingou"} + output = self.app.get("/_dg/bzoverrides/somenamespace/test3") + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # set both assignees to foo + datainput = {"epel_assignee": "foo", "fedora_assignee": "foo"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + self.assertEqual(output.status_code, 200) + output = self.app.get("/_dg/bzoverrides/somenamespace/test3") + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, datainput) + + # Change both assignees back to default + datainput = {"epel_assignee": "", "fedora_assignee": None} + expected_result = {"epel_assignee": "pingou", "fedora_assignee": "pingou"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + self.assertEqual(output.status_code, 200) + output = self.app.get("/_dg/bzoverrides/somenamespace/test3") + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # Change only one assignee, then change assignee back to default + expected_result = {"epel_assignee": "pingou", "fedora_assignee": "pingou"} + datainput = {"epel_assignee": "foo", "fedora_assignee": "pingou"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + self.assertEqual(output.status_code, 200) + datainput = {"epel_assignee": "", "fedora_assignee": "pingou"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput, headers=headers + ) + self.assertEqual(output.status_code, 200) + output = self.app.get("/_dg/bzoverrides/somenamespace/test3") + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + user = tests.FakeUser(username="pingou") + with tests.user_set(self.app.application, user): + # change one assignee + expected_result = {"epel_assignee": "foo", "fedora_assignee": "pingou"} + datainput = {"epel_assignee": "foo", "fedora_assignee": "pingou"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # change one assignee + expected_result = {"epel_assignee": "foo", "fedora_assignee": "foo"} + datainput = {"epel_assignee": "foo","fedora_assignee": "foo"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # reset assignees + expected_result = {"epel_assignee": "pingou", "fedora_assignee": "pingou"} + datainput = {"epel_assignee": None, "fedora_assignee": ""} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # change both assignees to foo, then change only one back + # As the other is considered empty, both should be back to pingou + expected_result = {"epel_assignee": "foo", "fedora_assignee": "foo"} + datainput = {"epel_assignee": "foo","fedora_assignee": "foo"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + expected_result = {"epel_assignee": "pingou", "fedora_assignee": "pingou"} + datainput = {"epel_assignee": "pingou"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + # Invalid datainput resets to default + datainput = {"epel_assignee": "foo","fedora_assignee": "foo"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + datainput = {} # This is invalid input + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, expected_result) + + datainput = {"epel_assignee": "foo","fedora_assignee": "foo"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + self.assertEqual(output.status_code, 200) + datainput = None # Invalid input + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=datainput + ) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual(output.status_code, 200) + + user = tests.FakeUser(username="foo") + with tests.user_set(self.app.application, user): + expected_result = {"epel_assignee": "pingou", "fedora_assignee": "pingou"} + output = self.app.post( + "/_dg/bzoverrides/somenamespace/test3", data=expected_result + ) + self.assertEqual(output.status_code, 401) diff --git a/createdb.py b/createdb.py index 39c93df..84e4189 100644 --- a/createdb.py +++ b/createdb.py @@ -29,7 +29,7 @@ if args.config: os.environ["PAGURE_CONFIG"] = config import pagure.config -from pagure_distgit.model import BASE, PagureAnitya +from pagure_distgit.model import BASE, PagureAnitya, PagureBZOverride db_url = pagure.config.config.get("DB_URL") if db_url.startswith("postgres"): @@ -37,4 +37,4 @@ if db_url.startswith("postgres"): else: engine = create_engine(db_url, echo=True) -BASE.metadata.create_all(engine, tables=[PagureAnitya.__table__]) +BASE.metadata.create_all(engine, tables=[PagureAnitya.__table__, PagureBZOverride.__table__]) diff --git a/pagure_distgit/forms.py b/pagure_distgit/forms.py index 2f892a6..d75e7f0 100644 --- a/pagure_distgit/forms.py +++ b/pagure_distgit/forms.py @@ -5,6 +5,7 @@ Authors: Pierre-Yves Chibon + Karsten Hopp """ @@ -27,3 +28,17 @@ class AnityaForm(pagure.forms.PagureForm): ("monitoring-with-scratch", "monitoring-with-scratch"), ], ) + + +class BZOverrideForm(pagure.forms.PagureForm): + """ Form to configure the Fedora/EPEL maintainer for a project. """ + + fedora_assignee = wtforms.StringField( + "Maintainer name", + [ wtforms.validators.optional(strip_whitespace=True) ], + ) + epel_assignee = wtforms.StringField( + "EPEL Maintainer name", + [ wtforms.validators.optional(strip_whitespace=True) ], + ) + diff --git a/pagure_distgit/model.py b/pagure_distgit/model.py index 4ffecf0..61537e1 100644 --- a/pagure_distgit/model.py +++ b/pagure_distgit/model.py @@ -57,3 +57,39 @@ class PagureAnitya(BASE): uselist=False, ), ) + + +class PagureBZOverride(BASE): + """ Stores information about default assignees (Fedora vs. EPEL) + + Table -- pagure_bzoverride + """ + + __tablename__ = "pagure_bzoverride" + + id = sa.Column(sa.Integer, primary_key=True) + project_id = sa.Column( + sa.Integer, + sa.ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + unique=True, + index=True, + ) + + epel_assignee = sa.Column( + sa.String(255), unique=False + ) + fedora_assignee = sa.Column( + sa.String(255), unique=False + ) + + project = relation( + "Project", + remote_side=[Project.id], + backref=backref( + "bzoverride", + cascade="delete, delete-orphan", + single_parent=True, + uselist=False, + ), + ) diff --git a/pagure_distgit/plugin.py b/pagure_distgit/plugin.py index 3809ec8..9d08d58 100644 --- a/pagure_distgit/plugin.py +++ b/pagure_distgit/plugin.py @@ -5,6 +5,7 @@ Authors: Pierre-Yves Chibon + Karsten Hopp """ @@ -374,3 +375,74 @@ def bodhi_updates_endpoint(namespace, repo): return html_output else: return flask.jsonify({"releases": releases, "updates": updates}) + + +@DISTGIT_NS.route("/bzoverrides//", methods=["GET"]) +@api_method +def bzoverride_get_endpoint(repo, namespace): + """ Returns the current default assignee(s) of this package. + Defaults to the repo user if unset. + """ + repo = _get_repo(repo, namespace=namespace) + output = {"fedora_assignee": repo.user.username, + "epel_assignee": repo.user.username } + if repo.bzoverride: + if repo.bzoverride.fedora_assignee: + output["fedora_assignee"] = repo.bzoverride.fedora_assignee + if repo.bzoverride.epel_assignee: + output["epel_assignee"] = repo.bzoverride.epel_assignee + return flask.jsonify(output) + + +@DISTGIT_NS.route("/bzoverrides//", methods=["POST"]) +@api_method +@api_login_required(acls=["modify_project"]) +def bzoverride_patch_endpoint(repo, namespace): + """ Updates the default assignees of this package. + """ + + repo = _get_repo(repo, namespace=namespace) + + is_site_admin = pagure.utils.is_admin() + # Only allow the main admin and Pagure site admins to modify projects' + # monitoring, even if the user has the right ACLs on their token + if ( + flask.g.fas_user.username != repo.user.username + and not is_site_admin + ): + raise pagure.exceptions.APIError( + 401, error_code=APIERROR.EMODIFYPROJECTNOTALLOWED + ) + + form = pagure_distgit.forms.BZOverrideForm(csrf_enabled=False) + if form.validate_on_submit(): + if form.fedora_assignee.data: + fedora_assignee = form.fedora_assignee.data.strip() + else: + fedora_assignee = None + if form.epel_assignee.data: + epel_assignee = form.epel_assignee.data.strip() + else: + epel_assignee = None + try: + if repo.bzoverride: + repo.bzoverride.fedora_assignee = fedora_assignee + repo.bzoverride.epel_assignee = epel_assignee + flask.g.session.add(repo) + else: + mapping = model.PagureBZOverride( + project_id=repo.id, epel_assignee=epel_assignee, + fedora_assignee=fedora_assignee + ) + flask.g.session.add(mapping) + flask.g.session.commit() + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) + else: + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.EINVALIDREQ, errors=form.errors + ) + + return bzoverride_get_endpoint(repo=repo.name, namespace=namespace)