Skip to content

Commit

Permalink
Concurrency-safe unpacking of TF providers.
Browse files Browse the repository at this point in the history
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: #33479
  • Loading branch information
pietrodn committed Nov 23, 2024
1 parent b4a634c commit 565c605
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
7 changes: 7 additions & 0 deletions internal/getproviders/filesystem_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
47 changes: 46 additions & 1 deletion internal/providercache/package_install.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ package providercache
import (
"context"
"fmt"
"io/fs"
"net/url"
"os"
"path"
"path/filepath"

getter "github.com/hashicorp/go-getter"
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 565c605

Please sign in to comment.