From ac10ea381e740b246a304d6694f99773bc9070b5 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 08 2018 08:10:04 +0000 Subject: [PATCH 1/3] Allow admins to select to ignore existing repositories Signed-off-by: Patrick Uiterwijk --- diff --git a/doc/configuration.rst b/doc/configuration.rst index eb35b98..6b1c056 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -1062,6 +1062,17 @@ prevent users from deleting branches in their git repositories. Defaults to: ``True``. +ALLOW_ADMIN_IGNORE_EXISTING_REPOS +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This enables a checkbox "Ignore existing repos" for admins when creating a new +project. When this is checkbox is checked, existing repositories will not cause +project creation to fail. +This could be used to assume responsibility of existing repositories. + +Defaults to: ``False``. + + LOCAL_SSH_KEY ~~~~~~~~~~~~~ diff --git a/pagure/default_config.py b/pagure/default_config.py index 05b4c86..45569d8 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -78,6 +78,9 @@ PRIVATE_PROJECTS = True # Enable / Disable deleting branches in the UI ALLOW_DELETE_BRANCH = True +# Allow admins to ignore existing repos when creating a new project +ALLOW_ADMIN_IGNORE_EXISTING_REPOS = False + # Enable / Disable having pagure manage the user's ssh keys LOCAL_SSH_KEY = True diff --git a/pagure/forms.py b/pagure/forms.py index c91879b..5e5ff3b 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -167,6 +167,11 @@ class ProjectForm(ProjectFormSimplified): choices=[], coerce=convert_value, ) + ignore_existing_repos = wtforms.BooleanField( + "Ignore existing repositories", + [wtforms.validators.optional()], + false_values=FALSE_VALUES, + ) repospanner_region = wtforms.SelectField( "repoSpanner Region", [wtforms.validators.optional()], @@ -202,6 +207,13 @@ class ProjectForm(ProjectFormSimplified): ] if not pagure_config.get("USER_NAMESPACE", False): self.namespace.choices.insert(0, ("", "")) + + if not ( + is_admin() + and pagure_config.get("ALLOW_ADMIN_IGNORE_EXISTING_REPOS") + ): + self.ignore_existing_repos = None + if not ( is_admin() and pagure_config.get("REPOSPANNER_NEW_REPO_ADMIN_OVERRIDE") diff --git a/pagure/lib/git.py b/pagure/lib/git.py index cfc5598..5a8429b 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -2280,6 +2280,8 @@ def _create_project_repo(project, region, templ, ignore_existing, repotype): resp = resp.json() _log.debug("Response json: %s", resp) if not resp["Success"]: + if ignore_existing and "already exists" in resp["Error"]: + return None raise Exception( "Error in repoSpanner API call: %s" % resp["Error"] ) @@ -2292,10 +2294,13 @@ def _create_project_repo(project, region, templ, ignore_existing, repotype): if repodir is None: # This repo type is disabled return None - if os.path.exists(repodir) and not ignore_existing: - raise pagure.exceptions.RepoExistsException( - "The %s repo %s already exists" % (repotype, project.path) - ) + if os.path.exists(repodir): + if not ignore_existing: + raise pagure.exceptions.RepoExistsException( + "The %s repo %s already exists" % (repotype, project.path) + ) + else: + return None if repotype == "main": pygit2.init_repository(repodir, bare=True, template_path=templ) diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 51e46c2..d07ad6c 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -1034,6 +1034,10 @@ def new_project(): repospanner_region = form.repospanner_region.data else: repospanner_region = None + if form.ignore_existing_repos: + ignore_existing_repos = form.ignore_existing_repos.data + else: + ignore_existing_repos = False try: task = pagure.lib.new_project( @@ -1054,6 +1058,7 @@ def new_project(): "OLD_VIEW_COMMIT_ENABLED", False ), user_ns=pagure_config.get("USER_NAMESPACE", False), + ignore_existing_repo=ignore_existing_repos, ) flask.g.session.commit() return pagure.utils.wait_for_task(task) From 6876a8df93478ef0e92c2fbf75e4b536181d1f5d Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 08 2018 08:10:45 +0000 Subject: [PATCH 2/3] If a repo existed, clean up the project instance Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 5a8429b..f8de106 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -2280,8 +2280,11 @@ def _create_project_repo(project, region, templ, ignore_existing, repotype): resp = resp.json() _log.debug("Response json: %s", resp) if not resp["Success"]: - if ignore_existing and "already exists" in resp["Error"]: - return None + if "already exists" in resp["Error"]: + if ignore_existing: + return None + else: + raise pagure.exceptions.RepoExistsException(resp["Error"]) raise Exception( "Error in repoSpanner API call: %s" % resp["Error"] ) diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index ea23871..00f6822 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -290,9 +290,17 @@ def create_project( else: _log.debug(" Using template at: %s", templ) - pagure.lib.git.create_project_repos( - project, project.repospanner_region, templ, ignore_existing_repo - ) + try: + pagure.lib.git.create_project_repos( + project, + project.repospanner_region, + templ, + ignore_existing_repo, + ) + except pagure.exceptions.RepoExistsException: + session.delete(project) + session.commit() + raise if add_readme: with pagure.lib.git.TemporaryClone( From 335383797bba4d7308d27d966f3153eabf127652 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Oct 08 2018 08:48:44 +0000 Subject: [PATCH 3/3] Add a test case for adopting existing repos Signed-off-by: Patrick Uiterwijk --- diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index cbcb038..8048ac2 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -364,6 +364,35 @@ class PagureFlaskApptests(tests.Modeltests): self.assertTrue(os.path.exists( os.path.join(self.path, 'repos', 'requests', 'project-1.git'))) + @patch.dict('pagure.config.config', {'PAGURE_ADMIN_USERS': ['pingou'], + 'ALLOW_ADMIN_IGNORE_EXISTING_REPOS': True}) + def test_adopt_repos(self): + """ Test the new_project endpoint with existing git repo. """ + # Before + projects = pagure.lib.search_projects(self.session) + self.assertEqual(len(projects), 0) + tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + tests.add_content_git_repo(os.path.join(self.path, 'repos', 'test.git')) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + data = { + 'csrf_token': self.get_csrf(), + 'name': 'test', + 'description': 'Project #1', + } + + output = self.app.post('/new/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('The main repo test.git already exists', output_text) + + data['ignore_existing_repos'] = 'y' + output = self.app.post('/new/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn("Alice Author", output_text) + @patch.dict('pagure.config.config', {'PROJECT_NAME_REGEX': '^1[a-z]*$'}) def test_new_project_diff_regex(self): """ Test the new_project endpoint with a different regex. """ diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 875e6b7..e3bab98 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -1701,7 +1701,6 @@ class PagureLibtests(tests.Modeltests): self.assertIn( 'already exists', str(task.get(propagate=False))) - self.session.rollback() self.assertFalse(os.path.exists(gitrepo)) self.assertTrue(os.path.exists(docrepo)) @@ -1710,19 +1709,18 @@ class PagureLibtests(tests.Modeltests): # Drop the doc repo and try again shutil.rmtree(docrepo) - self.assertRaises( - pagure.exceptions.PagureException, - pagure.lib.new_project, - session=self.session, - user='pingou', - name='testproject', - repospanner_region=None, - blacklist=[], - allowed_prefix=[], - description='description for testproject', - parent_id=None - ) - self.session.rollback() + with self.assertRaises(pagure.exceptions.RepoExistsException): + task = pagure.lib.new_project( + session=self.session, + user='pingou', + name='testproject', + repospanner_region=None, + blacklist=[], + allowed_prefix=[], + description='description for testproject', + parent_id=None, + ) + task.get() self.assertFalse(os.path.exists(gitrepo)) self.assertFalse(os.path.exists(docrepo)) self.assertTrue(os.path.exists(ticketrepo)) @@ -1730,19 +1728,18 @@ class PagureLibtests(tests.Modeltests): # Drop the request repo and try again shutil.rmtree(ticketrepo) - self.assertRaises( - pagure.exceptions.PagureException, - pagure.lib.new_project, - session=self.session, - user='pingou', - name='testproject', - repospanner_region=None, - blacklist=[], - allowed_prefix=[], - description='description for testproject', - parent_id=None - ) - self.session.rollback() + with self.assertRaises(pagure.exceptions.RepoExistsException): + task = pagure.lib.new_project( + session=self.session, + user='pingou', + name='testproject', + repospanner_region=None, + blacklist=[], + allowed_prefix=[], + description='description for testproject', + parent_id=None, + ) + task.get() self.assertFalse(os.path.exists(gitrepo)) self.assertFalse(os.path.exists(docrepo)) self.assertFalse(os.path.exists(ticketrepo)) diff --git a/tests/test_pagure_repospanner.py b/tests/test_pagure_repospanner.py index 6f50174..8ab2058 100644 --- a/tests/test_pagure_repospanner.py +++ b/tests/test_pagure_repospanner.py @@ -473,6 +473,78 @@ class PagureRepoSpannerTestsNewRepoDefault(PagureRepoSpannerTests): output_text = output.get_data(as_text=True) self.assertEqual(output_text, 'foo\n bar\n baz') + @patch.dict('pagure.config.config', {'PAGURE_ADMIN_USERS': ['pingou'], + 'ALLOW_ADMIN_IGNORE_EXISTING_REPOS': True}) + @patch('pagure.ui.app.admin_session_timedout') + def test_adopt_project(self, ast): + """ Test adopting a project in repoSpanner works. """ + ast.return_value = False + + user = tests.FakeUser(username='foo') + with tests.user_set(self.app.application, user): + output = self.app.get('/new/') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Create new Project', output_text) + + data = { + 'name': 'project-1', + 'description': 'Project #1', + 'create_readme': 'y', + 'csrf_token': self.get_csrf(), + } + + output = self.app.post('/new/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
\nProject #1', + output_text) + self.assertIn( + 'Overview - project-1 - Pagure', output_text) + self.assertIn('Added the README', output_text) + + output = self.app.get('/project-1/settings') + self.assertIn( + 'This repository is on repoSpanner region default', + output.get_data(as_text=True)) + + # Delete the project instance so that the actual repo remains + project = pagure.lib._get_project(self.session, 'project-1') + self.session.delete(project) + self.session.commit() + shutil.rmtree(os.path.join(self.path, 'repos', 'pseudo')) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + output = self.app.get('/project-1/') + self.assertEqual(output.status_code, 404) + + data = { + 'name': 'project-1', + 'description': 'Recreated project #1', + 'create_readme': 'false', + 'csrf_token': self.get_csrf(), + } + output = self.app.post('/new/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Repo pagure/main/project-1 already exists', + output_text) + + data['ignore_existing_repos'] = 'y' + output = self.app.post('/new/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
\nRecreated project #1', + output_text) + self.assertIn( + 'Overview - project-1 - Pagure', output_text) + self.assertIn('Added the README', output_text) + if __name__ == '__main__': unittest.main(verbosity=2)