diff --git a/op-service/github/release/provider.go b/op-service/github/release/provider.go index ff61d353495..9a17ec08512 100644 --- a/op-service/github/release/provider.go +++ b/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) { diff --git a/op-service/github/release/provider_test.go b/op-service/github/release/provider_test.go index 085f4eb8a14..606b7be3998 100644 --- a/op-service/github/release/provider_test.go +++ b/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 +}