# HG changeset patch # User Georges Racinet <georges.racinet@octobus.net> # Date 1583858952 -3600 # Tue Mar 10 17:49:12 2020 +0100 # Node ID a9f1a24b967984864cd4331fc909aef005236e56 # Parent cac20dfe000d1c5af3221a9e83b3db8c7adf3150 GitLab hooks: post-receive directly through internal REST API This implements the `post-receive` hook by a direct HTTP call to the internal API endpoint. For this, two new config parameters in the `heptapod` section are introduced: - gitlab-internal-api-secret-file - gitlab-internal-api-url We're keeping the previous structure, with a `name` that's now almost useless and leads us to the complication of the `_hook_methods` dict. This keeps compatibility for the caller (also part of this package since Heptapod 0.12), allowing us to refactor in a separate step. diff --git a/heptapod/gitlab/__init__.py b/heptapod/gitlab/__init__.py --- a/heptapod/gitlab/__init__.py +++ b/heptapod/gitlab/__init__.py @@ -4,8 +4,12 @@ # GNU General Public License version 2 or any later version. # # SPDX-License-Identifier: GPL-2.0-or-later +import logging import os -import subprocess +import requests +import requests_unixsocket + +logger = logging.getLogger(__name__) def git_hook_format_changes(changes): @@ -20,6 +24,74 @@ for ref, (old_gitsha, new_gitsha) in changes.items()) +def parse_broadcast_msg(msg, text_length): + """Cut into several lines of at most text_length length """ + if len(msg) <= text_length: + return [msg] + + lines = [] + current = [] + current_len = 0 + for word in msg.split(): + if current_len + len(word) < text_length: + current.append(word) + current_len += len(word) + 1 + else: + lines.append(' '.join(current)) + current = [word] + current_len = 0 + + if current: + lines.append(' '.join(current)) + return lines + + +def format_alert_message(message): + """Directly translated from gitlab_post_receive.rb in gitlab-shell.""" + # A standard terminal window is (at least) 80 characters wide. + total_width = 80 + + # Git prefixes remote messages with "remote: ", so this width is subtracted + # from the width available to us. TODO recheck for Mercurial + total_width -= len("remote: ") + + # Our centered text shouldn't start or end right at the edge of the window, + # so we add some horizontal padding: 2 chars on either side. + text_width = total_width - 2 * 2 + + # Automatically wrap message at text_width (= 68) characters: + # Splits the message up into the longest possible chunks matching + # "<between 0 and text_width characters><space or end-of-line>". + lines = ["", "=" * total_width, ""] + for msg_line in parse_broadcast_msg(message, text_width): + padding = max((total_width - len(msg_line)) / 2, 0) + lines.append(" " * padding + msg_line) + + lines.extend(("", "=" * total_width, "")) + + return lines + + +def format_warnings(warnings): + return format_alert_message("\n".join(("WARNINGS", warnings))) + + +def format_mr_links(mrs): + if not mrs: + return () + + out_lines = [""] + for mr in mrs: + if mr.get("new_merge_request"): + fmt = "To create a merge request for {branch_name}, visit:" + else: + fmt = "View merge request for {branch_name}:" + fmt += "\n {url}" + out_lines.append(fmt.format(**mr)) + print(repr(out_lines)) + return out_lines + + SKIP = object() @@ -34,15 +106,33 @@ at the right stage of Mercurial transaction end. """ + _hook_methods = { + 'post-receive': 'post_receive', + } + def __init__(self, name, repo): self.name = name self.repo = repo self.git_fs_path = self.repo.root[:-3] + '.git' + url = str(repo.ui.config(b'heptapod', b'gitlab-internal-api-url', + 'http://localhost:8080')) + self.gitlab_internal_api_endpoint = url + '/api/v4/internal' + + self.gitlab_url = url shell = repo.ui.config(b'heptapod', b'gitlab-shell') if not shell: raise RuntimeError("Path to GitLab Shell is unknown") self.gitlab_shell = shell + secret = repo.ui.config(b'heptapod', + b'gitlab-internal-api-secret-file') + + if not secret: + raise RuntimeError("heptapod.gitlab-internal-api-secret-file " + "config parameter is missing.") + with open(secret, 'r') as secretf: + self.gitlab_secret = secretf.read().strip() + def __str__(self): return "GitLab %r hook wrapper" % self.name @@ -72,6 +162,52 @@ env['GL_REPOSITORY'] = 'project-' + project_id return env + def gitlab_internal_api_request( + self, meth, + subpath, params, **kwargs): # pragma: no cover + params['secret_token'] = self.gitlab_secret + with requests_unixsocket.monkeypatch(): + return requests.request( + meth, + '/'.join((self.gitlab_internal_api_endpoint, + subpath)), + data=params, + **kwargs) + + def post_receive_params(self, changes, env=None): + if env is None: + env = self.environ() + return dict(gl_repository=env['GL_REPOSITORY'], + identifier=env['GL_ID'], + changes=git_hook_format_changes(changes)) + + def post_receive(self, changes, env=None): + """Call GitLab internal API post-receive endpoint.""" + params = self.post_receive_params(changes, env=env) + resp = self.gitlab_internal_api_request('POST', + subpath='post_receive', + params=params) + ok = resp.status_code == 200 + as_json = resp.json() + logger.debug("post_receive for changes %r, response: %r", + changes, as_json) + return ok, self.post_receive_handle_response(as_json), '' + + def post_receive_handle_response(self, as_json): + out_lines = [] + broadcast_message = as_json.get('broadcast_message') + if broadcast_message is not None: + out_lines.extend(format_alert_message(broadcast_message)) + out_lines.extend(format_mr_links(as_json.get('merge_request_urls'))) + for msg_name in ('redirected_message', 'project_created_message', ): + msg = as_json.get(msg_name) + if msg is not None: + out_lines.append(msg) + warnings = as_json.get('warnings') + if warnings is not None: + out_lines.extend(format_warnings(warnings)) + return '\n'.join(out_lines) + def __call__(self, changes): """Call GitLab Hook for the given changes. @@ -95,13 +231,9 @@ self.repo.ui.note('%s: bailing (explicitely told not to send)') return 0, '', '' - self.repo.ui.note("%s: sending changes %r" % (self, changes)) - pr = subprocess.Popen( - [os.path.join(self.gitlab_shell, 'hooks', self.name)], - env=env, - cwd=self.git_fs_path, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=subprocess.PIPE) - out, err = pr.communicate(input=git_hook_format_changes(changes)) - return pr.returncode, out, err + meth_name = self._hook_methods.get(self.name) + if meth_name is None: + raise NotImplementedError(self.name) + + ok, out, err = getattr(self, meth_name)(changes, env=env) + return 0 if ok else 1, out, err diff --git a/heptapod/gitlab/tests/test_hooks.py b/heptapod/gitlab/tests/test_hooks.py --- a/heptapod/gitlab/tests/test_hooks.py +++ b/heptapod/gitlab/tests/test_hooks.py @@ -4,12 +4,20 @@ # GNU General Public License version 2 or any later version. # # SPDX-License-Identifier: GPL-2.0-or-later +import io +import json import pytest +import requests + from heptapod.testhelpers import ( LocalRepoWrapper, ) from .. import ( + format_alert_message, + format_warnings, + format_mr_links, git_hook_format_changes, + parse_broadcast_msg, Hook, ) @@ -26,11 +34,93 @@ } +def test_parse_broadcast_msg(): + msg = "Short message" + assert parse_broadcast_msg(msg, 80) == [msg] + + msg = "A somewhat longer message, cut here" + assert parse_broadcast_msg(msg, 30) == ["A somewhat longer message,", + "cut here"] + + +def test_format_alert_message(): + # here the point of the test is mostly to make sure that we don't + # get stupid errors, we don't care that much of the exact amount of + # padding or leading and trailing separators + msg = "Short message" + assert (format_alert_message(msg)[3] + == + ' Short message' + ) + msg = ' '.join('word%d' % i for i in range(20)) + formatted = format_alert_message(msg) + assert (formatted[3] + == + (' word0 word1 word2 word3 word4 word5 word6 word7 word8 ' + 'word9 word10') + ) + assert (formatted[4] + == + (' word11 word12 word13 word14 word15 ' + 'word16 word17 word18 word19') + ) + + +def test_format_warnings(): + """Test format_warnings for non breakage, not for precise content wording. + """ + assert 'WARNINGS' in format_warnings('Short warning')[3] + + +def fmt_mr_links(mrs): + """Call and convert return into a tuple. + + The goal is that assertions don't depend onto the precise return type. + """ + return tuple(format_mr_links(mrs)) + + +def test_format_mr_links(): + """Test format_mr_links for non breakage, not for precise content wording. + """ + assert fmt_mr_links(None) == () + assert fmt_mr_links([]) == () + assert fmt_mr_links(()) == () + + seen_create = False + seen_view = False + + for line in fmt_mr_links( + [dict(new_merge_request=True, + branch_name='heptapod-next', + url="https://heptapod.example?query=something"), + dict(new_merge_request=False, + branch_name='heptapod-stable', + url="https://heptapod.example?query=other"), + ]): + line = line.lower() + if 'view merge request' in line: + seen_view = True + assert "heptapod-stable" in line + # query string passed through + assert '?query=other' in line + elif 'create a merge request' in line: + seen_create = True + assert "heptapod-next" in line + assert '?query=something' in line + assert seen_view, "Did not get message to view existing MR" + assert seen_create, "Did not get message to create new MR" + + def make_repo(base_dir, name): + secret = base_dir.join('secret') + secret.write("TEHTOKEN") wrapper = LocalRepoWrapper.init( base_dir.join(name), - config=dict(heptapod={'gitlab-shell': str(base_dir)}), - ) + config=dict(heptapod={'gitlab-shell': str(base_dir), + 'gitlab-internal-api-secret-file': str(secret), + }), + ) # by default, ui.environ IS os.environ (shared) but that isn't true # in WSGI contexts. In that case, it is a plain dict, the WSGI environment # indeed. For these tests, we need them to be independent. @@ -38,62 +128,179 @@ return wrapper -def test_hook(tmpdir): +def request_recorder(records, responses): + def request(hook, meth, subpath, params, **kwargs): + records.append((meth, subpath, params, kwargs)) + return responses.pop(0) + + return request + + +def test_post_receive_params(tmpdir): + wrapper = make_repo(tmpdir, 'proj.hg') + repo = wrapper.repo + hook = Hook('post-receive', repo) + + repo.ui.environ.update( + HEPTAPOD_USERINFO_ID='23', + HEPTAPOD_PROJECT_ID='1024', + ) + changes = { + 'branch/newbr': (ZERO_SHA, "beef1234dead" + "a" * 8), + 'topic/default/zz': ("23cafe45" + "0" * 12, "1234" + "b" * 16), + } + + params = hook.post_receive_params(changes) + assert params['identifier'] == 'user-23' + assert params['changes'].splitlines() == [ + '23cafe45000000000000 1234bbbbbbbbbbbbbbbb topic/default/zz', + '00000000000000000000 beef1234deadaaaaaaaa branch/newbr'] + assert params['gl_repository'] == 'project-1024' + + +def test_post_receive_handle_response(tmpdir): wrapper = make_repo(tmpdir, 'proj.hg') repo = wrapper.repo - hook = Hook('post-something', repo) + hook = Hook('post-receive', repo) + + out = hook.post_receive_handle_response( + # example taken from the functional tests + {u'broadcast_message': None, + u'reference_counter_decreased': True, + u'merge_request_urls': [ + {u'url': u'http://localhost:3000/root/' + 'test_project_1583330760_9216309/merge_requests/1', + u'branch_name': u'topic/default/antelope', + u'new_merge_request': False} + ]}) + out = [l.strip() for l in out.splitlines() if l.strip()] + assert out == [ + 'View merge request for topic/default/antelope:', + 'http://localhost:3000/root/test_project_1583330760_9216309/' + 'merge_requests/1'] + + broadcast = "Check out the new tutorial" + out = hook.post_receive_handle_response( + {u'broadcast_message': broadcast, + u'reference_counter_decreased': True, + }) + out = (l.strip() for l in out.splitlines() if l.strip()) + assert any(broadcast in l for l in out) + + project_created = "Congrats!" + out = hook.post_receive_handle_response( + {u'project_created_message': project_created, + u'reference_counter_decreased': True, + }) + out = (l.strip() for l in out.splitlines() if l.strip()) + assert any(project_created in l for l in out) + + # GitLab internal API gives warnings as a unique string + # see lib/api/helpers/internal_helpers.rb + warnings = "You probably shouldn't have" + out = hook.post_receive_handle_response( + {u'warnings': warnings, + u'reference_counter_decreased': True, + }) + out = (l.strip() for l in out.splitlines() if l.strip()) + assert any(warnings in l for l in out) + + +def make_response(status_code, body): + """Body is bytes""" + resp = requests.models.Response() + resp.status_code = status_code + resp.raw = io.BytesIO(body) + return resp + + +def make_json_response(obj, status_code=200): + return make_response(status_code, json.dumps(obj).encode()) + + +def test_integration_post_receive(tmpdir, monkeypatch): + records = [] + resp = make_json_response( + {u'broadcast_message': None, + u'reference_counter_decreased': True, + u'merge_request_urls': [ + {u'url': u'http://localhost:3000/root/' + 'test_project_1583330760_9216309/merge_requests/1', + u'branch_name': u'topic/default/antelope', + u'new_merge_request': False} + ]}) + responses = [resp] + monkeypatch.setattr(Hook, 'gitlab_internal_api_request', + request_recorder(records, responses)) + + repo_wrapper = make_repo(tmpdir, 'repo.hg') + repo = repo_wrapper.repo repo.ui.environ.update( HEPTAPOD_USERINFO_ID='23', HEPTAPOD_PROJECT_ID='1024', ) - assert str(hook) == "GitLab 'post-something' hook wrapper" - env = hook.environ() - - assert env.get('HG_GIT_SYNC') == 'yes' - assert env.get('GL_ID') == 'user-23' - assert env.get('GL_REPOSITORY') == 'project-1024' + hook = Hook('post-receive', repo_wrapper.repo) + changes = { + 'branch/newbr': (ZERO_SHA, "beef1234dead" + "a" * 8), + 'topic/default/zz': ("23cafe45" + "0" * 12, "1234" + "b" * 16), + } - # we didn't pollute original environment - assert 'GL_ID' not in repo.ui.environ + return_code, out, err = hook(changes) + assert return_code == 0 + out = [l.strip() for l in out.splitlines() if l.strip()] + assert out == [ + 'View merge request for topic/default/antelope:', + 'http://localhost:3000/root/test_project_1583330760_9216309/' + 'merge_requests/1'] - # calling with empty changes - assert hook(()) == (0, '', '') - - # now with an executable - # we need a pseudo Git repo - tmpdir.join('proj.git').ensure(dir=True) - hook_bin = tmpdir.ensure('hooks', dir=True).join('post-something') - hook_bin.write('\n'.join(( - "#!/usr/bin/env python", - "import sys", - "sys.stderr.write('stderr of the post something hook')", - "for line in sys.stdin.readlines():", - " sys.stdout.write(line)", - "sys.exit(3)", - ))) - hook_bin.chmod(0o700) - changes = {'branch/newbr': (ZERO_SHA, "beef1234dead" + "a" * 8), - 'topic/default/zz': ("23cafe45" + "0" * 12, "1234" + "b" * 16), - } +def test_empty_changes(tmpdir): + hook = Hook('post-receive', make_repo(tmpdir, 'repo.hg').repo) + code, out, err = hook(()) + assert code == 0 + assert not out + assert not err + + +def test_unknown_hook(tmpdir): + # that's a typo, the most likely way for it to happen + name = 'post-recieve' - code, out, err = hook(changes) - assert code, err == (3, 'stderr of the post something hoko') - assert set(out.splitlines()) == { - '00000000000000000000 beef1234deadaaaaaaaa branch/newbr', - '23cafe45000000000000 1234bbbbbbbbbbbbbbbb topic/default/zz' + repo = make_repo(tmpdir, 'repo.hg').repo + hook = Hook(name, repo) + changes = { + 'branch/newbr': (ZERO_SHA, "beef1234dead" + "a" * 8), + 'topic/default/zz': ("23cafe45" + "0" * 12, "1234" + "b" * 16), } + repo.ui.environ.update( + HEPTAPOD_USERINFO_ID='23', + HEPTAPOD_PROJECT_ID='1024', + ) + with pytest.raises(NotImplementedError) as exc_info: + hook(changes) + assert exc_info.value.args[0] == name + def test_missing_gitlab_shell(tmpdir): repo = LocalRepoWrapper.init(tmpdir).repo + repo.ui.setconfig(b'heptapod', b'gitlab-internal-api-secret-file', + b'/some/path') with pytest.raises(RuntimeError) as exc_info: - Hook('post-something', repo) + Hook('post-receive', repo) assert "Path to GitLab Shell" in exc_info.value.args[0] +def test_missing_secret(tmpdir): + repo = LocalRepoWrapper.init(tmpdir).repo + repo.ui.setconfig(b'heptapod', b'gitlab-shell', b'/some/path') + with pytest.raises(RuntimeError) as exc_info: + Hook('post-receive', repo) + assert "secret-file" in exc_info.value.args[0] + + def test_hook_missing_user(tmpdir): wrapper = make_repo(tmpdir, 'proj2.hg') repo = wrapper.repo diff --git a/install-requirements.txt b/install-requirements.txt --- a/install-requirements.txt +++ b/install-requirements.txt @@ -1,3 +1,5 @@ Mercurial>=5.2 hg-evolve>=9.2.1 hg-git>=0.8.13 +requests +requests_unixsocket==0.2.0