From f74e6bbf8d70e957cebb2a47f30f9e3c9c301338 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Feb 2018 11:30:25 -0800 Subject: [PATCH] Make "phabricator.silent" disable build steps which rely on external services Summary: Depends on D19084. Fixes T13078. When `phabricator.silent` is enabled, immediately fail the "HTTP Request", "CircleCI" and "Buildkite" build steps. This doesn't feel quite as clean as most of the other behavior of `phabricator.silent`, since these calls are not exactly notifications in the same way that email is, and failing to make these calls means that builds run differently (whereas failing to deliver email doesn't really do anything). However, I suspect that this behavior is almost always reasonable/correct, and that we can probably get away with it until this grey area between "notifications" and "external service calls" is more clearly defined. Test Plan: - Created a build with HTTP, CircleCI, and Buildkite steps. - Put install in `phabricator.silent` mode: all three steps failed with "declining, because silent" messages. - Put install back in normal mode: all three steps made HTTP requests. - Read updated documentation. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13078 Differential Revision: https://secure.phabricator.com/D19085 --- .../option/PhabricatorCoreConfigOptions.php | 34 +++++++++++-------- .../HarbormasterBuildStepImplementation.php | 12 +++++++ ...masterBuildkiteBuildStepImplementation.php | 5 +++ ...rmasterCircleCIBuildStepImplementation.php | 5 +++ ...sterHTTPRequestBuildStepImplementation.php | 6 ++++ 5 files changed, 47 insertions(+), 15 deletions(-) diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php index 6efd27d9a3..0c9e69ebf2 100644 --- a/src/applications/config/option/PhabricatorCoreConfigOptions.php +++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php @@ -37,6 +37,24 @@ final class PhabricatorCoreConfigOptions $proto_doc_name = pht('User Guide: Prototype Applications'); $applications_app_href = '/applications/'; + $silent_description = $this->deformat(pht(<<newOption('phabricator.base-uri', 'string', null) ->setLocked(true) @@ -232,21 +250,7 @@ final class PhabricatorCoreConfigOptions pht('Run Normally'), )) ->setSummary(pht('Stop Phabricator from sending any email, etc.')) - ->setDescription( - pht( - 'This option allows you to stop Phabricator from sending '. - 'any data to external services. Among other things, it will '. - 'disable email, SMS, repository mirroring, and HTTP hooks.'. - "\n\n". - 'This option is intended to allow a Phabricator instance to '. - 'be exported, copied, imported, and run in a test environment '. - 'without impacting users. For example, if you are migrating '. - 'to new hardware, you could perform a test migration first, '. - 'make sure things work, and then do a production cutover '. - 'later with higher confidence and less disruption. Without '. - 'this flag, users would receive duplicate email during the '. - 'time the test instance and old production instance were '. - 'both in operation.')), + ->setDescription($silent_description), ); } diff --git a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php index ca6f30bd36..47af992630 100644 --- a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php @@ -295,6 +295,18 @@ abstract class HarbormasterBuildStepImplementation extends Phobject { ->append($body); } + protected function logSilencedCall( + HarbormasterBuild $build, + HarbormasterBuildTarget $build_target, + $label) { + + $build_target + ->newLog($label, 'silenced') + ->append( + pht( + 'Declining to make service call because `phabricator.silent` is '. + 'enabled in configuration.')); + } /* -( Automatic Targets )-------------------------------------------------- */ diff --git a/src/applications/harbormaster/step/HarbormasterBuildkiteBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildkiteBuildStepImplementation.php index 3ecd6553da..89d4002eef 100644 --- a/src/applications/harbormaster/step/HarbormasterBuildkiteBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterBuildkiteBuildStepImplementation.php @@ -72,6 +72,11 @@ EOTEXT HarbormasterBuildTarget $build_target) { $viewer = PhabricatorUser::getOmnipotentUser(); + if (PhabricatorEnv::getEnvConfig('phabricator.silent')) { + $this->logSilencedCall($build, $build_target, pht('Buildkite')); + throw new HarbormasterBuildFailureException(); + } + $buildable = $build->getBuildable(); $object = $buildable->getBuildableObject(); diff --git a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php index e74e25a395..f0104779ff 100644 --- a/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterCircleCIBuildStepImplementation.php @@ -84,6 +84,11 @@ EOTEXT HarbormasterBuildTarget $build_target) { $viewer = PhabricatorUser::getOmnipotentUser(); + if (PhabricatorEnv::getEnvConfig('phabricator.silent')) { + $this->logSilencedCall($build, $build_target, pht('CircleCI')); + throw new HarbormasterBuildFailureException(); + } + $buildable = $build->getBuildable(); $object = $buildable->getBuildableObject(); diff --git a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php index 016f29642e..59866d3b73 100644 --- a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php @@ -43,6 +43,12 @@ final class HarbormasterHTTPRequestBuildStepImplementation HarbormasterBuildTarget $build_target) { $viewer = PhabricatorUser::getOmnipotentUser(); + + if (PhabricatorEnv::getEnvConfig('phabricator.silent')) { + $this->logSilencedCall($build, $build_target, pht('HTTP Request')); + throw new HarbormasterBuildFailureException(); + } + $settings = $this->getSettings(); $variables = $build_target->getVariables();