diff --git a/internal/envoy/install.go b/internal/envoy/install.go index dd44085e..1cb50b86 100644 --- a/internal/envoy/install.go +++ b/internal/envoy/install.go @@ -82,15 +82,35 @@ func InstallIfNeeded(ctx context.Context, o *globals.GlobalOpts) (string, error) if mtime, err = time.Parse("2006-01-02", string(releaseDate)); err != nil { return "", fmt.Errorf("couldn't find releaseDate of version %q for platform %q: %w", v, o.Platform, err) } - if err = os.MkdirAll(installPath, 0o750); err != nil { - return "", fmt.Errorf("unable to create directory %q: %w", installPath, err) + // Unarchive into a staging directory next to the install path, and only move it into place + // once the SHA-256 sum matches. Otherwise an interrupted download leaves a partial binary + // at the install path, which later runs treat as already downloaded and never repair. + versionsDir := o.EnvoyVersionsDir() + if err = os.MkdirAll(versionsDir, 0o750); err != nil { + return "", fmt.Errorf("unable to create directory %q: %w", versionsDir, err) } + // The staging directory shares a filesystem with the install path, so the move is atomic, + // and its name is not a version, so "func-e versions" ignores it if anything is left behind. + stageDir, err := os.MkdirTemp(versionsDir, ".download-") + if err != nil { + return "", fmt.Errorf("unable to create directory in %q: %w", versionsDir, err) + } + defer os.RemoveAll(stageDir) //nolint:errcheck // best effort cleanup of a partial download + stagePath := filepath.Join(stageDir, v.String()) + o.Logf("downloading %s\n", tarballURL) - if err := untarEnvoy(ctx, o.HTTPClient, installPath, tarballURL, sha256Sum, o.UserAgent); err != nil { + if err := untarEnvoy(ctx, o.HTTPClient, stagePath, tarballURL, sha256Sum, o.UserAgent); err != nil { return "", err } - if err = os.Chtimes(installPath, mtime, mtime); err != nil { // overwrite the mtime to preserve it in the list - return "", fmt.Errorf("unable to set date of directory %q: %w", installPath, err) + if err = os.Chtimes(stagePath, mtime, mtime); err != nil { // overwrite the mtime to preserve it in the list + return "", fmt.Errorf("unable to set date of directory %q: %w", stagePath, err) + } + // A re-download of "dev" has an existing directory in the way of the move. + if err = os.RemoveAll(installPath); err != nil { + return "", fmt.Errorf("unable to remove directory %q: %w", installPath, err) + } + if err = os.Rename(stagePath, installPath); err != nil { + return "", fmt.Errorf("unable to move directory %q to %q: %w", stagePath, installPath, err) } case err == nil: o.Logf("%s is already downloaded\n", v) diff --git a/internal/envoy/install_test.go b/internal/envoy/install_test.go index dee560a4..e0270c42 100644 --- a/internal/envoy/install_test.go +++ b/internal/envoy/install_test.go @@ -271,3 +271,54 @@ func setupInstallTest(t *testing.T, v version.PatchVersion) *installTest { setup.GetEnvoyVersions = NewGetVersions(setup.HTTPClient, setup.EnvoyVersionsURL, setup.UserAgent) return setup } + +// An interrupted download must leave nothing at the install path: a partial binary there +// makes every later run report "already downloaded" and reuse it, with no way to self-heal. +func TestInstallIfNeeded_InterruptedDownload(t *testing.T) { + o := setupInstallTest(t, version.LastKnownEnvoy) + o.EnvoyVersion = version.LastKnownEnvoy + tarball, _ := test.RequireFakeEnvoyTarGz(t, version.LastKnownEnvoy) + + // Serve a truncated tarball to simulate the connection dropping mid-download. + versionsHandler := test.NewEnvoyVersionsHandler(t, "http://"+admin.ServerAddr, version.LastKnownEnvoy) + o.HTTPClient = httptest.HTTPClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if filepath.Ext(r.URL.Path) == ".json" { + versionsHandler.ServeHTTP(w, r) + return + } + w.WriteHeader(http.StatusOK) + w.Write(tarball[:len(tarball)/2]) + })) + o.GetEnvoyVersions = NewGetVersions(o.HTTPClient, o.EnvoyVersionsURL, o.UserAgent) + + _, err := InstallIfNeeded(o.ctx, &o.GlobalOpts) + require.ErrorContains(t, err, "error untarring") + + installPath := filepath.Join(o.EnvoyVersionsDir(), version.LastKnownEnvoy.String()) + require.NoDirExists(t, installPath, "a failed download must not leave a partial install") + staged, err := filepath.Glob(filepath.Join(o.EnvoyVersionsDir(), ".download-*")) + require.NoError(t, err) + require.Empty(t, staged, "the staging directory must be removed on failure") + + // With a healthy upstream, the retry must download instead of reusing a partial install. + o.HTTPClient = httptest.HTTPClient(versionsHandler) + o.GetEnvoyVersions = NewGetVersions(o.HTTPClient, o.EnvoyVersionsURL, o.UserAgent) + o.Out = new(bytes.Buffer) + envoyPath, err := InstallIfNeeded(o.ctx, &o.GlobalOpts) + require.NoError(t, err) + require.FileExists(t, envoyPath) + require.Contains(t, o.Out.(*bytes.Buffer).String(), "downloading") +} + +func TestInstallIfNeeded_InstallDirectoryMode(t *testing.T) { + o := setupInstallTest(t, version.LastKnownEnvoy) + o.EnvoyVersion = version.LastKnownEnvoy + + _, err := InstallIfNeeded(o.ctx, &o.GlobalOpts) + require.NoError(t, err) + + installPath := filepath.Join(o.EnvoyVersionsDir(), version.LastKnownEnvoy.String()) + stat, err := os.Stat(installPath) + require.NoError(t, err) + require.Equal(t, os.FileMode(0o750), stat.Mode().Perm()) +}