launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25170
[Merge] ~pappacena/turnip:enable-protocol-v2-on-http into turnip:master
Thiago F. Pappacena has proposed merging ~pappacena/turnip:enable-protocol-v2-on-http into turnip:master.
Commit message:
Enabling protocol v2 compatibility on HTTP frontend
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/389309
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:enable-protocol-v2-on-http into turnip:master.
diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 763d5d6..9be4e90 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -77,7 +77,7 @@ class UnstoppableProducerWrapper(object):
pass
-class PackProtocol(protocol.Protocol):
+class PackProtocol(protocol.Protocol, object):
paused = False
raw = False
@@ -415,7 +415,7 @@ class PackProxyServerProtocol(PackServerProtocol):
def resumeProducing(self):
# Send our translated request and then open the gate to the client.
self.sendNextCommand()
- PackServerProtocol.resumeProducing(self)
+ super(PackProxyServerProtocol, self).resumeProducing()
def readConnectionLost(self):
# Forward the closed stdin down the stack.
@@ -456,7 +456,12 @@ class PackBackendProtocol(PackServerProtocol):
self.resumeProducing()
return
+ cmd_env = {}
write_operation = False
+ version = params.get(b'version', 0)
+ cmd_env["GIT_PROTOCOL"] = 'version=%s' % version
+ if version == b'2':
+ params.pop(b'turnip-advertise-refs', None)
if command == b'git-upload-pack':
subcmd = b'upload-pack'
elif command == b'git-receive-pack':
@@ -472,13 +477,14 @@ class PackBackendProtocol(PackServerProtocol):
if params.pop(b'turnip-advertise-refs', None):
args.append(b'--advertise-refs')
args.append(self.path)
- self.spawnGit(subcmd,
- args,
- write_operation=write_operation,
- auth_params=auth_params)
+ self.spawnGit(
+ subcmd, args,
+ write_operation=write_operation, auth_params=auth_params,
+ cmd_env=cmd_env)
def spawnGit(self, subcmd, extra_args, write_operation=False,
- send_path_as_option=False, auth_params=None):
+ send_path_as_option=False, auth_params=None,
+ cmd_env=None):
cmd = b'git'
args = [b'git']
if send_path_as_option:
@@ -487,6 +493,7 @@ class PackBackendProtocol(PackServerProtocol):
args.extend(extra_args)
env = {}
+ env.update((cmd_env or {}))
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.
diff --git a/turnip/pack/helpers.py b/turnip/pack/helpers.py
index 9cf411d..4a3758c 100644
--- a/turnip/pack/helpers.py
+++ b/turnip/pack/helpers.py
@@ -7,6 +7,7 @@ from __future__ import (
unicode_literals,
)
+from collections import OrderedDict
import enum
import hashlib
import os.path
@@ -23,8 +24,9 @@ import six
import yaml
import turnip.pack.hooks
+from turnip.version_info import version_info
-
+DELIM_PKT = object()
PKT_LEN_SIZE = 4
PKT_PAYLOAD_MAX = 65520
INCOMPLETE_PKT = object()
@@ -34,6 +36,8 @@ def encode_packet(payload):
if payload is None:
# flush-pkt.
return b'0000'
+ if payload is DELIM_PKT:
+ return b'0001'
else:
# data-pkt
if len(payload) > PKT_PAYLOAD_MAX:
@@ -45,6 +49,8 @@ def encode_packet(payload):
def decode_packet(input):
"""Consume a packet, returning the payload and any unconsumed tail."""
+ if input.startswith(b'0001'):
+ return (DELIM_PKT, input[PKT_LEN_SIZE:])
if len(input) < PKT_LEN_SIZE:
return (INCOMPLETE_PKT, input)
if input.startswith(b'0000'):
@@ -80,7 +86,7 @@ def decode_request(data):
if len(bits) < 2 or bits[-1] != b'':
raise ValueError('Invalid git-proto-request')
pathname = bits[0]
- params = {}
+ params = OrderedDict()
for index, param in enumerate(bits[1:-1]):
if param == b'':
if (index < len(bits) - 1):
@@ -94,7 +100,7 @@ def decode_request(data):
if name in params:
raise ValueError('Parameters must not be repeated')
params[name] = value
- return (command, pathname, params)
+ return command, pathname, params
def encode_request(command, pathname, params):
@@ -105,7 +111,7 @@ def encode_request(command, pathname, params):
if b' ' in command or b'\0' in pathname:
raise ValueError('Metacharacter in arguments')
bits = [pathname]
- for name in sorted(params):
+ for name in params:
value = params[name]
value = six.ensure_binary(value) if value is not None else b''
name = six.ensure_binary(name)
@@ -226,3 +232,22 @@ def translate_xmlrpc_fault(code):
else:
result = TurnipFaultCode.INTERNAL_SERVER_ERROR
return result
+
+
+def get_capabilities_advertisement(version='1'):
+ """Returns the capability advertisement binary string to be sent to
+ clients for a given protocol version requested.
+
+ If no binary data is sent, no advertisement is done and we declare to
+ not be compatible with that specific version."""
+ if version != '2':
+ return b""
+ turnip_version = six.ensure_binary(version_info.get("revision_id", '-1'))
+ return (
+ encode_packet(b"version 2\n") +
+ encode_packet(b"agent=turnip/%s\n" % turnip_version) +
+ encode_packet(b"ls-refs\n") +
+ encode_packet(b"fetch=shallow\n") +
+ encode_packet(b"server-option\n") +
+ b'0000'
+ )
diff --git a/turnip/pack/http.py b/turnip/pack/http.py
index c0becf0..50fd5b8 100644
--- a/turnip/pack/http.py
+++ b/turnip/pack/http.py
@@ -29,6 +29,7 @@ from paste.auth.cookie import (
decode as decode_cookie,
encode as encode_cookie,
)
+import six
from twisted.internet import (
defer,
error,
@@ -57,9 +58,10 @@ from turnip.pack.git import (
from turnip.pack.helpers import (
encode_packet,
encode_request,
+ get_capabilities_advertisement,
translate_xmlrpc_fault,
TurnipFaultCode,
- )
+)
try:
from turnip.version_info import version_info
except ImportError:
@@ -80,6 +82,16 @@ def fail_request(request, message, code=http.INTERNAL_SERVER_ERROR):
return b''
+def get_protocol_version_from_request(request):
+ version_header = request.requestHeaders.getRawHeaders(
+ 'git-protocol', ['version=1'])[0]
+ try:
+ return version_header.split('version=', 1)[1]
+ except IndexError:
+ pass
+ return 1
+
+
class HTTPPackClientProtocol(PackProtocol):
"""Abstract bridge between a Git pack connection and a smart HTTP request.
@@ -99,7 +111,11 @@ class HTTPPackClientProtocol(PackProtocol):
def startGoodResponse(self):
"""Prepare the HTTP response for forwarding from the backend."""
- raise NotImplementedError()
+ self.factory.http_request.write(
+ get_capabilities_advertisement(self.getProtocolVersion()))
+
+ def getProtocolVersion(self):
+ return get_protocol_version_from_request(self.factory.http_request)
def backendConnected(self):
"""Called when the backend is connected and has sent a good packet."""
@@ -209,6 +225,7 @@ class HTTPPackClientRefsProtocol(HTTPPackClientProtocol):
self.factory.http_request.setHeader(
b'Content-Type',
b'application/x-%s-advertisement' % self.factory.command)
+ super(HTTPPackClientRefsProtocol, self).startGoodResponse()
def backendConnected(self):
HTTPPackClientProtocol.backendConnected(self)
@@ -221,10 +238,11 @@ class HTTPPackClientCommandProtocol(HTTPPackClientProtocol):
def startGoodResponse(self):
"""Prepare the HTTP response for forwarding from the backend."""
- self.factory.http_request.setResponseCode(http.OK)
- self.factory.http_request.setHeader(
- b'Content-Type',
- b'application/x-%s-result' % self.factory.command)
+ if self.getProtocolVersion() != b'2':
+ self.factory.http_request.setResponseCode(http.OK)
+ self.factory.http_request.setHeader(
+ b'Content-Type',
+ b'application/x-%s-result' % self.factory.command)
class HTTPPackClientFactory(protocol.ClientFactory):
@@ -282,6 +300,8 @@ class BaseSmartHTTPResource(resource.Resource):
params = {
b'turnip-can-authenticate': b'yes',
b'turnip-request-id': str(uuid.uuid4()),
+ b'version': six.ensure_binary(
+ get_protocol_version_from_request(request))
}
authenticated_params = yield self.authenticateUser(request)
for key, value in authenticated_params.items():
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index 27bc26e..56a6c17 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -83,6 +83,7 @@ class FunctionalTestMixin(WithScenarios):
run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=30)
scenarios = [
+ ('v0 protocol', {"protocol_version": b"0"}),
('v1 protocol', {"protocol_version": b"1"}),
('v2 protocol', {"protocol_version": b"2"}),
]
@@ -815,16 +816,19 @@ class TestSmartHTTPFrontendWithAuthFunctional(TestSmartHTTPFrontendFunctional):
test_root = self.useFixture(TempDir()).path
clone = os.path.join(test_root, 'clone')
yield self.assertCommandSuccess((b'git', b'clone', self.ro_url, clone))
+ expected_requests = 1 if self.protocol_version in ('0', '1') else 2
self.assertEqual(
- [(b'test-user', b'test-password')], self.virtinfo.authentications)
- self.assertThat(self.virtinfo.translations, MatchesListwise([
- MatchesListwise([
+ [(b'test-user', b'test-password')] * expected_requests,
+ self.virtinfo.authentications)
+ self.assertEqual(expected_requests, len(self.virtinfo.translations))
+ for translation in self.virtinfo.translations:
+ self.assertThat(translation, MatchesListwise([
Equals(b'/test'), Equals(b'read'),
MatchesDict({
b'can-authenticate': Is(True),
b'request-id': Not(Is(None)),
- b'user': Equals(b'test-user'),
- })])]))
+ b'user': Equals(b'test-user')})
+ ]))
@defer.inlineCallbacks
def test_authenticated_push(self):
diff --git a/turnip/pack/tests/test_git.py b/turnip/pack/tests/test_git.py
index 8d4ed32..d1a8162 100644
--- a/turnip/pack/tests/test_git.py
+++ b/turnip/pack/tests/test_git.py
@@ -229,8 +229,9 @@ class TestPackBackendProtocol(TestCase):
[('foo.git', )], self.virtinfo.confirm_repo_creation_call_args)
self.assertEqual(
- (b'git', [b'git', b'upload-pack', full_path], {}),
- self.proto.test_process)
+ (b'git', [b'git', b'upload-pack', full_path], {
+ 'GIT_PROTOCOL': 'version=0'
+ }), self.proto.test_process)
@defer.inlineCallbacks
def test_create_repo_fails_to_confirm(self):
@@ -267,7 +268,7 @@ class TestPackBackendProtocol(TestCase):
self.assertEqual(
(b'git',
[b'git', b'upload-pack', full_path],
- {}),
+ {'GIT_PROTOCOL': 'version=0'}),
self.proto.test_process)
def test_git_receive_pack_calls_spawnProcess(self):
diff --git a/turnip/pack/tests/test_helpers.py b/turnip/pack/tests/test_helpers.py
index a7a3870..ec89370 100644
--- a/turnip/pack/tests/test_helpers.py
+++ b/turnip/pack/tests/test_helpers.py
@@ -9,21 +9,29 @@ from __future__ import (
import os.path
import re
-import stat
-import sys
-from textwrap import dedent
-import time
+import shutil
+import subprocess
+import tempfile
from fixtures import TempDir
from pygit2 import (
Config,
init_repository,
)
+import six
+import stat
+import sys
from testtools import TestCase
+from textwrap import dedent
+import time
from turnip.pack import helpers
+from turnip.pack.helpers import (
+ encode_packet,
+ get_capabilities_advertisement,
+ )
import turnip.pack.hooks
-
+from turnip.version_info import version_info
TEST_DATA = b'0123456789abcdef'
TEST_PKT = b'00140123456789abcdef'
@@ -303,3 +311,30 @@ class TestEnsureHooks(TestCase):
self.assertEqual(expected_bytes, actual.read())
# The hook is executable.
self.assertTrue(os.stat(self.hook('hook.py')).st_mode & stat.S_IXUSR)
+
+
+class TestCapabilityAdvertisement(TestCase):
+ def test_returning_same_output_as_git_command(self):
+ """Make sure that our hard-coded feature advertisement matches what
+ our git command advertises."""
+ root = tempfile.mkdtemp(prefix=b'turnip-test-root-')
+ self.addCleanup(shutil.rmtree, root, ignore_errors=True)
+ # Create a dummy repository
+ subprocess.call(['git', 'init', root])
+
+ git_version = subprocess.check_output(['git', '--version'])
+ git_version_num = six.ensure_binary(git_version.split(' ')[-1].strip())
+ git_agent = encode_packet(b"agent=git/%s\n" % git_version_num)
+
+ proc = subprocess.Popen(
+ ['git', 'upload-pack', root], env={"GIT_PROTOCOL": "version=2"},
+ stdout=subprocess.PIPE, stdin=subprocess.PIPE)
+ git_advertised_capabilities, _ = proc.communicate()
+
+ turnip_capabilities = get_capabilities_advertisement(version=b'2')
+ turnip_agent = encode_packet(
+ b"agent=turnip/%s\n" % version_info["revision_id"])
+
+ self.assertEqual(
+ turnip_capabilities,
+ git_advertised_capabilities.replace(git_agent, turnip_agent))
diff --git a/turnip/pack/tests/test_http.py b/turnip/pack/tests/test_http.py
index 26e2754..3471cd6 100644
--- a/turnip/pack/tests/test_http.py
+++ b/turnip/pack/tests/test_http.py
@@ -24,7 +24,10 @@ from turnip.pack import (
helpers,
http,
)
+from turnip.pack.helpers import encode_packet
from turnip.pack.tests.fake_servers import FakeVirtInfoService
+from turnip.tests.compat import mock
+from turnip.version_info import version_info
class LessDummyRequest(requesthelper.DummyRequest):
@@ -74,12 +77,14 @@ class FakeRoot(object):
def __init__(self):
self.backend_transport = None
+ self.client_factory = None
self.backend_connected = defer.Deferred()
def authenticateWithPassword(self, user, password):
return {}
def connectToBackend(self, client_factory):
+ self.client_factory = client_factory
self.backend_transport = testing.StringTransportWithDisconnection()
p = client_factory.buildProtocol(None)
self.backend_transport.protocol = p
@@ -186,6 +191,36 @@ class TestSmartHTTPRefsResource(ErrorTestMixin, TestCase):
'And I am raw, since we got a good packet to start with.',
self.request.value)
+ @defer.inlineCallbacks
+ def test_good_v2_included_version_and_capabilities(self):
+ self.request.requestHeaders.addRawHeader("Git-Protocol", "version=2")
+ yield self.performRequest(
+ helpers.encode_packet(b'I am git protocol data.') +
+ b'And I am raw, since we got a good packet to start with.')
+ self.assertEqual(200, self.request.responseCode)
+ self.assertEqual(self.root.client_factory.params, {
+ 'version': '2',
+ 'turnip-advertise-refs': 'yes',
+ 'turnip-can-authenticate': 'yes',
+ 'turnip-request-id': mock.ANY,
+ 'turnip-stateless-rpc': 'yes'})
+
+ ver = version_info["revision_id"]
+ capabilities = (
+ encode_packet(b'version 2\n') +
+ encode_packet(b'agent=turnip/%s\n' % ver) +
+ encode_packet(b'ls-refs\n') +
+ encode_packet(b'fetch=shallow\n') +
+ encode_packet(b'server-option\n') +
+ b'0000'
+ )
+ self.assertEqual(
+ capabilities +
+ '001e# service=git-upload-pack\n'
+ '0000001bI am git protocol data.'
+ 'And I am raw, since we got a good packet to start with.',
+ self.request.value)
+
class TestSmartHTTPCommandResource(ErrorTestMixin, TestCase):
