Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions config/prod/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ services:
- ../../src/DataStore/YamlStore.php
- ../../src/DataStore/JsonDataStore.php
- ../../src/CloudApi/AccessTokenConnector.php
# SourceConfig is instantiated by the command with the SAS client.
- ../../src/SasApi/SourceConfig.php
- ../../src/Command/App/From/**
public: true
resource: ../../src
Expand Down Expand Up @@ -79,6 +81,9 @@ services:
acsf.credentials:
class: Acquia\Cli\AcsfApi\AcsfCredentials

sas.credentials:
class: Acquia\Cli\SasApi\SasCredentials

# AcquiaCloudApi services.
Acquia\Cli\Command\Api\ApiCommandFactory: ~
Acquia\Cli\Command\Api\ApiBaseCommand:
Expand Down Expand Up @@ -132,6 +137,22 @@ services:
arguments:
Acquia\Cli\ApiCredentialsInterface: '@acsf.credentials'

# Sites Aggregation Service (SAS) API services.
# SAS shares the Accounts authentication layer with the Cloud API, so it
# reuses the standard cloud credentials; only the base URI differs.
Acquia\Cli\SasApi\SasConnectorFactory:
arguments:
$config:
# @see https://symfony.com/doc/current/service_container/expression_language.html
key: '@=service("cloud.credentials").getCloudKey()'
secret: '@=service("cloud.credentials").getCloudSecret()'
accessToken: '@=service("cloud.credentials").getCloudAccessToken()'
accessTokenExpiry: '@=service("cloud.credentials").getCloudAccessTokenExpiry()'
$baseUri: '@=service("sas.credentials").getBaseUri()'
$accountsUri: '@=service("cloud.credentials").getAccountsUri()'
Acquia\Cli\SasApi\SasConnector:
alias: Acquia\Cli\SasApi\SasConnectorFactory

# Symfony services.
Acquia\Cli\Application:
arguments:
Expand Down
190 changes: 190 additions & 0 deletions src/Command/Source/ConfigPushCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
<?php

declare(strict_types=1);

namespace Acquia\Cli\Command\Source;

use Acquia\Cli\ApiCredentialsInterface;
use Acquia\Cli\Attribute\RequireAuth;
use Acquia\Cli\CloudApi\ClientService;
use Acquia\Cli\Command\CommandBase;
use Acquia\Cli\DataStore\AcquiaCliDatastore;
use Acquia\Cli\DataStore\CloudDataStore;
use Acquia\Cli\Exception\AcquiaCliException;
use Acquia\Cli\Helpers\LocalMachineHelper;
use Acquia\Cli\Helpers\LoopHelper;
use Acquia\Cli\Helpers\SshHelper;
use Acquia\Cli\Helpers\TelemetryHelper;
use Acquia\Cli\SasApi\SasClientService;
use Acquia\Cli\SasApi\SourceConfig;
use Psr\Log\LoggerInterface;
use SelfUpdate\SelfUpdateManager;
use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Finder\Finder;
use Symfony\Component\Yaml\Yaml;

/**
* Push local Source configuration to a site via the Sites Aggregation Service.
*
* Reads every .yml file under .acquia/config/ in the current project and
* assembles them into a single YAML document keyed by config collection (the
* root directory is the default collection) and then by config name. This
* mirrors the structure produced by `drush source:config:dump --single-yaml`,
* which is what a future source:config:pull command writes out.
*/
#[RequireAuth]
#[AsCommand(name: 'source:config:push', description: 'Push Source configuration from .acquia/config to a site')]
final class ConfigPushCommand extends CommandBase
{
/**
* The directory (relative to the project root) holding config files.
*/
private const CONFIG_DIR = '.acquia/config';

public function __construct(
LocalMachineHelper $localMachineHelper,
CloudDataStore $datastoreCloud,
AcquiaCliDatastore $datastoreAcli,
ApiCredentialsInterface $cloudCredentials,
TelemetryHelper $telemetryHelper,
string $projectDir,
ClientService $cloudApiClientService,
SshHelper $sshHelper,
string $sshDir,
LoggerInterface $logger,
SelfUpdateManager $selfUpdateManager,
private readonly SasClientService $sasClient,
) {
parent::__construct(
$localMachineHelper,
$datastoreCloud,
$datastoreAcli,
$cloudCredentials,
$telemetryHelper,
$projectDir,
$cloudApiClientService,
$sshHelper,
$sshDir,
$logger,
$selfUpdateManager,
);
}

protected function configure(): void
{
$this

Check warning on line 79 in src/Command/Source/ConfigPushCommand.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "MethodCallRemoval": @@ @@ protected function configure(): void { - $this - ->acceptEnvironmentId() - ->acceptSiteInstanceId() - ->addOption('force', 'f', InputOption::VALUE_NONE, 'Do not ask for confirmation before pushing'); + } protected function execute(InputInterface $input, OutputInterface $output): int
->acceptEnvironmentId()
->acceptSiteInstanceId()
->addOption('force', 'f', InputOption::VALUE_NONE, 'Do not ask for confirmation before pushing');
}

protected function execute(InputInterface $input, OutputInterface $output): int
{
$this->setDirAndRequireProjectCwd($input);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point on the convention, but we're deliberately holding off on an execute()-level test for now: the SAS endpoint doesn't exist yet, so any test would just cement a placeholder request/response shape we'd have to redo once the real API lands. The payload assembly (the novel logic) is covered. We'll add command-level coverage once the endpoint's contract is settled — tracked as part of DXBE-20.

$siteInstance = $this->determineSiteInstance($input);
if ($siteInstance === null) {
throw new AcquiaCliException(
'Could not determine a Source site instance. Run this command from a repository linked to an Acquia Cloud application, or pass --siteInstanceId.'
);
}

$environment = $siteInstance->environment;

$payload = $this->assemblePayload();
if ($payload === []) {
throw new AcquiaCliException(sprintf('No configuration files found in %s.', self::CONFIG_DIR));
}
$yaml = Yaml::dump($payload, 10, 2);

if (!$input->getOption('force')) {
$answer = $this->io->confirm(
sprintf('Push configuration from %s to the %s environment?', self::CONFIG_DIR, $environment->name),
false,
);
if (!$answer) {
return Command::SUCCESS;
}
}

$sourceConfig = new SourceConfig($this->sasClient->getClient());

$response = $sourceConfig->push($environment->uuid, $yaml);
// @todo DXBE-20: Confirm the operation ID field name with the SAS team.
$operationId = $response->id ?? null;
if (!is_string($operationId)) {
throw new AcquiaCliException('The SAS API response did not include an operation ID.');
}

$this->io->writeln(sprintf('Config push submitted (operation %s). Waiting for it to complete...', $operationId));

return $this->waitForPush($sourceConfig, $operationId) ? Command::SUCCESS : Command::FAILURE;
}

/**
* Assemble the payload from the config files on disk.
*
* Returns a structure keyed by collection name (the default collection is
* the empty string; subdirectories become dotted collection names like
* "language.es"), then by config name (the file name minus .yml).
* Collections with no config files are omitted.
*
* @return array<string, array<string, mixed>>
*/
private function assemblePayload(): array
{
$configDir = $this->dir . '/' . self::CONFIG_DIR;
if (!is_dir($configDir)) {
return [];
}

$finder = new Finder();
$finder->files()->in($configDir)->name('*.yml');

$payload = [];
foreach ($finder as $file) {
$relativeDir = $file->getRelativePath();
// The root directory maps to the default collection ("").
// Subdirectories map to dotted collection names: language/es
// becomes language.es.
$collection = $relativeDir === '' ? '' : str_replace('/', '.', $relativeDir);
$name = $file->getBasename('.yml');
$payload[$collection][$name] = Yaml::parseFile($file->getPathname());
}

return $payload;
}

/**
* Poll the operation until it leaves the in-progress states.
*
* @todo DXBE-20: Confirm the status field name and its values with the
* SAS team. Assumes a `status` field mirroring the task gateway's
* phases (pending/running/succeeded/failed).
*/
private function waitForPush(SourceConfig $sourceConfig, string $operationId): bool
{
$status = null;
$checkStatus = static function () use ($sourceConfig, $operationId, &$status): bool {
$response = $sourceConfig->getPushStatus($operationId);
$status = $response->status ?? 'unknown';
return !in_array($status, ['pending', 'running'], true);
};
$onDone = static function (): void {
};

LoopHelper::getLoopy($this->output, $this->io, 'Pushing configuration...', $checkStatus, $onDone);

if ($status === 'succeeded') {
$this->io->success('Configuration pushed successfully.');
return true;
}

$this->io->error(sprintf('Config push ended with status: %s', $status));
return false;
}
Comment on lines +26 to +29

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operationLabel() is the whole of the push wording, and ConfigCommandBase.php:116 renders it as Importing configuration on the <env> environment?. Per acquia/sites-aggregation-tasks#261, confirming it puts the site in maintenance mode, backs up the database, checkpoints config, imports, and on failure reverts the checkpoint or restores the database. acquia/sites-aggregation-service#971 opens its risk section with "This triggers a destructive operation on a live site … The site is offline for the duration."

Someone pointing this at production can't tell from that sentence that the site goes down. EnvMirrorCommand.php:61 and PushDatabaseCommand.php:48 are the models — they name what gets overwritten. Please say that the site goes into maintenance mode and is unavailable while this runs, and that a database backup is taken first.

Anchoring here because the warning is push-specific: pull doesn't take the site offline. But the base's sprintf('%s on the %s environment?', ...) template only has room for a short label, so either that grows or the base needs a hook for the extra warning text.

}
16 changes: 16 additions & 0 deletions src/SasApi/SasClient.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Acquia\Cli\SasApi;

use AcquiaCloudApi\Connector\Client;

/**
* Client for the Sites Aggregation Service (SAS) API.
*
* Response processing is inherited unchanged from the Cloud API client.
*/
class SasClient extends Client

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class SasClient extends Client {}. AcsfClient exists because it overrides processResponse(); this overrides nothing. Use AcquiaCloudApi\Connector\Client directly — SasClientService is still the prophecy seam the tests need.

{
}
25 changes: 25 additions & 0 deletions src/SasApi/SasClientService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace Acquia\Cli\SasApi;

use Acquia\Cli\ApiCredentialsInterface;
use Acquia\Cli\Application;
use Acquia\Cli\CloudApi\ClientService;

class SasClientService extends ClientService
{
public function __construct(SasConnectorFactory $connectorFactory, Application $application, ApiCredentialsInterface $credentials)
{
parent::__construct($connectorFactory, $application, $credentials);

Check warning on line 15 in src/SasApi/SasClientService.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "MethodCallRemoval": @@ @@ { public function __construct(SasConnectorFactory $connectorFactory, Application $application, ApiCredentialsInterface $credentials) { - parent::__construct($connectorFactory, $application, $credentials); + } public function getClient(): SasClient
}

public function getClient(): SasClient
{
$client = SasClient::factory($this->connector);
$this->configureClient($client);

return $client;
}
}
25 changes: 25 additions & 0 deletions src/SasApi/SasConnector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace Acquia\Cli\SasApi;

use AcquiaCloudApi\Connector\Connector;

/**
* Connector for the Sites Aggregation Service (SAS) API.
*
* SAS shares the Accounts authentication layer with the Cloud API, so the
* parent class provides OAuth2 client-credentials tokens (Bearer auth) with
* no changes. The only difference is the base URI requests are sent to.
*/
class SasConnector extends Connector
{
/**
* @param array<string, string> $config
*/
public function __construct(array $config, ?string $baseUri = null, ?string $urlAccessToken = null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constructor forwards all three arguments unchanged and the class adds nothing. AcsfConnector swaps in basic auth, which is why it's a subclass.

Deleting this also deletes SasConnectorTest and the services.yml:166 alias mapping a Connector type to a factory that isn't one.

{
Comment thread
phenaproxima marked this conversation as resolved.
parent::__construct($config, $baseUri, $urlAccessToken);

Check warning on line 23 in src/SasApi/SasConnector.php

View workflow job for this annotation

GitHub Actions / Mutation Testing

Escaped Mutant for Mutator "MethodCallRemoval": @@ @@ */ public function __construct(array $config, ?string $baseUri = null, ?string $urlAccessToken = null) { - parent::__construct($config, $baseUri, $urlAccessToken); + } }
}
}
23 changes: 23 additions & 0 deletions src/SasApi/SasConnectorFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Acquia\Cli\SasApi;

use Acquia\Cli\ConnectorFactoryInterface;
use AcquiaCloudApi\Connector\ConnectorInterface;

class SasConnectorFactory implements ConnectorFactoryInterface
{
/**
* @param array<string, string|null> $config
*/
public function __construct(protected array $config, protected ?string $baseUri = null, protected ?string $accountsUri = null)
{
}

public function createConnector(): ConnectorInterface
{
return new SasConnector($this->config, $this->baseUri, $this->accountsUri);
}
}
Comment thread
phenaproxima marked this conversation as resolved.
46 changes: 46 additions & 0 deletions src/SasApi/SasCredentials.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

declare(strict_types=1);

namespace Acquia\Cli\SasApi;

use Acquia\Cli\ApiCredentialsInterface;

/**
* Configuration for the Sites Aggregation Service (SAS) API.
*
* Authentication is identical to the Cloud API (the same Accounts-issued
* key/secret and access token), which is why the services file feeds this
* class's data from the standard cloud credentials. This class exists to
* provide the SAS base URI, the only piece of configuration unique to SAS.
*/
class SasCredentials implements ApiCredentialsInterface
{
public function getCloudKey(): ?string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCloudKey() and getCloudSecret() return null with // Unused: comments. The class implements ApiCredentialsInterface only to be allowed to exist, and is used solely for getBaseUri().

There's a trap in that: SasClientService doesn't override checkAuthentication(), so it inherits ClientService.php:79, which calls getCloudAccessToken() — not declared on that interface. It works only because autowiring supplies CloudCredentials via services.yml:88. Wire sas.credentials in and it throws a fatal error. AcsfClientService overrides checkAuthentication() for this reason.

Dropping implements ApiCredentialsInterface and the two null methods leaves a class that just provides getBaseUri(), which is all the @=service(...) expression needs. Also clears most of the codecov/patch failure.

{
// Unused: the SAS connector is configured from cloud.credentials
// directly. See config/prod/services.yml.
return null;
}

public function getCloudSecret(): ?string
{
// Unused: see getCloudKey().
return null;
}

/**
* Get the SAS API base URI.
*
* @todo DXBE-20: Confirm the env var name and the production URI with the
* SAS team. Follows the ACLI_CLOUD_API_BASE_URI convention.
*/
public function getBaseUri(): ?string
{
if ($uri = getenv('ACLI_SAS_API_BASE_URI')) {
return $uri;
}

return 'https://sites-aggregation-service.acquia.com/api';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things.

The value: sites-aggregation-service.acquia.com appears in no source I can find. The SAS repo itself only carries sites-aggregation-service.acquia.test/api (local) and a relative url: / in docs/sas-spec.yaml, so it offers no production host either. The only one with evidence behind it is DXBE-19's working curl: https://sites-aggregation-service-prod.prod.cicd.acquia.io/api. Use that unless the SAS team names a stable alias — prod.cicd.acquia.io reads like a deployment host, so it's worth one question before hardcoding.

The fallback: getenv() matches CloudCredentials and AcsfCredentials and is right. Hardcoding a production host doesn't — both of those return null. They can afford to: Cloud's SDK default is already correct, and ACSF's URI is required per-customer input. Neither holds for SAS, so an unset env var sends SAS traffic to cloud.acquia.com. SasConnectorTest::testConstructorDefaultsToCloudBaseUriWhenNotOverridden() documents that.

Either hardcode a confirmed host, or throw an exception when the env var is unset.

}
}
Loading
Loading