feat(gateway): _redirects file support
Context: https://github.com/ipfs/specs/pull/290, part of https://github.com/ipfs/specs/issues/257
This PR implements https://github.com/ipfs/specs/pull/290.
Details
Now that https://github.com/ipfs/go-ipfs/pull/8885 has landed, I've moved my refactored _redirects changes here from https://github.com/ipfs/go-ipfs/pull/8816. This is on my personal account now, so the CircleCI permission issues should be resolved. 😅
@lidel, as mentioned at https://github.com/ipfs/go-ipfs/pull/8816#issuecomment-1091022658, there are some challenges with trying to fully move _redirects logic out of gateway_handler.go as you requested. The changes in this PR aren't ready (full review not needed yet), but would you be able to take a look and let me know if you approve of how I've split code into handleUnixfsPathResolution and handleNonUnixfsPathResolution?
Also, note that I've intentionally not moved 404 related functions from gateway_handler.go to gateway_handler_unixfs__redirects.go yet, to avoid extra PR noise.
@lidel When you have time, gentle bump on getting your feedback to the comments above. I really appreciate the time and feedback you've given me already. 🙏
At this point I have what feels like the right MVP functionality working (from to [status][!] including 200 rewrite for SPA support). I have more tests to write but would really like your thoughts on how close this code is to done so I can try to get this over the finish line.
Also, related to your go-redirects comments... it feels to me like we should not use a library dedicated to parsing Netlify's _redirect file, and instead have our own library whose specific goal is to support the functionality in the redirect IPFS spec (to be written). For now I'm thinking this is essentially a fork of go-redirects that only supports from to [status][!] and the first version of the spec will only include this functionality. All of this functionality is working now on this PR's branch and justincjohnson/go-ipfs-redirects. If you are okay with this approach, I can keep this project in my personal account or we could move it to the ipfs org before this ships if that is preferable.
If you disagree and want me to stick with a repo that specifically supports Netlify's syntax, I still had to fork the repo since it is no longer maintained, and it seems to me there wouldn't be any reason to add all the existing Netlify parsing logic right away, since the MVP won't support all of those features anyway.
For convenience for anyone wanting to evaluate the changes...
# Build and run
make build
GOLOG_LOG_LEVEL="core/server=debug" ./cmd/ipfs/ipfs daemon
# web site
mkdir -p ~/testredirect/
echo "index" > ~/testredirect/index.html
echo "one" > ~/testredirect/one.html
echo "two" > ~/testredirect/two.html
echo "404" > ~/testredirect/404.html
echo "existing" > ~/testredirect/existing.html
echo "forced redirect, never to be seen" > ~/testredirect/forced-redirect.html
mkdir -p ~/testredirect/articles/2022/04/25/hello-world
echo "hello world" > ~/testredirect/articles/2022/04/25/hello-world/index.html
# _redirects file
echo "/redirect-one /one.html" > ~/testredirect/_redirects
echo "/301-redirect-one /one.html 301" >> ~/testredirect/_redirects
echo "/302-redirect-two /two.html 302" >> ~/testredirect/_redirects
echo "/200-index /index.html 200" >> ~/testredirect/_redirects
# existing file with no force, so won't redirect
echo "/existing.html /two.html 200" >> ~/testredirect/_redirects
# existing file with force, so will redirect
echo '/forced-redirect.html /index.html 301!' >> ~/testredirect/_redirects
# Redirect with placeholder. Try /posts/2022/04/25/hello-world in browser
echo "/posts/:year/:month/:day/:title /articles/:year/:month/:day/:title 301" >> ~/testredirect/_redirects
# Try /splat/index.html or /splat/one.html
echo "/splat/* /:splat 301" >> ~/testredirect/_redirects
# Pretty 404 logic
echo "/en/* /404.html 404" >> ~/testredirect/_redirects
# SPA, as last rule
echo "/* /index.html 200" >> ~/testredirect/_redirects
CID=$(./cmd/ipfs/ipfs add -r ~/testredirect -Q)
open http://$(echo $CID | ./cmd/ipfs/ipfs cid base32).ipfs.localhost:8080
FYI, I'll be out for a few weeks but will be checking email in case there is any PR feedback. I'm sure there will be things to address but I'm going to mark this as ready for review now.
After some discussion with @b5 and @dignifiedquire, I'm leaning toward pulling out forced redirect support to avoid a performance hit for anything other than non-existent paths (i.e. read _redirects file and check for matching redirects if and only if the path didn't resolve). If someone wants to force a redirect, they can just remove or rename the existing path that's preventing redirect rules from firing.
Review status... @lidel is wrapping up specs for existing gateway functionality and then hopes to get to this, possibly next week or the week after.
@lidel I believe I've addressed all feedback now. From my perspective the only question that is outstanding is https://github.com/ipfs/specs/pull/290#discussion_r944384586. I did add code to handle these extra http status codes, but it still feels to me like it may not make sense.
My hope is that we are finally close to having this pushed across the finish line. 🤞
Thanks again for your assistance.
@lidel regarding broken tests, I'm waiting for you to merge changes in go-ipfs-redirects-file first. 🙏
I'll take a look at the diff you sent, and today I learned about git push --force-with-lease . 🥳
I would love this to be included in the release, but understand the lack of bandwidth.
@lidel your diff's are now pushed on my branch. All tests pass locally for me when go-ipfs-redirects-file is replaced with my local clone.
- Tagged and switched to release version of the lib: https://github.com/ipfs/go-ipfs-redirects-file/releases/tag/v0.1.0
- Rebased on master, resolved conflicts
- Things are green, I will do final pass on this before EOW (making sure we did not miss anything like we did with cache-control).
- If nothing unexpected but there is high chance this will land in 0.16 (which got moved to Monday) :crossed_fingers:
- I want to merge specs (https://github.com/ipfs/specs/pull/290) AFTER this PR lands
Thanks for all the work here all! For being included in 0.16 (https://github.com/ipfs/kubo/issues/9237), we'll need to have this ready for review today (2022-09-23) because the RC gets cut on 2022-09-26 Europe time.
The changes are in and ready for review @BigLep 🙏
@lidel thanks a ton for all the work you put into this!
@justincjohnson fysa we are writing end-user docs for docs.ipfs.tech too, PR draft in https://github.com/ipfs/ipfs-docs/pull/1275