Update bootstrappers

[dirkmc]

As part of Fixing Bootstrapping we need to add a repo migration that

• removes bootstrap nodes with small keys
• adds new nodes with appropriately large keys

TODO:

• write sharness tests

[ribasushi]

See one comment, not sure if actionable

[ribasushi]

@dirkmc is it intentional that you are looking only for the .82 host here, but above ( line 96 ) there are 2 "old style" hosts?

Approving, as the rest looks good and the above is likely intentional

[dirkmc]

Good question - yes, in the migrate forward we move from oldConfig to expectedMigrateForward. In the migrate backwards we move from newConfig to expectedMigrateBackward for which I wanted a test that includes one peer that is "known" (in the list of peers with acceptably large keys) and one that is not in that list, to make sure both get through.

[dirkmc]

I updated the var names to make it a little clearer what's going on, and added some more explicit tests

[aschmahmann]

@dirkmc overall seems reasonable a couple comments/thoughts.

1. Repo migrations look really painful, it also looks like there's a bunch of boilerplate required to do a really simple change
2. Doing a repo migration to update bootstrappers seems a little wacky. I understand that if we need this now then we've just go to do it, but is there a better way to deal with this? If we have to do a migration anyway maybe we should add a UseDefaults boolean and just remove all of our bootstrappers, this will let us push updates in code instead of forcing repo migrations.
   • Note: we probably want to be able to push default config updates in code in the future anyway config: defaults difficult to update kubo#8776

[Stebalien]

General comment: we should try to respect user choices as much as possible. We need to add our new bootstrappers if the user hasn't explicitly removed our old bootstrappers and we should re-add the old bootstrappers if the user hasn't explicitly removed our new bootstrappers. Otherwise, the user is on their own.

Otherwise, we could cause problems for users on private/separate networks.

[dirkmc]

@aschmahmann you're right that there's a lot of boilerplate. Much of this code is copied from https://github.com/ipfs/fs-repo-migrations/tree/master/ipfs-5-to-6 (and much of that is copied from earlier version changes)
Because the code is intended to be used at one point in time and remain static thereafter I'm not sure if this is a big issue.
With respect to doing a repo migration to update bootstrappers I agree it is a little strange. It may be worth talking through a better way to apply config-only changes if this is something that happens often.

[aschmahmann]

@dirkmc as I mentioned in the issue I linked we really need to up our config file game so that we can deal with default values. Currently we just give people a config file on init and the only way to upgrade it is a repo migration or having users manually change the files.

We have some experimental features that can be enabled optionally via config (e.g. pubsub) that when we want to enable by default (we already do) we don't have any choices other than modifying the user's config file (e.g. via a repo migration)

[dirkmc]

@Stebalien I think I've addressed all your suggestions, let me know if the logic looks right to you now

[Stebalien]

Almost there. I have one somewhat paranoid nit (my network actually has an ipfs.local node so I wouldn't be surprised if people use /dns4/ipfs.my-domain.com as a bootstrapper).

[Stebalien]

@dirkmc how's your internet? To test this, I think we need to:

Build ipfs/kubo#6814 with https://github.com/ipfs/distributions:

1. ipfs get -o releases /ipns/dist.ipfs.io. This will download 10GiB of crap and add it to your local IPFS repo, FYI. However, you'll only have to download it once.
2. (./dist.sh add-version go-ipfs $HASH_OF_THAT_PR).
3. make publish (let's call the final hash MY_DIST_PATH

(checkout the readme in github.com/ipfs/distributions, it should walk you through this process)

This will publish a local release of IPFS with $HASH_OF_THAT_PR as the version. Note: Given that we normally use, you know, sane version numbers, I'm not sure if this is going to actually work. We may need a step 1.5 that moves the built release to some real version (v0.5.0-beta?). If so, you'll probably need to rename a bunch of files and edit the dist.json file.

Once you have everything built, you can write the sharness test. Model it after t0110-migration-6-to-7.sh:

1. Set IPFS_DIST_PATH to MY_DIST_PATH
2. Use $HASH_OF_THAT_PR as the new version and v0.4.22 as the old version.

If everything works, pin MY_DIST_PATH using pinbot. We don't need to publish this release, we just need it to stick around.

[Stebalien]

Merging as there's no reason to leave this sitting in a PR. It won't be used until we bump the repo version and cut a release.

[dirkmc]

Thanks 👍

I messed around with trying to get sharness tests working with a temp release but was having trouble with the old, vendored dependencies (eg ipfs-update).

It may make sense to wait till we have a RC so as to avoid having to cut a temp release and the versioning issues that raises when trying to write sharness tests.

[Stebalien]

Yeah, there's no point in wasting time on this. I'm going to see if I can clean up the deps at all but I won't spend too much time at it.