gateway: remote car gateway backend

[hacdias]

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:

• feat: remove range test for dag-cbor as text/html gateway-conformance#202

[codecov]

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%)	⬆️
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]

@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]

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

[lidel]

Thank you for salvaging this work @hacdias ❤️

On only-if-cached

Disabling tests for backend_car" for now sgtm.

💭 Click for Rationale

In theory, we could wire it up to in-memory cache, and pre-warm the cache so it passes the test, but it feels like a time sink not worth the effort.

In practice, Cache-Control: only-if-cached woudl be handled by CDN/cache in front of gateway, so it is reasonable for gateway backed by a remote backend to always return HTTP 412. HTTP 200s will be returned only by the cache that sits in front of it.

---

On Range requests for DAG-CBOR converted to HTML

Thank you for surfacing this one. Took me a moment to understand what is happening here.

The way I see it, the conformance test of (dag-cbor + Accept with text/html + Range) in path_gateway_dag_test.go#L718-L731 makes no sense conformance-wise, and should be removed from conformance suite itself, and remove any hacks we added to make it pass.

Lmk if below missed the mark, but I think removing this edge case is the best way of handling it.

💭 Click for (long) Rationale why

Making HTTP Range request with Accept that includes text/html make little sense.
The only meaningful Range tests for dag-cbor are ones that fetch subset of CBOR bytes.
So if Accept is missing, */* or application/vnd.ipld.dag-cbor application/vnd.ipld.cbor.
The case where Accept is text/html is not meaningful.

This HTML is generated, getting subset of bytes of generated HTML preview page, while technically possible for HTTP client, is.. meh? Not useful at all, not related to receiving actual content-addressed data.

It would be a-okay regression test in Kubo, but should not be part of conformance.

I think the reason it ended up in conformance tests is that our approach to conformance was to play it safe and "better to test too much than too little". Which is good, and I still think was a good call.

When it comes to conformance, we have two opposite problems:

1. We test Kubo-specific behaviors. As we have implementations, we identify these, and over time relax, make generic, or remove.
2. We have no tests for implicit behaviors related to generic HTTP due to always using net/http from Golang
   • For example: for a long time we had HTTP Ranges working everywhere "for free" because net/http from golang handled things like Range and Content-Length seamlessly. But since gateway/serve_http_content.go was introduced in feat(gateway): more explicit IPFSBackend and no multi-range #369 we do all these things by hand, and only where it brings any value. This created a gaps, for example, we are lacking some of basic Range request tests (Add tests for HTTP Range requests gateway-conformance#154)

Hopefully this is useful context.
Over time, conformance tests will improve. The more we use them, the better.

[hacdias]

@lidel great, I do agree. I opened a PR to remove the conformance test (ipfs/gateway-conformance#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]

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]

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]

@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 2ce7502.

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

[lidel]

💭 i guess we have to live with both now.

iiuc we need v1 for WithErrorOnEmptyRoots which was introduced in ipld/go-car#461, and v2 does not support everything v1 does, and v3 mentioned in ipld/go-car#461 (comment) never happened.

[lidel]

Thank you for porting and cleaning this up @hacdias!

I think it is good enough: we have examples that show how to use it, and we dogfood both backends in conformance tests, which is a neat way to ensure they don't break without notice.

Lgtm, let's merge it.

For anyone following this work: the next step will be to expose support for block and car proxy modes in rainbow (ipfs/rainbow#88).