launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/turnip:run-v2-commands into turnip:master with ~pappacena/turnip:git-backend-stdin as a prerequisite.

Commit message:
Running, at the backend, v2 commands when requested

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/389118

This MP makes the backend run a git using the v2 protocol when a frontend connection requests it. For now, the execution is controlled by `get_capabilities_advertisement` method, which enables the v2 execution path only if we are advertising v2 compatibility.

get_capabilities_advertisement should be changed in a future MP in order to enable compatibility only for certain frontend types.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:run-v2-commands into turnip:master.

diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 6ae6dd5..1b34ac0 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -31,11 +31,14 @@ from turnip.config import config
 from turnip.helpers import compose_path
 from turnip.pack.helpers import (
     decode_packet,
+    DELIM_PKT,
     decode_request,
     encode_packet,
     encode_request,
     ensure_config,
     ensure_hooks,
+    FLUSH_PKT,
+    get_capabilities_advertisement,
     INCOMPLETE_PKT,
     translate_xmlrpc_fault,
     )
@@ -77,7 +80,7 @@ class UnstoppableProducerWrapper(object):
         pass
 
 
-class PackProtocol(protocol.Protocol):
+class PackProtocol(protocol.Protocol, object):
 
     paused = False
     raw = False
@@ -420,7 +423,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.
@@ -437,6 +440,24 @@ class PackBackendProtocol(PackServerProtocol):
     hookrpc_key = None
     expect_set_symbolic_ref = False
 
+    def getV2CommandInput(self, params):
+        """Reconstruct what should be sent to git's stdin from the
+        parameters received."""
+        cmd_input = encode_packet(b"command=%s\n" % params.get(b'command'))
+        for capability in params["capabilities"].split(b"\n"):
+            cmd_input += encode_packet(b"%s\n" % capability)
+        cmd_input += DELIM_PKT
+        ignore_keys = (b'capabilities', b'version')
+        for k, v in params.items():
+            k = six.ensure_binary(k)
+            if k.startswith(b"turnip-") or k in ignore_keys:
+                continue
+            for param_value in v.split(b'\n'):
+                value = (b"" if not param_value else b" %s" % param_value)
+                cmd_input += encode_packet(b"%s%s\n" % (k, value))
+        cmd_input += FLUSH_PKT
+        return cmd_input
+
     @defer.inlineCallbacks
     def requestReceived(self, command, raw_pathname, params):
         self.extractRequestMeta(command, raw_pathname, params)
@@ -465,14 +486,27 @@ class PackBackendProtocol(PackServerProtocol):
         cmd_input = None
         cmd_env = {}
         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
+        if not get_capabilities_advertisement(params.get(b'version', 1)):
+            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
         else:
-            self.die(b'Unsupported command in request')
-            return
+            v2_command = params.get(b'command')
+            if command == b'git-upload-pack' and not v2_command:
+                self.expectNextCommand()
+                self.transport.loseConnection()
+                return
+            subcmd = b'upload-pack'
+            cmd_env["GIT_PROTOCOL"] = 'version=2'
+            send_path_as_option = True
+            # Do not include "advertise-refs" parameter.
+            params.pop(b'turnip-advertise-refs', None)
+            cmd_input = self.getV2CommandInput(params)
 
         args = []
         if params.pop(b'turnip-stateless-rpc', None):
diff --git a/turnip/pack/helpers.py b/turnip/pack/helpers.py
index e0d95a1..93aba33 100644
--- a/turnip/pack/helpers.py
+++ b/turnip/pack/helpers.py
@@ -289,3 +289,13 @@ 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."""
+    # XXX pappacena 2020-08-11: Return the correct data for protocol v2.
+    return b""
diff --git a/turnip/pack/tests/test_git.py b/turnip/pack/tests/test_git.py
index 5adffb1..b69049c 100644
--- a/turnip/pack/tests/test_git.py
+++ b/turnip/pack/tests/test_git.py
@@ -9,6 +9,7 @@ from __future__ import (
 
 import hashlib
 import os.path
+from collections import OrderedDict
 
 from fixtures import TempDir, MonkeyPatch
 from pygit2 import init_repository
@@ -283,6 +284,45 @@ class TestPackBackendProtocol(TestCase):
              {}),
             self.proto.test_process)
 
+    def test_git_upload_pack_v2_calls_spawnProcess(self):
+        # If the command is git-upload-pack using v2 protocol, requestReceived
+        # calls spawnProcess with appropriate arguments.
+        advertise_capabilities = mock.Mock()
+        self.useFixture(
+            MonkeyPatch("turnip.pack.git.get_capabilities_advertisement",
+                        advertise_capabilities))
+        advertise_capabilities.return_value = b'fake capability'
+
+        self.proto.requestReceived(
+            b'git-upload-pack', b'/foo.git', OrderedDict([
+                (b'turnip-x', b'yes'),
+                (b'turnip-request-id', b'123'),
+                (b'version', b'2'),
+                (b'command', b'ls-refs'),
+                (b'capabilities', b'agent=git/2.25.1'),
+                (b'peel', b''),
+                (b'symrefs', b''),
+                (b'ref-prefix', b'HEAD\nrefs/heads/\nrefs/tags/')
+                ]))
+        full_path = os.path.join(six.ensure_binary(self.root), b'foo.git')
+        self.assertEqual(
+            (b'git',
+             [b'git', b'-C', full_path, b'upload-pack', full_path],
+             {'GIT_PROTOCOL': 'version=2'}),
+            self.proto.test_process)
+        stdin_content = (
+            b'0014command=ls-refs\n'
+            b'0015agent=git/2.25.1\n'
+            b'00010014command ls-refs\n'
+            b'0009peel\n'
+            b'000csymrefs\n'
+            b'0014ref-prefix HEAD\n'
+            b'001bref-prefix refs/heads/\n'
+            b'001aref-prefix refs/tags/\n'
+            b'0000'
+        )
+        self.assertEqual(stdin_content, self.proto.peer.cmd_input)
+
     def test_git_receive_pack_calls_spawnProcess(self):
         # If the command is git-receive-pack, requestReceived calls
         # spawnProcess with appropriate arguments.