launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21143
[Merge] ~cjwatson/turnip:set-symbolic-ref into turnip:master
Colin Watson has proposed merging ~cjwatson/turnip:set-symbolic-ref into turnip:master.
Commit message:
Add turnip-set-symbolic-ref extension service
This makes it possible to set a repository's HEAD by talking directly to
turnip over HTTPS, which is needed for git-to-git code imports.
LP: #1469459
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1469459 in turnip: "import external code into a LP git repo (natively)"
https://bugs.launchpad.net/turnip/+bug/1469459
For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/309702
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:set-symbolic-ref into turnip:master.
diff --git a/README b/README
index 7b26510..e274c12 100644
--- a/README
+++ b/README
@@ -68,6 +68,21 @@ The only additional parameters implemented today are
'turnip-stateless-rpc' and 'turnip-advertise-refs', which are used by
the smart HTTP server to proxy to the standard pack protocol.
+turnip implements one externally-visible extension: a
+'turnip-set-symbolic-ref' service that sets a symbolic ref (currently only
+'HEAD' is permitted) to a given target. This may be used over the various
+protocols (git, SSH, smart HTTP), requesting the service in the same way as
+the existing 'git-upload-pack' and 'git-receive-pack' services.
+
+ turnip-set-symbolic-ref-request = set-symbolic-ref-line
+ flush-pkt
+ set-symbolic-ref-line = PKT-LINE(refname SP refname)
+
+The server replies with an ACK indicating the symbolic ref name that was
+changed, or an error message.
+
+ turnip-set-symbolic-ref-response = set-symbolic-ref-ack / error-line
+ set-symbolic-ref-ack = PKT-LINE("ACK" SP refname)
Development
===========
diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 04d6770..fc01527 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -272,7 +272,7 @@ class GitProcessProtocol(protocol.ProcessProtocol):
self.transport.closeChildFD(0)
def processEnded(self, status):
- self.peer.transport.loseConnection()
+ self.peer.processEnded(status)
def pauseProducing(self):
self.transport.pauseProducing()
@@ -386,43 +386,114 @@ class PackBackendProtocol(PackServerProtocol):
"""
hookrpc_key = None
+ expect_set_symbolic_ref = False
def requestReceived(self, command, raw_pathname, params):
self.extractRequestMeta(command, raw_pathname, params)
+ self.command = command
+ self.raw_pathname = raw_pathname
+ self.path = compose_path(self.factory.root, self.raw_pathname)
+
+ if command == b'turnip-set-symbolic-ref':
+ self.expect_set_symbolic_ref = True
+ self.resumeProducing()
+ return
- path = compose_path(self.factory.root, raw_pathname)
+ write_operation = False
if command == b'git-upload-pack':
subcmd = b'upload-pack'
elif command == b'git-receive-pack':
subcmd = b'receive-pack'
+ write_operation = True
else:
self.die(b'Unsupported command in request')
return
- cmd = b'git'
- args = [b'git', subcmd]
+ args = []
if params.pop(b'turnip-stateless-rpc', None):
args.append(b'--stateless-rpc')
if params.pop(b'turnip-advertise-refs', None):
args.append(b'--advertise-refs')
- args.append(path)
+ args.append(self.path)
+ self.spawnGit(subcmd, args, write_operation=write_operation)
+
+ def spawnGit(self, subcmd, extra_args, write_operation=False,
+ send_path_as_option=False):
+ cmd = b'git'
+ args = [b'git']
+ if send_path_as_option:
+ args.extend([b'-C', self.path])
+ args.append(subcmd)
+ args.extend(extra_args)
env = {}
- if subcmd == b'receive-pack' and self.factory.hookrpc_handler:
+ if write_operation and self.factory.hookrpc_handler:
# This is a write operation, so prepare config, hooks, the hook
# RPC server, and the environment variables that link them up.
- ensure_config(path)
+ ensure_config(self.path)
self.hookrpc_key = str(uuid.uuid4())
self.factory.hookrpc_handler.registerKey(
- self.hookrpc_key, raw_pathname, [])
- ensure_hooks(path)
+ self.hookrpc_key, self.raw_pathname, [])
+ ensure_hooks(self.path)
env[b'TURNIP_HOOK_RPC_SOCK'] = self.factory.hookrpc_sock
env[b'TURNIP_HOOK_RPC_KEY'] = self.hookrpc_key
self.log.info('Spawning {args}', args=args)
self.peer = GitProcessProtocol(self)
+ self.spawnProcess(cmd, args, env=env)
+
+ def spawnProcess(self, cmd, args, env=None):
reactor.spawnProcess(self.peer, cmd, args, env=env)
+ def packetReceived(self, data):
+ if self.expect_set_symbolic_ref:
+ if data is None:
+ self.die(b'Bad request: flush-pkt instead')
+ return
+ self.pauseProducing()
+ self.expect_set_symbolic_ref = False
+ if b' ' not in data:
+ self.die(b'Invalid set-symbolic-ref-line')
+ return
+ name, target = data.split(b' ', 1)
+ # Be careful about extending this to anything other than HEAD.
+ # We use "git symbolic-ref" because it gives us locking and
+ # logging, but it doesn't prevent writing a ref to ../something.
+ # Fortunately it does at least refuse to point HEAD outside of
+ # refs/.
+ if name != b'HEAD':
+ self.die(b'Symbolic ref name must be "HEAD"')
+ return
+ if target.startswith(b'-'):
+ self.die(b'Symbolic ref target may not start with "-"')
+ return
+ self.symbolic_ref_name = name
+ self.spawnGit(
+ b'symbolic-ref', [name, target], write_operation=True,
+ send_path_as_option=True)
+ return
+
+ PackServerProtocol.packetReceived(self, data)
+
+ @defer.inlineCallbacks
+ def processEnded(self, reason):
+ message = None
+ if self.command == b'turnip-set-symbolic-ref':
+ if reason.check(error.ProcessDone):
+ try:
+ yield self.factory.hookrpc_handler.notify(self.path)
+ self.sendPacket(b'ACK %s' % self.symbolic_ref_name)
+ except Exception as e:
+ message = str(e)
+ else:
+ message = (
+ 'git symbolic-ref exited with status %d' %
+ reason.value.exitCode)
+ if message is None:
+ self.transport.loseConnection()
+ else:
+ self.die(message)
+
def readConnectionLost(self):
# Forward the closed stdin down the stack.
if self.peer is not None:
diff --git a/turnip/pack/hookrpc.py b/turnip/pack/hookrpc.py
index d273a9c..bd3bc0e 100644
--- a/turnip/pack/hookrpc.py
+++ b/turnip/pack/hookrpc.py
@@ -126,11 +126,15 @@ class HookRPCHandler(object):
return [rule.decode('utf-8') for rule in self.ref_rules[args['key']]]
@defer.inlineCallbacks
+ def notify(self, path):
+ proxy = xmlrpc.Proxy(self.virtinfo_url, allowNone=True)
+ yield proxy.callRemote(b'notify', path)
+
+ @defer.inlineCallbacks
def notifyPush(self, proto, args):
"""Notify the virtinfo service about a push."""
path = self.ref_paths[args['key']]
- proxy = xmlrpc.Proxy(self.virtinfo_url, allowNone=True)
- yield proxy.callRemote(b'notify', path)
+ yield self.notify(path)
class HookRPCServerFactory(RPCServerFactory):
diff --git a/turnip/pack/http.py b/turnip/pack/http.py
index ada6b5f..9c209d2 100644
--- a/turnip/pack/http.py
+++ b/turnip/pack/http.py
@@ -664,7 +664,8 @@ class HTTPAuthResource(resource.Resource):
class SmartHTTPFrontendResource(resource.Resource):
"""HTTP resource to translate Git smart HTTP requests to pack protocol."""
- allowed_services = frozenset((b'git-upload-pack', b'git-receive-pack'))
+ allowed_services = frozenset((
+ b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref'))
def __init__(self, backend_host, config):
resource.Resource.__init__(self)
@@ -721,7 +722,9 @@ class SmartHTTPFrontendResource(resource.Resource):
content_type = request.getHeader(b'Content-Type')
if content_type is None:
return False
- return content_type.startswith(b'application/x-git-')
+ return (
+ content_type.startswith(b'application/x-git-') or
+ content_type.startswith(b'application/x-turnip-'))
def getChild(self, path, request):
if self._isGitRequest(request):
diff --git a/turnip/pack/ssh.py b/turnip/pack/ssh.py
index 006331d..b753220 100644
--- a/turnip/pack/ssh.py
+++ b/turnip/pack/ssh.py
@@ -120,7 +120,8 @@ class SSHPackClientFactory(protocol.ClientFactory):
class SmartSSHSession(DoNothingSession):
"""SSH session allowing only Git smart SSH requests."""
- allowed_services = frozenset((b'git-upload-pack', b'git-receive-pack'))
+ allowed_services = frozenset((
+ b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref'))
def __init__(self, *args, **kwargs):
super(SmartSSHSession, self).__init__(*args, **kwargs)
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index e30b59f..374213c 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -8,8 +8,10 @@ from __future__ import (
unicode_literals,
)
+import base64
from collections import defaultdict
import hashlib
+import io
import os
import shutil
import stat
@@ -45,10 +47,12 @@ from twisted.internet import (
from twisted.web import (
client,
error,
+ http_headers,
server,
xmlrpc,
)
+from turnip.pack import helpers
from turnip.pack.git import (
PackBackendFactory,
PackFrontendFactory,
@@ -441,14 +445,68 @@ class TestSmartHTTPFrontendFunctional(FrontendFunctionalTestMixin, TestCase):
@defer.inlineCallbacks
def test_root_revision_header(self):
- factory = client.HTTPClientFactory(
- b'http://localhost:%d/' % self.port, method=b'HEAD',
- followRedirect=False)
- reactor.connectTCP(b'localhost', self.port, factory)
- yield assert_fails_with(factory.deferred, error.PageRedirect)
+ response = yield client.Agent(reactor).request(
+ b'HEAD', b'http://localhost:%d/' % self.port)
+ self.assertEqual(302, response.code)
self.assertEqual(
[version_info['revision_id']],
- factory.response_headers[b'x-turnip-revision'])
+ response.headers.getRawHeaders(b'X-Turnip-Revision'))
+
+ def make_set_symbolic_ref_request(self, line):
+ parsed_url = urlsplit(self.url)
+ url = urlunsplit([
+ parsed_url.scheme,
+ b'%s:%d' % (parsed_url.hostname, parsed_url.port),
+ parsed_url.path + b'/turnip-set-symbolic-ref', b'', b''])
+ headers = {
+ b'Content-Type': [
+ b'application/x-turnip-set-symbolic-ref-request',
+ ],
+ }
+ if parsed_url.username:
+ headers[b'Authorization'] = [
+ b'Basic ' + base64.b64encode(
+ b'%s:%s' % (parsed_url.username, parsed_url.password)),
+ ]
+ data = helpers.encode_packet(line) + helpers.encode_packet(None)
+ return client.Agent(reactor).request(
+ b'POST', url, headers=http_headers.Headers(headers),
+ bodyProducer=client.FileBodyProducer(io.BytesIO(data)))
+
+ @defer.inlineCallbacks
+ def get_symbolic_ref(self, path, name):
+ out = yield utils.getProcessOutput(
+ b'git', (b'symbolic-ref', name), env=os.environ, path=path)
+ defer.returnValue(out.rstrip(b'\n'))
+
+ @defer.inlineCallbacks
+ def test_turnip_set_symbolic_ref(self):
+ repo = os.path.join(self.root, self.internal_name)
+ head_target = yield self.get_symbolic_ref(repo, b'HEAD')
+ self.assertEqual(b'refs/heads/master', head_target)
+ response = yield self.make_set_symbolic_ref_request(
+ b'HEAD refs/heads/new-head')
+ self.assertEqual(200, response.code)
+ body = yield client.readBody(response)
+ self.assertEqual((b'ACK HEAD', ''), helpers.decode_packet(body))
+ head_target = yield self.get_symbolic_ref(repo, b'HEAD')
+ self.assertEqual(b'refs/heads/new-head', head_target)
+
+ @defer.inlineCallbacks
+ def test_turnip_set_symbolic_ref_error(self):
+ repo = os.path.join(self.root, self.internal_name)
+ head_target = yield self.get_symbolic_ref(repo, b'HEAD')
+ self.assertEqual(b'refs/heads/master', head_target)
+ response = yield self.make_set_symbolic_ref_request(b'HEAD --evil')
+ # This is a little weird since an error occurred, but it's
+ # consistent with other HTTP pack protocol responses.
+ self.assertEqual(200, response.code)
+ body = yield client.readBody(response)
+ self.assertEqual(
+ (b'ERR Symbolic ref target may not start with "-"\n', ''),
+ helpers.decode_packet(body))
+ head_target = yield self.get_symbolic_ref(repo, b'HEAD')
+ self.assertEqual(b'refs/heads/master', head_target)
class TestSmartHTTPFrontendWithAuthFunctional(TestSmartHTTPFrontendFunctional):
diff --git a/turnip/pack/tests/test_git.py b/turnip/pack/tests/test_git.py
index d19574d..8d0b1cf 100644
--- a/turnip/pack/tests/test_git.py
+++ b/turnip/pack/tests/test_git.py
@@ -7,13 +7,23 @@ from __future__ import (
unicode_literals,
)
+import os.path
+
+from fixtures import TempDir
+from pygit2 import init_repository
from testtools import TestCase
+from testtools.matchers import (
+ ContainsDict,
+ Equals,
+ MatchesListwise,
+ )
from twisted.test import proto_helpers
from turnip.pack import (
git,
helpers,
)
+from turnip.pack.tests.test_hooks import MockHookRPCHandler
class DummyPackServerProtocol(git.PackServerProtocol):
@@ -87,3 +97,103 @@ class TestPackServerProtocol(TestCase):
# dropped.
self.proto.dataReceived(b'0000')
self.assertKilledWith(b'Bad request: flush-pkt instead')
+
+
+class DummyPackBackendProtocol(git.PackBackendProtocol):
+
+ test_process = None
+
+ def spawnProcess(self, cmd, args, env=None):
+ if self.test_process is not None:
+ raise AssertionError('Process already spawned.')
+ self.test_process = (cmd, args, env)
+
+
+class TestPackBackendProtocol(TestCase):
+ """Test the Git pack backend protocol."""
+
+ def setUp(self):
+ super(TestPackBackendProtocol, self).setUp()
+ self.root = self.useFixture(TempDir()).path
+ self.hookrpc_handler = MockHookRPCHandler()
+ self.hookrpc_sock = os.path.join(self.root, 'hookrpc_sock')
+ self.factory = git.PackBackendFactory(
+ self.root, self.hookrpc_handler, self.hookrpc_sock)
+ self.proto = DummyPackBackendProtocol()
+ self.proto.factory = self.factory
+ self.transport = proto_helpers.StringTransportWithDisconnection()
+ self.transport.protocol = self.proto
+ self.proto.makeConnection(self.transport)
+
+ def assertKilledWith(self, message):
+ self.assertFalse(self.transport.connected)
+ self.assertEqual(
+ (b'ERR ' + message + b'\n', b''),
+ helpers.decode_packet(self.transport.value()))
+
+ def test_git_upload_pack_calls_spawnProcess(self):
+ # If the command is git-upload-pack, requestReceived calls
+ # spawnProcess with appropriate arguments.
+ self.proto.requestReceived(
+ b'git-upload-pack', b'/foo.git', {b'host': b'example.com'})
+ self.assertEqual(
+ (b'git',
+ [b'git', b'upload-pack', os.path.join(self.root, b'foo.git')],
+ {}),
+ self.proto.test_process)
+
+ def test_git_receive_pack_calls_spawnProcess(self):
+ # If the command is git-receive-pack, requestReceived calls
+ # spawnProcess with appropriate arguments.
+ repo_dir = os.path.join(self.root, 'foo.git')
+ init_repository(repo_dir, bare=True)
+ self.proto.requestReceived(
+ b'git-receive-pack', b'/foo.git', {b'host': b'example.com'})
+ self.assertThat(
+ self.proto.test_process, MatchesListwise([
+ Equals(b'git'),
+ Equals([b'git', b'receive-pack', repo_dir.encode('utf-8')]),
+ ContainsDict(
+ {b'TURNIP_HOOK_RPC_SOCK': Equals(self.hookrpc_sock)})]))
+
+ def test_turnip_set_symbolic_ref_calls_spawnProcess(self):
+ # If the command is turnip-set-symbolic-ref, requestReceived does
+ # not spawn a process, but packetReceived calls spawnProcess with
+ # appropriate arguments.
+ repo_dir = os.path.join(self.root, 'foo.git')
+ init_repository(repo_dir, bare=True)
+ self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {})
+ self.assertIsNone(self.proto.test_process)
+ self.proto.packetReceived(b'HEAD refs/heads/master')
+ self.assertThat(
+ self.proto.test_process, MatchesListwise([
+ Equals(b'git'),
+ Equals([
+ b'git', b'-C', repo_dir.encode('utf-8'), b'symbolic-ref',
+ b'HEAD', b'refs/heads/master']),
+ ContainsDict(
+ {b'TURNIP_HOOK_RPC_SOCK': Equals(self.hookrpc_sock)})]))
+
+ def test_turnip_set_symbolic_ref_requires_valid_line(self):
+ # The turnip-set-symbolic-ref command requires a valid
+ # set-symbolic-ref-line packet.
+ self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {})
+ self.assertIsNone(self.proto.test_process)
+ self.proto.packetReceived(b'HEAD')
+ self.assertKilledWith(b'Invalid set-symbolic-ref-line')
+
+ def test_turnip_set_symbolic_ref_name_must_be_HEAD(self):
+ # The turnip-set-symbolic-ref command's "name" parameter must be
+ # "HEAD".
+ self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {})
+ self.assertIsNone(self.proto.test_process)
+ self.proto.packetReceived(b'another-symref refs/heads/master')
+ self.assertKilledWith(b'Symbolic ref name must be "HEAD"')
+
+ def test_turnip_set_symbolic_ref_target_not_option(self):
+ # The turnip-set-symbolic-ref command's "target" parameter may not
+ # start with "-".
+ self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {})
+ self.assertIsNone(self.proto.test_process)
+ self.proto.packetReceived(b'HEAD --evil')
+ self.assertKilledWith(b'Symbolic ref target may not start with "-"')
diff --git a/turnip/pack/tests/test_http.py b/turnip/pack/tests/test_http.py
index 8e0418f..4eb6ada 100644
--- a/turnip/pack/tests/test_http.py
+++ b/turnip/pack/tests/test_http.py
@@ -64,7 +64,8 @@ def render_resource(resource, request):
class FakeRoot(object):
- allowed_services = frozenset((b'git-upload-pack', b'git-receive-pack'))
+ allowed_services = frozenset((
+ b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref'))
def __init__(self):
self.backend_transport = None
