chore: Github release binary provider: Version check & static cache pather

op-service/github/release/provider.go

@@ -14,9 +14,12 @@ import (
 	"io"
 	"log"
 	"os"
+	"os/exec"
 	"path"
 	"runtime"
+	"strings"
 
+	"github.com/Masterminds/semver/v3"
 	"github.com/ethereum-optimism/optimism/op-service/httputil"
 	"github.com/ethereum-optimism/optimism/op-service/ioutil"
 )
@@ -28,6 +31,21 @@ type BinaryProvider interface {
 	Get(ctx context.Context, version string) (string, error)
 }
 
+// BinaryVersionCheckerFactory is a factory function that creates a BinaryVersionChecker for a
+// specific requested version and OS/architecture pair. This allows for customizable version
+// checking logic that may vary based on the target platform or version.
+type BinaryVersionCheckerFactory func(requestedVersion string, os string, arch string) (BinaryVersionChecker, error)
+
+// BinaryVersionChecker is a function that verifies the version of a downloaded binary matches
+// the requested version. It takes a context and the path to the binary as input and returns
+// an error if the version check fails (e.g., version mismatch or command execution error).
+type BinaryVersionChecker func(ctx context.Context, binary string) error
+
+// BinaryVersionComparator is a function that compares a requested version with the actual
+// version extracted from a binary. It returns an error if the versions do not match according
+// to the comparison logic (e.g., semver equality check).
+type BinaryVersionComparator func(requestedVersion string, actualVersion string) error
+
 // GithubReleaseChecksummer reads the downloaded archive data and validates its
 // checksum. It should return an error if the checksum does not match.
 type GithubReleaseChecksummer func(r io.Reader) error
@@ -94,12 +112,63 @@ type GithubReleaseDownloader struct {
 	// logger is optional and used for informational logging during the
 	// download/extract process. The `WithLogger` option sets this field.
 	logger *log.Logger
+
+	// versionCheckerFactory is optional and used to verify that the downloaded
+	// binary matches the requested version. If set, it will be invoked after
+	// successful download and extraction. The `WithVersionCheckerFactory` option
+	// sets this field.
+	versionCheckerFactory BinaryVersionCheckerFactory
 }
 
 var _ BinaryProvider = (*GithubReleaseDownloader)(nil)
 
 type GithubReleaseDownloaderOption func(*GithubReleaseDownloader)
 
+func WithVersionCheckerFactory(f BinaryVersionCheckerFactory) GithubReleaseDownloaderOption {
+	return func(d *GithubReleaseDownloader) {
+		d.versionCheckerFactory = f
+	}
+}
+
+func NewStaticCommandVersionCheckerFactory(args []string, outputParser func(stdout string) (string, error), comparator BinaryVersionComparator) BinaryVersionCheckerFactory {
+	return func(requestedVersion string, os string, arch string) (BinaryVersionChecker, error) {
+		return func(ctx context.Context, binary string) error {
+			cmd := exec.CommandContext(ctx, binary, args...)
+			out, err := cmd.Output()
+			if err != nil {
+				return fmt.Errorf("version check failed: command '%s %s' returned an error: %w", binary, strings.Join(args, " "), err)
+			}
+
+			actualVersion, err := outputParser(string(out))
+			if err != nil {
+				return fmt.Errorf("version check failed: could not parse version from output '%s': %w", string(out), err)
+			}
+
+			return comparator(requestedVersion, actualVersion)
+		}, nil
+	}
+}
+
+func NewSemverEqualityComparator() BinaryVersionComparator {
+	return func(requestedVersion string, actualVersion string) error {
+		requestedVersionSemver, err := semver.NewVersion(requestedVersion)
+		if err != nil {
+			return fmt.Errorf("failed to convert version %s to semver: %w", requestedVersion, err)
+		}
+
+		actualVersionSemver, err := semver.NewVersion(actualVersion)
+		if err != nil {
+			return fmt.Errorf("failed to convert version %s to semver: %w", actualVersion, err)
+		}
+
+		if requestedVersionSemver.Compare(actualVersionSemver) != 0 {
+			return fmt.Errorf("requested version %s does not match the actual one %s", requestedVersion, actualVersion)
+		}
+
+		return nil
+	}
+}
+
 func WithChecksummerFactory(c GithubReleaseChecksummerFactory) GithubReleaseDownloaderOption {
 	return func(d *GithubReleaseDownloader) {
 		d.checksummerFactory = c
@@ -147,6 +216,12 @@ func NewHomeDirCachePather(namespace string) GithubReleaseCachePather {
 	}
 }
 
+func NewStaticCachePather(cacheDir string) GithubReleaseCachePather {
+	return func() (string, error) {
+		return cacheDir, nil
+	}
+}
+
 func WithOSGetter(c GithubReleaseOSGetter) GithubReleaseDownloaderOption {
 	return func(d *GithubReleaseDownloader) {
 		d.osGetter = c
@@ -232,15 +307,30 @@ func (d *GithubReleaseDownloader) Get(ctx context.Context, version string) (stri
 		return "", fmt.Errorf("failed to get destination path: %w", err)
 	}
 
-	cached, err := d.getCached(ctx, version, destinationPath)
-	if err == nil {
-		// getCached returning an error indicates the name is not available
-		// (or not valid) in the cache; return the empty cached path and let
-		// the caller proceed to perform the download step.
-		return cached, nil
+	var binary string
+
+	binary, err = d.getCached(ctx, version, destinationPath)
+	if err != nil {
+		binary, err = d.download(ctx, version, releaseOS, releaseArch, destinationPath)
+	}
+
+	if err != nil {
+		return "", fmt.Errorf("failed to get binary %s of version %s: %w", d.name, version, err)
+	}
+
+	if d.versionCheckerFactory != nil {
+		versionChecker, err := d.versionCheckerFactory(version, releaseOS, releaseArch)
+		if err != nil {
+			return "", fmt.Errorf("failed to create version checker for binary %s of version %s: %w", d.name, version, err)
+		}
+
+		err = versionChecker(ctx, binary)
+		if err != nil {
+			return "", fmt.Errorf("version check failed for binary %s of version %s: %w", d.name, version, err)
+		}
 	}
 
-	return d.download(ctx, version, releaseOS, releaseArch, destinationPath)
+	return binary, nil
 }
 
 func (d *GithubReleaseDownloader) getDestinationPath(version string, os string, arch string) (string, error) {

op-service/github/release/provider_test.go

@@ -3,6 +3,8 @@ package release
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -39,7 +41,7 @@ func TestGithubReleaseDownloader_Forge(t *testing.T) {
 					"foundry",
 					"forge",
 					WithChecksummerFactory(NewStaticChecksummerFactory(checksums)),
-					WithCachePather(newStaticCachePather(cacheDir)),
+					WithCachePather(NewStaticCachePather(cacheDir)),
 					WithOSGetter(newStaticOSGetter(tgtOS, tgtArch)),
 					WithURLGetter(newForgeURLGetter()),
 				)
@@ -54,6 +56,78 @@ func TestGithubReleaseDownloader_Forge(t *testing.T) {
 		}
 	})
 
+	t.Run("version check", func(t *testing.T) {
+		var checksums map[string]string
+		err := json.Unmarshal(versionJSON, &checksums)
+		require.NoError(t, err)
+
+		cacheDir := t.TempDir()
+
+		t.Run("should fail if the command fails", func(t *testing.T) {
+			provider := NewGithubReleaseDownloader(
+				"foundry-rs",
+				"foundry",
+				"forge",
+				WithChecksummerFactory(NewStaticChecksummerFactory(checksums)),
+				WithCachePather(NewStaticCachePather(cacheDir)),
+				WithOSGetter(NewDefaultOSGetter()),
+				WithURLGetter(newForgeURLGetter()),
+				WithVersionCheckerFactory(NewStaticCommandVersionCheckerFactory([]string{"-idontexist"}, parseForgeVersionOutput, NewSemverEqualityComparator())),
+			)
+
+			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+			defer cancel()
+
+			binPath, err := provider.Get(ctx, "v1.1.0")
+			require.ErrorContains(t, err, "version check failed for binary forge of version v1.1.0: version check failed")
+			require.Empty(t, binPath)
+		})
+
+		t.Run("should fail if the versions do not match", func(t *testing.T) {
+			returnStaticUnmatchingForgeVersion := func(out string) (string, error) {
+				return "4.0.0", nil
+			}
+
+			provider := NewGithubReleaseDownloader(
+				"foundry-rs",
+				"foundry",
+				"forge",
+				WithChecksummerFactory(NewStaticChecksummerFactory(checksums)),
+				WithCachePather(NewStaticCachePather(cacheDir)),
+				WithOSGetter(NewDefaultOSGetter()),
+				WithURLGetter(newForgeURLGetter()),
+				WithVersionCheckerFactory(NewStaticCommandVersionCheckerFactory([]string{"-V"}, returnStaticUnmatchingForgeVersion, NewSemverEqualityComparator())),
+			)
+
+			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+			defer cancel()
+
+			binPath, err := provider.Get(ctx, "v1.1.0")
+			require.ErrorContains(t, err, "version check failed for binary forge of version v1.1.0: requested version v1.1.0 does not match the actual one 4.0.0")
+			require.Empty(t, binPath)
+		})
+
+		t.Run("should succeed if the versions match", func(t *testing.T) {
+			provider := NewGithubReleaseDownloader(
+				"foundry-rs",
+				"foundry",
+				"forge",
+				WithChecksummerFactory(NewStaticChecksummerFactory(checksums)),
+				WithCachePather(NewStaticCachePather(cacheDir)),
+				WithOSGetter(NewDefaultOSGetter()),
+				WithURLGetter(newForgeURLGetter()),
+				WithVersionCheckerFactory(NewStaticCommandVersionCheckerFactory([]string{"-V"}, parseForgeVersionOutput, NewSemverEqualityComparator())),
+			)
+
+			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+			defer cancel()
+
+			binPath, err := provider.Get(ctx, "v1.1.0")
+			require.NoError(t, err)
+			require.NotEmpty(t, binPath)
+		})
+	})
+
 	t.Run("invalid checksum", func(t *testing.T) {
 		cacheDir := t.TempDir()
 		provider := NewGithubReleaseDownloader(
@@ -63,7 +137,7 @@ func TestGithubReleaseDownloader_Forge(t *testing.T) {
 			WithChecksummerFactory(NewStaticChecksummerFactory(map[string]string{
 				"darwin_amd64": "invalidchecksum",
 			})),
-			WithCachePather(newStaticCachePather(cacheDir)),
+			WithCachePather(NewStaticCachePather(cacheDir)),
 			WithOSGetter(newStaticOSGetter("darwin", "amd64")),
 			WithURLGetter(newForgeURLGetter()),
 		)
@@ -83,7 +157,7 @@ func TestGithubReleaseDownloader_Forge(t *testing.T) {
 			"foundry",
 			"forge",
 			WithChecksummerFactory(NewStaticChecksummerFactory(map[string]string{})),
-			WithCachePather(newStaticCachePather(cacheDir)),
+			WithCachePather(NewStaticCachePather(cacheDir)),
 			WithOSGetter(newStaticOSGetter("linux", "amd64")),
 			WithURLGetter(newForgeURLGetter()),
 		)
@@ -107,7 +181,7 @@ func TestGithubReleaseDownloader_OpDeployer(t *testing.T) {
 		"ethereum-optimism",
 		"optimism",
 		"op-deployer",
-		WithCachePather(newStaticCachePather(cacheDir)),
+		WithCachePather(NewStaticCachePather(cacheDir)),
 		WithURLGetter(NewOPStackURLGetter()),
 		WithBinaryLocator(NewOPStackBinaryLocator()),
 	)
@@ -124,12 +198,6 @@ func newStaticOSGetter(os, arch string) GithubReleaseOSGetter {
 	}
 }
 
-func newStaticCachePather(cachePath string) GithubReleaseCachePather {
-	return func() (string, error) {
-		return cachePath, nil
-	}
-}
-
 func newForgeURLGetter() GithubReleaseURLGetter {
 	defaultURLGetter := NewDefaultURLGetter()
 
@@ -141,3 +209,12 @@ func newForgeURLGetter() GithubReleaseURLGetter {
 		return defaultURLGetter(owner, repo, "foundry", version, os, arch)
 	}
 }
+
+func parseForgeVersionOutput(out string) (string, error) {
+	re := regexp.MustCompile(`(\d+\.\d+\.\d+)`)
+	m := re.FindStringSubmatch(out)
+	if len(m) < 2 {
+		return "", fmt.Errorf("could not parse version tag from: %q", out)
+	}
+	return "v" + m[1], nil
+}