Skip to content
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

Open
Luflosi opened this issue Jan 11, 2022 · 4 comments
Open

fs-repo-migrations is difficult to package #148

Luflosi opened this issue Jan 11, 2022 · 4 comments
Labels
need/triage Needs initial labeling and prioritization

Comments

@Luflosi
Copy link
Contributor

Luflosi commented Jan 11, 2022

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.

@Luflosi Luflosi added the need/triage Needs initial labeling and prioritization label Jan 11, 2022
@Luflosi
Copy link
Contributor Author

Luflosi commented Feb 23, 2022

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.

Luflosi added a commit to Luflosi/nixpkgs that referenced this issue Feb 25, 2022
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.
jonringer pushed a commit to NixOS/nixpkgs that referenced this issue Feb 25, 2022
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.
@lemmi
Copy link

lemmi commented Jul 1, 2022

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 $repo/cmd/fs-repo-$X-to-$Y layout.
Keeping the current layout is proably also fine, but there needs to be a toplevel module, otherwise you manually need to change into every folder and do the build there instead of just being able to do a go build ./fs-repo-X-to-Y.
Packing everything into a subdirectory should make it possible to run a single go build ./cmd/..., utilising available parallelism even better.

Additionally this desperately needs a new release.

void-linux/void-packages#37775

@CMB
Copy link

CMB commented Sep 22, 2022

As it stands now, the 11-to-12 migration won't run
when built for Void Linux because of issue 156.
Void only offers Go 1.19.

@ajakk
Copy link

ajakk commented Sep 15, 2024

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.

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.

Luflosi added a commit to Luflosi/nixpkgs that referenced this issue Oct 4, 2024
- 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`
Luflosi added a commit to Luflosi/nixpkgs that referenced this issue Oct 26, 2024
- 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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

4 participants