From 09e818b642d84b549a15b993f561230b873d1f5d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:01 +0000 Subject: [PATCH 1/10] Add a hook_type to each hook This will allow having different types of hooks --- diff --git a/pagure/hooks/__init__.py b/pagure/hooks/__init__.py index c8a2c42..400bdd2 100644 --- a/pagure/hooks/__init__.py +++ b/pagure/hooks/__init__.py @@ -42,6 +42,7 @@ class BaseHook(object): name = None form = None description = None + hook_type = 'post-receive' @classmethod def set_up(cls, project): @@ -66,10 +67,10 @@ class BaseHook(object): os.makedirs(hookfolder) # Install the main post-receive file - postreceive = os.path.join(hookfolder, 'post-receive') + postreceive = os.path.join(hookfolder, cls.hook_type) if not os.path.exists(postreceive): shutil.copyfile( - os.path.join(hook_files, 'post-receive'), + os.path.join(hook_files, cls.hook_type), postreceive) os.chmod(postreceive, 0755) From b1f2a1a759d4b727fd9ea09797f0f8e8f43c6953 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:01 +0000 Subject: [PATCH 2/10] Adjust the variable names in is_forced_push to fit with the documentation --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 9629774..ecf0f18 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -858,11 +858,13 @@ def get_revs_between(torev, fromrev, abspath, forced=False): return pagure.lib.git.read_git_lines(cmd, abspath) -def is_forced_push(torev, fromrev, abspath): - """ Returns wether there was a force push between HEAD and BASE. """ +def is_forced_push(oldrev, newrev, abspath): + """ Returns wether there was a force push between HEAD and BASE. + Doc: http://stackoverflow.com/a/12258773 + """ # Returns if there was any commits deleted in the changeset - cmd = ['rev-list', '%s' % torev, '^%s' % (fromrev)] + cmd = ['rev-list', '%s' % oldrev, '^%s' % newrev] out = pagure.lib.git.read_git_lines(cmd, abspath) return len(out) > 0 From 54f8c4b55835df17b563820de166970cfc7931ee Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:01 +0000 Subject: [PATCH 3/10] Add a generic pre-receive hook This hook will call all the pre-receive.foo hooks installed --- diff --git a/pagure/hooks/files/pre-receive b/pagure/hooks/files/pre-receive new file mode 100644 index 0000000..8a63e5c --- /dev/null +++ b/pagure/hooks/files/pre-receive @@ -0,0 +1,23 @@ +#!/bin/bash +# +# author: orefalo + +hookname=`basename $0` + + +FILE=`mktemp` +trap 'rm -f $FILE' EXIT +cat - > $FILE + +for hook in $GIT_DIR/hooks/$hookname.* +do + if test -x "$hook"; then + cat $FILE | $hook "$@" + status=$? + + if test $status -ne 0; then + echo Hook $hook failed with error code $status + exit $status + fi + fi +done From 200af3388a5c81aa8872b5acb441ecbee6e085c6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:01 +0000 Subject: [PATCH 4/10] Add a pre-receive plugin allowing to block non fast-forward pushes on a per branch basis --- diff --git a/pagure/hooks/files/pagure_force_commit_hook.py b/pagure/hooks/files/pagure_force_commit_hook.py new file mode 100755 index 0000000..a1bacd4 --- /dev/null +++ b/pagure/hooks/files/pagure_force_commit_hook.py @@ -0,0 +1,82 @@ +#! /usr/bin/env python2 + + +"""Pagure specific hook to add comment on issues if the commits fixes or +relates to an issue. +""" + +import os +import sys + +from sqlalchemy.exc import SQLAlchemyError + + +if 'PAGURE_CONFIG' not in os.environ \ + and os.path.exists('/etc/pagure/pagure.cfg'): + os.environ['PAGURE_CONFIG'] = '/etc/pagure/pagure.cfg' + + +import pagure +import pagure.exceptions +import pagure.lib.link +import pagure.ui.plugins + + +abspath = os.path.abspath(os.environ['GIT_DIR']) + + +def run_as_pre_receive_hook(): + reponame = pagure.lib.git.get_repo_name(abspath) + username = pagure.lib.git.get_username(abspath) + if pagure.APP.config.get('HOOK_DEBUG', False): + print 'repo:', reponame + print 'user:', username + + repo = pagure.lib.get_project(pagure.SESSION, reponame, user=username) + if not repo: + print 'Unknown repo %s of username: %s' % (reponame, username) + sys.exit(1) + + plugin = pagure.ui.plugins.get_plugin('Block non fast-forward pushes') + dbobj = plugin.db_object() + # Get the list of branches + branches = [ + branch.strip() + for branch in repo.pagure_force_commit_hook[0].branches.split(',') + if repo.pagure_force_commit_hook] + + # Remove empty branches + branches = [ + branch.strip() + for branch in branches + if branch] + + for line in sys.stdin: + if pagure.APP.config.get('HOOK_DEBUG', False): + print line + (oldrev, newrev, refname) = line.strip().split(' ', 2) + + refname = refname.replace('refs/heads/', '') + if refname in branches: + if pagure.APP.config.get('HOOK_DEBUG', False): + print ' -- Old rev' + print oldrev + print ' -- New rev' + print newrev + print ' -- Ref name' + print refname + + if set(newrev) == set(['0']): + print "Deletion is forbidden" + sys.exit(1) + elif pagure.lib.git.is_forced_push(oldrev, newrev, abspath): + print "Non fast-forward push are forbidden" + sys.exit(1) + + +def main(args): + run_as_pre_receive_hook() + + +if __name__ == '__main__': + main(sys.argv[1:]) diff --git a/pagure/hooks/pagure_force_commit.py b/pagure/hooks/pagure_force_commit.py new file mode 100644 index 0000000..09b8930 --- /dev/null +++ b/pagure/hooks/pagure_force_commit.py @@ -0,0 +1,128 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2016 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +import os + +import sqlalchemy as sa +import pygit2 +import wtforms +from flask.ext import wtf +from sqlalchemy.orm import relation +from sqlalchemy.orm import backref + +from pagure.hooks import BaseHook, RequiredIf +from pagure.lib.model import BASE, Project +from pagure import APP, get_repo_path + + +class PagureForceCommitTable(BASE): + """ Stores information about the pagure hook deployed on a project. + + Table -- hook_pagure_force_commit + """ + + __tablename__ = 'hook_pagure_force_commit' + + id = sa.Column(sa.Integer, primary_key=True) + project_id = sa.Column( + sa.Integer, + sa.ForeignKey('projects.id', onupdate='CASCADE'), + nullable=False, + unique=True, + index=True) + + branches = sa.Column(sa.Text, nullable=False) + + active = sa.Column(sa.Boolean, nullable=False, default=False) + + project = relation( + 'Project', foreign_keys=[project_id], remote_side=[Project.id], + backref=backref( + 'pagure_force_commit_hook', cascade="delete, delete-orphan", + single_parent=True) + ) + + +class PagureForceCommitForm(wtf.Form): + ''' Form to configure the pagure hook. ''' + branches = wtforms.TextField( + 'Branches', + [RequiredIf('active')] + ) + + active = wtforms.BooleanField( + 'Active', + [wtforms.validators.Optional()] + ) + + +class PagureForceCommitHook(BaseHook): + ''' PagurPagureForceCommit hook. ''' + + name = 'Block non fast-forward pushes' + description = 'Using this hook you can block any non-fast-forward '\ + 'commit forced pushed to one or more branches' + form = PagureForceCommitForm + db_object = PagureForceCommitTable + backref = 'pagure_force_commit_hook' + form_fields = ['branches', 'active'] + hook_type = 'pre-receive' + + @classmethod + def install(cls, project, dbobj): + ''' Method called to install the hook for a project. + + :arg project: a ``pagure.model.Project`` object to which the hook + should be installed + + ''' + repopaths = [get_repo_path(project)] + for folder in [ + APP.config.get('DOCS_FOLDER'), + APP.config.get('REQUESTS_FOLDER')]: + repopaths.append( + os.path.join(folder, project.path) + ) + + hook_files = os.path.join( + os.path.dirname(os.path.realpath(__file__)), 'files') + hook_file = os.path.join(hook_files, 'pagure_force_commit_hook.py') + + for repopath in repopaths: + # Init the git repo in case + pygit2.Repository(repopath) + + # Install the hook itself + hook_path = os.path.join( + repopath, 'hooks', 'pre-receive.pagureforcecommit') + if not os.path.exists(hook_path): + os.symlink(hook_file, hook_path) + + @classmethod + def remove(cls, project): + ''' Method called to remove the hook of a project. + + :arg project: a ``pagure.model.Project`` object to which the hook + should be installed + + ''' + repopaths = [get_repo_path(project)] + for folder in [ + APP.config.get('DOCS_FOLDER'), + APP.config.get('REQUESTS_FOLDER')]: + repopaths.append( + os.path.join(folder, project.path) + ) + + for repopath in repopaths: + hook_path = os.path.join( + repopath, 'hooks', 'pre-receive.pagureforcecommit') + if os.path.exists(hook_path): + os.unlink(hook_path) From 27549ed16c93c6915c6b8489f58aabcff4d70014 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:01 +0000 Subject: [PATCH 5/10] Small code-style change --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 2288385..a680848 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1228,7 +1228,8 @@ def update_project_settings(session, repo, settings, user): update.append(key) if key == 'Minimum_score_to_merge_pull-request': try: - settings[key] = int(settings[key]) if settings[key] else -1 + settings[key] = int(settings[key]) \ + if settings[key] else -1 except ValueError: raise pagure.exceptions.PagureException( "Please enter a numeric value for the 'minimum " From 573708186c089f1a9f76c05286cd16592a5124b2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:02 +0000 Subject: [PATCH 6/10] The no-FF git hook is only applicable to the main git repo --- diff --git a/pagure/hooks/pagure_force_commit.py b/pagure/hooks/pagure_force_commit.py index 09b8930..4715653 100644 --- a/pagure/hooks/pagure_force_commit.py +++ b/pagure/hooks/pagure_force_commit.py @@ -83,27 +83,20 @@ class PagureForceCommitHook(BaseHook): should be installed ''' - repopaths = [get_repo_path(project)] - for folder in [ - APP.config.get('DOCS_FOLDER'), - APP.config.get('REQUESTS_FOLDER')]: - repopaths.append( - os.path.join(folder, project.path) - ) + repopath = get_repo_path(project) hook_files = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'files') hook_file = os.path.join(hook_files, 'pagure_force_commit_hook.py') - for repopath in repopaths: - # Init the git repo in case - pygit2.Repository(repopath) + # Init the git repo in case + pygit2.Repository(repopath) - # Install the hook itself - hook_path = os.path.join( - repopath, 'hooks', 'pre-receive.pagureforcecommit') - if not os.path.exists(hook_path): - os.symlink(hook_file, hook_path) + # Install the hook itself + hook_path = os.path.join( + repopath, 'hooks', 'pre-receive.pagureforcecommit') + if not os.path.exists(hook_path): + os.symlink(hook_file, hook_path) @classmethod def remove(cls, project): From b879091358a0f05a4056ae06cd75a6b3b5ed341b Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:02 +0000 Subject: [PATCH 7/10] Adjust unit-test for the new no-FF plugin --- diff --git a/tests/test_pagure_flask_ui_plugins.py b/tests/test_pagure_flask_ui_plugins.py index 17b89c4..2c57287 100644 --- a/tests/test_pagure_flask_ui_plugins.py +++ b/tests/test_pagure_flask_ui_plugins.py @@ -69,8 +69,8 @@ class PagureFlaskPluginstests(tests.Modeltests): names = pagure.ui.plugins.get_plugin_names() self.assertEqual( sorted(names), - ['Fedmsg', 'IRC', 'Mail', 'pagure', 'pagure requests', - 'pagure tickets']) + ['Block non fast-forward pushes', 'Fedmsg', 'IRC', 'Mail', + 'pagure', 'pagure requests', 'pagure tickets']) def test_get_plugin(self): """ Test the get_plugin function. """ From b6ab03c5a0a6a00570b6d25bb3c09aa691141356 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:02 +0000 Subject: [PATCH 8/10] Add unit-test for the new no-FF plugin --- diff --git a/tests/test_pagure_flask_ui_plugins_noff.py b/tests/test_pagure_flask_ui_plugins_noff.py new file mode 100644 index 0000000..039350f --- /dev/null +++ b/tests/test_pagure_flask_ui_plugins_noff.py @@ -0,0 +1,237 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2016 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import json +import unittest +import shutil +import sys +import os + +import pygit2 +from mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure.lib +import tests + + +class PagureFlaskPluginNoFFtests(tests.Modeltests): + """ Tests for Block non fast-forward pushes plugin of pagure """ + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskPluginNoFFtests, self).setUp() + + pagure.APP.config['TESTING'] = True + pagure.SESSION = self.session + pagure.ui.SESSION = self.session + pagure.ui.app.SESSION = self.session + pagure.ui.plugins.SESSION = self.session + pagure.ui.repo.SESSION = self.session + pagure.ui.filters.SESSION = self.session + + pagure.APP.config['GIT_FOLDER'] = tests.HERE + pagure.APP.config['FORK_FOLDER'] = os.path.join( + tests.HERE, 'forks') + pagure.APP.config['TICKETS_FOLDER'] = os.path.join( + tests.HERE, 'tickets') + pagure.APP.config['DOCS_FOLDER'] = os.path.join( + tests.HERE, 'docs') + self.app = pagure.APP.test_client() + + def test_plugin_noff(self): + """ Test the noff plugin on/off endpoint. """ + + tests.create_projects(self.session) + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + output = self.app.get( + '/test/settings/Block non fast-forward pushes') + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n' + 'test project #1
', output.data) + self.assertIn( + '

Block non fast-forward pushes settings

', + output.data) + self.assertIn( + '', output.data) + self.assertTrue( + '' + in output.data) + + csrf_token = output.data.split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + data = {} + + output = self.app.post( + '/test/settings/Block non fast-forward pushes', data=data) + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n' + 'test project #1
', output.data) + self.assertIn( + '

Block non fast-forward pushes settings

', + output.data) + self.assertIn( + '', output.data) + self.assertTrue( + '' + in output.data) + + data['csrf_token'] = csrf_token + # No git found + output = self.app.post( + '/test/settings/Block non fast-forward pushes', data=data) + self.assertEqual(output.status_code, 404) + + tests.create_projects_git(tests.HERE) + + # With the git repo + output = self.app.post( + '/test/settings/Block non fast-forward pushes', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n

Settings for test

', + output.data) + self.assertTrue( + '\n Hook Block non ' + 'fast-forward pushes inactived' in output.data) + + output = self.app.get( + '/test/settings/Block non fast-forward pushes') + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n' + 'test project #1
', output.data) + self.assertIn( + '

Block non fast-forward pushes settings

', + output.data) + self.assertIn( + '', output.data) + self.assertTrue( + '' + in output.data) + + self.assertFalse(os.path.exists(os.path.join( + tests.HERE, 'test.git', 'hooks', 'post-receive.mail'))) + + # Missing the required mail_to + data = {'csrf_token': csrf_token, 'active': 'y'} + + output = self.app.post( + '/test/settings/Block non fast-forward pushes', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n' + 'test project #1
', output.data) + self.assertIn( + '

Block non fast-forward pushes settings

', + output.data) + self.assertNotIn( + '\n Hook activated', + output.data) + self.assertIn( + '', output.data) + self.assertTrue( + '' in output.data) + + self.assertFalse(os.path.exists(os.path.join( + tests.HERE, 'test.git', 'hooks', + 'pre-receive.pagureforcecommit'))) + + # Activate hook + data = { + 'csrf_token': csrf_token, + 'active': 'y', + 'branches': 'master', + } + + output = self.app.post( + '/test/settings/Block non fast-forward pushes', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '

Settings for test

', + output.data) + self.assertIn( + '\n Hook Block non ' + 'fast-forward pushes activated', output.data) + + output = self.app.get( + '/test/settings/Block non fast-forward pushes') + self.assertIn( + '
\n' + 'test project #1
', output.data) + self.assertIn( + '

Block non fast-forward pushes settings

', + output.data) + self.assertIn( + '', output.data) + self.assertIn( + '', output.data) + + self.assertTrue(os.path.exists(os.path.join( + tests.HERE, 'test.git', 'hooks', + 'pre-receive.pagureforcecommit'))) + + # De-Activate hook + data = {'csrf_token': csrf_token} + output = self.app.post( + '/test/settings/Block non fast-forward pushes', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n

Settings for test

', + output.data) + self.assertIn( + '\n Hook Block non ' + 'fast-forward pushes inactived', output.data) + + output = self.app.get( + '/test/settings/Block non fast-forward pushes') + self.assertIn( + '
\n' + 'test project #1
', output.data) + self.assertIn( + '

Block non fast-forward pushes settings

', + output.data) + self.assertIn( + '', output.data) + self.assertIn( + '', output.data) + + self.assertFalse(os.path.exists(os.path.join( + tests.HERE, 'test.git', 'hooks', + 'pre-receive.pagureforcecommit'))) + + +if __name__ == '__main__': + SUITE = unittest.TestLoader().loadTestsFromTestCase( + PagureFlaskPluginNoFFtests) + unittest.TextTestRunner(verbosity=2).run(SUITE) From d6e5e62df06a3258fb5485f62415facf81a5f455 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:02 +0000 Subject: [PATCH 9/10] Simplify removing the non-ff plugin, it's only present in the main repo --- diff --git a/pagure/hooks/pagure_force_commit.py b/pagure/hooks/pagure_force_commit.py index 4715653..0d23f2e 100644 --- a/pagure/hooks/pagure_force_commit.py +++ b/pagure/hooks/pagure_force_commit.py @@ -106,16 +106,8 @@ class PagureForceCommitHook(BaseHook): should be installed ''' - repopaths = [get_repo_path(project)] - for folder in [ - APP.config.get('DOCS_FOLDER'), - APP.config.get('REQUESTS_FOLDER')]: - repopaths.append( - os.path.join(folder, project.path) - ) - - for repopath in repopaths: - hook_path = os.path.join( - repopath, 'hooks', 'pre-receive.pagureforcecommit') - if os.path.exists(hook_path): - os.unlink(hook_path) + repopaths = get_repo_path(project) + hook_path = os.path.join( + repopath, 'hooks', 'pre-receive.pagureforcecommit') + if os.path.exists(hook_path): + os.unlink(hook_path) From 52e6fb8915da60281b5d72ac936837f57a14e2c3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 16 2016 09:00:02 +0000 Subject: [PATCH 10/10] Fix typo in the variable name, thanks unit-tests! --- diff --git a/pagure/hooks/pagure_force_commit.py b/pagure/hooks/pagure_force_commit.py index 0d23f2e..3f6725c 100644 --- a/pagure/hooks/pagure_force_commit.py +++ b/pagure/hooks/pagure_force_commit.py @@ -106,7 +106,7 @@ class PagureForceCommitHook(BaseHook): should be installed ''' - repopaths = get_repo_path(project) + repopath = get_repo_path(project) hook_path = os.path.join( repopath, 'hooks', 'pre-receive.pagureforcecommit') if os.path.exists(hook_path):