From b786e89b4216c6d3f29e6d1118a4998161cc3382 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Mon, 24 Aug 2026 15:27:20 -0700 Subject: [PATCH] Take the target version from the binary when there is no route out preflight asked the GitHub release API which version it was upgrading to, so a host with no internet failed there even with the target binary already on disk - and `run` never passed its own --target-binary down to preflight, so supplying one did not help. Read the version from the binary instead when one is given, and fail if it is not the version the run targets. Found by the first preflight against the production clone, which is deliberately cut off from the network. --- cmd/stalwart-migrate/preflight.go | 24 +++++----- cmd/stalwart-migrate/run.go | 2 +- internal/preflight/checks.go | 40 ++++++++++++---- internal/preflight/checks_test.go | 80 +++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 21 deletions(-) diff --git a/cmd/stalwart-migrate/preflight.go b/cmd/stalwart-migrate/preflight.go index 591427b..b16f403 100644 --- a/cmd/stalwart-migrate/preflight.go +++ b/cmd/stalwart-migrate/preflight.go @@ -24,6 +24,7 @@ func runPreflight(args []string) error { adminPassword := fs.String("admin-password", os.Getenv("STALWART_MIGRATE_ADMIN_PASSWORD"), "admin password for the reachability check (or set STALWART_MIGRATE_ADMIN_PASSWORD)") targetVersion := fs.String("target", "latest", `target Stalwart version, or "latest"`) + targetBinary := fs.String("target-binary", "", "read the target version from this already-downloaded binary instead of the release API (for a host with no route to the internet)") minFree := fs.Float64("min-free-multiple", 2.0, "required free disk space as a multiple of the data directory size") stateDir := fs.String("state-dir", checkpoint.DefaultBaseDir, "directory to store run checkpoints in") pythonPath := fs.String("python", "python3", "path to python3, needed by migrate_v016.py") @@ -39,17 +40,18 @@ func runPreflight(args []string) error { } checker := preflight.New(preflight.Options{ - BinaryPath: *binaryPath, - ConfigPath: *configPath, - DataDir: *dataDir, - ContainerName: *containerName, - AdminURL: *adminURL, - AdminUser: *adminUser, - AdminPassword: *adminPassword, - TargetVersion: *targetVersion, - MinFreeMultiple: *minFree, - CLIPath: *stalwartCLI, - PythonPath: *pythonPath, + BinaryPath: *binaryPath, + ConfigPath: *configPath, + DataDir: *dataDir, + ContainerName: *containerName, + AdminURL: *adminURL, + AdminUser: *adminUser, + AdminPassword: *adminPassword, + TargetVersion: *targetVersion, + TargetBinaryPath: *targetBinary, + MinFreeMultiple: *minFree, + CLIPath: *stalwartCLI, + PythonPath: *pythonPath, }) report, err := checker.Run(context.Background(), store, rs) diff --git a/cmd/stalwart-migrate/run.go b/cmd/stalwart-migrate/run.go index 45eb743..67a7803 100644 --- a/cmd/stalwart-migrate/run.go +++ b/cmd/stalwart-migrate/run.go @@ -157,7 +157,7 @@ func runRun(args []string) (err error) { pfReport, err := preflight.New(preflight.Options{ BinaryPath: *binaryPath, ConfigPath: *configPath, DataDir: *dataDir, ContainerName: *containerName, AdminURL: *adminURL, AdminUser: *adminUser, AdminPassword: *adminPassword, - TargetVersion: *targetVersion, MinFreeMultiple: *minFree, HTTPClient: httpClient, + TargetVersion: *targetVersion, TargetBinaryPath: *targetBinary, MinFreeMultiple: *minFree, HTTPClient: httpClient, CLIPath: *stalwartCLI, PythonPath: *pythonPath, }).Run(ctx, store, rs) fmt.Print(pfReport.String()) diff --git a/internal/preflight/checks.go b/internal/preflight/checks.go index 68e5367..e5813ef 100644 --- a/internal/preflight/checks.go +++ b/internal/preflight/checks.go @@ -18,15 +18,19 @@ import ( // Options configures a Checker. Every field has a conservative default // applied by New except the ones that must name a real path on this host. type Options struct { - BinaryPath string // installed stalwart binary, e.g. /usr/local/bin/stalwart - ConfigPath string // its config file (TOML pre-0.16, JSON 0.16+) - DataDir string // data directory to size/space-check - ContainerName string // docker container name, if applicable - AdminURL string // base URL for the JMAP reachability check; empty skips it - AdminUser string - AdminPassword string - TargetVersion string // e.g. "0.16.14" or "latest" - MinFreeMultiple float64 + BinaryPath string // installed stalwart binary, e.g. /usr/local/bin/stalwart + ConfigPath string // its config file (TOML pre-0.16, JSON 0.16+) + DataDir string // data directory to size/space-check + ContainerName string // docker container name, if applicable + AdminURL string // base URL for the JMAP reachability check; empty skips it + AdminUser string + AdminPassword string + TargetVersion string // e.g. "0.16.14" or "latest" + // TargetBinaryPath, when set, is read for the target version instead of + // asking the release API - the only way a host with no route out can + // pass this check. + TargetBinaryPath string + MinFreeMultiple float64 // CLIPath and PythonPath are the external programs the migration // shells out to. Checked before anything is touched - see // CheckExternalTools. @@ -103,6 +107,24 @@ func (c *Checker) Run(ctx context.Context, store *checkpoint.Store, rs *checkpoi } targetOutcome, err := runCheck("target-release", func() (CheckResult, string) { + // A binary already on disk answers the question the release API was + // being asked - which version are we upgrading to - without needing + // a route to the internet. A host that has none cannot reach the + // API at all, and failing here would stop it migrating even though + // everything it needs is present. + if c.opts.TargetBinaryPath != "" { + got, err := DetectVersion(ctx, c.opts.TargetBinaryPath) + if err != nil { + return CheckResult{Status: StatusFail, Detail: fmt.Sprintf("couldn't read the version of %s: %v", c.opts.TargetBinaryPath, err)}, "" + } + want := strings.TrimPrefix(c.opts.TargetVersion, "v") + if want != "" && want != "latest" && want != got { + return CheckResult{Status: StatusFail, Detail: fmt.Sprintf( + "%s reports version %s, but this run targets %s - migrating to a version nobody planned for", + c.opts.TargetBinaryPath, got, want)}, "" + } + return CheckResult{Status: StatusOK, Detail: fmt.Sprintf("target is %s, taken from %s (no release lookup needed)", got, c.opts.TargetBinaryPath)}, got + } rel, err := ResolveRelease(ctx, c.opts.HTTPClient, c.opts.TargetVersion) if err != nil { return CheckResult{Status: StatusFail, Detail: err.Error()}, "" diff --git a/internal/preflight/checks_test.go b/internal/preflight/checks_test.go index d2e97d0..ef68876 100644 --- a/internal/preflight/checks_test.go +++ b/internal/preflight/checks_test.go @@ -329,3 +329,83 @@ func TestCheckerRunCapturesAccountSnapshotWhenAdminURLSet(t *testing.T) { t.Errorf("resumed PreflightSnapshot = %+v, want AccountCount 1", resumed.PreflightSnapshot) } } + +// A host with no route to the internet cannot reach the release API, and +// failing there would stop it migrating even though the binary it needs is +// already on disk. The binary itself answers the question. +func TestTargetReleaseReadsALocalBinaryInsteadOfTheAPI(t *testing.T) { + fakeTool(t, "stalwart-cli", "stalwart-cli 1.0.12") + fakeTool(t, "python3", "Python 3.13.5") + counter := filepath.Join(t.TempDir(), "invocations") + current := writeFakeBinary(t, "0.15.5", counter) + target := writeFakeBinary(t, "0.16.19", counter) + + configPath := filepath.Join(t.TempDir(), "config.toml") + if err := os.WriteFile(configPath, []byte("[store.\"rocksdb\"]\ntype = \"rocksdb\"\n"), 0o644); err != nil { + t.Fatal(err) + } + dataDir := t.TempDir() + + // Point the release API at a listener that fails the test if used. + withFakeGithub(t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("the release API must not be consulted when a local target binary was given (%s)", r.URL.Path) + w.WriteHeader(http.StatusInternalServerError) + }) + + store := checkpoint.NewStore(t.TempDir()) + rs, err := store.Create("", "0.16.19") + if err != nil { + t.Fatal(err) + } + report, err := New(Options{ + BinaryPath: current, ConfigPath: configPath, DataDir: dataDir, + TargetVersion: "0.16.19", TargetBinaryPath: target, MinFreeMultiple: 0.0001, + }).Run(context.Background(), store, rs) + if err != nil { + t.Fatalf("preflight: %v", err) + } + var found *CheckResult + for i := range report.Results { + if report.Results[i].Name == "target-release" { + found = &report.Results[i] + } + } + if found == nil || found.Status != StatusOK { + t.Fatalf("target-release = %+v, want OK without touching the network\n%s", found, report.String()) + } + if !strings.Contains(found.Detail, "0.16.19") { + t.Fatalf("detail should name the version it read, got %q", found.Detail) + } +} + +func TestTargetReleaseRejectsAMismatchedLocalBinary(t *testing.T) { + fakeTool(t, "stalwart-cli", "stalwart-cli 1.0.12") + fakeTool(t, "python3", "Python 3.13.5") + counter := filepath.Join(t.TempDir(), "invocations") + configPath := filepath.Join(t.TempDir(), "config.toml") + if err := os.WriteFile(configPath, []byte("[store.\"rocksdb\"]\ntype = \"rocksdb\"\n"), 0o644); err != nil { + t.Fatal(err) + } + withFakeGithub(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) }) + store := checkpoint.NewStore(t.TempDir()) + rs, err := store.Create("", "0.16.19") + if err != nil { + t.Fatal(err) + } + report, err := New(Options{ + BinaryPath: writeFakeBinary(t, "0.15.5", counter), ConfigPath: configPath, DataDir: t.TempDir(), + TargetVersion: "0.16.19", TargetBinaryPath: writeFakeBinary(t, "0.16.14", counter), MinFreeMultiple: 0.0001, + }).Run(context.Background(), store, rs) + if err != nil { + t.Fatalf("preflight: %v", err) + } + for _, r := range report.Results { + if r.Name == "target-release" { + if r.Status != StatusFail { + t.Fatalf("target-release = %+v, want FAIL for a binary that is not the target", r) + } + return + } + } + t.Fatal("no target-release check in the report") +}