From 158243d1464657442c1f7f8275190344adfeae12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Banaszewski?= Date: Tue, 18 Nov 2025 09:52:44 +0100 Subject: [PATCH] fix: add cache for terraform installer files (#20776) Replaces not working mocks by simple proxy that caches terraform files using test cache https://github.com/coder/coder/blob/16b8e6072fd84f45404e3f84bb2b6fea2424b090/testutil/cache.go#L13 Fixes: https://github.com/coder/internal/issues/1126 --- provisioner/terraform/install.go | 9 +- provisioner/terraform/install_test.go | 230 +++++++++----------------- provisioner/terraform/serve.go | 2 +- 3 files changed, 82 insertions(+), 159 deletions(-) diff --git a/provisioner/terraform/install.go b/provisioner/terraform/install.go index 19bf7c6a8d..83791abfc1 100644 --- a/provisioner/terraform/install.go +++ b/provisioner/terraform/install.go @@ -34,7 +34,7 @@ var ( // operation. // //nolint:revive // verbose is a control flag that controls the verbosity of the log output. -func Install(ctx context.Context, log slog.Logger, verbose bool, dir string, wantVersion *version.Version, baseUrl string, verifyChecksums bool) (string, error) { +func Install(ctx context.Context, log slog.Logger, verbose bool, dir string, wantVersion *version.Version, baseUrl string) (string, error) { err := os.MkdirAll(dir, 0o750) if err != nil { return "", err @@ -63,10 +63,9 @@ func Install(ctx context.Context, log slog.Logger, verbose bool, dir string, wan } installer := &releases.ExactVersion{ - InstallDir: dir, - Product: product.Terraform, - Version: TerraformVersion, - SkipChecksumVerification: !verifyChecksums, + InstallDir: dir, + Product: product.Terraform, + Version: TerraformVersion, } installer.SetLogger(slog.Stdlib(ctx, log, slog.LevelDebug)) if baseUrl != "" { diff --git a/provisioner/terraform/install_test.go b/provisioner/terraform/install_test.go index edbc758043..c259ccd2d2 100644 --- a/provisioner/terraform/install_test.go +++ b/provisioner/terraform/install_test.go @@ -6,12 +6,12 @@ package terraform_test import ( - "archive/zip" "context" - "encoding/json" - "fmt" + "errors" + "io" "net" "net/http" + "net/url" "os" "path/filepath" "strings" @@ -28,17 +28,8 @@ import ( ) const ( - // simple script that mocks `./terraform version -json` - terraformExecutableTemplate = `#!/bin/bash -cat < zip contains 'terraform' binary and sometimes 'LICENSE.txt' -func createFakeTerraformInstallationFiles(t *testing.T) string { - tmpDir := t.TempDir() - - mij := mustMarshal(t, mainJSON(version1, version2)) - jv1 := mustMarshal(t, versionedJSON(version1)) - jv2 := mustMarshal(t, versionedJSON(version2)) - - // `index.json` - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "index.json"), mij, 0o400)) - - // `${version1}/index.json` - require.NoError(t, os.Mkdir(filepath.Join(tmpDir, version1.String()), 0o700)) - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, version1.String(), "index.json"), jv1, 0o400)) - - // `${version2}/index.json` - require.NoError(t, os.Mkdir(filepath.Join(tmpDir, version2.String()), 0o700)) - require.NoError(t, os.WriteFile(filepath.Join(tmpDir, version2.String(), "index.json"), jv2, 0o400)) - - // `${version1}/linux_amd64.zip` - zip1, err := os.Create(filepath.Join(tmpDir, version1.String(), zipFilename(version1))) - require.NoError(t, err) - zip1Writer := zip.NewWriter(zip1) - - // `${version1}/linux_amd64.zip/terraform` - exe1, err := zip1Writer.Create("terraform") - require.NoError(t, err) - n, err := exe1.Write(exeContent(version1)) - require.NoError(t, err) - require.NotZero(t, n) - - // `${version1}/linux_amd64.zip/LICENSE.txt` - lic1, err := zip1Writer.Create("LICENSE.txt") - require.NoError(t, err) - n, err = lic1.Write([]byte("some license")) - require.NoError(t, err) - require.NotZero(t, n) - require.NoError(t, zip1Writer.Close()) - - // `${version2}/linux_amd64.zip` - zip2, err := os.Create(filepath.Join(tmpDir, version2.String(), zipFilename(version2))) - require.NoError(t, err) - zip2Writer := zip.NewWriter(zip2) - - // `${version1}/linux_amd64.zip/terraform` - exe2, err := zip2Writer.Create("terraform") - require.NoError(t, err) - n, err = exe2.Write(exeContent(version2)) - require.NoError(t, err) - require.NotZero(t, n) - require.NoError(t, zip2Writer.Close()) - - return tmpDir -} - -// starts http server serving fake terraform installation files -func startFakeTerraformServer(t *testing.T, tmpDir string) string { listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("failed to create listener") } + proxy.listener = listener - mux := http.NewServeMux() - fs := http.FileServer(http.Dir(tmpDir)) - mux.Handle("/terraform/", http.StripPrefix("/terraform", fs)) + m := http.NewServeMux() + m.HandleFunc("GET /", proxy.handleGet) - srv := http.Server{ - ReadHeaderTimeout: time.Second, - Handler: mux, + proxy.srv = &http.Server{ + WriteTimeout: 30 * time.Second, + ReadTimeout: 30 * time.Second, + Handler: m, } - go srv.Serve(listener) - t.Cleanup(func() { - if err := srv.Close(); err != nil { - t.Errorf("failed to close server: %v", err) + return &proxy +} + +func uriToFilename(u url.URL) string { + return strings.ReplaceAll(u.RequestURI(), "/", "_") +} + +func (p *terraformProxy) handleGet(w http.ResponseWriter, r *http.Request) { + p.mutex.Lock() + defer p.mutex.Unlock() + + filename := uriToFilename(*r.URL) + path := filepath.Join(p.cacheRoot, filename) + if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + require.NoError(p.t, os.MkdirAll(p.cacheRoot, os.ModeDir|0o700)) + + // Update cache + req, err := http.NewRequestWithContext(p.t.Context(), "GET", terraformURL+r.URL.Path, nil) + require.NoError(p.t, err) + + resp, err := p.httpClient.Do(req) + require.NoError(p.t, err) + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(p.t, err) + + // update index.json so urls in it point to proxy by making them relative + // "https://releases.hashicorp.com/terraform/1.13.4/terraform_1.13.4_windows_amd64.zip" -> "/terraform/1.13.4/terraform_1.13.4_windows_amd64.zip" + if strings.HasSuffix(r.URL.Path, "index.json") { + body = []byte(strings.ReplaceAll(string(body), terraformURL, "")) } - }) - return "http://" + listener.Addr().String() + require.NoError(p.t, os.WriteFile(path, body, 0o400)) + } else if err != nil { + p.t.Errorf("unexpected error when trying to read file from cache: %v", err) + } + + // Serve from cache + r.URL.Path = filename + r.URL.RawPath = filename + p.fsHandler.ServeHTTP(w, r) } func TestInstall(t *testing.T) { @@ -205,8 +126,11 @@ func TestInstall(t *testing.T) { dir := t.TempDir() log := testutil.Logger(t) - tmpDir := createFakeTerraformInstallationFiles(t) - addr := startFakeTerraformServer(t, tmpDir) + proxy := persistentlyCachedProxy(t) + go proxy.srv.Serve(proxy.listener) + t.Cleanup(func() { + require.NoError(t, proxy.srv.Close()) + }) // Install spins off 8 installs with Version and waits for them all // to complete. The locking mechanism within Install should @@ -219,7 +143,7 @@ func TestInstall(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - p, err := terraform.Install(ctx, log, false, dir, version, addr, false) + p, err := terraform.Install(ctx, log, false, dir, version, "http://"+proxy.listener.Addr().String()) assert.NoError(t, err) paths <- p }() diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index a927f288fa..32b5343f6f 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -103,7 +103,7 @@ func Serve(ctx context.Context, options *ServeOptions) error { slog.F("min_version", minTerraformVersion.String())) } - binPath, err := Install(ctx, options.Logger, options.ExternalProvisioner, options.CachePath, TerraformVersion, "", true) + binPath, err := Install(ctx, options.Logger, options.ExternalProvisioner, options.CachePath, TerraformVersion, "") if err != nil { return xerrors.Errorf("install terraform: %w", err) }