fix: stage Envoy downloads and only move verified installs into place

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)

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())
+}