From d8550c114d70e363c70be9b9777419322532622c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 27 Jan 2015 14:51:48 -0800 Subject: [PATCH] Promote instance identity to the upstream and pass it to commit hooks Summary: Fixes T7019. In a cluster environment, pushes currently fail because the commit hook can't identify the instance. For web processes, the hostname identifies the instance -- but we don't have a hostname in the hook. For CLI processes, the environment identifies the instance -- but we don't have an environment in the hook under SVN. Promote the instance identifier into the upstream and pack/unpack it explicitly for hooks. This is probably not useful for anyone but us, but the amount of special-purpose code we're introducing is very small. I poked at trying to do this in a more general way, but: - We MUST know this BEFORE we run code, so the normal subclassing stuff is useless. - I couldn't come up with any other parameter which might ever be useful to pass in. Test Plan: Used `git push` to push code through proxied HTTP, got a clean push. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7019 Differential Revision: https://secure.phabricator.com/D11495 --- scripts/repository/commit_hook.php | 19 +++++++++++++++++ .../PhabricatorClusterConfigOptions.php | 12 +++++++++++ .../PhabricatorRepositoryPullEngine.php | 21 +++++++++++++++---- src/infrastructure/env/PhabricatorEnv.php | 9 ++++++++ 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/scripts/repository/commit_hook.php b/scripts/repository/commit_hook.php index 2c7fe4289b..4132df941e 100755 --- a/scripts/repository/commit_hook.php +++ b/scripts/repository/commit_hook.php @@ -1,6 +1,25 @@ #!/usr/bin/env php 1) { + $context = $argv[1]; + $context = explode(':', $context, 2); + $argv[1] = $context[0]; + + if (count($context) > 1) { + $_ENV['PHABRICATOR_INSTANCE'] = $context[1]; + putenv('PHABRICATOR_INSTANCE='.$context[1]); + } +} + $root = dirname(dirname(dirname(__FILE__))); require_once $root.'/scripts/__init_script__.php'; diff --git a/src/applications/config/option/PhabricatorClusterConfigOptions.php b/src/applications/config/option/PhabricatorClusterConfigOptions.php index 830208823d..23d0827c18 100644 --- a/src/applications/config/option/PhabricatorClusterConfigOptions.php +++ b/src/applications/config/option/PhabricatorClusterConfigOptions.php @@ -52,6 +52,18 @@ final class PhabricatorClusterConfigOptions '0.0.0.0/0', ), pht('Allow Any Host (Insecure!)')), + $this->newOption('cluster.instance', 'string', null) + ->setLocked(true) + ->setSummary(pht('Instance identifier for multi-tenant clusters.')) + ->setDescription( + pht( + 'WARNING: This is a very advanced option, and only useful for '. + 'hosting providers running multi-tenant clusters.'. + "\n\n". + 'If you provide an instance identifier here (normally by '. + 'injecting it with a `PhabricatorConfigSiteSource`), Phabricator '. + 'will pass it to subprocesses and commit hooks in the '. + '`PHABRICATOR_INSTANCE` environmental variable.')), ); } diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php index 8ac193fad7..6acc21fe05 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -161,7 +161,7 @@ final class PhabricatorRepositoryPullEngine $this->log('%s', pht('Installing commit hook to "%s"...', $path)); $repository = $this->getRepository(); - $callsign = $repository->getCallsign(); + $identifier = $this->getHookContextIdentifier($repository); $root = dirname(phutil_get_library_root('phabricator')); $bin = $root.'/bin/commit-hook'; @@ -171,7 +171,7 @@ final class PhabricatorRepositoryPullEngine 'exec %s -f %s -- %s "$@"', $full_php_path, $bin, - $callsign); + $identifier); $hook = "#!/bin/sh\nexport TERM=dumb\n{$cmd}\n"; @@ -190,6 +190,17 @@ final class PhabricatorRepositoryPullEngine Filesystem::writeFile($path.'/README', $readme); } + private function getHookContextIdentifier(PhabricatorRepository $repository) { + $identifier = $repository->getCallsign(); + + $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); + if (strlen($instance)) { + $identifier = "{$identifier}:{$instance}"; + } + + return $identifier; + } + /* -( Pulling Git Working Copies )----------------------------------------- */ @@ -412,6 +423,8 @@ final class PhabricatorRepositoryPullEngine $repository = $this->getRepository(); $path = $repository->getLocalPath().'/.hg/hgrc'; + $identifier = $this->getHookContextIdentifier($repository); + $root = dirname(phutil_get_library_root('phabricator')); $bin = $root.'/bin/commit-hook'; @@ -422,14 +435,14 @@ final class PhabricatorRepositoryPullEngine $data[] = csprintf( 'pretxnchangegroup.phabricator = %s %s %s', $bin, - $repository->getCallsign(), + $identifier, 'pretxnchangegroup'); // This one handles creating bookmarks. $data[] = csprintf( 'prepushkey.phabricator = %s %s %s', $bin, - $repository->getCallsign(), + $identifier, 'prepushkey'); $data[] = null; diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index ae40ba4a5c..7e266ef469 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -112,6 +112,15 @@ final class PhabricatorEnv { // subprocess environments. $_ENV['PATH'] = $env_path; + + // If an instance identifier is defined, write it into the environment so + // it's available to subprocesses. + $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); + if (strlen($instance)) { + putenv('PHABRICATOR_INSTANCE='.$instance); + $_ENV['PHABRICATOR_INSTANCE'] = $instance; + } + PhabricatorEventEngine::initialize(); $translation = PhabricatorEnv::newObjectFromConfig('translation.provider');