Unpack TF plugins in a more atomic way.

The original implementation of plugin cache handling unpacks plugin
archives in a way that can result in race conditions when multiple
terraform processes are accessing the same plugin cache. Since the
archives are being decompressed in chunks and output files are written
directly to the cache, we observed following manifestations of the issue:

- `text file busy` errors if other terraform processes are already
  running the plugin and another process tries to replace it.
- various plugin checksum errors triggered likely by simultaneous checksumming
  and rewriting the file.

This PR changes the zip archives with plugins are handled:

1. Instead of writing directly to the target directory,
   `installFromLocalArchive` is now writing to a temporary staging
   directory prefixed with`.temp` that resides in the plugin cache dir
   to ensure this is on the same filesystem.
2. After unpacking, the directory structure of the staging directory is
   replicated in the `targetDir`. The directories are created as needed
   and any files are moved using `os.Rename`. After this, the staging
   directory is removed.
3. Since the temporary staging directories can be picked up by
   `SearchLocalDirectory` and make it fail in the situation when they're
   removed during directory traversal, the function has been modified to
   skip any entry that starts with dot.

Signed-off-by: Milan Plzik <milan.plzik@grafana.com>

Author: mplzik

internal/getproviders/filesystem_search.go

@@ -45,8 +45,25 @@ func SearchLocalDirectory(baseDir string) (map[addrs.Provider]PackageMetaList, e
 		log.Printf("[TRACE] getproviders.SearchLocalDirectory: failed to resolve symlinks for %s: %s", baseDir, err)
 	}
 
-	err := filepath.Walk(baseDir, func(fullPath string, info os.FileInfo, err error) error {
+	err := filepath.Walk(baseDir, func(fullPath string, info os.FileInfo, werr error) error {
+		fsPath, err := filepath.Rel(baseDir, fullPath)
 		if err != nil {
+			// This should never happen because the filepath.Walk contract is
+			// for the paths to include the base path.
+			log.Printf("[TRACE] getproviders.SearchLocalDirectory: ignoring malformed path %q during walk: %s", fullPath, err)
+			return nil
+		}
+
+		if filepath.Base(fsPath)[0] == '.' && fsPath != "." {
+			// Dot-files should not occur in the provider cache at all.
+			// Dot-directories are used by e.g. temporary directories to
+			// unpack the packed providers. These might disappear at any
+			// moment, making the traversal fairly brittle. Just skip them.
+			log.Printf("[TRACE] getproviders.SearchLocalDirectory: skipping dotfile %s", fsPath)
+			return filepath.SkipDir
+		}
+
+		if werr != nil {
 			return fmt.Errorf("cannot search %s: %s", fullPath, err)
 		}
 
@@ -56,13 +73,7 @@ func SearchLocalDirectory(baseDir string) (map[addrs.Provider]PackageMetaList, e
 		//
 		// Both of these give us enough information to identify the package
 		// metadata.
-		fsPath, err := filepath.Rel(baseDir, fullPath)
-		if err != nil {
-			// This should never happen because the filepath.Walk contract is
-			// for the paths to include the base path.
-			log.Printf("[TRACE] getproviders.SearchLocalDirectory: ignoring malformed path %q during walk: %s", fullPath, err)
-			return nil
-		}
+
 		relPath := filepath.ToSlash(fsPath)
 		parts := strings.Split(relPath, "/")
 

internal/providercache/installer_test.go

@@ -2299,7 +2299,7 @@ func TestEnsureProviderVersions_local_source(t *testing.T) {
 			provider: "null",
 			version:  "2.1.0",
 			wantHash: getproviders.NilHash,
-			err:      "zip: not a valid zip file",
+			err:      "failed to decompress: zip: not a valid zip file",
 		},
 		"version-constraint-unmet": {
 			provider: "null",

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,48 @@ 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)
+	err := os.MkdirAll(targetDir, 0777)
+	if err != nil {
+		return authResult, fmt.Errorf("failed to create new directory: %w", err)
+	}
+
+	stagingDir, err := os.MkdirTemp(path.Dir(targetDir), ".temp")
+	if err != nil {
+		return authResult, fmt.Errorf("failed to create a temp dir: %w", 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
 	}