mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-18 19:40:55 +01:00
Use Drydock authorizations when acquiring leases
Summary: Ref T9519. When acquiring leases on resources: - Only consider resources created by authorized blueprints. - Only consider authorized blueprints when creating new resources. - Fail with a tailored error if no blueprints are allowed. - Fail with a tailored error if missing authorizations are causing acquisition failure. One somewhat-substantial issue with this is that it's pretty hard to figure out from the Harbormaster side. Specifically, the Build step UI does not show field value anywhere, so the presence of unapproved blueprints is not communicated. This is much more clear in Drydock. I'll plan to address this in future changes to Harbormaster, since there are other related/similar issues anyway. Test Plan: {F872527} Reviewers: hach-que, chad Reviewed By: chad Maniphest Tasks: T9519 Differential Revision: https://secure.phabricator.com/D14254
This commit is contained in:
parent
dac16264e4
commit
1bdf225354
17 changed files with 232 additions and 18 deletions
2
resources/sql/autopatches/20151010.drydock.auth.2.sql
Normal file
2
resources/sql/autopatches/20151010.drydock.auth.2.sql
Normal file
|
@ -0,0 +1,2 @@
|
|||
ALTER TABLE {$NAMESPACE}_drydock.drydock_lease
|
||||
ADD authorizingPHID VARBINARY(64) NOT NULL;
|
|
@ -847,6 +847,8 @@ phutil_register_library_map(array(
|
|||
'DrydockLeaseDestroyedLogType' => 'applications/drydock/logtype/DrydockLeaseDestroyedLogType.php',
|
||||
'DrydockLeaseListController' => 'applications/drydock/controller/DrydockLeaseListController.php',
|
||||
'DrydockLeaseListView' => 'applications/drydock/view/DrydockLeaseListView.php',
|
||||
'DrydockLeaseNoAuthorizationsLogType' => 'applications/drydock/logtype/DrydockLeaseNoAuthorizationsLogType.php',
|
||||
'DrydockLeaseNoBlueprintsLogType' => 'applications/drydock/logtype/DrydockLeaseNoBlueprintsLogType.php',
|
||||
'DrydockLeasePHIDType' => 'applications/drydock/phid/DrydockLeasePHIDType.php',
|
||||
'DrydockLeaseQuery' => 'applications/drydock/query/DrydockLeaseQuery.php',
|
||||
'DrydockLeaseQueuedLogType' => 'applications/drydock/logtype/DrydockLeaseQueuedLogType.php',
|
||||
|
@ -4611,6 +4613,8 @@ phutil_register_library_map(array(
|
|||
'DrydockLeaseDestroyedLogType' => 'DrydockLogType',
|
||||
'DrydockLeaseListController' => 'DrydockLeaseController',
|
||||
'DrydockLeaseListView' => 'AphrontView',
|
||||
'DrydockLeaseNoAuthorizationsLogType' => 'DrydockLogType',
|
||||
'DrydockLeaseNoBlueprintsLogType' => 'DrydockLogType',
|
||||
'DrydockLeasePHIDType' => 'PhabricatorPHIDType',
|
||||
'DrydockLeaseQuery' => 'DrydockQuery',
|
||||
'DrydockLeaseQueuedLogType' => 'DrydockLogType',
|
||||
|
|
|
@ -292,7 +292,8 @@ abstract class DrydockBlueprintImplementation extends Phobject {
|
|||
}
|
||||
|
||||
protected function newLease(DrydockBlueprint $blueprint) {
|
||||
return DrydockLease::initializeNewLease();
|
||||
return DrydockLease::initializeNewLease()
|
||||
->setAuthorizingPHID($blueprint->getPHID());
|
||||
}
|
||||
|
||||
protected function requireActiveLease(DrydockLease $lease) {
|
||||
|
|
|
@ -118,10 +118,13 @@ final class DrydockWorkingCopyBlueprintImplementation
|
|||
|
||||
$resource_phid = $resource->getPHID();
|
||||
|
||||
$blueprint_phids = $blueprint->getFieldValue('blueprintPHIDs');
|
||||
|
||||
$host_lease = $this->newLease($blueprint)
|
||||
->setResourceType('host')
|
||||
->setOwnerPHID($resource_phid)
|
||||
->setAttribute('workingcopy.resourcePHID', $resource_phid);
|
||||
->setAttribute('workingcopy.resourcePHID', $resource_phid)
|
||||
->setAllowedBlueprintPHIDs($blueprint_phids);
|
||||
|
||||
$resource
|
||||
->setAttribute('host.leasePHID', $host_lease->getPHID())
|
||||
|
|
|
@ -116,6 +116,14 @@ final class DrydockLeaseViewController extends DrydockLeaseController {
|
|||
}
|
||||
$view->addProperty(pht('Owner'), $owner_display);
|
||||
|
||||
$authorizing_phid = $lease->getAuthorizingPHID();
|
||||
if ($authorizing_phid) {
|
||||
$authorizing_display = $viewer->renderHandle($authorizing_phid);
|
||||
} else {
|
||||
$authorizing_display = phutil_tag('em', array(), pht('None'));
|
||||
}
|
||||
$view->addProperty(pht('Authorized By'), $authorizing_display);
|
||||
|
||||
$resource_phid = $lease->getResourcePHID();
|
||||
if ($resource_phid) {
|
||||
$resource_display = $viewer->renderHandle($resource_phid);
|
||||
|
|
|
@ -0,0 +1,26 @@
|
|||
<?php
|
||||
|
||||
final class DrydockLeaseNoAuthorizationsLogType extends DrydockLogType {
|
||||
|
||||
const LOGCONST = 'core.lease.no-authorizations';
|
||||
|
||||
public function getLogTypeName() {
|
||||
return pht('No Authorizations');
|
||||
}
|
||||
|
||||
public function getLogTypeIcon(array $data) {
|
||||
return 'fa-map-o red';
|
||||
}
|
||||
|
||||
public function renderLog(array $data) {
|
||||
$viewer = $this->getViewer();
|
||||
$authorizing_phid = idx($data, 'authorizingPHID');
|
||||
|
||||
return pht(
|
||||
'The object which authorized this lease (%s) is not authorized to use '.
|
||||
'any of the blueprints the lease lists. Approve the authorizations '.
|
||||
'before using the lease.',
|
||||
$viewer->renderHandle($authorizing_phid)->render());
|
||||
}
|
||||
|
||||
}
|
|
@ -0,0 +1,19 @@
|
|||
<?php
|
||||
|
||||
final class DrydockLeaseNoBlueprintsLogType extends DrydockLogType {
|
||||
|
||||
const LOGCONST = 'core.lease.no-blueprints';
|
||||
|
||||
public function getLogTypeName() {
|
||||
return pht('No Blueprints');
|
||||
}
|
||||
|
||||
public function getLogTypeIcon(array $data) {
|
||||
return 'fa-map-o red';
|
||||
}
|
||||
|
||||
public function renderLog(array $data) {
|
||||
return pht('This lease does not list any usable blueprints.');
|
||||
}
|
||||
|
||||
}
|
|
@ -28,7 +28,7 @@ final class DrydockManagementLeaseWorkflow
|
|||
}
|
||||
|
||||
public function execute(PhutilArgumentParser $args) {
|
||||
$console = PhutilConsole::getConsole();
|
||||
$viewer = $this->getViewer();
|
||||
|
||||
$resource_type = $args->getArg('type');
|
||||
if (!$resource_type) {
|
||||
|
@ -59,6 +59,23 @@ final class DrydockManagementLeaseWorkflow
|
|||
$lease = id(new DrydockLease())
|
||||
->setResourceType($resource_type);
|
||||
|
||||
$drydock_phid = id(new PhabricatorDrydockApplication())->getPHID();
|
||||
$lease->setAuthorizingPHID($drydock_phid);
|
||||
|
||||
// TODO: This is not hugely scalable, although this is a debugging workflow
|
||||
// so maybe it's fine. Do we even need `bin/drydock lease` in the long run?
|
||||
$all_blueprints = id(new DrydockBlueprintQuery())
|
||||
->setViewer($viewer)
|
||||
->execute();
|
||||
$allowed_phids = mpull($all_blueprints, 'getPHID');
|
||||
if (!$allowed_phids) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'No blueprints exist which can plausibly allocate resources to '.
|
||||
'satisfy the requested lease.'));
|
||||
}
|
||||
$lease->setAllowedBlueprintPHIDs($allowed_phids);
|
||||
|
||||
if ($attributes) {
|
||||
$lease->setAttributes($attributes);
|
||||
}
|
||||
|
|
|
@ -8,6 +8,14 @@ final class DrydockBlueprintPHIDType extends PhabricatorPHIDType {
|
|||
return pht('Blueprint');
|
||||
}
|
||||
|
||||
public function getPHIDTypeApplicationClass() {
|
||||
return 'PhabricatorDrydockApplication';
|
||||
}
|
||||
|
||||
public function getTypeIcon() {
|
||||
return 'fa-map-o';
|
||||
}
|
||||
|
||||
public function newObject() {
|
||||
return new DrydockBlueprint();
|
||||
}
|
||||
|
|
|
@ -8,6 +8,14 @@ final class DrydockLeasePHIDType extends PhabricatorPHIDType {
|
|||
return pht('Drydock Lease');
|
||||
}
|
||||
|
||||
public function getPHIDTypeApplicationClass() {
|
||||
return 'PhabricatorDrydockApplication';
|
||||
}
|
||||
|
||||
public function getTypeIcon() {
|
||||
return 'fa-link';
|
||||
}
|
||||
|
||||
public function newObject() {
|
||||
return new DrydockLease();
|
||||
}
|
||||
|
|
|
@ -8,6 +8,14 @@ final class DrydockResourcePHIDType extends PhabricatorPHIDType {
|
|||
return pht('Drydock Resource');
|
||||
}
|
||||
|
||||
public function getPHIDTypeApplicationClass() {
|
||||
return 'PhabricatorDrydockApplication';
|
||||
}
|
||||
|
||||
public function getTypeIcon() {
|
||||
return 'fa-map';
|
||||
}
|
||||
|
||||
public function newObject() {
|
||||
return new DrydockResource();
|
||||
}
|
||||
|
|
|
@ -7,6 +7,7 @@ final class DrydockBlueprintQuery extends DrydockQuery {
|
|||
private $blueprintClasses;
|
||||
private $datasourceQuery;
|
||||
private $disabled;
|
||||
private $authorizedPHIDs;
|
||||
|
||||
public function withIDs(array $ids) {
|
||||
$this->ids = $ids;
|
||||
|
@ -33,10 +34,19 @@ final class DrydockBlueprintQuery extends DrydockQuery {
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function withAuthorizedPHIDs(array $phids) {
|
||||
$this->authorizedPHIDs = $phids;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function newResultObject() {
|
||||
return new DrydockBlueprint();
|
||||
}
|
||||
|
||||
protected function getPrimaryTableAlias() {
|
||||
return 'blueprint';
|
||||
}
|
||||
|
||||
protected function loadPage() {
|
||||
return $this->loadStandardPage($this->newResultObject());
|
||||
}
|
||||
|
@ -63,39 +73,66 @@ final class DrydockBlueprintQuery extends DrydockQuery {
|
|||
if ($this->ids !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'id IN (%Ld)',
|
||||
'blueprint.id IN (%Ld)',
|
||||
$this->ids);
|
||||
}
|
||||
|
||||
if ($this->phids !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'phid IN (%Ls)',
|
||||
'blueprint.phid IN (%Ls)',
|
||||
$this->phids);
|
||||
}
|
||||
|
||||
if ($this->datasourceQuery !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'blueprintName LIKE %>',
|
||||
'blueprint.blueprintName LIKE %>',
|
||||
$this->datasourceQuery);
|
||||
}
|
||||
|
||||
if ($this->blueprintClasses !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'className IN (%Ls)',
|
||||
'blueprint.className IN (%Ls)',
|
||||
$this->blueprintClasses);
|
||||
}
|
||||
|
||||
if ($this->disabled !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'isDisabled = %d',
|
||||
'blueprint.isDisabled = %d',
|
||||
(int)$this->disabled);
|
||||
}
|
||||
|
||||
return $where;
|
||||
}
|
||||
|
||||
protected function shouldGroupQueryResultRows() {
|
||||
if ($this->authorizedPHIDs !== null) {
|
||||
return true;
|
||||
}
|
||||
return parent::shouldGroupQueryResultRows();
|
||||
}
|
||||
|
||||
protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
|
||||
$joins = parent::buildJoinClauseParts($conn);
|
||||
|
||||
if ($this->authorizedPHIDs !== null) {
|
||||
$joins[] = qsprintf(
|
||||
$conn,
|
||||
'JOIN %T authorization
|
||||
ON authorization.blueprintPHID = blueprint.phid
|
||||
AND authorization.objectPHID IN (%Ls)
|
||||
AND authorization.objectAuthorizationState = %s
|
||||
AND authorization.blueprintAuthorizationState = %s',
|
||||
id(new DrydockAuthorization())->getTableName(),
|
||||
$this->authorizedPHIDs,
|
||||
DrydockAuthorization::OBJECTAUTH_ACTIVE,
|
||||
DrydockAuthorization::BLUEPRINTAUTH_AUTHORIZED);
|
||||
}
|
||||
|
||||
return $joins;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -7,6 +7,7 @@ final class DrydockLease extends DrydockDAO
|
|||
protected $resourceType;
|
||||
protected $until;
|
||||
protected $ownerPHID;
|
||||
protected $authorizingPHID;
|
||||
protected $attributes = array();
|
||||
protected $status = DrydockLeaseStatus::STATUS_PENDING;
|
||||
|
||||
|
@ -141,6 +142,25 @@ final class DrydockLease extends DrydockDAO
|
|||
pht('Only new leases may be queued for activation!'));
|
||||
}
|
||||
|
||||
if (!$this->getAuthorizingPHID()) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Trying to queue a lease for activation without an authorizing '.
|
||||
'object. Use "%s" to specify the PHID of the authorizing object. '.
|
||||
'The authorizing object must be approved to use the allowed '.
|
||||
'blueprints.',
|
||||
'setAuthorizingPHID()'));
|
||||
}
|
||||
|
||||
if (!$this->getAllowedBlueprintPHIDs()) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Trying to queue a lease for activation without any allowed '.
|
||||
'Blueprints. Use "%s" to specify allowed blueprints. The '.
|
||||
'authorizing object must be approved to use the allowed blueprints.',
|
||||
'setAllowedBlueprintPHIDs()'));
|
||||
}
|
||||
|
||||
$this
|
||||
->setStatus(DrydockLeaseStatus::STATUS_PENDING)
|
||||
->save();
|
||||
|
@ -376,6 +396,15 @@ final class DrydockLease extends DrydockDAO
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function setAllowedBlueprintPHIDs(array $phids) {
|
||||
$this->setAttribute('internal.blueprintPHIDs', $phids);
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getAllowedBlueprintPHIDs() {
|
||||
return $this->getAttribute('internal.blueprintPHIDs', array());
|
||||
}
|
||||
|
||||
private function didActivate() {
|
||||
$viewer = PhabricatorUser::getOmnipotentUser();
|
||||
$need_update = false;
|
||||
|
|
|
@ -300,11 +300,46 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
|
|||
return array();
|
||||
}
|
||||
|
||||
$blueprints = id(new DrydockBlueprintQuery())
|
||||
$query = id(new DrydockBlueprintQuery())
|
||||
->setViewer($viewer)
|
||||
->withBlueprintClasses(array_keys($impls))
|
||||
->withDisabled(false)
|
||||
->execute();
|
||||
->withDisabled(false);
|
||||
|
||||
$blueprint_phids = $lease->getAllowedBlueprintPHIDs();
|
||||
if (!$blueprint_phids) {
|
||||
$lease->logEvent(DrydockLeaseNoBlueprintsLogType::LOGCONST);
|
||||
return array();
|
||||
}
|
||||
|
||||
// The Drydock application itself is allowed to authorize anything. This
|
||||
// is primarily used for leases generated by CLI administrative tools.
|
||||
$drydock_phid = id(new PhabricatorDrydockApplication())->getPHID();
|
||||
|
||||
$authorizing_phid = $lease->getAuthorizingPHID();
|
||||
if ($authorizing_phid != $drydock_phid) {
|
||||
$blueprints = id(clone $query)
|
||||
->withAuthorizedPHIDs(array($authorizing_phid))
|
||||
->execute();
|
||||
if (!$blueprints) {
|
||||
// If we didn't hit any blueprints, check if this is an authorization
|
||||
// problem: re-execute the query without the authorization constraint.
|
||||
// If the second query hits blueprints, the overall configuration is
|
||||
// fine but this is an authorization problem. If the second query also
|
||||
// comes up blank, this is some other kind of configuration issue so
|
||||
// we fall through to the default pathway.
|
||||
$all_blueprints = $query->execute();
|
||||
if ($all_blueprints) {
|
||||
$lease->logEvent(
|
||||
DrydockLeaseNoAuthorizationsLogType::LOGCONST,
|
||||
array(
|
||||
'authorizingPHID' => $authorizing_phid,
|
||||
));
|
||||
return array();
|
||||
}
|
||||
}
|
||||
} else {
|
||||
$blueprints = $query->execute();
|
||||
}
|
||||
|
||||
$keep = array();
|
||||
foreach ($blueprints as $key => $blueprint) {
|
||||
|
|
|
@ -67,11 +67,6 @@ final class HarbormasterBuildStepCoreCustomField
|
|||
$object->setDetail($key, $value);
|
||||
}
|
||||
|
||||
public function applyApplicationTransactionExternalEffects(
|
||||
PhabricatorApplicationTransaction $xaction) {
|
||||
return;
|
||||
}
|
||||
|
||||
public function getBuildTargetFieldValue() {
|
||||
return $this->getProxy()->getFieldValue();
|
||||
}
|
||||
|
|
|
@ -28,9 +28,13 @@ final class HarbormasterBuildStepPHIDType extends PhabricatorPHIDType {
|
|||
foreach ($handles as $phid => $handle) {
|
||||
$build_step = $objects[$phid];
|
||||
|
||||
$id = $build_step->getID();
|
||||
$name = $build_step->getName();
|
||||
|
||||
$handle->setName($name);
|
||||
$handle
|
||||
->setName($name)
|
||||
->setFullName(pht('Build Step %d: %s', $id, $name))
|
||||
->setURI("/harbormaster/step/{$id}/edit/");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -41,9 +41,14 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
|
|||
$working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation())
|
||||
->getType();
|
||||
|
||||
$allowed_phids = $build_target->getFieldValue('repositoryPHIDs');
|
||||
$authorizing_phid = $build_target->getBuildStep()->getPHID();
|
||||
|
||||
$lease = DrydockLease::initializeNewLease()
|
||||
->setResourceType($working_copy_type)
|
||||
->setOwnerPHID($build_target->getPHID());
|
||||
->setOwnerPHID($build_target->getPHID())
|
||||
->setAuthorizingPHID($authorizing_phid)
|
||||
->setAllowedBlueprintPHIDs($allowed_phids);
|
||||
|
||||
$map = $this->buildRepositoryMap($build_target);
|
||||
|
||||
|
@ -104,6 +109,11 @@ final class HarbormasterLeaseWorkingCopyBuildStepImplementation
|
|||
'type' => 'text',
|
||||
'required' => true,
|
||||
),
|
||||
'blueprintPHIDs' => array(
|
||||
'name' => pht('Use Blueprints'),
|
||||
'type' => 'blueprints',
|
||||
'required' => true,
|
||||
),
|
||||
'repositoryPHIDs' => array(
|
||||
'name' => pht('Also Clone'),
|
||||
'type' => 'datasource',
|
||||
|
|
Loading…
Reference in a new issue