mirror of
https://we.phorge.it/source/phorge.git
synced 2025-01-08 22:01:03 +01:00
Make ConduitCall always local/in-process
Summary: Ref T2783. ConduitCall currently has logic to pick a random remote server, but this is ultimately not appropriate: we always want to send requests to a specific server. For example, we want to send repository requests to a server which has that repository locally. The repository tier is not homogenous, so we can't do this below the call level. Make ConduitCall always-local; logic above it will select ConduitCall for an in-process request or do service selection for an off-host request via ConduitClient. Test Plan: - Browsed some pages using ConduitCall, everything worked. - Grepped for removed stuff. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2783 Differential Revision: https://secure.phabricator.com/D10959
This commit is contained in:
parent
d151c88040
commit
db51d7d92a
5 changed files with 1 additions and 89 deletions
|
@ -1419,7 +1419,6 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorConduitAPIController' => 'applications/conduit/controller/PhabricatorConduitAPIController.php',
|
'PhabricatorConduitAPIController' => 'applications/conduit/controller/PhabricatorConduitAPIController.php',
|
||||||
'PhabricatorConduitApplication' => 'applications/conduit/application/PhabricatorConduitApplication.php',
|
'PhabricatorConduitApplication' => 'applications/conduit/application/PhabricatorConduitApplication.php',
|
||||||
'PhabricatorConduitCertificateToken' => 'applications/conduit/storage/PhabricatorConduitCertificateToken.php',
|
'PhabricatorConduitCertificateToken' => 'applications/conduit/storage/PhabricatorConduitCertificateToken.php',
|
||||||
'PhabricatorConduitConfigOptions' => 'applications/conduit/config/PhabricatorConduitConfigOptions.php',
|
|
||||||
'PhabricatorConduitConnectionLog' => 'applications/conduit/storage/PhabricatorConduitConnectionLog.php',
|
'PhabricatorConduitConnectionLog' => 'applications/conduit/storage/PhabricatorConduitConnectionLog.php',
|
||||||
'PhabricatorConduitConsoleController' => 'applications/conduit/controller/PhabricatorConduitConsoleController.php',
|
'PhabricatorConduitConsoleController' => 'applications/conduit/controller/PhabricatorConduitConsoleController.php',
|
||||||
'PhabricatorConduitController' => 'applications/conduit/controller/PhabricatorConduitController.php',
|
'PhabricatorConduitController' => 'applications/conduit/controller/PhabricatorConduitController.php',
|
||||||
|
@ -4530,7 +4529,6 @@ phutil_register_library_map(array(
|
||||||
'PhabricatorConduitAPIController' => 'PhabricatorConduitController',
|
'PhabricatorConduitAPIController' => 'PhabricatorConduitController',
|
||||||
'PhabricatorConduitApplication' => 'PhabricatorApplication',
|
'PhabricatorConduitApplication' => 'PhabricatorApplication',
|
||||||
'PhabricatorConduitCertificateToken' => 'PhabricatorConduitDAO',
|
'PhabricatorConduitCertificateToken' => 'PhabricatorConduitDAO',
|
||||||
'PhabricatorConduitConfigOptions' => 'PhabricatorApplicationConfigOptions',
|
|
||||||
'PhabricatorConduitConnectionLog' => 'PhabricatorConduitDAO',
|
'PhabricatorConduitConnectionLog' => 'PhabricatorConduitDAO',
|
||||||
'PhabricatorConduitConsoleController' => 'PhabricatorConduitController',
|
'PhabricatorConduitConsoleController' => 'PhabricatorConduitController',
|
||||||
'PhabricatorConduitController' => 'PhabricatorController',
|
'PhabricatorConduitController' => 'PhabricatorController',
|
||||||
|
|
|
@ -107,7 +107,6 @@ abstract class AphrontApplicationConfiguration {
|
||||||
$prod_uri = PhabricatorEnv::getEnvConfig('phabricator.production-uri');
|
$prod_uri = PhabricatorEnv::getEnvConfig('phabricator.production-uri');
|
||||||
$file_uri = PhabricatorEnv::getEnvConfig(
|
$file_uri = PhabricatorEnv::getEnvConfig(
|
||||||
'security.alternate-file-domain');
|
'security.alternate-file-domain');
|
||||||
$conduit_uris = PhabricatorEnv::getEnvConfig('conduit.servers');
|
|
||||||
$allowed_uris = PhabricatorEnv::getEnvConfig('phabricator.allowed-uris');
|
$allowed_uris = PhabricatorEnv::getEnvConfig('phabricator.allowed-uris');
|
||||||
|
|
||||||
$uris = array_merge(
|
$uris = array_merge(
|
||||||
|
@ -115,7 +114,6 @@ abstract class AphrontApplicationConfiguration {
|
||||||
$base_uri,
|
$base_uri,
|
||||||
$prod_uri,
|
$prod_uri,
|
||||||
),
|
),
|
||||||
$conduit_uris,
|
|
||||||
$allowed_uris);
|
$allowed_uris);
|
||||||
|
|
||||||
$cdn_routes = array(
|
$cdn_routes = array(
|
||||||
|
|
|
@ -13,14 +13,10 @@ final class ConduitCall {
|
||||||
private $method;
|
private $method;
|
||||||
private $request;
|
private $request;
|
||||||
private $user;
|
private $user;
|
||||||
private $servers;
|
|
||||||
private $forceLocal;
|
|
||||||
|
|
||||||
public function __construct($method, array $params) {
|
public function __construct($method, array $params) {
|
||||||
$this->method = $method;
|
$this->method = $method;
|
||||||
$this->handler = $this->buildMethodHandler($method);
|
$this->handler = $this->buildMethodHandler($method);
|
||||||
$this->servers = PhabricatorEnv::getEnvConfig('conduit.servers');
|
|
||||||
$this->forceLocal = false;
|
|
||||||
|
|
||||||
$invalid_params = array_diff_key(
|
$invalid_params = array_diff_key(
|
||||||
$params,
|
$params,
|
||||||
|
@ -31,16 +27,6 @@ final class ConduitCall {
|
||||||
implode("', '", array_keys($invalid_params))."'.");
|
implode("', '", array_keys($invalid_params))."'.");
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->servers) {
|
|
||||||
$current_host = AphrontRequest::getHTTPHeader('HOST');
|
|
||||||
foreach ($this->servers as $server) {
|
|
||||||
if ($current_host === id(new PhutilURI($server))->getDomain()) {
|
|
||||||
$this->forceLocal = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
$this->request = new ConduitAPIRequest($params);
|
$this->request = new ConduitAPIRequest($params);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -53,15 +39,6 @@ final class ConduitCall {
|
||||||
return $this->user;
|
return $this->user;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function setForceLocal($force_local) {
|
|
||||||
$this->forceLocal = $force_local;
|
|
||||||
return $this;
|
|
||||||
}
|
|
||||||
|
|
||||||
public function shouldForceLocal() {
|
|
||||||
return $this->forceLocal;
|
|
||||||
}
|
|
||||||
|
|
||||||
public function shouldRequireAuthentication() {
|
public function shouldRequireAuthentication() {
|
||||||
return $this->handler->shouldRequireAuthentication();
|
return $this->handler->shouldRequireAuthentication();
|
||||||
}
|
}
|
||||||
|
@ -137,35 +114,7 @@ final class ConduitCall {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!$this->shouldForceLocal() && $this->servers) {
|
return $this->handler->executeMethod($this->request);
|
||||||
$server = $this->pickRandomServer($this->servers);
|
|
||||||
$client = new ConduitClient($server);
|
|
||||||
$params = $this->request->getAllParameters();
|
|
||||||
|
|
||||||
$params['__conduit__']['isProxied'] = true;
|
|
||||||
|
|
||||||
if ($this->handler->shouldRequireAuthentication()) {
|
|
||||||
$client->callMethodSynchronous(
|
|
||||||
'conduit.connect',
|
|
||||||
array(
|
|
||||||
'client' => 'PhabricatorConduit',
|
|
||||||
'clientVersion' => '1.0',
|
|
||||||
'user' => $this->getUser()->getUserName(),
|
|
||||||
'certificate' => $this->getUser()->getConduitCertificate(),
|
|
||||||
'__conduit__' => $params['__conduit__'],
|
|
||||||
));
|
|
||||||
}
|
|
||||||
|
|
||||||
return $client->callMethodSynchronous(
|
|
||||||
$this->method,
|
|
||||||
$params);
|
|
||||||
} else {
|
|
||||||
return $this->handler->executeMethod($this->request);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
protected function pickRandomServer($servers) {
|
|
||||||
return $servers[array_rand($servers)];
|
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function buildMethodHandler($method_name) {
|
protected function buildMethodHandler($method_name) {
|
||||||
|
|
|
@ -4,7 +4,6 @@ final class ConduitCallTestCase extends PhabricatorTestCase {
|
||||||
|
|
||||||
public function testConduitPing() {
|
public function testConduitPing() {
|
||||||
$call = new ConduitCall('conduit.ping', array());
|
$call = new ConduitCall('conduit.ping', array());
|
||||||
$call->setForceLocal(true);
|
|
||||||
$result = $call->execute();
|
$result = $call->execute();
|
||||||
|
|
||||||
$this->assertFalse(empty($result));
|
$this->assertFalse(empty($result));
|
||||||
|
@ -12,7 +11,6 @@ final class ConduitCallTestCase extends PhabricatorTestCase {
|
||||||
|
|
||||||
public function testConduitAuth() {
|
public function testConduitAuth() {
|
||||||
$call = new ConduitCall('user.whoami', array(), true);
|
$call = new ConduitCall('user.whoami', array(), true);
|
||||||
$call->setForceLocal(true);
|
|
||||||
|
|
||||||
$caught = null;
|
$caught = null;
|
||||||
try {
|
try {
|
||||||
|
|
|
@ -1,31 +0,0 @@
|
||||||
<?php
|
|
||||||
|
|
||||||
final class PhabricatorConduitConfigOptions
|
|
||||||
extends PhabricatorApplicationConfigOptions {
|
|
||||||
|
|
||||||
public function getName() {
|
|
||||||
return pht('Conduit');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function getDescription() {
|
|
||||||
return pht('Configure conduit.');
|
|
||||||
}
|
|
||||||
|
|
||||||
public function getOptions() {
|
|
||||||
return array(
|
|
||||||
$this->newOption('conduit.servers', 'list<string>', array())
|
|
||||||
->setLocked(true)
|
|
||||||
->setSummary(pht('Servers that conduit can connect to.'))
|
|
||||||
->setDescription(
|
|
||||||
pht(
|
|
||||||
"Set an array of servers where conduit can connect to. This is ".
|
|
||||||
"an advanced option. Don't touch this unless you know what you ".
|
|
||||||
"are doing."))
|
|
||||||
->addExample(
|
|
||||||
'["http://phabricator2.example.com/", '.
|
|
||||||
'"http://phabricator3.example.com/]"',
|
|
||||||
pht('Valid Setting')),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
}
|
|
Loading…
Reference in a new issue