Skip to content

Move API docs redirects to CloudFront #366

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 13, 2025
Merged

Move API docs redirects to CloudFront #366

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5b46ca9
Move API docs redirects to CloudFront
matiasgarciaisaia Jul 14, 2025
1e619fa
Rename make target back to `dist-redirect_latest`
matiasgarciaisaia Jul 15, 2025
3a3f48f
Move function to be post-S3-request
matiasgarciaisaia Jul 17, 2025
9a34488
Add S3-object-level redirections to the function
matiasgarciaisaia Jul 24, 2025
9c56ad8
Publish the redirect function after update
matiasgarciaisaia Jul 24, 2025
9e56b54
Make the function work upon viewer request
matiasgarciaisaia Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions docs/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,12 @@ dist-docs_versions: $(OUTPUT_DIR)/versions-$(CRYSTAL_VERSION).json ## Update `/v

$(OUTPUT_DIR)/versions-$(CRYSTAL_VERSION).json: $(OUTPUT_DIR)/$(OUTPUT_DOCS_BASE_NAME).tar.gz

.PHONY: dist-redirect_latest
dist-redirect_latest: $(OUTPUT_DIR)/aws-config.json ## Apply redirect from `api/latest` to `api/$(CRYSTAL_VERSION)` in S3 (needs to be manually applied)
$(AWS_CLI) s3api put-bucket-website --bucket "$(AWS_BUCKET)" --website-configuration "file://$<"
.PHONY: cloudfront-redirect_latest
cloudfront-redirect_latest: $(OUTPUT_DIR)/aws-cloudfront-redirect.js ## Apply redirect from `api/latest` to `api/$(CRYSTAL_VERSION)` in CloudFront (needs to be manually applied)
$(AWS_CLI) cloudfront update-function --name crystal-api-docs-redirects --if-match "$$($(AWS_CLI) cloudfront describe-function --name crystal-api-docs-redirects --output text --query 'ETag')" --function-config '{"Runtime":"cloudfront-js-2.0","Comment":"Redirects for Crystal API docs $(CRYSTAL_VERSION)"}' --function-code "$(shell base64 -i $(OUTPUT_DIR)/aws-cloudfront-redirect.js)"

.PHONY: get-website-configuration
get-website-configuration: ## Pull website configuration from S3 bucket (run as `make pull-website-configuration -s > aws-config.json`)
$(AWS_CLI) s3api get-bucket-website --bucket "$(AWS_BUCKET)"

.PHONY: $(OUTPUT_DIR)/aws-config.json
$(OUTPUT_DIR)/aws-config.json: aws-config.json $(OUTPUT_DIR)
.PHONY: $(OUTPUT_DIR)/aws-cloudfront-redirect.js
$(OUTPUT_DIR)/aws-cloudfront-redirect.js: aws-cloudfront-redirect.js $(OUTPUT_DIR)
sed 's/$${CRYSTAL_VERSION}/$(CRYSTAL_VERSION)/g' "$<" > $@

.PHONY: help
Expand Down
21 changes: 21 additions & 0 deletions docs/aws-cloudfront-redirect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
function handler(event) {
var requestPath = event.request.uri
var redirectUri = null

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}/')
}
Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Member

@straight-shoota straight-shoota Jul 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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


if(redirectUri != null) {
return {
statusCode: 302,
headers: {
'location': { value: redirectUri }
}
}
}

return event.request
}
57 changes: 0 additions & 57 deletions docs/aws-config.json
View file
Open in desktop

This file was deleted.