Skip to content

Commit 9b359af

Browse files
author
epriestley
committedJan 28, 2015
Prepare SSH connections for proxying
Summary: Ref T7034. In a cluster environment, when a user connects with a VCS request over SSH (like `git pull`), the receiving server may need to proxy it to a server which can actually satisfy the request. In order to proxy the request, we need to know which repository the user is interested in accessing. Split the SSH workflow into two steps: # First, identify the repository. # Then, execute the operation. In the future, this will allow us to put a possible "proxy the whole thing somewhere else" step in the middle, mirroring the behavior of Conduit. This is trivially easy in `git` and `hg`. Both identify the repository on the commmand line. This is fiendishly complex in `svn`, for the same reasons that hosting SVN was hard in the first place. Specifically: - The client doesn't tell us what it's after. - To get it to tell us, we have to send it a server capabilities string //first//. - We can't just start an `svnserve` process and read the repository out after a little while, because we may need to proxy the request once we figure out the repository. - We can't consume the client protocol frame that tells us what the client wants, because when we start the real server request it won't know what the client is after if it never receives that frame. - On the other hand, we must consume the second copy of the server protocol frame that would be sent to the client, or they'll get two "HELLO" messages and not know what to do. The approach here is straightforward, but the implementation is not trivial. Roughly: - Start `svnserve`, read the "hello" frame from it. - Kill `svnserve`. - Send the "hello" to the client. - Wait for the client to send us "I want repository X". - Save the message it sent us in the "peekBuffer". - Return "this is a request for repository X", so we can proxy it. Then, to continue the request: - Start the real `svnserve`. - Read the "hello" frame from it and throw it away. - Write the data in the "peekBuffer" to it, as though we'd just received it from the client. - State of the world is normal again, so we can continue. Also fixed some other issues: - SVN could choke if `repository.default-local-path` contained extra slashes. - PHP might emit some complaints when executing the commit hook; silence those. Test Plan: Pushed and pulled repositories in SVN, Mercurial and Git. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7034 Differential Revision: https://secure.phabricator.com/D11541
1 parent 834079f commit 9b359af

9 files changed

+223
-33
lines changed
 

‎resources/celerity/map.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88
return array(
99
'names' => array(
10-
'core.pkg.css' => '04a24e98',
10+
'core.pkg.css' => '8815f87d',
1111
'core.pkg.js' => 'efa12ecc',
1212
'darkconsole.pkg.js' => '8ab24e01',
1313
'differential.pkg.css' => '8af45893',
@@ -125,7 +125,7 @@
125125
'rsrc/css/phui/phui-action-list.css' => '9ee9910a',
126126
'rsrc/css/phui/phui-box.css' => '7b3a2eed',
127127
'rsrc/css/phui/phui-button.css' => '008ba5e2',
128-
'rsrc/css/phui/phui-crumbs-view.css' => '3e362700',
128+
'rsrc/css/phui/phui-crumbs-view.css' => '646a8830',
129129
'rsrc/css/phui/phui-document.css' => 'bbeb1890',
130130
'rsrc/css/phui/phui-feed-story.css' => 'c9f3a0b5',
131131
'rsrc/css/phui/phui-fontkit.css' => '9c3d2dce',
@@ -137,7 +137,7 @@
137137
'rsrc/css/phui/phui-info-panel.css' => '27ea50a1',
138138
'rsrc/css/phui/phui-list.css' => '53deb25c',
139139
'rsrc/css/phui/phui-object-box.css' => '0d47b3c8',
140-
'rsrc/css/phui/phui-object-item-list-view.css' => '832c58fe',
140+
'rsrc/css/phui/phui-object-item-list-view.css' => '2686a80e',
141141
'rsrc/css/phui/phui-pinboard-view.css' => '3dd4a269',
142142
'rsrc/css/phui/phui-property-list-view.css' => '51480060',
143143
'rsrc/css/phui/phui-remarkup-preview.css' => '19ad512b',
@@ -770,7 +770,7 @@
770770
'phui-calendar-day-css' => 'de035c8a',
771771
'phui-calendar-list-css' => 'c1d0ca59',
772772
'phui-calendar-month-css' => 'a92e47d2',
773-
'phui-crumbs-view-css' => '3e362700',
773+
'phui-crumbs-view-css' => '646a8830',
774774
'phui-document-view-css' => 'bbeb1890',
775775
'phui-feed-story-css' => 'c9f3a0b5',
776776
'phui-font-icon-base-css' => '3dad2ae3',
@@ -783,7 +783,7 @@
783783
'phui-info-panel-css' => '27ea50a1',
784784
'phui-list-view-css' => '53deb25c',
785785
'phui-object-box-css' => '0d47b3c8',
786-
'phui-object-item-list-view-css' => '832c58fe',
786+
'phui-object-item-list-view-css' => '2686a80e',
787787
'phui-pinboard-view-css' => '3dd4a269',
788788
'phui-property-list-view-css' => '51480060',
789789
'phui-remarkup-preview-css' => '19ad512b',

‎scripts/repository/commit_hook.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1-
#!/usr/bin/env php
1+
#!/usr/bin/env TERM=dumb php
22
<?php
33

4+
// NOTE: Note that we're specifying TERM=dumb above when invoking the PHP
5+
// interpreter. This suppresses an error which looks like this:
6+
//
7+
// No entry for terminal type "unknown";
8+
// using dumb terminal settings.
9+
//
10+
// This arises from somewhere in the PHP startup machinery if TERM is not
11+
// set to a recognized value.
12+
413
// Commit hooks execute in an unusual context where the environment may be
514
// unavailable, particularly in SVN. The first parameter to this script is
615
// either a bare repository identifier ("X"), or a repository identifier

‎src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ protected function didConstruct() {
1414
}
1515

1616
protected function executeRepositoryOperations() {
17-
$args = $this->getArgs();
18-
$path = head($args->getArg('dir'));
19-
$repository = $this->loadRepository($path);
17+
$repository = $this->getRepository();
2018

2119
// This is a write, and must have write access.
2220
$this->requireWriteAccess();

‎src/applications/diffusion/ssh/DiffusionGitSSHWorkflow.php

+6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ protected function writeError($message) {
77
return parent::writeError($message."\n");
88
}
99

10+
protected function identifyRepository() {
11+
$args = $this->getArgs();
12+
$path = head($args->getArg('dir'));
13+
return $this->loadRepositoryWithPath($path);
14+
}
15+
1016
protected function waitForGitClient() {
1117
$io_channel = $this->getIOChannel();
1218

‎src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ protected function didConstruct() {
1414
}
1515

1616
protected function executeRepositoryOperations() {
17-
$args = $this->getArgs();
18-
$path = head($args->getArg('dir'));
19-
$repository = $this->loadRepository($path);
17+
$repository = $this->getRepository();
2018

2119
$command = csprintf('git-upload-pack -- %s', $repository->getLocalPath());
2220
$command = PhabricatorDaemon::sudoCommandAsDaemonUser($command);

‎src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php

+5-2
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,14 @@ protected function didConstruct() {
2424
));
2525
}
2626

27-
protected function executeRepositoryOperations() {
27+
protected function identifyRepository() {
2828
$args = $this->getArgs();
2929
$path = $args->getArg('repository');
30-
$repository = $this->loadRepository($path);
30+
return $this->loadRepositoryWithPath($path);
31+
}
3132

33+
protected function executeRepositoryOperations() {
34+
$repository = $this->getRepository();
3235
$args = $this->getArgs();
3336

3437
if (!$args->getArg('stdio')) {

‎src/applications/diffusion/ssh/DiffusionSSHWorkflow.php

+17-4
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,16 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
88

99
public function getRepository() {
1010
if (!$this->repository) {
11-
throw new Exception('Call loadRepository() before getRepository()!');
11+
throw new Exception(pht('Repository is not available yet!'));
1212
}
1313
return $this->repository;
1414
}
1515

16+
private function setRepository(PhabricatorRepository $repository) {
17+
$this->repository = $repository;
18+
return $this;
19+
}
20+
1621
public function getArgs() {
1722
return $this->args;
1823
}
@@ -33,6 +38,10 @@ public function getEnvironment() {
3338
return $env;
3439
}
3540

41+
/**
42+
* Identify and load the affected repository.
43+
*/
44+
abstract protected function identifyRepository();
3645
abstract protected function executeRepositoryOperations();
3746

3847
protected function writeError($message) {
@@ -43,6 +52,12 @@ protected function writeError($message) {
4352
final public function execute(PhutilArgumentParser $args) {
4453
$this->args = $args;
4554

55+
$repository = $this->identifyRepository();
56+
$this->setRepository($repository);
57+
58+
// TODO: Here, we would make a proxying decision, had I implemented
59+
// proxying yet.
60+
4661
try {
4762
return $this->executeRepositoryOperations();
4863
} catch (Exception $ex) {
@@ -51,7 +66,7 @@ final public function execute(PhutilArgumentParser $args) {
5166
}
5267
}
5368

54-
protected function loadRepository($path) {
69+
protected function loadRepositoryWithPath($path) {
5570
$viewer = $this->getUser();
5671

5772
$regex = '@^/?diffusion/(?P<callsign>[A-Z]+)(?:/|\z)@';
@@ -88,8 +103,6 @@ protected function loadRepository($path) {
88103
pht('This repository is not available over SSH.'));
89104
}
90105

91-
$this->repository = $repository;
92-
93106
return $repository;
94107
}
95108

0 commit comments

Comments
 (0)
Failed to load comments.