From 1f53017f1f46c4c5b6a5c6a1ac5981f4694dd7cd Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Thu, 5 Dec 2013 08:16:33 +1100 Subject: [PATCH] Validate resource attributes for preallocated hosts before executing leases Summary: This prevents issues when the user hasn't provided the appropriate attributes for a preallocated host. Test Plan: Attempted to lease against a resource with omitted attributes, got an exception thrown before any SSH commands occurred. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley CC: Korvin, epriestley, aran Maniphest Tasks: T1049 Differential Revision: https://secure.phabricator.com/D7695 --- ...reallocatedHostBlueprintImplementation.php | 21 ++++++++++++++++--- .../drydock/storage/DrydockResource.php | 4 ++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php index 7260ba8999..03a381c3d6 100644 --- a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php @@ -37,6 +37,21 @@ final class DrydockPreallocatedHostBlueprintImplementation DrydockResource $resource, DrydockLease $lease) { + // Because preallocated resources are manually created, we should verify + // we have all the information we need. + PhutilTypeSpec::checkMap( + $resource->getAttributesForTypeSpec( + array('platform', 'host', 'port', 'user', 'path')), + array( + 'platform' => 'string', + 'host' => 'string', + 'port' => 'string', // Value is a string from the command line + 'user' => 'string', + 'path' => 'string', + )); + $v_platform = $resource->getAttribute('platform'); + $v_path = $resource->getAttribute('path'); + // Similar to DrydockLocalHostBlueprint, we create a folder // on the remote host that the lease can use. @@ -46,18 +61,18 @@ final class DrydockPreallocatedHostBlueprintImplementation // the platform we're currently running on, not the platform we are // remoting to. $separator = '/'; - if ($lease->getAttribute('platform') === 'windows') { + if ($v_platform === 'windows') { $separator = '\\'; } // Clean up the directory path a little. - $base_path = rtrim($resource->getAttribute('path'), '/'); + $base_path = rtrim($v_path, '/'); $base_path = rtrim($base_path, '\\'); $full_path = $base_path.$separator.$lease_id; $cmd = $lease->getInterface('command'); - if ($lease->getAttribute('platform') !== 'windows') { + if ($v_platform !== 'windows') { $cmd->execx('mkdir %s', $full_path); } else { // Windows is terrible. The mkdir command doesn't even support putting diff --git a/src/applications/drydock/storage/DrydockResource.php b/src/applications/drydock/storage/DrydockResource.php index 4e847883c1..87e7f7dc9a 100644 --- a/src/applications/drydock/storage/DrydockResource.php +++ b/src/applications/drydock/storage/DrydockResource.php @@ -35,6 +35,10 @@ final class DrydockResource extends DrydockDAO return idx($this->attributes, $key, $default); } + public function getAttributesForTypeSpec(array $attribute_names) { + return array_select_keys($this->attributes, $attribute_names); + } + public function setAttribute($key, $value) { $this->attributes[$key] = $value; return $this;