From 1898864b6c453076da02dfe15f7b831e852627d1 Mon Sep 17 00:00:00 2001 From: Aviv Eyal Date: Wed, 4 Nov 2015 18:32:18 +0000 Subject: [PATCH] add initiator.phid parameter to HM builds Summary: Fix T9662. Record who initiated the build, and allow this information as a parameter. In this implementation, a 're-run' keeps the original initiator, which we maybe not desired? Test Plan: Make a HTTP step with initiator.phid, trigger manually, via HM, via ./bin/harbormaster build. Look at requests made. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Maniphest Tasks: T9662 Differential Revision: https://secure.phabricator.com/D14380 --- .../autopatches/20151030.harbormaster.initiator.sql | 2 ++ .../controller/HarbormasterPlanRunController.php | 2 +- .../harbormaster/engine/HarbormasterBuildRequest.php | 10 ++++++++++ .../harbormaster/engine/HarbormasterTargetEngine.php | 8 ++++++-- .../herald/HarbormasterRunBuildPlansHeraldAction.php | 7 ++++--- .../management/HarbormasterManagementBuildWorkflow.php | 7 ++++++- .../harbormaster/storage/HarbormasterBuildable.php | 10 ++++++++-- .../harbormaster/storage/build/HarbormasterBuild.php | 7 +++++++ 8 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 resources/sql/autopatches/20151030.harbormaster.initiator.sql diff --git a/resources/sql/autopatches/20151030.harbormaster.initiator.sql b/resources/sql/autopatches/20151030.harbormaster.initiator.sql new file mode 100644 index 0000000000..f8ba3f6757 --- /dev/null +++ b/resources/sql/autopatches/20151030.harbormaster.initiator.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_build + ADD COLUMN initiatorPHID VARBINARY(64); diff --git a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php index 6655fd6806..fb664dc084 100644 --- a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php @@ -59,7 +59,7 @@ final class HarbormasterPlanRunController extends HarbormasterController { if (!$errors) { $buildable->save(); - $buildable->applyPlan($plan, array()); + $buildable->applyPlan($plan, array(), $viewer->getPHID()); $buildable_uri = '/B'.$buildable->getID(); return id(new AphrontRedirectResponse())->setURI($buildable_uri); diff --git a/src/applications/harbormaster/engine/HarbormasterBuildRequest.php b/src/applications/harbormaster/engine/HarbormasterBuildRequest.php index 1874f08b27..71721fdaf2 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildRequest.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildRequest.php @@ -14,6 +14,7 @@ final class HarbormasterBuildRequest extends Phobject { private $buildPlanPHID; + private $initiatorPHID; private $buildParameters = array(); public function setBuildPlanPHID($build_plan_phid) { @@ -34,4 +35,13 @@ final class HarbormasterBuildRequest extends Phobject { return $this->buildParameters; } + public function setInitiatorPHID($phid) { + $this->initiatorPHID = $phid; + return $this; + } + + public function getInitiatorPHID() { + return $this->initiatorPHID; + } + } diff --git a/src/applications/harbormaster/engine/HarbormasterTargetEngine.php b/src/applications/harbormaster/engine/HarbormasterTargetEngine.php index b201891b6b..d9efb8b9fd 100644 --- a/src/applications/harbormaster/engine/HarbormasterTargetEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterTargetEngine.php @@ -6,7 +6,7 @@ final class HarbormasterTargetEngine extends Phobject { private $object; private $autoTargetKeys; - public function setViewer($viewer) { + public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; return $this; } @@ -163,6 +163,10 @@ final class HarbormasterTargetEngine extends Phobject { array $step_map) { $viewer = $this->getViewer(); + $initiator_phid = null; + if (!$viewer->isOmnipotent()) { + $initiator_phid = $viewer->getPHID(); + } $plan_map = mgroup($step_map, 'getBuildPlanPHID'); $builds = id(new HarbormasterBuildQuery()) @@ -206,7 +210,7 @@ final class HarbormasterTargetEngine extends Phobject { // resource and "own" it, so we don't try to handle this, but may need // to be more careful here if use of autotargets expands. - $build = $buildable->applyPlan($plan, array()); + $build = $buildable->applyPlan($plan, array(), $initiator_phid); PhabricatorWorker::setRunAllTasksInProcess(false); } catch (Exception $ex) { PhabricatorWorker::setRunAllTasksInProcess(false); diff --git a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php index de399e5976..76bdf9ccc3 100644 --- a/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php +++ b/src/applications/harbormaster/herald/HarbormasterRunBuildPlansHeraldAction.php @@ -16,7 +16,7 @@ final class HarbormasterRunBuildPlansHeraldAction return ($adapter instanceof HarbormasterBuildableAdapterInterface); } - protected function applyBuilds(array $phids) { + protected function applyBuilds(array $phids, HeraldRule $rule) { $adapter = $this->getAdapter(); $allowed_types = array( @@ -32,7 +32,8 @@ final class HarbormasterRunBuildPlansHeraldAction foreach ($phids as $phid) { $request = id(new HarbormasterBuildRequest()) - ->setBuildPlanPHID($phid); + ->setBuildPlanPHID($phid) + ->setInitiatorPHID($rule->getPHID()); $adapter->queueHarbormasterBuildRequest($request); } @@ -68,7 +69,7 @@ final class HarbormasterRunBuildPlansHeraldAction } public function applyEffect($object, HeraldEffect $effect) { - return $this->applyBuilds($effect->getTarget()); + return $this->applyBuilds($effect->getTarget(), $effect->getRule()); } public function getHeraldActionStandardType() { diff --git a/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php b/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php index 7110d98d6e..6fd3cf2ffd 100644 --- a/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php +++ b/src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php @@ -98,7 +98,12 @@ final class HarbormasterManagementBuildWorkflow PhabricatorWorker::setRunAllTasksInProcess(true); } - $buildable->applyPlan($plan, array()); + if ($viewer->isOmnipotent()) { + $initiator = id(new PhabricatorHarbormasterApplication())->getPHID(); + } else { + $initiator = $viewer->getPHID(); + } + $buildable->applyPlan($plan, array(), $initiator); $console->writeOut("%s\n", pht('Done.')); diff --git a/src/applications/harbormaster/storage/HarbormasterBuildable.php b/src/applications/harbormaster/storage/HarbormasterBuildable.php index e74471e879..3a7df73f52 100644 --- a/src/applications/harbormaster/storage/HarbormasterBuildable.php +++ b/src/applications/harbormaster/storage/HarbormasterBuildable.php @@ -154,11 +154,14 @@ final class HarbormasterBuildable extends HarbormasterDAO } $parameters = $request->getBuildParameters(); - $buildable->applyPlan($plan, $parameters); + $buildable->applyPlan($plan, $parameters, $request->getInitiatorPHID()); } } - public function applyPlan(HarbormasterBuildPlan $plan, array $parameters) { + public function applyPlan( + HarbormasterBuildPlan $plan, + array $parameters, + $initiator_phid) { $viewer = PhabricatorUser::getOmnipotentUser(); $build = HarbormasterBuild::initializeNewBuild($viewer) @@ -166,6 +169,9 @@ final class HarbormasterBuildable extends HarbormasterDAO ->setBuildPlanPHID($plan->getPHID()) ->setBuildParameters($parameters) ->setBuildStatus(HarbormasterBuild::STATUS_PENDING); + if ($initiator_phid) { + $build->setInitiatorPHID($initiator_phid); + } $auto_key = $plan->getPlanAutoKey(); if ($auto_key) { diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 154681d93f..c96d0c2710 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -10,6 +10,7 @@ final class HarbormasterBuild extends HarbormasterDAO protected $buildStatus; protected $buildGeneration; protected $buildParameters = array(); + protected $initiatorPHID; protected $planAutoKey; private $buildable = self::ATTACHABLE; @@ -164,6 +165,7 @@ final class HarbormasterBuild extends HarbormasterDAO 'buildStatus' => 'text32', 'buildGeneration' => 'uint32', 'planAutoKey' => 'text32?', + 'initiatorPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_buildable' => array( @@ -260,6 +262,7 @@ final class HarbormasterBuild extends HarbormasterDAO 'repository.uri' => null, 'step.timestamp' => null, 'build.id' => null, + 'initiator.phid' => null, ); foreach ($this->getBuildParameters() as $key => $value) { @@ -275,6 +278,7 @@ final class HarbormasterBuild extends HarbormasterDAO $results['step.timestamp'] = time(); $results['build.id'] = $this->getID(); + $results['initiator.phid'] = $this->getInitiatorPHID(); return $results; } @@ -289,6 +293,9 @@ final class HarbormasterBuild extends HarbormasterDAO 'step.timestamp' => pht('The current UNIX timestamp.'), 'build.id' => pht('The ID of the current build.'), 'target.phid' => pht('The PHID of the current build target.'), + 'initiator.phid' => pht( + 'The PHID of the user or Object that initiated the build, '. + 'if applicable.'), ); foreach ($objects as $object) {