feat: add hlx6 support for api.aem.live content routing - #242
Conversation
| const cached = hlx6Cache.get(key); | ||
| if (cached !== undefined) return cached; | ||
|
|
||
| const resp = await fetch(aemApiSourceUrl(org, site, '/'), { |
There was a problem hiding this comment.
no, this is the wrong "ping". every helix5 project is automatically a helix6 project....
you need to request: https://admin.hlx.page/ping/{org}/{site} and then check if the
x-api-upgrade-available header is true
There was a problem hiding this comment.
and even then, it is not guaranteed that the respective site has the source bus configured.
we probably need a better way to detect this.
@bosschaert what about returning 405 for /{org}/sites/{site}/source/ if the content source is not correct?
The HTTP 405 Method Not Allowed client error response status code indicates that the server knows the request method, but the target resource doesn't support this method.
if we would implement this, you're check would also need to allow 404
| const cached = hlx6Cache.get(key); | ||
| if (cached !== undefined) return cached; | ||
|
|
||
| const resp = await fetch(aemApiSourceUrl(org, site, '/'), { |
There was a problem hiding this comment.
and even then, it is not guaranteed that the respective site has the source bus configured.
we probably need a better way to detect this.
@bosschaert what about returning 405 for /{org}/sites/{site}/source/ if the content source is not correct?
The HTTP 405 Method Not Allowed client error response status code indicates that the server knows the request method, but the target resource doesn't support this method.
if we would implement this, you're check would also need to allow 404
Would it not be easier to send out two fetches in parallel?
waiting on their response and then doing a fallback. That way we don't need a special detection mechanism. Would work even for projects that may end up with mixed content between DA Admin and HLX6 (for whatever edge case). |
@andreituicu, with helix-admin#3687 and #3689, |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
rebased on main and adjusted to changes in #240 |
e285b78 to
1eb95c9
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
sure, not at all against the probing solution. it is the faster and less wasteful solution with cache, so clearly more optiomal. in terms of safest, in my mind the getting from both places was the safest option, because the content is either there in one of the two places or its a 404. there is no possibility of the probe being wrong, the content being mixed between Helix and DA, site being in migration, or any of the multiple edge cases, or in-between states that might be encountered during the Helix 6 transition. that's just to explain why I proposed it, but still, nothing against if go the probe route. |
|
@andreituicu Agreed on simplicity. my concern was rollback: if a site moves back to DA but the migrated copy is still in source bus, an H6-first read would keep returning it even if I have also removed the routing cache locally. probably YAGNI, and it is per CF isolate anyway, so different isolates could route the same site differently during cutover. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| const daPath = `${path}.${ext}`; | ||
| let hlx6 = await probeHlx6(org, site); | ||
| if (hlx6 === undefined) { | ||
| hlx6 = await resolveUncertainWriteBackend({ |
There was a problem hiding this comment.
I don't understand this check. if it's not hlx6, then it's not hlx6...
There was a problem hiding this comment.
undefined means that the probe failed, in which case resolveUncertainWriteBackend will HEAD both backends and use api.aem.live if the document exists there, otherwise da-admin. I'll add JSDoc to explain, yes.
There was a problem hiding this comment.
but if the probe fails, why should the HEAD request then be a good alternative?
and for a new file, resolveUncertainWriteBackend it will also return false, because its a 404.
Co-authored-by: Tobias Bocanegra <tripod@bocanegra.ch>
Co-authored-by: Tobias Bocanegra <tripod@bocanegra.ch>
Co-authored-by: Tobias Bocanegra <tripod@bocanegra.ch>
Co-authored-by: Tobias Bocanegra <tripod@bocanegra.ch>
| ); | ||
| let sourceResp; | ||
| if (hlx6) { | ||
| const sourceUrl = aemApiSourceUrl(org, site, `${path}.${ext}`); |
There was a problem hiding this comment.
but path already has the ext, no?
tripodsan
left a comment
There was a problem hiding this comment.
the entire daCtx path / ext / html / ... handling is uninuitive and not documented.
I would first (in a different PR) harmonize this.
sketched the harmonization in #256 |
PR is #261 |
Description
This change routes source GET, HEAD, and POST requests to
api.aem.livefor source-bus sites.Legacy sites continue through the da-admin service binding.
Each source operation sends an unauthenticated
GETtoadmin.hlx.page/ping/{org}/{site}.The
x-api-upgrade-available: trueheader selects the source bus.The worker does not cache the result.
helix-admin#3687 sets this header for source-bus sites.
It checks whether
content.source.urlstarts withhttps://api.aem.live/.helix-admin#3689 added the
/pingroute.If the probe fails, reads use da-admin.
If the probe fails before a write, the worker sends parallel HEAD requests to both backends.
The backend that answers 200 wins.
Two 404 responses select da-admin for a new page.
Two 200 responses select
api.aem.liveand log a warning.A rejected HEAD request selects da-admin.
Legacy sites are the majority and do not need
admin.hlx.page.Failing them closed on a ping error would cause the larger outage.
Source responses with 401, 403, or 5xx pass through unchanged.
Only 404 uses the new-page template.
Related Issue
Motivation and Context
Helix 6 stores media in the source bus until preview.
The preview proxy had no source-bus read path, so source-bus media returned 404.
DA Live uses the preview domain to keep IMS credentials out of page scripts.
Known limits
/pinground trip.cache-control: no-store, private, must-revalidate, so CDN caching does not help.x-ratelimit-limit: 10.getSiteConfig.da-live,da-admin, and this worker.How Has This Been Tested?
npm run lintreports no errors.main: the preview image request returned 404 with 398 bytes.hlx6-ping: local preview returned 200 with 3,669,975 bytes.admin.hlx.page/ping/benpeter/hlx6-testreturned 200 withx-api-upgrade-available: true.benpeter/hlx6-test/index.htmlreturned 200 with 150 bytes.Screenshots
Not applicable.
Types of changes
Checklist