Page MenuHomePhabricator
Files F22585151

D19357.id46306.diff
No OneTemporary
Actions

D19357.id46306.diff
View Options

diff --git a/src/applications/almanac/servicetype/AlmanacClusterRepositoryServiceType.php b/src/applications/almanac/servicetype/AlmanacClusterRepositoryServiceType.php
--- a/src/applications/almanac/servicetype/AlmanacClusterRepositoryServiceType.php
+++ b/src/applications/almanac/servicetype/AlmanacClusterRepositoryServiceType.php
@@ -57,6 +57,11 @@
'protocol' => id(new PhabricatorSelectEditField())
->setOptions(ipull($protocols, 'value', 'value'))
->setValue($default_value),
+ 'writable' => id(new PhabricatorBoolEditField())
+ ->setOptions(
+ pht('Prevent Writes'),
+ pht('Allow Writes'))
+ ->setValue(true),
);
}
diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php
--- a/src/applications/diffusion/controller/DiffusionServeController.php
+++ b/src/applications/diffusion/controller/DiffusionServeController.php
@@ -437,6 +437,7 @@
'http',
'https',
),
+ 'writable' => !$this->isReadOnlyRequest($repository),
));
if ($uri) {
$future = $this->getRequest()->newClusterProxyFuture($uri);
diff --git a/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php
@@ -29,7 +29,7 @@
->setLog($this);
if ($this->shouldProxy()) {
- $command = $this->getProxyCommand();
+ $command = $this->getProxyCommand(true);
$did_synchronize = false;
if ($device) {
diff --git a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionGitUploadPackSSHWorkflow.php
@@ -22,7 +22,7 @@
$is_proxy = $this->shouldProxy();
if ($is_proxy) {
- $command = $this->getProxyCommand();
+ $command = $this->getProxyCommand(false);
if ($device) {
$this->writeClusterEngineLogMessage(
diff --git a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionMercurialServeSSHWorkflow.php
@@ -45,7 +45,10 @@
}
if ($this->shouldProxy()) {
- $command = $this->getProxyCommand();
+ // NOTE: For now, we're always requesting a writable node. The request
+ // may not actually need one, but we can't currently determine whether
+ // it is read-only or not at this phase of evaluation.
+ $command = $this->getProxyCommand(true);
} else {
$command = csprintf(
'hg -R %s serve --stdio',
diff --git a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionSSHWorkflow.php
@@ -5,7 +5,7 @@
private $args;
private $repository;
private $hasWriteAccess;
- private $proxyURI;
+ private $shouldProxy;
private $baseRequestPath;
public function getRepository() {
@@ -69,18 +69,34 @@
return php_uname('n');
}
- protected function getTargetDeviceName() {
- // TODO: This should use the correct device identity.
- $uri = new PhutilURI($this->proxyURI);
- return $uri->getDomain();
- }
-
protected function shouldProxy() {
- return (bool)$this->proxyURI;
+ return $this->shouldProxy;
}
- protected function getProxyCommand() {
- $uri = new PhutilURI($this->proxyURI);
+ protected function getProxyCommand($for_write) {
+ $viewer = $this->getSSHUser();
+ $repository = $this->getRepository();
+
+ $is_cluster_request = $this->getIsClusterRequest();
+
+ $uri = $repository->getAlmanacServiceURI(
+ $viewer,
+ array(
+ 'neverProxy' => $is_cluster_request,
+ 'protocols' => array(
+ 'ssh',
+ ),
+ 'writable' => $for_write,
+ ));
+
+ if (!$uri) {
+ throw new Exception(
+ pht(
+ 'Failed to generate an intracluster proxy URI even though this '.
+ 'request was routed as a proxy request.'));
+ }
+
+ $uri = new PhutilURI($uri);
$username = AlmanacKeys::getClusterSSHUser();
if ($username === null) {
@@ -148,6 +164,15 @@
$repository = $this->identifyRepository();
$this->setRepository($repository);
+ // NOTE: Here, we're just figuring out if this is a proxyable request to
+ // a clusterized repository or not. We don't (and can't) use the URI we get
+ // back directly.
+
+ // For example, we may get a read-only URI here but be handling a write
+ // request. We only care if we get back `null` (which means we should
+ // handle the request locally) or anything else (which means we should
+ // proxy it to an appropriate device).
+
$is_cluster_request = $this->getIsClusterRequest();
$uri = $repository->getAlmanacServiceURI(
$viewer,
@@ -157,10 +182,7 @@
'ssh',
),
));
-
- if ($uri) {
- $this->proxyURI = $uri;
- }
+ $this->shouldProxy = (bool)$uri;
try {
return $this->executeRepositoryOperations();
diff --git a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php
--- a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php
+++ b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php
@@ -148,7 +148,10 @@
}
if ($this->shouldProxy()) {
- $command = $this->getProxyCommand();
+ // NOTE: We're always requesting a writable device here. The request
+ // might be read-only, but we can't currently tell, and SVN requests
+ // can mix reads and writes.
+ $command = $this->getProxyCommand(true);
$this->isProxying = true;
$cwd = null;
} else {
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -1909,10 +1909,12 @@
array(
'neverProxy' => 'bool',
'protocols' => 'list<string>',
+ 'writable' => 'optional bool',
));
$never_proxy = $options['neverProxy'];
$protocols = $options['protocols'];
+ $writable = idx($options, 'writable', false);
$cache_key = $this->getAlmanacServiceCacheKey();
if (!$cache_key) {
@@ -1958,7 +1960,7 @@
}
if (isset($protocol_map[$uri['protocol']])) {
- $results[] = new PhutilURI($uri['uri']);
+ $results[] = $uri;
}
}
@@ -1989,8 +1991,29 @@
}
}
+ // If we require a writable device, remove URIs which aren't writable.
+ if ($writable) {
+ foreach ($results as $key => $uri) {
+ if (!$uri['writable']) {
+ unset($results[$key]);
+ }
+ }
+
+ if (!$results) {
+ throw new Exception(
+ pht(
+ 'This repository ("%s") is not writable with the given '.
+ 'protocols (%s). The Almanac service for this repository has no '.
+ 'writable bindings that support these protocols.',
+ $this->getDisplayName(),
+ implode(', ', $protocols)));
+ }
+ }
+
shuffle($results);
- return head($results);
+
+ $result = head($results);
+ return $result['uri'];
}
public function supportsSynchronization() {
@@ -2009,7 +2032,14 @@
}
$repository_phid = $this->getPHID();
- return "diffusion.repository({$repository_phid}).service({$service_phid})";
+
+ $parts = array(
+ "repo({$repository_phid})",
+ "serv({$service_phid})",
+ 'v2',
+ );
+
+ return implode('.', $parts);
}
private function buildAlmanacServiceURIs() {
@@ -2038,6 +2068,7 @@
'protocol' => $protocol,
'uri' => (string)$uri,
'device' => $device_name,
+ 'writable' => (bool)$binding->getAlmanacPropertyValue('writable'),
);
}
@@ -2091,6 +2122,10 @@
'http',
'https',
),
+
+ // At least today, no Conduit call can ever write to a repository,
+ // so it's fine to send anything to a read-only node.
+ 'writable' => false,
));
if ($uri === null) {
return null;

File Metadata

Mime Type
text/plain
Expires
Tue, Sep 29, 12:22 AM (10 h, 44 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
16887653
Default Alt Text
D19357.id46306.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility