Migrate "Run Command" to use Drydock hosts

Summary: This migrates the "Run Remote Command" build step over to use Drydock hosts and Harbormaster artifacts.

Test Plan:
Created a build plan with a "Lease Host" step and a "Run Command" step.  Configured the "Run Command" step to use the artifact from the "Lease Host" step.

Saw the results:

{F87377}

{F87378}

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049, T4111

Differential Revision: https://secure.phabricator.com/D7707

src/__phutil_library_map__.php

@@ -102,6 +102,7 @@ phutil_register_library_map(array(
     'CelerityResourceTransformerTestCase' => 'infrastructure/celerity/__tests__/CelerityResourceTransformerTestCase.php',
     'CeleritySpriteGenerator' => 'infrastructure/celerity/CeleritySpriteGenerator.php',
     'CelerityStaticResourceResponse' => 'infrastructure/celerity/CelerityStaticResourceResponse.php',
+    'CommandBuildStepImplementation' => 'applications/harbormaster/step/CommandBuildStepImplementation.php',
     'ConduitAPIMethod' => 'applications/conduit/method/ConduitAPIMethod.php',
     'ConduitAPIRequest' => 'applications/conduit/protocol/ConduitAPIRequest.php',
     'ConduitAPIResponse' => 'applications/conduit/protocol/ConduitAPIResponse.php',
@@ -2322,7 +2323,6 @@ phutil_register_library_map(array(
     'ReleephStatusFieldSpecification' => 'applications/releeph/field/specification/ReleephStatusFieldSpecification.php',
     'ReleephSummaryFieldSpecification' => 'applications/releeph/field/specification/ReleephSummaryFieldSpecification.php',
     'ReleephUserView' => 'applications/releeph/view/user/ReleephUserView.php',
-    'RemoteCommandBuildStepImplementation' => 'applications/harbormaster/step/RemoteCommandBuildStepImplementation.php',
     'ShellLogView' => 'applications/harbormaster/view/ShellLogView.php',
     'SleepBuildStepImplementation' => 'applications/harbormaster/step/SleepBuildStepImplementation.php',
     'SlowvoteEmbedView' => 'applications/slowvote/view/SlowvoteEmbedView.php',
@@ -2442,6 +2442,7 @@ phutil_register_library_map(array(
     'CelerityResourceController' => 'PhabricatorController',
     'CelerityResourceGraph' => 'AbstractDirectedGraph',
     'CelerityResourceTransformerTestCase' => 'PhabricatorTestCase',
+    'CommandBuildStepImplementation' => 'VariableBuildStepImplementation',
     'ConduitAPIMethod' =>
     array(
       0 => 'Phobject',
@@ -4950,7 +4951,6 @@ phutil_register_library_map(array(
     'ReleephStatusFieldSpecification' => 'ReleephFieldSpecification',
     'ReleephSummaryFieldSpecification' => 'ReleephFieldSpecification',
     'ReleephUserView' => 'AphrontView',
-    'RemoteCommandBuildStepImplementation' => 'VariableBuildStepImplementation',
     'ShellLogView' => 'AphrontView',
     'SleepBuildStepImplementation' => 'BuildStepImplementation',
     'SlowvoteEmbedView' => 'AphrontView',

src/applications/harbormaster/config/PhabricatorHarbormasterConfigOptions.php

@@ -12,24 +12,7 @@ final class PhabricatorHarbormasterConfigOptions
   }
 
   public function getOptions() {
-    return array(
-      $this->newOption(
-        'harbormaster.temporary.hosts.whitelist',
-        'list<string>',
-        array())
-        ->setSummary('Temporary configuration value.')
-        ->setLocked(true)
-        ->setDescription(
-          pht(
-            "This specifies a whitelist of remote hosts that the \"Run ".
-            "Remote Command\" may connect to.  This is a temporary ".
-            "configuration option as Drydock is not yet available.".
-            "\n\n".
-            "**This configuration option will be removed in the future and ".
-            "your build configuration will no longer work when Drydock ".
-            "replaces this option.  There is ABSOLUTELY NO SUPPORT for ".
-            "using this functionality!**"))
-    );
+    return array();
   }
 
 }

src/applications/harbormaster/controller/HarbormasterBuildViewController.php

@@ -73,10 +73,10 @@ final class HarbormasterBuildViewController
         ->setHeader($header)
         ->addPropertyList($properties);
 
+      $targets[] = $this->buildArtifacts($build_target);
       $targets[] = $this->buildLog($build, $build_target);
     }
 
-
     return $this->buildApplicationPage(
       array(
         $crumbs,
@@ -89,6 +89,35 @@ final class HarbormasterBuildViewController
       ));
   }
 
+  private function buildArtifacts(HarbormasterBuildTarget $build_target) {
+    $request = $this->getRequest();
+    $viewer = $request->getUser();
+
+    $artifacts = id(new HarbormasterBuildArtifactQuery())
+      ->setViewer($viewer)
+      ->withBuildTargetPHIDs(array($build_target->getPHID()))
+      ->execute();
+
+    if (count($artifacts) === 0) {
+      return null;
+    }
+
+    $list = new PHUIObjectItemListView();
+
+    foreach ($artifacts as $artifact) {
+      $list->addItem($artifact->getObjectItemView($viewer));
+    }
+
+    $header = id(new PHUIHeaderView())
+      ->setHeader(pht('Build Artifacts'))
+      ->setUser($viewer);
+
+    $box = id(new PHUIObjectBoxView())
+      ->setHeader($header);
+
+    return array($box, $list);
+  }
+
   private function buildLog(
     HarbormasterBuild $build,
     HarbormasterBuildTarget $build_target) {

src/applications/harbormaster/controller/HarbormasterStepEditController.php

@@ -91,6 +91,23 @@ final class HarbormasterStepEditController
             ->setName($name)
             ->setValue($value);
           break;
+        case BuildStepImplementation::SETTING_TYPE_ARTIFACT:
+          $filter = $opt['artifact_type'];
+          $available_artifacts =
+            BuildStepImplementation::getAvailableArtifacts(
+              $plan,
+              $step,
+              $filter);
+          $options = array();
+          foreach ($available_artifacts as $key => $type) {
+            $options[$key] = $key;
+          }
+          $control = id(new AphrontFormSelectControl())
+            ->setLabel($this->getReadableName($name, $opt))
+            ->setName($name)
+            ->setValue($value)
+            ->setOptions($options);
+          break;
         default:
           throw new Exception("Unable to render field with unknown type.");
       }
@@ -145,6 +162,7 @@ final class HarbormasterStepEditController
   public function getValueFromRequest(AphrontRequest $request, $name, $type) {
     switch ($type) {
       case BuildStepImplementation::SETTING_TYPE_STRING:
+      case BuildStepImplementation::SETTING_TYPE_ARTIFACT:
         return $request->getStr($name);
         break;
       case BuildStepImplementation::SETTING_TYPE_INTEGER:

src/applications/harbormaster/query/HarbormasterBuildArtifactQuery.php

@@ -7,6 +7,7 @@ final class HarbormasterBuildArtifactQuery
   private $buildTargetPHIDs;
   private $artifactTypes;
   private $artifactKeys;
+  private $keyBuildPHID;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -23,7 +24,8 @@ final class HarbormasterBuildArtifactQuery
     return $this;
   }
 
-  public function withArtifactKeys(array $artifact_keys) {
+  public function withArtifactKeys($build_phid, array $artifact_keys) {
+    $this->keyBuildPHID = $build_phid;
     $this->artifactKeys = $artifact_keys;
     return $this;
   }
@@ -95,7 +97,8 @@ final class HarbormasterBuildArtifactQuery
     if ($this->artifactKeys) {
       $indexes = array();
       foreach ($this->artifactKeys as $key) {
-        $indexes[] = PhabricatorHash::digestForIndex($key);
+        $indexes[] = PhabricatorHash::digestForIndex(
+          $this->keyBuildPHID.$key);
       }
 
       $where[] = qsprintf(

src/applications/harbormaster/step/BuildStepImplementation.php

@@ -7,6 +7,7 @@ abstract class BuildStepImplementation {
   const SETTING_TYPE_STRING = 'string';
   const SETTING_TYPE_INTEGER = 'integer';
   const SETTING_TYPE_BOOLEAN = 'boolean';
+  const SETTING_TYPE_ARTIFACT = 'artifact';
 
   public static function getImplementations() {
     $symbols = id(new PhutilSymbolLoader())
@@ -117,23 +118,18 @@ abstract class BuildStepImplementation {
    * Returns a list of all artifacts made available by previous build steps.
    */
   public static function getAvailableArtifacts(
+    HarbormasterBuildPlan $build_plan,
     HarbormasterBuildStep $current_build_step,
     $artifact_type) {
 
-    $build_plan_phid = $current_build_step->getBuildPlanPHID();
-
-    $build_plan = id(new HarbormasterBuildPlanQuery())
-      ->withPHIDs(array($build_plan_phid))
-      ->executeOne();
-
     $build_steps = $build_plan->loadOrderedBuildSteps();
 
     $previous_implementations = array();
-    for ($i = 0; $i < count($build_steps); $i++) {
-      if ($build_steps[$i]->getPHID() === $current_build_step->getPHID()) {
+    foreach ($build_steps as $build_step) {
+      if ($build_step->getPHID() === $current_build_step->getPHID()) {
         break;
       }
-      $previous_implementations[$i] = $build_steps[$i]->getStepImplementation();
+      $previous_implementations[] = $build_step->getStepImplementation();
     }
 
     $artifact_arrays = mpull($previous_implementations, 'getArtifactMappings');

src/applications/harbormaster/step/CommandBuildStepImplementation.php

@@ -1,14 +1,14 @@
 <?php
 
-final class RemoteCommandBuildStepImplementation
+final class CommandBuildStepImplementation
   extends VariableBuildStepImplementation {
 
   public function getName() {
-    return pht('Run Remote Command');
+    return pht('Run Command');
   }
 
   public function getGenericDescription() {
-    return pht('Run a command on another machine.');
+    return pht('Run a command on Drydock host.');
   }
 
   public function getDescription() {
@@ -17,7 +17,7 @@ final class RemoteCommandBuildStepImplementation
     return pht(
       'Run \'%s\' on \'%s\'.',
       $settings['command'],
-      $settings['sshhost']);
+      $settings['hostartifact']);
   }
 
   public function execute(
@@ -32,22 +32,35 @@ final class RemoteCommandBuildStepImplementation
       $settings['command'],
       $variables);
 
-    $future = null;
-    if (empty($settings['sshkey'])) {
-      $future = new ExecFuture(
-        'ssh -o "StrictHostKeyChecking no" -p %s %s %s',
-        $settings['sshport'],
-        $settings['sshuser'].'@'.$settings['sshhost'],
-        $command);
-    } else {
-      $future = new ExecFuture(
-        'ssh -o "StrictHostKeyChecking no" -p %s -i %s %s %s',
-        $settings['sshport'],
-        $settings['sshkey'],
-        $settings['sshuser'].'@'.$settings['sshhost'],
-        $command);
+    $artifact = id(new HarbormasterBuildArtifactQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withArtifactKeys(
+        $build->getPHID(),
+        array($settings['hostartifact']))
+      ->executeOne();
+    if ($artifact === null) {
+      throw new Exception("Associated Drydock host artifact not found!");
     }
 
+    $data = $artifact->getArtifactData();
+
+    // FIXME: Is there a better way of doing this?
+    $lease = id(new DrydockLease())->load(
+      $data['drydock-lease']);
+    if ($lease === null) {
+      throw new Exception("Associated Drydock lease not found!");
+    }
+    $resource = id(new DrydockResource())->load(
+      $lease->getResourceID());
+    if ($resource === null) {
+      throw new Exception("Associated Drydock resource not found!");
+    }
+    $lease->attachResource($resource);
+
+    $interface = $lease->getInterface('command');
+
+    $future = $interface->getExecFuture('%C', $command);
+
     $log_stdout = $build->createLog($build_target, "remote", "stdout");
     $log_stderr = $build->createLog($build_target, "remote", "stderr");
 
@@ -98,25 +111,12 @@ final class RemoteCommandBuildStepImplementation
     if ($settings['command'] === null || !is_string($settings['command'])) {
       return false;
     }
-    if ($settings['sshhost'] === null || !is_string($settings['sshhost'])) {
-      return false;
-    }
-    if ($settings['sshuser'] === null || !is_string($settings['sshuser'])) {
-      return false;
-    }
-    if ($settings['sshkey'] === null || !is_string($settings['sshkey'])) {
-      return false;
-    }
-    if ($settings['sshport'] === null || !is_int($settings['sshport']) ||
-        $settings['sshport'] <= 0 || $settings['sshport'] >= 65536) {
+    if ($settings['hostartifact'] === null ||
+      !is_string($settings['hostartifact'])) {
       return false;
     }
 
-    $whitelist = PhabricatorEnv::getEnvConfig(
-      'harbormaster.temporary.hosts.whitelist');
-    if (!in_array($settings['sshhost'], $whitelist)) {
-      return false;
-    }
+    // TODO: Check if the host artifact is provided by previous build steps.
 
     return true;
   }
@@ -127,25 +127,13 @@ final class RemoteCommandBuildStepImplementation
         'name' => 'Command',
         'description' => 'The command to execute on the remote machine.',
         'type' => BuildStepImplementation::SETTING_TYPE_STRING),
-      'sshhost' => array(
-        'name' => 'SSH Host',
-        'description' => 'The SSH host that the command will be run on.',
-        'type' => BuildStepImplementation::SETTING_TYPE_STRING),
-      'sshport' => array(
-        'name' => 'SSH Port',
-        'description' => 'The SSH port to connect to.',
-        'type' => BuildStepImplementation::SETTING_TYPE_INTEGER,
-        'default' => 22), // TODO: 'default' doesn't do anything yet..
-      'sshuser' => array(
-        'name' => 'SSH Username',
-        'description' => 'The SSH username to use.',
-        'type' => BuildStepImplementation::SETTING_TYPE_STRING),
-      'sshkey' => array(
-        'name' => 'SSH Identity File',
+      'hostartifact' => array(
+        'name' => 'Host Artifact',
         'description' =>
-          'The path to the SSH identity file (private key) '.
-          'on the local web server.',
-        'type' => BuildStepImplementation::SETTING_TYPE_STRING));
+          'The host artifact that determines what machine the command '.
+          'will run on.',
+        'type' => BuildStepImplementation::SETTING_TYPE_ARTIFACT,
+        'artifact_type' => HarbormasterBuildArtifact::TYPE_HOST));
   }
 
 }

src/applications/harbormaster/step/LeaseHostBuildStepImplementation.php

@@ -15,8 +15,10 @@ final class LeaseHostBuildStepImplementation
     $settings = $this->getSettings();
 
     return pht(
-      'Obtain a lease on a Drydock host whose platform is \'%s\'.',
-      $settings['platform']);
+      'Obtain a lease on a Drydock host whose platform is \'%s\' and store '.
+      'the resulting lease in a host artifact called \'%s\'.',
+      $settings['platform'],
+      $settings['name']);
   }
 
   public function execute(
@@ -41,12 +43,19 @@ final class LeaseHostBuildStepImplementation
     $artifact = $build->createArtifact(
       $build_target,
       $settings['name'],
-      'host');
+      HarbormasterBuildArtifact::TYPE_HOST);
     $artifact->setArtifactData(array(
       'drydock-lease' => $lease->getID()));
     $artifact->save();
   }
 
+  public function getArtifactMappings() {
+    $settings = $this->getSettings();
+
+    return array(
+      $settings['name'] => 'host');
+  }
+
   public function validateSettings() {
     $settings = $this->getSettings();
 

src/applications/harbormaster/step/VariableBuildStepImplementation.php

@@ -37,10 +37,11 @@ abstract class VariableBuildStepImplementation extends BuildStepImplementation {
   }
 
   public function getSettingRemarkupInstructions() {
+    $variables = HarbormasterBuild::getAvailableBuildVariables();
     $text = '';
     $text .= pht('The following variables are available: ')."\n";
     $text .= "\n";
-    foreach ($this->getAvailableVariables() as $name => $desc) {
+    foreach ($variables as $name => $desc) {
       $text .= '  - `'.$name.'`: '.$desc."\n";
     }
     $text .= "\n";

src/applications/harbormaster/storage/build/HarbormasterBuild.php

@@ -113,7 +113,7 @@ final class HarbormasterBuild extends HarbormasterDAO
 
     $artifact =
       HarbormasterBuildArtifact::initializeNewBuildArtifact($build_target);
-    $artifact->setArtifactKey($artifact_key);
+    $artifact->setArtifactKey($this->getPHID(), $artifact_key);
     $artifact->setArtifactType($artifact_type);
     $artifact->save();
     return $artifact;
@@ -172,7 +172,7 @@ final class HarbormasterBuild extends HarbormasterDAO
     return $results;
   }
 
-  public function getAvailableBuildVariables() {
+  public static function getAvailableBuildVariables() {
     return array(
       'buildable.diff' =>
         pht('The differential diff ID, if applicable.'),

src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php

@@ -9,6 +9,11 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO
   protected $artifactKey;
   protected $artifactData = array();
 
+  private $buildTarget = self::ATTACHABLE;
+
+  const TYPE_FILE = 'file';
+  const TYPE_HOST = 'host';
+
   public static function initializeNewBuildArtifact(
     HarbormasterBuildTarget $build_target) {
     return id(new HarbormasterBuildArtifact())
@@ -23,22 +28,49 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO
     ) + parent::getConfiguration();
   }
 
-  public function attachBuildable(HarbormasterBuildable $buildable) {
-    $this->buildable = $buildable;
+  public function attachBuildTarget(HarbormasterBuildTarget $build_target) {
+    $this->buildTarget = $build_target;
     return $this;
   }
 
-  public function getBuildable() {
-    return $this->assertAttached($this->buildable);
+  public function getBuildTarget() {
+    return $this->assertAttached($this->buildTarget);
   }
 
-  public function setArtifactKey($key) {
+  public function setArtifactKey($build_phid, $key) {
     $this->artifactIndex =
-      PhabricatorHash::digestForIndex($this->buildTargetPHID.$key);
+      PhabricatorHash::digestForIndex($build_phid.$key);
     $this->artifactKey = $key;
     return $this;
   }
 
+  public function getObjectItemView(PhabricatorUser $viewer) {
+    $data = $this->getArtifactData();
+    switch ($this->getArtifactType()) {
+      case self::TYPE_FILE:
+        $handle = id(new PhabricatorHandleQuery())
+          ->setViewer($viewer)
+          ->withPHIDs($data)
+          ->executeOne();
+
+        return id(new PHUIObjectItemView())
+          ->setObjectName(pht('File'))
+          ->setHeader($handle->getFullName())
+          ->setHref($handle->getURI());
+      case self::TYPE_HOST:
+        $leases = id(new DrydockLeaseQuery())
+          ->withIDs(array($data["drydock-lease"]))
+          ->execute();
+        $lease = $leases[$data["drydock-lease"]];
+
+        return id(new PHUIObjectItemView())
+          ->setObjectName(pht('Drydock Lease'))
+          ->setHeader($lease->getID())
+          ->setHref('/drydock/lease/'.$lease->getID());
+      default:
+        return null;
+    }
+  }
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
@@ -50,11 +82,11 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO
   }
 
   public function getPolicy($capability) {
-    return $this->getBuildable()->getPolicy($capability);
+    return $this->getBuildTarget()->getPolicy($capability);
   }
 
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
-    return $this->getBuildable()->hasAutomaticCapability(
+    return $this->getBuildTarget()->hasAutomaticCapability(
       $capability,
       $viewer);
   }