Move API docs redirects to CloudFront by matiasgarciaisaia · Pull Request #366 · crystal-lang/distribution-scripts

matiasgarciaisaia · 2025-07-14T22:33:29Z

We've hit the limits of what S3 bucket redirections allow, so we're serving the 404 page with a 200 OK status code - breaking HTTP, and making [AI] crawlers go crazy coming up with paths.

We can now move that redirection logic to CloudFront, and be correct on our status codes.

We've hit the limits of what S3 bucket redirections allow, so we're serving the 404 page with a `200 OK` status code - breaking HTTP, and making [AI] crawlers go crazy coming up with paths. We can now move that redirection logic to CloudFront, and be correct on our status codes.

matiasgarciaisaia · 2025-07-14T22:34:19Z

Deployment note: we still need to "apply" the function to the current CloudFront distribution for it to take effect. Also, we'll manually drop the old S3 bucket rules.

docs/Makefile

straight-shoota · 2025-07-15T08:50:15Z

docs/aws-cloudfront-redirect.js

+    if(requestPath.startsWith('/api/latest/')) {
+      redirectUri = requestPath.replace('/api/latest/', '/api/${CRYSTAL_VERSION}/')
+    } else if(requestPath.startsWith('/api/') && !(requestPath.startsWith('/api/0') || requestPath.startsWith('/api/1'))) {
+      redirectUri = requestPath.replace('/api/', '/api/${CRYSTAL_VERSION}/')
+    }


issue: I think we also need to pass through paths with the /api/master/ prefix (for the HEAD version of the docs) and /api/versions.json (metadata).

In the current s3 setup, we also have the following redirects which we might want to incorporate into cloudfront:

/api -> /api/ (302)

/api/ -> /api/latest (301)

/api/index.html -> /api/latest (301)

/api/latest to /api/latest/ (301)

Maybe they should all redirect directly to /api/${CRYSTAL_VERSION}/ without intermediaries and all be 302s? Or 301s to /api/latest/...

Then we could delete the files index.html and latest in the s3 bucket.

Yes, they can all be 301 to /api/latest/ (trailing slash), which would avoid some redirects and make the move explicit 👍

Oohhh! Good catch.

The S3 rules don't mention anything about /api/master/ and /api/versions.json, and I was scratching my head trying to understand why does this work different, then?. But that's because S3 rules apply when S3 resolves to a 404 error. This CloudFront function was meant (so far, at least) to be applied before hitting S3.

My first instinct is to keep this function as simple as possible: move it to after the request went to S3, and only work with 404's (as we're doing now with S3); and keep the few redirects that @straight-shoota mentions that are files in the bucket (ie, not S3 "logic").

But if you have a strong reason to move all the logic to this function, I can do that too.

Yeah, I was also expecting this function to go before S3.
Because there's no point in bothering S3 when we can easily tell that the request wouldn't match anything.

But I'm not determined on whether that's actually better or not. I suppose most legitimate requests should not hit a 404 anyway. So maybe it's more efficient to have the rewrite only after S3.

I don't think so. But the different versions are easily accessible from the version selector on each API doc page.

I meant an internal doc to avoid situations where it's only in someone's 🧠.

Ah yeah, we probably should write that down somewhere. Although the CI workflows also document what's happening.

Btw. master is actually master, not nightly. It updates on every commit to the master branch, not just once per day.

From CloudFront - Restrictions on all edge functions:

CloudFront doesn't invoke edge functions for viewer response events when the origin returns HTTP status code 400 or higher.

🤦

So we can't say "send the request to S3, and we'll do our thing if that fails" 😞

Don't know why they put this limitation in place, but it changes the framing of the function. We'll need to apply the redirects before trying to hit the bucket.

Damn. Well, the rules are static and small. It's doable to hardcode them.

Co-authored-by: Johannes Müller <straightshoota@gmail.com>

The S3 bucket had two empty files that were redirection-only: - api/index.html --> /api/latest - api/latest --> /api/latest/ We now add them to the function's check.

CloudFront requires to publish the new version, and we have to get their ETag each time.

CloudFront doesn't run viewer response functions when the origin responds a 4xx or 5xx, so we have to hard-code which requests can pass straight to the origin. This function will need to get updated if we add extra files to the `/api/` directory, such as a 2.x release, or other new files - I predict this will be the source of a few bugs in the future.

matiasgarciaisaia · 2025-07-24T22:44:26Z

OK, I think the function is now working OK.

I've set https://next.crystal-lang.org/ as a CloudFront distribution matching the production one, and replaced its API docs origin with a clone of the production one - except it doesn't have the S3 redirection rules nor the "symlinks" files.

Feel free to test every URL you can think of, so we merge this and publish on production.

ysbaddaden · 2025-07-25T09:07:25Z

The API sounds good to me.

In #366 we moved the S3 redirections to CloudFront, but in doing so we stopped re-setting the bucket's ErrorDocument upon release - so a new release keeps using the old 404 page. We now re-add that part of the settings.

Hopefully the last fix after #366

matiasgarciaisaia requested a review from straight-shoota July 14, 2025 22:33

straight-shoota reviewed Jul 15, 2025

View reviewed changes

straight-shoota added the enhancement label Jul 15, 2025

matiasgarciaisaia and others added 2 commits July 15, 2025 16:52

Rename make target back to dist-redirect_latest

1e619fa

Co-authored-by: Johannes Müller <straightshoota@gmail.com>

Move function to be post-S3-request

3a3f48f

straight-shoota approved these changes Jul 17, 2025

View reviewed changes

matiasgarciaisaia added 3 commits July 23, 2025 23:14

Add S3-object-level redirections to the function

9a34488

The S3 bucket had two empty files that were redirection-only: - api/index.html --> /api/latest - api/latest --> /api/latest/ We now add them to the function's check.

Publish the redirect function after update

9c56ad8

CloudFront requires to publish the new version, and we have to get their ETag each time.

matiasgarciaisaia requested a review from straight-shoota July 25, 2025 13:21

straight-shoota approved these changes Aug 13, 2025

View reviewed changes

matiasgarciaisaia merged commit 6d3d4c6 into master Aug 13, 2025

This was referenced Oct 16, 2025

Fix: Set S3 bucket's 404 page on docs release #373

Merged

Return 404 HTTP status on not-found docs pages #317

Closed

matiasgarciaisaia added a commit that referenced this pull request Oct 24, 2025

Fix: build CloudFront config upon docs release

448f3b7

Hopefully the last fix after #366

matiasgarciaisaia mentioned this pull request Oct 24, 2025

Fix: build CloudFront config upon docs release #380

Merged

straight-shoota mentioned this pull request Oct 30, 2025

Update distribution-scripts crystal-lang/crystal#16301

Merged

Uh oh!

Conversation

matiasgarciaisaia commented Jul 14, 2025

Uh oh!

matiasgarciaisaia commented Jul 14, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

straight-shoota Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matiasgarciaisaia commented Jul 24, 2025

Uh oh!

ysbaddaden commented Jul 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

straight-shoota Jul 15, 2025 •

edited

Loading