DXBE-19: Add SAS API client service - #2039
Conversation
Introduce a dedicated client stack for the Sites Aggregation Service (SAS) API, a standalone service that shares the Cloud API auth layer: - SasApi\SasCredentials extends CloudApi\CloudCredentials, inheriting key/secret/token resolution and owning only the base URI (ACLI_SAS_BASE_URI; defaults to the prod SAS service URL). - SasApi\SasClientService extends CloudApi\ClientService, injected with a separately wired sas.connector_factory (same pattern as V3ClientService) so requests go to SAS instead of the Cloud API gateway. No connector subclass is needed: the SDK Connector handles the shared OAuth flow, and SAS paths (/sites/...) never match PathRewriteConnector rules in MEO contexts.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2039 +/- ##
=========================================
Coverage 92.54% 92.54%
- Complexity 2022 2025 +3
=========================================
Files 126 128 +2
Lines 7307 7312 +5
=========================================
+ Hits 6762 6767 +5
Misses 545 545 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/2039/acli.phar |
The service hostname includes a -prod suffix: sites-aggregation-service-prod.prod.cicd.acquia.io.
There was a problem hiding this comment.
Pull request overview
Adds a dedicated SAS API client stack sharing Cloud API authentication.
Changes:
- Adds SAS credentials and configurable base URI.
- Adds SAS client service and production wiring.
- Adds credential and client tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Review notes |
|---|---|
tests/phpunit/src/SasApi/SasClientServiceTest.php |
Moderate: Add PHPUnit’s serial group because tests mutate environment state. |
tests/phpunit/src/SasApi/EnvVarSasAuthenticationTest.php |
Moderate: Add PHPUnit’s serial group because tests mutate environment state. |
src/SasApi/SasCredentials.php |
Moderate: Treat an empty ACLI_SAS_BASE_URI value as unset and use the production default. |
src/SasApi/SasClientService.php |
No final review comments. |
config/prod/services.yml |
No final review comments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Treat a set-but-empty ACLI_SAS_BASE_URI the same as unset. - Put env-var-mutating SAS tests in the serial group per house rules.
|
Human-authored comment here. I think this PR makes sense, given what I discovered from talking to @danepowell and @justafish:
This PR is agent-generated, but it follows preexisting patterns for Cloud API v3. I'm unfamiliar with the intricacies of those patterns, so that's why this is vibe-coded. The next step is probably to test that I can actually have some sort of meaningful interaction with SAS via ACLI; a temporary dummy command might be the ticket there, using my staging account. But otherwise this is ready for review; I can answer for it on at least a basic level. |
danepowell
left a comment
There was a problem hiding this comment.
I'm no longer on the DevX team and don't have any official authority over this, but IMHO this should not be accepted. ACLI should not be talking to any API other than Cloud API (the fact that it does also happen to talk to ACSF API is a legacy that we should move away from). The reference architecture for Cloud API v3 and beyond is for all services to be composed under the unified cloud.acquia.com/api gateway.
You should consult with the architecture and CXAPI teams to confirm this before making a decision.
Adds a dedicated client stack for the Sites Aggregation Service (SAS) API, a standalone Acquia service that shares the Cloud API auth layer (same key/secret, same OAuth token endpoint).
SasApi\SasCredentialsextendsCloudApi\CloudCredentials, inheriting key/secret/token resolution and owning only the base URI:ACLI_SAS_BASE_URIenv var, defaulting to the prod SAS service URL.SasApi\SasClientServiceextendsCloudApi\ClientService, injected with a separately wiredsas.connector_factory— same pattern asV3ClientService— so requests go to SAS rather than the Cloud API gateway.No connector subclass is needed: the SDK Connector already implements the shared OAuth client-credentials flow, and SAS paths (
/sites/...) never matchPathRewriteConnectorrules in MEO contexts.This is groundwork for upcoming
source:cms:*commands that will call SAS endpoints (e.g.POST /api/sites/{siteId}/config-sync), which are not part of the Cloud API OpenAPI specs and therefore need a hand-written client.