Merge ~cjwatson/turnip:set-symbolic-ref into turnip:master
- Git
- lp:~cjwatson/turnip
- set-symbolic-ref
- Merge into master
Proposed by
Colin Watson
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | eea2842e739a4c3d3314f31c7d8937f7cc2542e2 |
Merged at revision: | cf713d479b3fd067cb2a837428b276565b33f0a4 |
Proposed branch: | ~cjwatson/turnip:set-symbolic-ref |
Merge into: | turnip:master |
Diff against target: |
497 lines (+297/-26) 8 files modified
README (+16/-0) turnip/pack/git.py (+83/-9) turnip/pack/hookrpc.py (+6/-2) turnip/pack/http.py (+5/-2) turnip/pack/ssh.py (+2/-1) turnip/pack/tests/test_functional.py (+65/-11) turnip/pack/tests/test_git.py (+118/-0) turnip/pack/tests/test_http.py (+2/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Otto Co-Pilot | Needs Fixing | ||
William Grant | code | Approve | |
Review via email: mp+309702@code.launchpad.net |
Commit message
Add turnip-
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
Description of the change
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) : | # |
review:
Approve
(code)
Revision history for this message
Colin Watson (cjwatson) wrote : | # |
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote : | # |
I tried to merge it but there are some problems. Typically you want to merge or rebase and try again.
review:
Needs Fixing
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/README b/README | |||
2 | index 7b26510..4ebc443 100644 | |||
3 | --- a/README | |||
4 | +++ b/README | |||
5 | @@ -68,6 +68,22 @@ The only additional parameters implemented today are | |||
6 | 68 | 'turnip-stateless-rpc' and 'turnip-advertise-refs', which are used by | 68 | 'turnip-stateless-rpc' and 'turnip-advertise-refs', which are used by |
7 | 69 | the smart HTTP server to proxy to the standard pack protocol. | 69 | the smart HTTP server to proxy to the standard pack protocol. |
8 | 70 | 70 | ||
9 | 71 | turnip implements one externally-visible extension: a | ||
10 | 72 | 'turnip-set-symbolic-ref' service that sets a symbolic ref (currently only | ||
11 | 73 | 'HEAD' is permitted) to a given target. This may be used over the various | ||
12 | 74 | protocols (git, SSH, smart HTTP), requesting the service in the same way as | ||
13 | 75 | the existing 'git-upload-pack' and 'git-receive-pack' services. | ||
14 | 76 | |||
15 | 77 | turnip-set-symbolic-ref-request = set-symbolic-ref-line | ||
16 | 78 | flush-pkt | ||
17 | 79 | set-symbolic-ref-line = PKT-LINE(refname SP refname) | ||
18 | 80 | |||
19 | 81 | The server replies with an ACK indicating the symbolic ref name that was | ||
20 | 82 | changed, or an error message. | ||
21 | 83 | |||
22 | 84 | turnip-set-symbolic-ref-response = set-symbolic-ref-ack / error-line | ||
23 | 85 | set-symbolic-ref-ack = PKT-LINE("ACK" SP refname) | ||
24 | 86 | |||
25 | 71 | 87 | ||
26 | 72 | Development | 88 | Development |
27 | 73 | =========== | 89 | =========== |
28 | diff --git a/turnip/pack/git.py b/turnip/pack/git.py | |||
29 | index 0e5d928..93bc91b 100644 | |||
30 | --- a/turnip/pack/git.py | |||
31 | +++ b/turnip/pack/git.py | |||
32 | @@ -278,7 +278,7 @@ class GitProcessProtocol(protocol.ProcessProtocol): | |||
33 | 278 | 'git exited {code} with no output; synthesising an error', | 278 | 'git exited {code} with no output; synthesising an error', |
34 | 279 | code=code) | 279 | code=code) |
35 | 280 | self.peer.sendPacket(ERROR_PREFIX + 'backend exited %d' % code) | 280 | self.peer.sendPacket(ERROR_PREFIX + 'backend exited %d' % code) |
37 | 281 | self.peer.transport.loseConnection() | 281 | self.peer.processEnded(reason) |
38 | 282 | 282 | ||
39 | 283 | def pauseProducing(self): | 283 | def pauseProducing(self): |
40 | 284 | self.transport.pauseProducing() | 284 | self.transport.pauseProducing() |
41 | @@ -392,43 +392,117 @@ class PackBackendProtocol(PackServerProtocol): | |||
42 | 392 | """ | 392 | """ |
43 | 393 | 393 | ||
44 | 394 | hookrpc_key = None | 394 | hookrpc_key = None |
45 | 395 | expect_set_symbolic_ref = False | ||
46 | 395 | 396 | ||
47 | 396 | def requestReceived(self, command, raw_pathname, params): | 397 | def requestReceived(self, command, raw_pathname, params): |
48 | 397 | self.extractRequestMeta(command, raw_pathname, params) | 398 | self.extractRequestMeta(command, raw_pathname, params) |
49 | 399 | self.command = command | ||
50 | 400 | self.raw_pathname = raw_pathname | ||
51 | 401 | self.path = compose_path(self.factory.root, self.raw_pathname) | ||
52 | 402 | |||
53 | 403 | if command == b'turnip-set-symbolic-ref': | ||
54 | 404 | self.expect_set_symbolic_ref = True | ||
55 | 405 | self.resumeProducing() | ||
56 | 406 | return | ||
57 | 398 | 407 | ||
59 | 399 | path = compose_path(self.factory.root, raw_pathname) | 408 | write_operation = False |
60 | 400 | if command == b'git-upload-pack': | 409 | if command == b'git-upload-pack': |
61 | 401 | subcmd = b'upload-pack' | 410 | subcmd = b'upload-pack' |
62 | 402 | elif command == b'git-receive-pack': | 411 | elif command == b'git-receive-pack': |
63 | 403 | subcmd = b'receive-pack' | 412 | subcmd = b'receive-pack' |
64 | 413 | write_operation = True | ||
65 | 404 | else: | 414 | else: |
66 | 405 | self.die(b'Unsupported command in request') | 415 | self.die(b'Unsupported command in request') |
67 | 406 | return | 416 | return |
68 | 407 | 417 | ||
71 | 408 | cmd = b'git' | 418 | args = [] |
70 | 409 | args = [b'git', subcmd] | ||
72 | 410 | if params.pop(b'turnip-stateless-rpc', None): | 419 | if params.pop(b'turnip-stateless-rpc', None): |
73 | 411 | args.append(b'--stateless-rpc') | 420 | args.append(b'--stateless-rpc') |
74 | 412 | if params.pop(b'turnip-advertise-refs', None): | 421 | if params.pop(b'turnip-advertise-refs', None): |
75 | 413 | args.append(b'--advertise-refs') | 422 | args.append(b'--advertise-refs') |
77 | 414 | args.append(path) | 423 | args.append(self.path) |
78 | 424 | self.spawnGit(subcmd, args, write_operation=write_operation) | ||
79 | 425 | |||
80 | 426 | def spawnGit(self, subcmd, extra_args, write_operation=False, | ||
81 | 427 | send_path_as_option=False): | ||
82 | 428 | cmd = b'git' | ||
83 | 429 | args = [b'git'] | ||
84 | 430 | if send_path_as_option: | ||
85 | 431 | args.extend([b'-C', self.path]) | ||
86 | 432 | args.append(subcmd) | ||
87 | 433 | args.extend(extra_args) | ||
88 | 415 | 434 | ||
89 | 416 | env = {} | 435 | env = {} |
91 | 417 | if subcmd == b'receive-pack' and self.factory.hookrpc_handler: | 436 | if write_operation and self.factory.hookrpc_handler: |
92 | 418 | # This is a write operation, so prepare config, hooks, the hook | 437 | # This is a write operation, so prepare config, hooks, the hook |
93 | 419 | # RPC server, and the environment variables that link them up. | 438 | # RPC server, and the environment variables that link them up. |
95 | 420 | ensure_config(path) | 439 | ensure_config(self.path) |
96 | 421 | self.hookrpc_key = str(uuid.uuid4()) | 440 | self.hookrpc_key = str(uuid.uuid4()) |
97 | 422 | self.factory.hookrpc_handler.registerKey( | 441 | self.factory.hookrpc_handler.registerKey( |
100 | 423 | self.hookrpc_key, raw_pathname, []) | 442 | self.hookrpc_key, self.raw_pathname, []) |
101 | 424 | ensure_hooks(path) | 443 | ensure_hooks(self.path) |
102 | 425 | env[b'TURNIP_HOOK_RPC_SOCK'] = self.factory.hookrpc_sock | 444 | env[b'TURNIP_HOOK_RPC_SOCK'] = self.factory.hookrpc_sock |
103 | 426 | env[b'TURNIP_HOOK_RPC_KEY'] = self.hookrpc_key | 445 | env[b'TURNIP_HOOK_RPC_KEY'] = self.hookrpc_key |
104 | 427 | 446 | ||
105 | 428 | self.log.info('Spawning {args}', args=args) | 447 | self.log.info('Spawning {args}', args=args) |
106 | 429 | self.peer = GitProcessProtocol(self) | 448 | self.peer = GitProcessProtocol(self) |
107 | 449 | self.spawnProcess(cmd, args, env=env) | ||
108 | 450 | |||
109 | 451 | def spawnProcess(self, cmd, args, env=None): | ||
110 | 430 | reactor.spawnProcess(self.peer, cmd, args, env=env) | 452 | reactor.spawnProcess(self.peer, cmd, args, env=env) |
111 | 431 | 453 | ||
112 | 454 | def packetReceived(self, data): | ||
113 | 455 | if self.expect_set_symbolic_ref: | ||
114 | 456 | if data is None: | ||
115 | 457 | self.die(b'Bad request: flush-pkt instead') | ||
116 | 458 | return | ||
117 | 459 | self.pauseProducing() | ||
118 | 460 | self.expect_set_symbolic_ref = False | ||
119 | 461 | if b' ' not in data: | ||
120 | 462 | self.die(b'Invalid set-symbolic-ref-line') | ||
121 | 463 | return | ||
122 | 464 | name, target = data.split(b' ', 1) | ||
123 | 465 | # Be careful about extending this to anything other than HEAD. | ||
124 | 466 | # We use "git symbolic-ref" because it gives us locking and | ||
125 | 467 | # logging, but it doesn't prevent writing a ref to ../something. | ||
126 | 468 | # Fortunately it does at least refuse to point HEAD outside of | ||
127 | 469 | # refs/. | ||
128 | 470 | if name != b'HEAD': | ||
129 | 471 | self.die(b'Symbolic ref name must be "HEAD"') | ||
130 | 472 | return | ||
131 | 473 | if target.startswith(b'-'): | ||
132 | 474 | self.die(b'Symbolic ref target may not start with "-"') | ||
133 | 475 | return | ||
134 | 476 | elif b' ' in target: | ||
135 | 477 | self.die(b'Symbolic ref target may not contain " "') | ||
136 | 478 | return | ||
137 | 479 | self.symbolic_ref_name = name | ||
138 | 480 | self.spawnGit( | ||
139 | 481 | b'symbolic-ref', [name, target], write_operation=True, | ||
140 | 482 | send_path_as_option=True) | ||
141 | 483 | return | ||
142 | 484 | |||
143 | 485 | PackServerProtocol.packetReceived(self, data) | ||
144 | 486 | |||
145 | 487 | @defer.inlineCallbacks | ||
146 | 488 | def processEnded(self, reason): | ||
147 | 489 | message = None | ||
148 | 490 | if self.command == b'turnip-set-symbolic-ref': | ||
149 | 491 | if reason.check(error.ProcessDone): | ||
150 | 492 | try: | ||
151 | 493 | yield self.factory.hookrpc_handler.notify(self.path) | ||
152 | 494 | self.sendPacket(b'ACK %s\n' % self.symbolic_ref_name) | ||
153 | 495 | except Exception as e: | ||
154 | 496 | message = str(e) | ||
155 | 497 | else: | ||
156 | 498 | message = ( | ||
157 | 499 | 'git symbolic-ref exited with status %d' % | ||
158 | 500 | reason.value.exitCode) | ||
159 | 501 | if message is None: | ||
160 | 502 | self.transport.loseConnection() | ||
161 | 503 | else: | ||
162 | 504 | self.die(message) | ||
163 | 505 | |||
164 | 432 | def readConnectionLost(self): | 506 | def readConnectionLost(self): |
165 | 433 | # Forward the closed stdin down the stack. | 507 | # Forward the closed stdin down the stack. |
166 | 434 | if self.peer is not None: | 508 | if self.peer is not None: |
167 | diff --git a/turnip/pack/hookrpc.py b/turnip/pack/hookrpc.py | |||
168 | index d273a9c..bd3bc0e 100644 | |||
169 | --- a/turnip/pack/hookrpc.py | |||
170 | +++ b/turnip/pack/hookrpc.py | |||
171 | @@ -126,11 +126,15 @@ class HookRPCHandler(object): | |||
172 | 126 | return [rule.decode('utf-8') for rule in self.ref_rules[args['key']]] | 126 | return [rule.decode('utf-8') for rule in self.ref_rules[args['key']]] |
173 | 127 | 127 | ||
174 | 128 | @defer.inlineCallbacks | 128 | @defer.inlineCallbacks |
175 | 129 | def notify(self, path): | ||
176 | 130 | proxy = xmlrpc.Proxy(self.virtinfo_url, allowNone=True) | ||
177 | 131 | yield proxy.callRemote(b'notify', path) | ||
178 | 132 | |||
179 | 133 | @defer.inlineCallbacks | ||
180 | 129 | def notifyPush(self, proto, args): | 134 | def notifyPush(self, proto, args): |
181 | 130 | """Notify the virtinfo service about a push.""" | 135 | """Notify the virtinfo service about a push.""" |
182 | 131 | path = self.ref_paths[args['key']] | 136 | path = self.ref_paths[args['key']] |
185 | 132 | proxy = xmlrpc.Proxy(self.virtinfo_url, allowNone=True) | 137 | yield self.notify(path) |
184 | 133 | yield proxy.callRemote(b'notify', path) | ||
186 | 134 | 138 | ||
187 | 135 | 139 | ||
188 | 136 | class HookRPCServerFactory(RPCServerFactory): | 140 | class HookRPCServerFactory(RPCServerFactory): |
189 | diff --git a/turnip/pack/http.py b/turnip/pack/http.py | |||
190 | index 5e5f87e..42fed27 100644 | |||
191 | --- a/turnip/pack/http.py | |||
192 | +++ b/turnip/pack/http.py | |||
193 | @@ -680,7 +680,8 @@ class HTTPAuthResource(resource.Resource): | |||
194 | 680 | class SmartHTTPFrontendResource(resource.Resource): | 680 | class SmartHTTPFrontendResource(resource.Resource): |
195 | 681 | """HTTP resource to translate Git smart HTTP requests to pack protocol.""" | 681 | """HTTP resource to translate Git smart HTTP requests to pack protocol.""" |
196 | 682 | 682 | ||
198 | 683 | allowed_services = frozenset((b'git-upload-pack', b'git-receive-pack')) | 683 | allowed_services = frozenset(( |
199 | 684 | b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref')) | ||
200 | 684 | 685 | ||
201 | 685 | def __init__(self, backend_host, config): | 686 | def __init__(self, backend_host, config): |
202 | 686 | resource.Resource.__init__(self) | 687 | resource.Resource.__init__(self) |
203 | @@ -737,7 +738,9 @@ class SmartHTTPFrontendResource(resource.Resource): | |||
204 | 737 | content_type = request.getHeader(b'Content-Type') | 738 | content_type = request.getHeader(b'Content-Type') |
205 | 738 | if content_type is None: | 739 | if content_type is None: |
206 | 739 | return False | 740 | return False |
208 | 740 | return content_type.startswith(b'application/x-git-') | 741 | return ( |
209 | 742 | content_type.startswith(b'application/x-git-') or | ||
210 | 743 | content_type.startswith(b'application/x-turnip-')) | ||
211 | 741 | 744 | ||
212 | 742 | def getChild(self, path, request): | 745 | def getChild(self, path, request): |
213 | 743 | if self._isGitRequest(request): | 746 | if self._isGitRequest(request): |
214 | diff --git a/turnip/pack/ssh.py b/turnip/pack/ssh.py | |||
215 | index 006331d..b753220 100644 | |||
216 | --- a/turnip/pack/ssh.py | |||
217 | +++ b/turnip/pack/ssh.py | |||
218 | @@ -120,7 +120,8 @@ class SSHPackClientFactory(protocol.ClientFactory): | |||
219 | 120 | class SmartSSHSession(DoNothingSession): | 120 | class SmartSSHSession(DoNothingSession): |
220 | 121 | """SSH session allowing only Git smart SSH requests.""" | 121 | """SSH session allowing only Git smart SSH requests.""" |
221 | 122 | 122 | ||
223 | 123 | allowed_services = frozenset((b'git-upload-pack', b'git-receive-pack')) | 123 | allowed_services = frozenset(( |
224 | 124 | b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref')) | ||
225 | 124 | 125 | ||
226 | 125 | def __init__(self, *args, **kwargs): | 126 | def __init__(self, *args, **kwargs): |
227 | 126 | super(SmartSSHSession, self).__init__(*args, **kwargs) | 127 | super(SmartSSHSession, self).__init__(*args, **kwargs) |
228 | diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py | |||
229 | index 61fd5c4..e6dbd20 100644 | |||
230 | --- a/turnip/pack/tests/test_functional.py | |||
231 | +++ b/turnip/pack/tests/test_functional.py | |||
232 | @@ -8,8 +8,10 @@ from __future__ import ( | |||
233 | 8 | unicode_literals, | 8 | unicode_literals, |
234 | 9 | ) | 9 | ) |
235 | 10 | 10 | ||
236 | 11 | import base64 | ||
237 | 11 | from collections import defaultdict | 12 | from collections import defaultdict |
238 | 12 | import hashlib | 13 | import hashlib |
239 | 14 | import io | ||
240 | 13 | import os | 15 | import os |
241 | 14 | import random | 16 | import random |
242 | 15 | import shutil | 17 | import shutil |
243 | @@ -33,10 +35,7 @@ from fixtures import ( | |||
244 | 33 | from lazr.sshserver.auth import NoSuchPersonWithName | 35 | from lazr.sshserver.auth import NoSuchPersonWithName |
245 | 34 | from testtools import TestCase | 36 | from testtools import TestCase |
246 | 35 | from testtools.content import text_content | 37 | from testtools.content import text_content |
251 | 36 | from testtools.deferredruntest import ( | 38 | from testtools.deferredruntest import AsynchronousDeferredRunTest |
248 | 37 | assert_fails_with, | ||
249 | 38 | AsynchronousDeferredRunTest, | ||
250 | 39 | ) | ||
252 | 40 | from testtools.matchers import StartsWith | 39 | from testtools.matchers import StartsWith |
253 | 41 | from twisted.internet import ( | 40 | from twisted.internet import ( |
254 | 42 | defer, | 41 | defer, |
255 | @@ -45,11 +44,12 @@ from twisted.internet import ( | |||
256 | 45 | ) | 44 | ) |
257 | 46 | from twisted.web import ( | 45 | from twisted.web import ( |
258 | 47 | client, | 46 | client, |
260 | 48 | error, | 47 | http_headers, |
261 | 49 | server, | 48 | server, |
262 | 50 | xmlrpc, | 49 | xmlrpc, |
263 | 51 | ) | 50 | ) |
264 | 52 | 51 | ||
265 | 52 | from turnip.pack import helpers | ||
266 | 53 | from turnip.pack.git import ( | 53 | from turnip.pack.git import ( |
267 | 54 | PackBackendFactory, | 54 | PackBackendFactory, |
268 | 55 | PackFrontendFactory, | 55 | PackFrontendFactory, |
269 | @@ -480,14 +480,68 @@ class TestSmartHTTPFrontendFunctional(FrontendFunctionalTestMixin, TestCase): | |||
270 | 480 | 480 | ||
271 | 481 | @defer.inlineCallbacks | 481 | @defer.inlineCallbacks |
272 | 482 | def test_root_revision_header(self): | 482 | def test_root_revision_header(self): |
278 | 483 | factory = client.HTTPClientFactory( | 483 | response = yield client.Agent(reactor).request( |
279 | 484 | b'http://localhost:%d/' % self.port, method=b'HEAD', | 484 | b'HEAD', b'http://localhost:%d/' % self.port) |
280 | 485 | followRedirect=False) | 485 | self.assertEqual(302, response.code) |
276 | 486 | reactor.connectTCP(b'localhost', self.port, factory) | ||
277 | 487 | yield assert_fails_with(factory.deferred, error.PageRedirect) | ||
281 | 488 | self.assertEqual( | 486 | self.assertEqual( |
282 | 489 | [version_info['revision_id']], | 487 | [version_info['revision_id']], |
284 | 490 | factory.response_headers[b'x-turnip-revision']) | 488 | response.headers.getRawHeaders(b'X-Turnip-Revision')) |
285 | 489 | |||
286 | 490 | def make_set_symbolic_ref_request(self, line): | ||
287 | 491 | parsed_url = urlsplit(self.url) | ||
288 | 492 | url = urlunsplit([ | ||
289 | 493 | parsed_url.scheme, | ||
290 | 494 | b'%s:%d' % (parsed_url.hostname, parsed_url.port), | ||
291 | 495 | parsed_url.path + b'/turnip-set-symbolic-ref', b'', b'']) | ||
292 | 496 | headers = { | ||
293 | 497 | b'Content-Type': [ | ||
294 | 498 | b'application/x-turnip-set-symbolic-ref-request', | ||
295 | 499 | ], | ||
296 | 500 | } | ||
297 | 501 | if parsed_url.username: | ||
298 | 502 | headers[b'Authorization'] = [ | ||
299 | 503 | b'Basic ' + base64.b64encode( | ||
300 | 504 | b'%s:%s' % (parsed_url.username, parsed_url.password)), | ||
301 | 505 | ] | ||
302 | 506 | data = helpers.encode_packet(line) + helpers.encode_packet(None) | ||
303 | 507 | return client.Agent(reactor).request( | ||
304 | 508 | b'POST', url, headers=http_headers.Headers(headers), | ||
305 | 509 | bodyProducer=client.FileBodyProducer(io.BytesIO(data))) | ||
306 | 510 | |||
307 | 511 | @defer.inlineCallbacks | ||
308 | 512 | def get_symbolic_ref(self, path, name): | ||
309 | 513 | out = yield utils.getProcessOutput( | ||
310 | 514 | b'git', (b'symbolic-ref', name), env=os.environ, path=path) | ||
311 | 515 | defer.returnValue(out.rstrip(b'\n')) | ||
312 | 516 | |||
313 | 517 | @defer.inlineCallbacks | ||
314 | 518 | def test_turnip_set_symbolic_ref(self): | ||
315 | 519 | repo = os.path.join(self.root, self.internal_name) | ||
316 | 520 | head_target = yield self.get_symbolic_ref(repo, b'HEAD') | ||
317 | 521 | self.assertEqual(b'refs/heads/master', head_target) | ||
318 | 522 | response = yield self.make_set_symbolic_ref_request( | ||
319 | 523 | b'HEAD refs/heads/new-head') | ||
320 | 524 | self.assertEqual(200, response.code) | ||
321 | 525 | body = yield client.readBody(response) | ||
322 | 526 | self.assertEqual((b'ACK HEAD\n', ''), helpers.decode_packet(body)) | ||
323 | 527 | head_target = yield self.get_symbolic_ref(repo, b'HEAD') | ||
324 | 528 | self.assertEqual(b'refs/heads/new-head', head_target) | ||
325 | 529 | |||
326 | 530 | @defer.inlineCallbacks | ||
327 | 531 | def test_turnip_set_symbolic_ref_error(self): | ||
328 | 532 | repo = os.path.join(self.root, self.internal_name) | ||
329 | 533 | head_target = yield self.get_symbolic_ref(repo, b'HEAD') | ||
330 | 534 | self.assertEqual(b'refs/heads/master', head_target) | ||
331 | 535 | response = yield self.make_set_symbolic_ref_request(b'HEAD --evil') | ||
332 | 536 | # This is a little weird since an error occurred, but it's | ||
333 | 537 | # consistent with other HTTP pack protocol responses. | ||
334 | 538 | self.assertEqual(200, response.code) | ||
335 | 539 | body = yield client.readBody(response) | ||
336 | 540 | self.assertEqual( | ||
337 | 541 | (b'ERR Symbolic ref target may not start with "-"\n', ''), | ||
338 | 542 | helpers.decode_packet(body)) | ||
339 | 543 | head_target = yield self.get_symbolic_ref(repo, b'HEAD') | ||
340 | 544 | self.assertEqual(b'refs/heads/master', head_target) | ||
341 | 491 | 545 | ||
342 | 492 | 546 | ||
343 | 493 | class TestSmartHTTPFrontendWithAuthFunctional(TestSmartHTTPFrontendFunctional): | 547 | class TestSmartHTTPFrontendWithAuthFunctional(TestSmartHTTPFrontendFunctional): |
344 | diff --git a/turnip/pack/tests/test_git.py b/turnip/pack/tests/test_git.py | |||
345 | index d19574d..4153234 100644 | |||
346 | --- a/turnip/pack/tests/test_git.py | |||
347 | +++ b/turnip/pack/tests/test_git.py | |||
348 | @@ -7,13 +7,23 @@ from __future__ import ( | |||
349 | 7 | unicode_literals, | 7 | unicode_literals, |
350 | 8 | ) | 8 | ) |
351 | 9 | 9 | ||
352 | 10 | import os.path | ||
353 | 11 | |||
354 | 12 | from fixtures import TempDir | ||
355 | 13 | from pygit2 import init_repository | ||
356 | 10 | from testtools import TestCase | 14 | from testtools import TestCase |
357 | 15 | from testtools.matchers import ( | ||
358 | 16 | ContainsDict, | ||
359 | 17 | Equals, | ||
360 | 18 | MatchesListwise, | ||
361 | 19 | ) | ||
362 | 11 | from twisted.test import proto_helpers | 20 | from twisted.test import proto_helpers |
363 | 12 | 21 | ||
364 | 13 | from turnip.pack import ( | 22 | from turnip.pack import ( |
365 | 14 | git, | 23 | git, |
366 | 15 | helpers, | 24 | helpers, |
367 | 16 | ) | 25 | ) |
368 | 26 | from turnip.pack.tests.test_hooks import MockHookRPCHandler | ||
369 | 17 | 27 | ||
370 | 18 | 28 | ||
371 | 19 | class DummyPackServerProtocol(git.PackServerProtocol): | 29 | class DummyPackServerProtocol(git.PackServerProtocol): |
372 | @@ -87,3 +97,111 @@ class TestPackServerProtocol(TestCase): | |||
373 | 87 | # dropped. | 97 | # dropped. |
374 | 88 | self.proto.dataReceived(b'0000') | 98 | self.proto.dataReceived(b'0000') |
375 | 89 | self.assertKilledWith(b'Bad request: flush-pkt instead') | 99 | self.assertKilledWith(b'Bad request: flush-pkt instead') |
376 | 100 | |||
377 | 101 | |||
378 | 102 | class DummyPackBackendProtocol(git.PackBackendProtocol): | ||
379 | 103 | |||
380 | 104 | test_process = None | ||
381 | 105 | |||
382 | 106 | def spawnProcess(self, cmd, args, env=None): | ||
383 | 107 | if self.test_process is not None: | ||
384 | 108 | raise AssertionError('Process already spawned.') | ||
385 | 109 | self.test_process = (cmd, args, env) | ||
386 | 110 | |||
387 | 111 | |||
388 | 112 | class TestPackBackendProtocol(TestCase): | ||
389 | 113 | """Test the Git pack backend protocol.""" | ||
390 | 114 | |||
391 | 115 | def setUp(self): | ||
392 | 116 | super(TestPackBackendProtocol, self).setUp() | ||
393 | 117 | self.root = self.useFixture(TempDir()).path | ||
394 | 118 | self.hookrpc_handler = MockHookRPCHandler() | ||
395 | 119 | self.hookrpc_sock = os.path.join(self.root, 'hookrpc_sock') | ||
396 | 120 | self.factory = git.PackBackendFactory( | ||
397 | 121 | self.root, self.hookrpc_handler, self.hookrpc_sock) | ||
398 | 122 | self.proto = DummyPackBackendProtocol() | ||
399 | 123 | self.proto.factory = self.factory | ||
400 | 124 | self.transport = proto_helpers.StringTransportWithDisconnection() | ||
401 | 125 | self.transport.protocol = self.proto | ||
402 | 126 | self.proto.makeConnection(self.transport) | ||
403 | 127 | |||
404 | 128 | def assertKilledWith(self, message): | ||
405 | 129 | self.assertFalse(self.transport.connected) | ||
406 | 130 | self.assertEqual( | ||
407 | 131 | (b'ERR ' + message + b'\n', b''), | ||
408 | 132 | helpers.decode_packet(self.transport.value())) | ||
409 | 133 | |||
410 | 134 | def test_git_upload_pack_calls_spawnProcess(self): | ||
411 | 135 | # If the command is git-upload-pack, requestReceived calls | ||
412 | 136 | # spawnProcess with appropriate arguments. | ||
413 | 137 | self.proto.requestReceived( | ||
414 | 138 | b'git-upload-pack', b'/foo.git', {b'host': b'example.com'}) | ||
415 | 139 | self.assertEqual( | ||
416 | 140 | (b'git', | ||
417 | 141 | [b'git', b'upload-pack', os.path.join(self.root, b'foo.git')], | ||
418 | 142 | {}), | ||
419 | 143 | self.proto.test_process) | ||
420 | 144 | |||
421 | 145 | def test_git_receive_pack_calls_spawnProcess(self): | ||
422 | 146 | # If the command is git-receive-pack, requestReceived calls | ||
423 | 147 | # spawnProcess with appropriate arguments. | ||
424 | 148 | repo_dir = os.path.join(self.root, 'foo.git') | ||
425 | 149 | init_repository(repo_dir, bare=True) | ||
426 | 150 | self.proto.requestReceived( | ||
427 | 151 | b'git-receive-pack', b'/foo.git', {b'host': b'example.com'}) | ||
428 | 152 | self.assertThat( | ||
429 | 153 | self.proto.test_process, MatchesListwise([ | ||
430 | 154 | Equals(b'git'), | ||
431 | 155 | Equals([b'git', b'receive-pack', repo_dir.encode('utf-8')]), | ||
432 | 156 | ContainsDict( | ||
433 | 157 | {b'TURNIP_HOOK_RPC_SOCK': Equals(self.hookrpc_sock)})])) | ||
434 | 158 | |||
435 | 159 | def test_turnip_set_symbolic_ref_calls_spawnProcess(self): | ||
436 | 160 | # If the command is turnip-set-symbolic-ref, requestReceived does | ||
437 | 161 | # not spawn a process, but packetReceived calls spawnProcess with | ||
438 | 162 | # appropriate arguments. | ||
439 | 163 | repo_dir = os.path.join(self.root, 'foo.git') | ||
440 | 164 | init_repository(repo_dir, bare=True) | ||
441 | 165 | self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {}) | ||
442 | 166 | self.assertIsNone(self.proto.test_process) | ||
443 | 167 | self.proto.packetReceived(b'HEAD refs/heads/master') | ||
444 | 168 | self.assertThat( | ||
445 | 169 | self.proto.test_process, MatchesListwise([ | ||
446 | 170 | Equals(b'git'), | ||
447 | 171 | Equals([ | ||
448 | 172 | b'git', b'-C', repo_dir.encode('utf-8'), b'symbolic-ref', | ||
449 | 173 | b'HEAD', b'refs/heads/master']), | ||
450 | 174 | ContainsDict( | ||
451 | 175 | {b'TURNIP_HOOK_RPC_SOCK': Equals(self.hookrpc_sock)})])) | ||
452 | 176 | |||
453 | 177 | def test_turnip_set_symbolic_ref_requires_valid_line(self): | ||
454 | 178 | # The turnip-set-symbolic-ref command requires a valid | ||
455 | 179 | # set-symbolic-ref-line packet. | ||
456 | 180 | self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {}) | ||
457 | 181 | self.assertIsNone(self.proto.test_process) | ||
458 | 182 | self.proto.packetReceived(b'HEAD') | ||
459 | 183 | self.assertKilledWith(b'Invalid set-symbolic-ref-line') | ||
460 | 184 | |||
461 | 185 | def test_turnip_set_symbolic_ref_name_must_be_HEAD(self): | ||
462 | 186 | # The turnip-set-symbolic-ref command's "name" parameter must be | ||
463 | 187 | # "HEAD". | ||
464 | 188 | self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {}) | ||
465 | 189 | self.assertIsNone(self.proto.test_process) | ||
466 | 190 | self.proto.packetReceived(b'another-symref refs/heads/master') | ||
467 | 191 | self.assertKilledWith(b'Symbolic ref name must be "HEAD"') | ||
468 | 192 | |||
469 | 193 | def test_turnip_set_symbolic_ref_target_not_option(self): | ||
470 | 194 | # The turnip-set-symbolic-ref command's "target" parameter may not | ||
471 | 195 | # start with "-". | ||
472 | 196 | self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {}) | ||
473 | 197 | self.assertIsNone(self.proto.test_process) | ||
474 | 198 | self.proto.packetReceived(b'HEAD --evil') | ||
475 | 199 | self.assertKilledWith(b'Symbolic ref target may not start with "-"') | ||
476 | 200 | |||
477 | 201 | def test_turnip_set_symbolic_ref_target_no_space(self): | ||
478 | 202 | # The turnip-set-symbolic-ref command's "target" parameter may not | ||
479 | 203 | # contain " ". | ||
480 | 204 | self.proto.requestReceived(b'turnip-set-symbolic-ref', b'/foo.git', {}) | ||
481 | 205 | self.assertIsNone(self.proto.test_process) | ||
482 | 206 | self.proto.packetReceived(b'HEAD evil lies') | ||
483 | 207 | self.assertKilledWith(b'Symbolic ref target may not contain " "') | ||
484 | diff --git a/turnip/pack/tests/test_http.py b/turnip/pack/tests/test_http.py | |||
485 | index 45cafad..6e16b77 100644 | |||
486 | --- a/turnip/pack/tests/test_http.py | |||
487 | +++ b/turnip/pack/tests/test_http.py | |||
488 | @@ -64,7 +64,8 @@ def render_resource(resource, request): | |||
489 | 64 | 64 | ||
490 | 65 | class FakeRoot(object): | 65 | class FakeRoot(object): |
491 | 66 | 66 | ||
493 | 67 | allowed_services = frozenset((b'git-upload-pack', b'git-receive-pack')) | 67 | allowed_services = frozenset(( |
494 | 68 | b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref')) | ||
495 | 68 | 69 | ||
496 | 69 | def __init__(self): | 70 | def __init__(self): |
497 | 70 | self.backend_transport = None | 71 | self.backend_transport = None |
All fixed, thanks.