ipfs / kubo

An IPFS implementation in Go
https://docs.ipfs.tech/how-to/command-line-quick-start/
Other
15.83k stars 2.96k forks source link

repo/fsrepo/migrations: verified HTTP migrations #10324

Closed hacdias closed 2 months ago

hacdias commented 3 months ago

Closes #9159. This changes the current HttpFetcher to use CAR files. Because we have /ipns/dist.ipfs.tech as default (even though not used), I also implemented DNSLink resolution, as well as IPNS record fetching and verification.

Go tests were updated such that the test server now uses a CAR file backed gateway. This CAR file is generated on the fly before the tests begin. The reasoning behind this versus a static CAR is that the migration file names depend on the platform that they are being run on.

You can also test this locally by spawning a repository, downgrading the version, and running the migrations:

# Build local version (this PR)
make build

# New Kubo repo
export IPFS_PATH=$(mktemp -d)
./cmd/ipfs/ipfs init

# Change version to older version
echo 13 > $IPFS_PATH/version

# Start daemon and accept migrations
./cmd/ipfs/ipfs daemon
hacdias commented 3 months ago

I can't figure out a way of "fixing" the Docker test that checks if Docker downloads the correct version. The problem here is that uses the hardcoded distribution CIDs in the code. But I don't have that CAR file to just serve with socat. And the file would be massive nevertheless.

I would suggest removing this test unless someone has a very good idea.

aschmahmann commented 3 months ago

@hacdias removing that test (or reworking it to not use docker) might be reasonable.

However, if you wanted to keep this test working roughly as is you might be able to work around the large CAR issue by setting the IPFS_DIST_PATH env var to the root CID of a small DAG that you can control + embed. https://github.com/ipfs/kubo/blob/4d3cc96c1eaf9e180487f0eceff7352e0b089acb/repo/fsrepo/migrations/fetcher.go#L19

hacdias commented 3 months ago

@aschmahmann I think the test uses docker on purpose: the idea seems to be that the Docker image fetches the correct migration for the correct arch and platform. I will try what you said and see how it goes. Otherwise, I will just remove it.

hacdias commented 3 months ago

@aschmahmann the environment variable seems to do the trick 😄