gateway: remote car gateway backend

hacdias commented 4 months ago

Closes #576. Generally, this imports the graph gateway, here named CarBackend, from our old bifrost-gateway repository. The following updates were also done:

Run Conformance Tests against the Remote Car Gateway
Re-use options between CarBackend and BlocksBackend
Update CarBackend such that CARs return useful blocks when paths are not not resolvable

Depends on:

https://github.com/ipfs/gateway-conformance/pull/202

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 52.14541% with 803 lines in your changes are missing coverage. Please review.

Project coverage is 59.65%. Comparing base (63c3ec6) to head (32ddd92).

@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
+ Coverage   59.52%   59.65%   +0.13%     
==========================================
  Files         233      238       +5     
  Lines       28276    29810    +1534     
==========================================
+ Hits        16830    17782     +952     
- Misses       9977    10415     +438     
- Partials     1469     1613     +144

Files	Coverage Δ
examples/gateway/car-file/main.go	`11.53% <ø> (ø)`
examples/gateway/proxy-blocks/main.go	`0.00% <0.00%> (ø)`
gateway/handler_unixfs_dir.go	`64.84% <50.00%> (ø)`
gateway/errors.go	`84.25% <37.50%> (+0.91%)`	:arrow_up:
gateway/value_store.go	`0.00% <0.00%> (ø)`
gateway/blockstore.go	`0.00% <0.00%> (ø)`
gateway/backend_car_fetcher.go	`78.72% <78.72%> (ø)`
gateway/backend_car_traversal.go	`72.72% <72.72%> (ø)`
examples/gateway/proxy-car/main.go	`0.00% <0.00%> (ø)`
gateway/backend_blocks.go	`44.71% <40.00%> (ø)`
... and 3 more

... and 16 files with indirect coverage changes

hacdias commented 3 months ago

@aschmahmann there are quite some tests failing for Gateway Conformance (Graph Gateway). I've already fixed some tests but maybe you have a better intuition for what might be failing here. I had to update the code quite a bit considering all the updates we've had in Boxo.

hacdias commented 3 months ago

We have now 10 failing conformance tests with the remote CAR gateway backend. I've managed to solve the other ones in the meanwhile.

`Cache-Control: only-if-cached`

This are currently failing because the Car Gateway does not have a local blockstore and does not cache anything. Therefore, we always return false to IsCached.

I think we should disable this test for this backend.

Range requests for DAG-CBOR converted to HTML

And this is a very interesting one. For the range requests, we initially do a request without a range and then accept either the correct range, the first range, or the whole content as response. But here's where it gets tricky!

When converting to HTML, Boxo does some magic to nicely display DAG-CBOR and DAG-JSON. When we have the whole data, we do it:

However, the Car Backend fetches a CAR with only the necessary blocks from the range request. Therefore, it does not have the necessary data to pretty print the DAG into HTML format:

And this makes the test fail (content of range request does not match range, nor the whole request).

I don't have a clear solution here: we could potentially try doing resolution first, and then acting depending on the codec of the CID. If the codec is not dag-pb and request accepts text/html (information currently not passed into .Get), then fetch everything without ranges. But this creates another problem: we'd be doing two requests and potentially more data than necessary for exotic formats.

Wdyt? I temporarily solved it with 6fe39c8 by adding a small exception.

cc @aschmahmann @lidel

hacdias commented 3 months ago

@lidel great, I do agree. I opened a PR to remove the conformance test (https://github.com/ipfs/gateway-conformance/pull/202) and I will revert the change here that I made in order to pass such test. We'll need to make a release of the conformance tests once merged.

hacdias commented 3 months ago

Besides removing the conformance test (PR here), this PR is ready to be reviewed. I just made a few cleanups, so here's a short summary of the most important:

You can now override the http.Client used by remote fetchers where we were always using the default newRemoteHTTPClient.
You can customize the GetFetchTimeout in the CarBackend
I made it clear that the CarBackend is not strictly remote. Just like the BlocksBackend, we can use it with different CarFetchers.
I made the utility NewRemoteCarBackend that creates a CarBackend with a RemoteCarFetcher.
I updated te CarFetcher interface to be more generic (path + CarParams) instead of the URL directly, making it more flexible.

hacdias commented 3 months ago

Note that there is one test that is "expected" [by github actions] (gateway-conformance). This needs to be updated in the configuration of the repository after merging this PR, since we updated the name of the test.

lidel commented 3 months ago

@hacdias thanks!

FYSA I've removed "gateway-conformance" workflow from required list for now.

Since boxo/gateway is de facto reference implementation of what is possible, gateway-conformance.yml workflow from this repo will be read by people who want to see prior art, learn how to integrate conformance tests into their projects.

I took a stab at cleaning it up + adding the missing test for "remote block backend" (where blocks are fetched one by one via ?format=raw) in https://github.com/ipfs/boxo/pull/587/commits/2ce7502f2b7b3af9eed59aa1c5958243a5d1d508.

(I've run out of time, will try to finish reviewing this PR tomorrow)

ipfs / boxo