fix(docs): use service origins and verify Swagger asset integrity - #135
csnitker-godaddy wants to merge 3 commits into
Conversation
Signed-off-by: Connor Snitker <csnitker@godaddy.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Replace the RA event-path server overrides with a single / server in both canonical and embedded documents.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Updates RA and TL OpenAPI documents for valid Swagger parsing and same-origin requests.
Changes:
- Removes the duplicate RA
Problemschema. - Uses
/v2for RA and/for TL server URLs. - Synchronizes canonical and embedded documents.
- Two moderate findings remain, each with 3 votes: the RA event-path override still exposes localhost and example-host URLs in both RA documents.
| File | Summary |
|---|---|
spec/api-spec-v2.yaml |
Canonical RA specification; event-path server override remains unresolved. |
spec/api-spec-tl-v2.yaml |
Canonical TL specification. |
internal/adapter/docsui/openapi/tl.yaml |
Embedded TL specification. |
internal/adapter/docsui/openapi/ra.yaml |
Embedded RA specification; event-path server override remains unresolved. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - url: /v2 | ||
| description: This Registration Authority (current service origin) |
There was a problem hiding this comment.
The /v1/agents/events override now uses only / in both canonical and embedded RA documents. The root RA server remains /v2, and TL uses /, so each route resolves against the serving origin.
| - url: /v2 | ||
| description: This Registration Authority (current service origin) |
There was a problem hiding this comment.
The feed override is now / in both RA copies. It no longer overrides the same-origin configuration with localhost or an example hostname.
Signed-off-by: Connor Snitker <csnitker@godaddy.com>
kperry-godaddy
left a comment
There was a problem hiding this comment.
The relative servers entries are the right call, and I checked the mechanics rather than trusting them: swagger-js resolves a relative server URL against the document URL and strips the trailing slash before it appends the operation path, so the TL's url: / produces clean https://host/v1/... requests. Removing the second Problem also kept the accurate copy: the generic 500 path in errors.go emits no code, so the deleted definition's required: code was the wrong one. Both documents now pass a duplicate-key-strict YAML parse, and the embedded copies are byte-identical to spec/.
One thing I'd settle before this merges, inline on the servers block. Two small notes below.
Not blocking, worth a look
- 97
$refs still point atErrorResponseand 15 atProblem, while every RA failure path emits Problem Details. The deleted duplicate carried the only note that this migration is tracked separately, and I couldn't find an issue for it. Opening one would keep the work item from disappearing with the comment. - Dropping the named hosts changes what a code generator seeds as its default base URL from
servers[0].url. If any SDK build reads this document directly, it will want an explicit base URL now.
| description: Main ANS API server (v2) | ||
| - url: https://test.ans.example.org/v2 | ||
| description: Test server (v2) | ||
| - url: /v2 |
There was a problem hiding this comment.
The root list is fixed, but the operation-level override on /v1/agents/events (lines 96-100) still carries http://localhost:18080 and https://api.ans.example.org. A path item's servers replaces the root list for that path (OAS 3.0.3, Path Item Object: https://spec.openapis.org/oas/v3.0.3#path-item-object), so on a deployed RA the events-feed "Try it out" still goes to localhost, which is the behaviour #132 describes. Same treatment there, then re-copy the embedded ra.yaml:
/v1/agents/events:
servers:
- url: /
description: This Registration Authority (the feed is served at the origin root, not under /v2)Worth fixing before this ships since it is the same defect on the one route that lives outside /v2; the change is four lines and the docsui test will catch a missed copy.
There was a problem hiding this comment.
Applied the root-relative feed override and kept canonical/embedded copies identical. Generated clients must receive an explicit deployment base URL; the relative URL is intentional for Swagger served by either service.
Signed-off-by: Connor Snitker <csnitker@godaddy.com>

Fix Swagger parsing and requests on deployed RA/TL hosts: remove the duplicate
Problem schema, use
/v2for RA operations and/for TL and the V1 eventsfeed, and keep canonical/embedded documents synchronized.
Pin the existing Swagger UI 5.17.14 CSS and JavaScript with SHA-384 Subresource
Integrity and anonymous CORS. Hashes were checked against the npm tarball's
verified distribution and the actual CDN responses. Generated clients should
receive an explicit deployment base URL.
Validation: static analysis, duplicate-key-aware YAML parsing, matching
canonical/embedded documents, and served Swagger attributes on Ubuntu.
Fixes #132.
AI assistance
Assisted-by: Codex (GPT-6), under Connor Snitker's direction.