Skip to content

feat: add sandbox audit API endpoint #1122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Kanishkavijay39
Copy link

@Kanishkavijay39 Kanishkavijay39 commented Aug 7, 2025

Overview
This PR introduces a new REST API endpoint that allows partners to trigger audits on sandbox sites without requiring Slack SpaceCat access. The endpoint provides a simple, programmatic way to initiate audits for sandbox environments.

Wiki: https://wiki.corp.adobe.com/display/AEMSites/AEM+Sites+Optimizer+-+Sandbox+Backend+APIs
Changes
New API Endpoint
Route: GET /sandbox/audit
Purpose: Trigger audits on sandbox sites programmatically
Key Features
✅ Single audit trigger: GET /sandbox/audit?baseURL=https://sandbox.example.com&auditType=meta-tags,alt-text
✅ Multiple audit trigger: GET /sandbox/audit?baseURL=https://sandbox.example.com (triggers all enabled audits)
✅ Sandbox validation: Only works with sites marked as isSandbox: true
✅ Comprehensive error handling: Validates URLs, audit types, site existence, and sandbox status
✅ Added rate limiting : fetch last audit run and did time diff by default kept it 4 hrs have created env. variable as well so we can update it if needed.

Supported Audit Types
meta-tags - Validate meta tags and SEO elements
alt-text - Check for missing alt text in images

Testing done on local
image
image
image
image
image
image

- Add GET /sandbox/audit endpoint for partners to trigger audits on sandbox sites
- Support single audit type or trigger all enabled audits
- Validate that only sandbox sites can use this endpoint
- Add comprehensive test coverage (17 test cases)
- Include complete OpenAPI documentation with examples
- Update route handlers and main application integration

Resolves need for partners to trigger audits without Slack access
- Fixed sandbox validation in sandbox-audit controller
- Changed from site.isSandbox (undefined) to site.getIsSandbox() (boolean)
- Updated test mocks to use getIsSandbox() method instead of isSandbox property
- Removed debug logging after issue was resolved
- All sandbox audit tests now passing (1190 passing)
Copy link

github-actions bot commented Aug 7, 2025

This PR will trigger a minor release when merged.

import { badRequest, notFound, ok } from '@adobe/spacecat-shared-http-utils';
import { sendAuditMessage } from '../support/utils.js';

const ALL_AUDITS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we take this from site config?

Copy link
Author

Choose a reason for hiding this comment

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

As discussed, will create a follow up task will add configuration object for sandbox audits.
have to add the audit-types array to spacecat-shared-data-access configuration.

@absarasw absarasw requested a review from solaris007 August 8, 2025 05:25
const triggerAuditForSiteAPI = async (site, auditType, context) => sendAuditMessage(
context.sqs,
context.env.AUDIT_JOBS_QUEUE_URL,
auditType,
Copy link
Contributor

@absarasw absarasw Aug 8, 2025

Choose a reason for hiding this comment

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

Can you make it a list?

Copy link
Author

Choose a reason for hiding this comment

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

Done

Kanishkavijay39 and others added 9 commits August 8, 2025 12:54
- Remove scoped_api_key authentication option
- Keep only ims_key for better security and user identification
- Aligns with other administrative endpoint patterns
…cat-api-service into SITES-sandbox-audit-api
…ing logic

* Added normalizeAuditTypes and enforceRateLimit helpers
* Consolidated triggerAllEnabledAudits & triggerSpecificAudits into triggerAudits
* Updated controller to use helpers and env-configurable SANDBOX_AUDIT_RATE_LIMIT_HOURS
* Extended tests and docs; maintains 100% coverage
…udit-service.js – removed redundant eslint suppression

• src/controllers/sandbox-audit.js – add c8 ignore for minutes/hours branch
• tests – added over/under-hour cases + helper test → 100 % branch & line coverage
• docs/openapi/sandbox-audit-api.yaml + docs/index.html – document auditsSkipped, nextAllowedIn, skippedDetail
• .vscode/launch.json – optional VS Code debug profile
**Restrictions:**
- Only works with sandbox sites (sites with `isSandbox: true`)
- Audits are rate-limited (default **4 hours** between runs for the same site/audit type). If the
limit is hit the endpoint responds with HTTP 400 and an explanatory message.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using 429?

Copy link
Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,176 @@
sandbox-audit:
get:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why using GET for something that modifies state?

Copy link
Author

@Kanishkavijay39 Kanishkavijay39 Aug 18, 2025

Choose a reason for hiding this comment

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

Backend already prevents re-runs in x h, so it felt idempotent, but you’re right it does create state. I’ll update it to POST.
Updated

README.md Outdated
entirely.

```plaintext
SANDBOX_AUDIT_RATE_LIMIT_HOURS=24
Copy link
Contributor

Choose a reason for hiding this comment

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

24 or 4?

Copy link
Author

@Kanishkavijay39 Kanishkavijay39 Aug 18, 2025

Choose a reason for hiding this comment

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

ack, I’ll recheck and will make it consistent.
done

@@ -102,6 +102,16 @@ SLACK_TOKEN_WORKSPACE_EXTERNAL_ELEVATED=Slack token for the external workspace,
SLACK_OPS_CHANNEL_WORKSPACE_EXTERNAL=channel ID to use for operations messages in the external workspace
```

Sandbox audit rate-limit (optional):

Minimum hours that must elapse before the same audit can run again for the same site.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the rate limit be like x audit triggers per y hours?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, we can enhance it this way. I will add in configuration site level for sandbox for audit types and in that audit i will set rate limit time as well.
Will modify.

The audit type(s) to trigger. Accepts:
• a single value (e.g. `meta-tags`)
• a comma-separated list (e.g. `meta-tags,alt-text`)
If omitted, all enabled audits are triggered.
Copy link
Contributor

Choose a reason for hiding this comment

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

enabled and supported for sandboxes, you mean, right?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, enabled and supported.

- `meta-tags`: Validate meta tags and SEO elements
- `alt-text`: Check for missing alt text in images
parameters:
- name: baseURL
Copy link
Contributor

Choose a reason for hiding this comment

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

Whh baseurl and not siteId?
As part of API route instead of query param?

Copy link
Author

@Kanishkavijay39 Kanishkavijay39 Aug 18, 2025

Choose a reason for hiding this comment

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

siteId is an internal primary key, exposing it is unnecessary for partners. but many apis do add site id as path variable i will also update.
Done.

```plaintext
SANDBOX_AUDIT_RATE_LIMIT_HOURS=24
```

Copy link
Contributor

Choose a reason for hiding this comment

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

How will UI know if triggered audit failed or succeeded?

Copy link
Author

@Kanishkavijay39 Kanishkavijay39 Aug 18, 2025

Choose a reason for hiding this comment

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

since triggering audit is async, we can't provide audit details in this api.
rather ui can call get latest audits and see the status.
https://opensource.adobe.com/spacecat-api-service/#tag/import/operation/getImportJobStatus
Or i can provide job id with specific audit.

partners who need to trigger audits programmatically on sandbox environments only.

**Behavior:**
- If `auditType` is specified: triggers that specific audit type
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if the audit is disabled?

Copy link
Author

Choose a reason for hiding this comment

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

Will give proper 400, detailed error with all the audits which are enabled.

Kanishka added 2 commits August 18, 2025 17:28
…ve rate-limit handling

• Route now uses siteId path parameter (avoids baseURL query)
• Response when rate limit hit is 429 (Too Many Requests)
• Default limit clarified as 4 h via SANDBOX_AUDIT_RATE_LIMIT_HOURS env var
• Updated OpenAPI docs and README
• Removed unused import warning
- Updated all test cases to use siteId path parameter instead of baseURL query parameter
- Fixed mock Site objects to include getBaseURL() method
- Updated rate limit test expectations to use 429 status code
- Fixed linter errors (trailing spaces)
- Updated route tests to include new dynamic route
- All sandbox audit tests now pass
Kanishka added 2 commits August 19, 2025 01:56
- Resolved conflicts in routes configuration
- Added LLMO customer-intent routes alongside sandbox audit route
- Updated test expectations to include both features
- Used main branch version of generated docs/index.html
@Kanishkavijay39 Kanishkavijay39 force-pushed the SITES-sandbox-audit-api branch from c3ddf88 to 7fe70e4 Compare August 21, 2025 18:35
@Kanishkavijay39 Kanishkavijay39 force-pushed the SITES-sandbox-audit-api branch from 9b43ae4 to e7687aa Compare August 22, 2025 09:17
@Kanishkavijay39 Kanishkavijay39 force-pushed the SITES-sandbox-audit-api branch from e7687aa to 5886a7d Compare August 22, 2025 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants