-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fs-repo-migrations is difficult to package #148
Comments
I think I figured it out. I built every migration individually and then added the resulting binaries to the PATH of fs-repo-migrations. See NixOS/nixpkgs#161547 for how this looks in detail. |
https://github.com/ipfs/fs-repo-migrations/releases/tag/v2.0.2 This is pretty much a complete rewrite of the ipfs-migrator package. In version 2.0.0 a major change was made to the way the migrator works. Before, there was one binary that contained every migration. Now every migration has its own binary. If fs-repo-migrations can't find a required binary in the PATH, it will download it off the internet. To prevent that, build every migration individually, symlink them all into one package and then wrap fs-repo-migrations so it finds the package with all the migrations. The change to the IPFS NixOS module and the IPFS package is needed because without explicitly specifying a repo version to migrate to, fs-repo-migrations will query the internet to find the latest version. This fails in the sandbox, for example when testing the ipfs passthru tests. While it may seem like the repoVersion and IPFS version are in sync and the code could be simplified, this is not the case. See https://github.com/ipfs/fs-repo-migrations#when-should-i-migrate for a table with the IPFS versions and corresponding repo versions. Go 1.17 breaks the migrations, so use Go 1.16 instead. This is also the Go version used in their CI, see https://github.com/ipfs/fs-repo-migrations/blob/3dc218e3006adac25e1cb5e969d7c9d961f15ddd/.github/workflows/test.yml#L4. See ipfs/fs-repo-migrations#140 (comment) for a previous mention of this issue. The issue manifests itself when doing anything with a migration, for example `fs-repo-11-to-12 --help`: ``` panic: qtls.ClientHelloInfo doesn't match goroutine 1 [running]: github.com/marten-seemann/qtls-go1-15.init.0() github.com/marten-seemann/[email protected]/unsafe.go:20 0x132 ``` Also add myself as a maintainer for this package. This fixes the test failure discovered in NixOS#160914. See ipfs/fs-repo-migrations#148 to read some of my struggles with updating this package.
https://github.com/ipfs/fs-repo-migrations/releases/tag/v2.0.2 This is pretty much a complete rewrite of the ipfs-migrator package. In version 2.0.0 a major change was made to the way the migrator works. Before, there was one binary that contained every migration. Now every migration has its own binary. If fs-repo-migrations can't find a required binary in the PATH, it will download it off the internet. To prevent that, build every migration individually, symlink them all into one package and then wrap fs-repo-migrations so it finds the package with all the migrations. The change to the IPFS NixOS module and the IPFS package is needed because without explicitly specifying a repo version to migrate to, fs-repo-migrations will query the internet to find the latest version. This fails in the sandbox, for example when testing the ipfs passthru tests. While it may seem like the repoVersion and IPFS version are in sync and the code could be simplified, this is not the case. See https://github.com/ipfs/fs-repo-migrations#when-should-i-migrate for a table with the IPFS versions and corresponding repo versions. Go 1.17 breaks the migrations, so use Go 1.16 instead. This is also the Go version used in their CI, see https://github.com/ipfs/fs-repo-migrations/blob/3dc218e3006adac25e1cb5e969d7c9d961f15ddd/.github/workflows/test.yml#L4. See ipfs/fs-repo-migrations#140 (comment) for a previous mention of this issue. The issue manifests itself when doing anything with a migration, for example `fs-repo-11-to-12 --help`: ``` panic: qtls.ClientHelloInfo doesn't match goroutine 1 [running]: github.com/marten-seemann/qtls-go1-15.init.0() github.com/marten-seemann/[email protected]/unsafe.go:20 0x132 ``` Also add myself as a maintainer for this package. This fixes the test failure discovered in #160914. See ipfs/fs-repo-migrations#148 to read some of my struggles with updating this package.
The current layout really isn't very well supported with the go toolchain and also with the build helpers voidlinux uses. The more usual way to do things is to have a single module at the toplevel of the repository and mimic the usual Additionally this desperately needs a new release. |
As it stands now, the 11-to-12 migration won't run |
Note that the functionality of executing binaries downloaded from the internet introduces a security problem - if an attacker is able to gain control of the domain hosting the binaries relative to a target (e.g. through squatting on the domain or local DNS shenanigans), then that attacker can feed downloaders whatever binaries they want that then get executed by go-ipfs. It seems like validating the checksums (at least) with checksums written into the application itself would mitigate this. |
- Migrate to pkgs/by-name - Format with nixfmt-rfc-style - Make it possible to remove support for very old migrations in the future by increasing the `minRepoVersion` parameter - Rename kubo-migrator-all-fs-repo-migrations to kubo-fs-repo-migrations since it may no longer include all migrations - Add an alias for kubo-migrator-all-fs-repo-migrations to keep backwards compatibility - Update descriptions to differentiate between kubo-migrator and kubo-migrator-unwrapped and better describe the purpose of the migrator - Add a description to every individual migration - Add a description to kubo-fs-repo-migrations - Fetch the source code of the individual migrations from their specific Git tags, like upstream intends - Enable tests for some migrations - Check that the migrations don't crash on startup - Mark two broken migrations as broken. They are not compatible with the latest Go versions and upstream is not interested in fixing this - Change code to allow most updates to be done by only changing three lines (add new version and change git tag and hash) - Add a stub for any disabled or broken migration to prevent downloading unsigned binaries from the internet, see ipfs/fs-repo-migrations#148 (comment) and ipfs/fs-repo-migrations#188 - Use `lib.getExe` instead of hardcoding the binary name in the kubo NixOS module - Use `substituteInPlace` with `--replace-fail` instead of `--replace`
- Migrate to pkgs/by-name - Format with nixfmt-rfc-style - Make it possible to remove support for very old migrations in the future by increasing the `minRepoVersion` parameter - Rename kubo-migrator-all-fs-repo-migrations to kubo-fs-repo-migrations since it may no longer include all migrations - Add an alias for kubo-migrator-all-fs-repo-migrations to keep backwards compatibility - Update descriptions to differentiate between kubo-migrator and kubo-migrator-unwrapped and better describe the purpose of the migrator - Add a description to every individual migration - Add a description to kubo-fs-repo-migrations - Fetch the source code of the individual migrations from their specific Git tags, like upstream intends - Enable tests for some migrations - Check that the migrations don't crash on startup - Mark two broken migrations as broken. They are not compatible with the latest Go versions and upstream is not interested in fixing this - Change code to allow most updates to be done by only changing three lines (add new version and change git tag and hash) - Add a stub for any disabled or broken migration to prevent downloading unsigned binaries from the internet, see ipfs/fs-repo-migrations#148 (comment) and ipfs/fs-repo-migrations#188 - Use `lib.getExe` instead of hardcoding the binary name in the kubo NixOS module - Use `substituteInPlace` with `--replace-fail` instead of `--replace`
As can be seen at https://repology.org/project/fs-repo-migrations/versions, the latest fs-repo-migrations version in every repository is still 1.7.1. Version 2.0.1 was released on 01 Apr 2021, which is now close to one year old, yet no repository has updated to this version.
Perhaps the reason for this is that it is very difficult to build fs-repo-migrations in a way that allows it to function without downloading binaries from the internet.
The reasons for splitting fs-repo-migrations into many independent pieces mentioned at #98 regarding growing size and poor maintainability seem valid but the way this was implemented and the seeming lack of documentation (I couldn't find what I needed) makes it difficult to build everything and make fs-repo-migrations use my own binaries instead of downloading them.
Downloading binaries off the internet is not really in the spirit of software/Linux distributions, also see https://drewdevault.com/2021/09/27/Let-distros-do-their-job.html.
I realize, that fs-repo-migrations is now "deprecated" according to #98 but I don't see a good alternative. NixOS for example has code to optionally automatically run the migrations before starting the IPFS daemon: NixOS/nixpkgs@8bce303. This automatically upgrades the IPFS repo without requiring manual intervention. I don't see how else the same thing can be achieved.
Please provide a way (and document that way) to build all the different pieces locally and allow fs-repo-migrations to use those pieces.
If this is already possible, I apologize but I couldn't figure out how to do it. But I'm also absolutely not an expert in building Go packages.
Thank you.
The text was updated successfully, but these errors were encountered: