From 62cf7738bb58fbcddd67f8f7cb7de48364b4ef39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Mon, 11 May 2026 11:49:05 +0200 Subject: [PATCH] refactor: extract shared func CLI download with checksum verification (#132) (cherry picked from commit 7d2fb0dfe918a781daac7ee00bddebe4f4f675fb) --- internal/funccli/download.go | 147 +++++++++++++++++ internal/funccli/download_test.go | 266 ++++++++++++++++++++++++++++++ internal/funccli/manager.go | 126 +------------- test/utils/func.go | 45 ++--- 4 files changed, 428 insertions(+), 156 deletions(-) create mode 100644 internal/funccli/download.go create mode 100644 internal/funccli/download_test.go diff --git a/internal/funccli/download.go b/internal/funccli/download.go new file mode 100644 index 0000000..c15779a --- /dev/null +++ b/internal/funccli/download.go @@ -0,0 +1,147 @@ +package funccli + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + goruntime "runtime" + "strings" + "time" +) + +// DownloadOptions configures the binary download behavior. +type DownloadOptions struct { + // HTTPClient is the HTTP client to use for downloading. + // If nil, a default client with a 30-second timeout is used. + HTTPClient *http.Client +} + +func (o *DownloadOptions) httpClient() *http.Client { + if o != nil && o.HTTPClient != nil { + return o.HTTPClient + } + return &http.Client{Timeout: 30 * time.Second} +} + +// AssetName returns the func CLI asset name for the current platform +// (e.g. "func_linux_amd64"). +func AssetName() string { + return fmt.Sprintf("func_%s_%s", goruntime.GOOS, goruntime.GOARCH) +} + +// DownloadAndInstall downloads a func CLI binary from binaryURL, verifies its +// SHA256 checksum against the checksums.txt at checksumURL, and atomically +// installs it to installPath. The parent directory of installPath must exist. +func DownloadAndInstall(ctx context.Context, binaryURL, checksumURL, installPath string, opts *DownloadOptions) error { + client := opts.httpClient() + assetName := filepath.Base(binaryURL) + + tmpFile := installPath + ".tmp" + if err := downloadToFile(ctx, client, binaryURL, tmpFile); err != nil { + return fmt.Errorf("failed to download binary: %w", err) + } + + if err := verifyFileChecksum(ctx, client, tmpFile, assetName, checksumURL); err != nil { + _ = os.Remove(tmpFile) + return fmt.Errorf("checksum verification failed: %w", err) + } + + if err := os.Chmod(tmpFile, 0o755); err != nil { + _ = os.Remove(tmpFile) + return fmt.Errorf("failed to make binary executable: %w", err) + } + + if err := os.Rename(tmpFile, installPath); err != nil { + _ = os.Remove(tmpFile) + return fmt.Errorf("failed to install binary: %w", err) + } + + return nil +} + +func downloadToFile(ctx context.Context, client *http.Client, url, path string) error { + req, err := http.NewRequestWithContext(ctx, "GET", url, nil) + if err != nil { + return err + } + + resp, err := client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("download failed with status %d", resp.StatusCode) + } + + out, err := os.Create(path) + if err != nil { + return err + } + defer out.Close() + + _, err = io.Copy(out, resp.Body) + return err +} + +func verifyFileChecksum(ctx context.Context, client *http.Client, filePath, assetName, checksumURL string) error { + expectedHash, err := fetchExpectedChecksum(ctx, client, assetName, checksumURL) + if err != nil { + return err + } + + f, err := os.Open(filePath) + if err != nil { + return fmt.Errorf("failed to open file for checksum: %w", err) + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return fmt.Errorf("failed to compute checksum: %w", err) + } + + actualHash := hex.EncodeToString(h.Sum(nil)) + if actualHash != expectedHash { + return fmt.Errorf("sha256 mismatch: expected %s, got %s", expectedHash, actualHash) + } + + return nil +} + +func fetchExpectedChecksum(ctx context.Context, client *http.Client, assetName, checksumURL string) (string, error) { + req, err := http.NewRequestWithContext(ctx, "GET", checksumURL, nil) + if err != nil { + return "", err + } + + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("failed to download checksums: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("checksums download failed with status %d", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("failed to read checksums: %w", err) + } + + for _, line := range strings.Split(strings.TrimSpace(string(body)), "\n") { + parts := strings.Fields(line) + if len(parts) == 2 && parts[1] == assetName { + return parts[0], nil + } + } + + return "", fmt.Errorf("no checksum found for asset %s", assetName) +} diff --git a/internal/funccli/download_test.go b/internal/funccli/download_test.go new file mode 100644 index 0000000..10eeb39 --- /dev/null +++ b/internal/funccli/download_test.go @@ -0,0 +1,266 @@ +package funccli + +import ( + "context" + "crypto/sha256" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "testing" + "time" +) + +func TestAssetName(t *testing.T) { + expected := fmt.Sprintf("func_%s_%s", runtime.GOOS, runtime.GOARCH) + if got := AssetName(); got != expected { + t.Errorf("AssetName() = %q, want %q", got, expected) + } +} + +func newTestServer(binaryContent []byte, checksumContent string) *httptest.Server { + mux := http.NewServeMux() + mux.HandleFunc("/binary", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(binaryContent) + }) + mux.HandleFunc("/checksums.txt", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(checksumContent)) + }) + return httptest.NewServer(mux) +} + +func sha256Hex(data []byte) string { + h := sha256.Sum256(data) + return fmt.Sprintf("%x", h) +} + +func TestDownloadAndInstall_Success(t *testing.T) { + binaryContent := []byte("#!/bin/sh\necho hello") + hash := sha256Hex(binaryContent) + checksums := fmt.Sprintf("%s binary\n", hash) + + srv := newTestServer(binaryContent, checksums) + defer srv.Close() + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + context.Background(), + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + &DownloadOptions{HTTPClient: srv.Client()}, + ) + if err != nil { + t.Fatalf("DownloadAndInstall() error = %v", err) + } + + got, err := os.ReadFile(installPath) + if err != nil { + t.Fatalf("reading installed file: %v", err) + } + if string(got) != string(binaryContent) { + t.Errorf("installed content = %q, want %q", got, binaryContent) + } + + info, err := os.Stat(installPath) + if err != nil { + t.Fatalf("stat installed file: %v", err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Errorf("installed file is not executable, mode = %v", info.Mode()) + } + + tmpFile := installPath + ".tmp" + if _, err := os.Stat(tmpFile); !os.IsNotExist(err) { + t.Errorf("temp file %s should not exist after successful install", tmpFile) + } +} + +func TestDownloadAndInstall_ChecksumMismatch(t *testing.T) { + binaryContent := []byte("real binary") + checksums := "0000000000000000000000000000000000000000000000000000000000000000 binary\n" + + srv := newTestServer(binaryContent, checksums) + defer srv.Close() + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + context.Background(), + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + &DownloadOptions{HTTPClient: srv.Client()}, + ) + if err == nil { + t.Fatal("expected error for checksum mismatch, got nil") + } + + if _, statErr := os.Stat(installPath); !os.IsNotExist(statErr) { + t.Error("install path should not exist after checksum mismatch") + } + + tmpFile := installPath + ".tmp" + if _, statErr := os.Stat(tmpFile); !os.IsNotExist(statErr) { + t.Error("temp file should be cleaned up after checksum mismatch") + } +} + +func TestDownloadAndInstall_BinaryDownloadFails(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/binary", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + }) + mux.HandleFunc("/checksums.txt", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("abc binary\n")) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + context.Background(), + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + &DownloadOptions{HTTPClient: srv.Client()}, + ) + if err == nil { + t.Fatal("expected error for binary download failure, got nil") + } +} + +func TestDownloadAndInstall_ChecksumDownloadFails(t *testing.T) { + binaryContent := []byte("binary data") + + mux := http.NewServeMux() + mux.HandleFunc("/binary", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(binaryContent) + }) + mux.HandleFunc("/checksums.txt", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + context.Background(), + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + &DownloadOptions{HTTPClient: srv.Client()}, + ) + if err == nil { + t.Fatal("expected error for checksum download failure, got nil") + } + + tmpFile := installPath + ".tmp" + if _, statErr := os.Stat(tmpFile); !os.IsNotExist(statErr) { + t.Error("temp file should be cleaned up after checksum download failure") + } +} + +func TestDownloadAndInstall_CustomHTTPClient(t *testing.T) { + clientUsed := false + + binaryContent := []byte("test binary") + hash := sha256Hex(binaryContent) + checksums := fmt.Sprintf("%s binary\n", hash) + + srv := newTestServer(binaryContent, checksums) + defer srv.Close() + + customTransport := roundTripFunc(func(req *http.Request) (*http.Response, error) { + clientUsed = true + return http.DefaultTransport.RoundTrip(req) + }) + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + context.Background(), + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + &DownloadOptions{HTTPClient: &http.Client{Transport: customTransport}}, + ) + if err != nil { + t.Fatalf("DownloadAndInstall() error = %v", err) + } + + if !clientUsed { + t.Error("custom HTTP client was not used") + } +} + +func TestDownloadAndInstall_NilOptions(t *testing.T) { + binaryContent := []byte("test binary") + hash := sha256Hex(binaryContent) + checksums := fmt.Sprintf("%s binary\n", hash) + + srv := newTestServer(binaryContent, checksums) + defer srv.Close() + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + context.Background(), + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + nil, + ) + if err != nil { + t.Fatalf("DownloadAndInstall(opts=nil) error = %v", err) + } + + if _, err := os.Stat(installPath); err != nil { + t.Errorf("installed file should exist: %v", err) + } +} + +func TestDownloadAndInstall_ContextCancelled(t *testing.T) { + binaryContent := []byte("test binary") + hash := sha256Hex(binaryContent) + checksums := fmt.Sprintf("%s binary\n", hash) + + srv := newTestServer(binaryContent, checksums) + defer srv.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) + defer cancel() + // ensure context is expired + time.Sleep(1 * time.Millisecond) + + dir := t.TempDir() + installPath := filepath.Join(dir, "func") + + err := DownloadAndInstall( + ctx, + srv.URL+"/binary", + srv.URL+"/checksums.txt", + installPath, + &DownloadOptions{HTTPClient: srv.Client()}, + ) + if err == nil { + t.Fatal("expected error for cancelled context, got nil") + } +} + +type roundTripFunc func(req *http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} diff --git a/internal/funccli/manager.go b/internal/funccli/manager.go index c0520b7..b9eb975 100644 --- a/internal/funccli/manager.go +++ b/internal/funccli/manager.go @@ -2,11 +2,8 @@ package funccli import ( "context" - "crypto/sha256" - "encoding/hex" "encoding/json" "fmt" - "io" "net/http" "os" "os/exec" @@ -344,7 +341,7 @@ func (m *managerImpl) getLatestRelease(ctx context.Context) (*GitHubRelease, err // downloadAndInstall downloads the appropriate binary, verifies its SHA256 checksum, and installs it func (m *managerImpl) downloadAndInstall(ctx context.Context, release *GitHubRelease) error { - assetName := m.getAssetName() + assetName := AssetName() var downloadURL, checksumURL string for _, asset := range release.Assets { @@ -365,127 +362,12 @@ func (m *managerImpl) downloadAndInstall(ctx context.Context, release *GitHubRel m.logger.Info("Downloading func CLI", "url", downloadURL, "asset", assetName) - tmpFile := filepath.Join(m.installPath, fmt.Sprintf(".%s.tmp", binaryName)) - if err := m.downloadFile(ctx, downloadURL, tmpFile); err != nil { - return err - } - - if err := m.verifyChecksum(ctx, tmpFile, assetName, checksumURL); err != nil { - if removeErr := os.Remove(tmpFile); removeErr != nil { - m.logger.Error(removeErr, "Failed to remove tmp file after checksum verification failure", "file", tmpFile) - } - return fmt.Errorf("checksum verification failed: %w", err) - } - - if err := os.Chmod(tmpFile, 0755); err != nil { - if removeErr := os.Remove(tmpFile); removeErr != nil { - m.logger.Error(removeErr, "Failed to remove tmp file while file could not be marked as executable", "file", tmpFile) - } - return fmt.Errorf("failed to make binary executable: %w", err) - } - finalPath := filepath.Join(m.installPath, binaryName) - if err := os.Rename(tmpFile, finalPath); err != nil { - if removeErr := os.Remove(tmpFile); removeErr != nil { - m.logger.Error(removeErr, "Failed to remove tmp file while file could not be renamed", "file", tmpFile) - } - return fmt.Errorf("failed to install binary: %w", err) - } - - return nil -} - -// verifyChecksum downloads the checksums.txt file and verifies the downloaded binary matches -func (m *managerImpl) verifyChecksum(ctx context.Context, filePath, assetName, checksumURL string) error { - expectedHash, err := m.fetchExpectedChecksum(ctx, assetName, checksumURL) - if err != nil { + if err := DownloadAndInstall(ctx, downloadURL, checksumURL, finalPath, &DownloadOptions{ + HTTPClient: m.httpClient, + }); err != nil { return err } - f, err := os.Open(filePath) - if err != nil { - return fmt.Errorf("failed to open file for checksum: %w", err) - } - defer f.Close() - - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return fmt.Errorf("failed to compute checksum: %w", err) - } - - actualHash := hex.EncodeToString(h.Sum(nil)) - if actualHash != expectedHash { - return fmt.Errorf("sha256 mismatch: expected %s, got %s", expectedHash, actualHash) - } - - m.logger.Info("Checksum verified", "asset", assetName, "sha256", actualHash) return nil } - -// fetchExpectedChecksum downloads checksums.txt and extracts the hash for the given asset -func (m *managerImpl) fetchExpectedChecksum(ctx context.Context, assetName, checksumURL string) (string, error) { - req, err := http.NewRequestWithContext(ctx, "GET", checksumURL, nil) - if err != nil { - return "", err - } - - resp, err := m.httpClient.Do(req) - if err != nil { - return "", fmt.Errorf("failed to download checksums: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("checksums download failed with status %d", resp.StatusCode) - } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", fmt.Errorf("failed to read checksums: %w", err) - } - - for _, line := range strings.Split(strings.TrimSpace(string(body)), "\n") { - parts := strings.Fields(line) - if len(parts) == 2 && parts[1] == assetName { - return parts[0], nil - } - } - - return "", fmt.Errorf("no checksum found for asset %s", assetName) -} - -// downloadFile downloads a file from the given URL -func (m *managerImpl) downloadFile(ctx context.Context, url, path string) error { - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) - if err != nil { - return err - } - - resp, err := m.httpClient.Do(req) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("download failed with status %d", resp.StatusCode) - } - - out, err := os.Create(path) - if err != nil { - return err - } - defer out.Close() - - _, err = io.Copy(out, resp.Body) - return err -} - -// getAssetName returns the appropriate asset name for the current platform -func (m *managerImpl) getAssetName() string { - goos := goruntime.GOOS - goarch := goruntime.GOARCH - - // Knative func uses naming like: func_linux_amd64 - return fmt.Sprintf("func_%s_%s", goos, goarch) -} diff --git a/test/utils/func.go b/test/utils/func.go index be2e95e..0c07253 100644 --- a/test/utils/func.go +++ b/test/utils/func.go @@ -17,15 +17,14 @@ limitations under the License. package utils import ( + "context" "fmt" - "io" - "net/http" "os" "os/exec" "path/filepath" - "runtime" "time" + "github.com/functions-dev/func-operator/internal/funccli" ginkgo "github.com/onsi/ginkgo/v2" ) @@ -202,42 +201,20 @@ func ensureFuncVersion(version string) (string, error) { // downloadFuncVersion downloads the specified func version from GitHub releases func downloadFuncVersion(version, versionDir, funcBinary string) error { - // Create version directory - if err := os.MkdirAll(versionDir, 0755); err != nil { + if err := os.MkdirAll(versionDir, 0o755); err != nil { return fmt.Errorf("failed to create version directory: %w", err) } - // Construct download URL - goos := runtime.GOOS - goarch := runtime.GOARCH - url := fmt.Sprintf("https://github.com/knative/func/releases/download/knative-%s/func_%s_%s", - version, goos, goarch) + asset := funccli.AssetName() + base := "https://github.com/knative/func/releases/download/knative-" + version + binaryURL := base + "/" + asset + checksumURL := base + "/checksums.txt" - // Download binary - resp, err := http.Get(url) - if err != nil { - return fmt.Errorf("failed to download func %s: %w", version, err) - } - defer resp.Body.Close() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("failed to download func %s: HTTP %d", version, resp.StatusCode) - } - - // Write to file - out, err := os.Create(funcBinary) - if err != nil { - return fmt.Errorf("failed to create binary file: %w", err) - } - defer out.Close() - - if _, err := io.Copy(out, resp.Body); err != nil { - return fmt.Errorf("failed to write binary: %w", err) - } - - // Make executable - if err := os.Chmod(funcBinary, 0755); err != nil { - return fmt.Errorf("failed to make binary executable: %w", err) + if err := funccli.DownloadAndInstall(ctx, binaryURL, checksumURL, funcBinary, nil); err != nil { + return fmt.Errorf("failed to download func %s: %w", version, err) } return nil