From 565c605ee08e3d9da2d3f89c3ce542366604c2ee Mon Sep 17 00:00:00 2001 From: Pietro De Nicolao Date: Sat, 23 Nov 2024 13:24:09 +0100 Subject: [PATCH] Concurrency-safe unpacking of TF providers. The original implementation unpacked the downloaded provider .zip files directly in the Terraform plugin cache directory. This made the `terraform init` command prone to race conditions when multiple Terraform modules using the same cache were downloading, checksumming, and validating the provider files at the same time. This is a serious problem in CI workflows, as it forces developers to choose between initializing modules serially and using the cache, or initializing modules in parallel, but download the same modules every time, increasing the consumed bandwidth. This change unpacks the .zip files in a temporary directory with a unique name inside the plugin cache directory, and only then moves the files to the expected location. This change is inspired by this PR: https://github.com/hashicorp/terraform/pull/33479 --- internal/getproviders/filesystem_search.go | 7 ++++ internal/providercache/package_install.go | 47 +++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/internal/getproviders/filesystem_search.go b/internal/getproviders/filesystem_search.go index f9a07f92abbe..869df909d09d 100644 --- a/internal/getproviders/filesystem_search.go +++ b/internal/getproviders/filesystem_search.go @@ -64,6 +64,13 @@ func SearchLocalDirectory(baseDir string) (map[addrs.Provider]PackageMetaList, e return nil } relPath := filepath.ToSlash(fsPath) + + // If the relative path contains ".terraform-temp-", ignore it. + // This is the temporary directory created by the provider installer + if strings.Contains(relPath, ".terraform-temp-") { + return nil + } + parts := strings.Split(relPath, "/") if len(parts) < 3 { diff --git a/internal/providercache/package_install.go b/internal/providercache/package_install.go index 762b149adbf5..7f73f1e876b2 100644 --- a/internal/providercache/package_install.go +++ b/internal/providercache/package_install.go @@ -6,8 +6,10 @@ package providercache import ( "context" "fmt" + "io/fs" "net/url" "os" + "path" "path/filepath" getter "github.com/hashicorp/go-getter" @@ -122,7 +124,50 @@ func installFromLocalArchive(ctx context.Context, meta getproviders.PackageMeta, // match the allowed hashes and so our caller should catch that after // we return if so. - err := unzip.Decompress(targetDir, filename, true, 0000) + // Create the destination directory + err := os.MkdirAll(targetDir, 0777) + if err != nil { + return authResult, fmt.Errorf("failed to create new directory: %w", err) + } + + // Create a unique temporary directory for unpacking + stagingDir, err := os.MkdirTemp(path.Dir(targetDir), ".terraform-temp-*") + if err != nil { + return authResult, fmt.Errorf("failed to create temporary directory for provider installation: %s", err) + } + defer os.RemoveAll(stagingDir) + + err = unzip.Decompress(stagingDir, filename, true, 0000) + if err != nil { + return authResult, fmt.Errorf("failed to decompress: %w", err) + } + + // Try to atomically move the files from stagingDir to targetDir. + err = filepath.Walk(stagingDir, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return fmt.Errorf("failed to copy path %s to target directory: %w", path, err) + } + relPath, err := filepath.Rel(stagingDir, path) + if err != nil { + return fmt.Errorf("failed to calculate relative path: %w", err) + } + + if info.IsDir() { + // Create the directory + err := os.MkdirAll(filepath.Join(targetDir, relPath), info.Mode().Perm()) + if err != nil { + return fmt.Errorf("failed to create path: %w", err) + } + } else { + // On supported platforms, this should perform atomic replacement of the file. + err := os.Rename(path, filepath.Join(targetDir, relPath)) + if err != nil { + return fmt.Errorf("failed to move '%s': %w", path, err) + } + } + return nil + }) + if err != nil { return authResult, err }