From ad317b3d9b3292369c922951ebfe90686864ba3d Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 11 2017 14:28:00 +0000
Subject: [PATCH 1/4] Blacklist creating a group named 'group'


This is otherwise an easy way to create a project that cannot be accessed
later, create a group 'group' and use it as namespace for another project
you won't be able to access that project.

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/default_config.py b/pagure/default_config.py
index c9522a2..cb1aee5 100644
--- a/pagure/default_config.py
+++ b/pagure/default_config.py
@@ -243,7 +243,7 @@ BLACKLISTED_PROJECTS = [
 ALLOWED_PREFIX = []
 
 # List of blacklisted group names
-BLACKLISTED_GROUPS = ['forks']
+BLACKLISTED_GROUPS = ['forks', 'group']
 
 
 ACLS = {

From 56e29ffabad7ba69dcd3ca5d3e28acb665102a9d Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 11 2017 14:28:00 +0000
Subject: [PATCH 2/4] Allow having a dedicated worker to compile the gitolite configuration file


This is useful in large pagure deployment where compiling the gitolite
configuration file can take a while.

This commit documents the new configuration key and how it can be used.

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/doc/configuration.rst b/doc/configuration.rst
index 19b836b..ed9deb3 100644
--- a/doc/configuration.rst
+++ b/doc/configuration.rst
@@ -279,6 +279,24 @@ Defaults to: ``gitolite3``
 .. note:: These options can be expended, cf :ref:`custom-gitolite`.
 
 
+GITOLITE_CELERY_QUEUE
+~~~~~~~~~~~~~~~~~~~~~
+
+This configuration is useful for large pagure deployment where recompiling
+the gitolite config file can take a long time. By default the compilation
+of gitolite's configuration file is done by the pagure_worker, which spawns
+by default 4 concurrent workers. If it takes a while to recompile the
+gitolite configuration file, these workers may be stepping on each others'
+toes.
+In this situation, this configuration key allows you to direct the messages
+asking for the gitolite configuration file to be compiled to a different
+queue which can then be handled by a different service/worker.
+
+Pagure provides a ``pagure_gitolite_worker.service`` systemd service file
+pre-configured to handles these messages if this configuration key is set
+to ``gitolite_queue``.
+
+
 **gitolite 2 only**
 ~~~~~~~~~~~~~~~~~~~
 
diff --git a/files/pagure.spec b/files/pagure.spec
index ed23424..e5a4993 100644
--- a/files/pagure.spec
+++ b/files/pagure.spec
@@ -233,7 +233,8 @@ cp -r alembic $RPM_BUILD_ROOT/%{_datadir}/pagure
 mkdir -p $RPM_BUILD_ROOT/%{_unitdir}
 install -m 644 files/pagure_worker.service \
     $RPM_BUILD_ROOT/%{_unitdir}/pagure_worker.service
-
+install -p -m 644 files/pagure_gitolite_worker.service \
+    $RPM_BUILD_ROOT/%{_unitdir}/pagure_gitolite_worker.service
 
 # Install the milter files
 mkdir -p $RPM_BUILD_ROOT/%{_localstatedir}/run/pagure
@@ -342,7 +343,7 @@ install -m 644 pagure-loadjson/pagure_loadjson.service \
 %{python_sitelib}/pagure*.egg-info
 %{_bindir}/pagure-admin
 %{_unitdir}/pagure_worker.service
-
+%{_unitdir}/pagure_gitolite_worker.service
 
 %files milters
 %license LICENSE
diff --git a/files/pagure_gitolite_worker.service b/files/pagure_gitolite_worker.service
new file mode 100644
index 0000000..00b3805
--- /dev/null
+++ b/files/pagure_gitolite_worker.service
@@ -0,0 +1,15 @@
+[Unit]
+Description=Pagure worker for backend git interaction
+After=redis.target
+Documentation=https://pagure.io/pagure
+
+[Service]
+ExecStart=/usr/bin/celery worker -A pagure.lib.tasks --loglevel=info --autoreload -c 1 -Q gitolite_queue
+Environment="PAGURE_CONFIG=/etc/pagure/pagure.cfg"
+Type=simple
+User=git
+Group=git
+Restart=on-failure
+
+[Install]
+WantedBy=multi-user.target
diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py
index bf171aa..4a942c3 100644
--- a/pagure/lib/tasks.py
+++ b/pagure/lib/tasks.py
@@ -67,7 +67,7 @@ def gc_clean():
     gc.collect()
 
 
-@conn.task
+@conn.task(queue=APP.config.get('GITOLITE_CELERY_QUEUE', None))
 def generate_gitolite_acls(namespace=None, name=None, user=None, group=None):
     """ Generate the gitolite configuration file either entirely or for a
     specific project.
@@ -220,10 +220,11 @@ def create_project(username, namespace, name, add_readme,
         plugin.install(project, dbobj)
         session.commit()
 
-    generate_gitolite_acls.delay(
+    task = generate_gitolite_acls.delay(
         namespace=project.namespace,
         name=project.name,
         user=project.user.user if project.is_fork else None)
+    _log.info('Refresh in queued in task: %s', task.id)
 
     session.remove()
     gc_clean()
@@ -418,11 +419,12 @@ def fork(name, namespace, user_owner, user_forker, editbranch, editfile):
     del frepo
     session.remove()
 
-    _log.info('Project created, refreshing auth')
-    generate_gitolite_acls(
+    _log.info('Project created, refreshing auth async')
+    task = generate_gitolite_acls.delay(
         namespace=repo_to.namespace,
         name=repo_to.name,
         user=repo_to.user.user if repo_to.is_fork else None)
+    _log.info('Refresh in queued in task: %s', task.id)
     gc_clean()
 
     if editfile is None:

From 0066db49b91f38ce4c0a9c20e10921f9f178b947 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 11 2017 14:28:00 +0000
Subject: [PATCH 3/4] Fix removing groups of a project


Turns out what we fixed for users is also needed for groups, surprising
he?

Anyway, this should make the relation writable and the effect is now
actually tested.

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/model.py b/pagure/lib/model.py
index a3be8b0..15aacc0 100644
--- a/pagure/lib/model.py
+++ b/pagure/lib/model.py
@@ -418,7 +418,6 @@ class Project(BASE):
                       func.lower(projects.c.name)"
         ),
         order_by="PagureGroup.group_name.asc()",
-        viewonly=True
     )
 
     admin_groups = relation(
diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py
index 28078ac..50327f2 100644
--- a/tests/test_pagure_flask_ui_repo.py
+++ b/tests/test_pagure_flask_ui_repo.py
@@ -850,15 +850,21 @@ class PagureFlaskRepotests(tests.Modeltests):
         self.session.commit()
         self.assertEqual(msg, 'Group added')
 
+        repo = pagure.get_authorized_project(self.session, 'test')
+        self.assertEqual(len(repo.groups), 1)
+
         with tests.user_set(pagure.APP, user):
             output = self.app.post('/test/dropgroup/1', follow_redirects=True)
             self.assertEqual(output.status_code, 200)
             self.assertIn(
                 '<title>Settings - test - Pagure</title>', output.data)
             self.assertIn('<h3>Settings for test</h3>', output.data)
+            self.assertIn('action="/test/dropgroup/1">', output.data)
             self.assertNotIn(
                 '</button>\n                      Group removed',
                 output.data)
+            repo = pagure.get_authorized_project(self.session, 'test')
+            self.assertEqual(len(repo.groups), 1)
 
             data = {'csrf_token': csrf_token}
             output = self.app.post(
@@ -870,6 +876,9 @@ class PagureFlaskRepotests(tests.Modeltests):
             self.assertIn(
                 '</button>\n                      Group removed',
                 output.data)
+            self.assertNotIn('action="/test/dropgroup/1">', output.data)
+            repo = pagure.get_authorized_project(self.session, 'test')
+            self.assertEqual(len(repo.groups), 0)
 
     @patch('pagure.ui.repo.admin_session_timedout')
     def test_update_project(self, ast):

From 91330b7bec90dc3c30e3ee35ae684f8646b5429b Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 11 2017 14:29:10 +0000
Subject: [PATCH 4/4] Adjust the wording in the logged message


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py
index 4a942c3..ef5f7b3 100644
--- a/pagure/lib/tasks.py
+++ b/pagure/lib/tasks.py
@@ -224,7 +224,7 @@ def create_project(username, namespace, name, add_readme,
         namespace=project.namespace,
         name=project.name,
         user=project.user.user if project.is_fork else None)
-    _log.info('Refresh in queued in task: %s', task.id)
+    _log.info('Refreshing gitolite config queued in task: %s', task.id)
 
     session.remove()
     gc_clean()
@@ -424,7 +424,7 @@ def fork(name, namespace, user_owner, user_forker, editbranch, editfile):
         namespace=repo_to.namespace,
         name=repo_to.name,
         user=repo_to.user.user if repo_to.is_fork else None)
-    _log.info('Refresh in queued in task: %s', task.id)
+    _log.info('Refreshing gitolite config queued in task: %s', task.id)
     gc_clean()
 
     if editfile is None:

