Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Vendoring Issues with Globs and Symlinks #984

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
376 changes: 376 additions & 0 deletions internal/exec/copy_glob.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,376 @@
package exec

import (
"fmt"
"io"
"os"
"path/filepath"
"strings"

l "github.com/charmbracelet/log"
"github.com/cloudposse/atmos/pkg/schema"
u "github.com/cloudposse/atmos/pkg/utils"
cp "github.com/otiai10/copy" // Using the optimized copy library when no filtering is required.
)

// copyFile copies a single file from src to dst while preserving file permissions.
func copyFile(src, dst string) error {
sourceFile, err := os.Open(src)
if err != nil {
return fmt.Errorf("opening source file %q: %w", src, err)
}
defer sourceFile.Close()

if err := os.MkdirAll(filepath.Dir(dst), os.ModePerm); err != nil {
return fmt.Errorf("creating destination directory for %q: %w", dst, err)
}

destinationFile, err := os.Create(dst)
if err != nil {
return fmt.Errorf("creating destination file %q: %w", dst, err)
}
defer destinationFile.Close()

if _, err := io.Copy(destinationFile, sourceFile); err != nil {
return fmt.Errorf("copying content from %q to %q: %w", src, dst, err)
}

info, err := os.Stat(src)
if err != nil {
return fmt.Errorf("getting file info for %q: %w", src, err)
}
if err := os.Chmod(dst, info.Mode()); err != nil {
return fmt.Errorf("setting permissions on %q: %w", dst, err)
}
return nil
}

// shouldSkipEntry determines whether to skip a file/directory based on its relative path to baseDir.
// If an error occurs during matching for an exclusion or inclusion pattern, it logs the error and proceeds.
func shouldSkipEntry(info os.FileInfo, srcPath, baseDir string, excluded, included []string) (bool, error) {
if info.Name() == ".git" {
return true, nil
}
relPath, err := filepath.Rel(baseDir, srcPath)
if err != nil {
l.Debug("Error computing relative path", "srcPath", srcPath, "error", err)
return true, nil // treat error as a signal to skip
}
// Ensure uniform path separator.
relPath = filepath.ToSlash(relPath)

// Process exclusion patterns.
// For directories, check with and without a trailing slash.
for _, pattern := range excluded {
// First check the plain relative path.
matched, err := u.PathMatch(pattern, relPath)
if err != nil {
l.Debug("Error matching exclusion pattern", "pattern", pattern, "path", relPath, "error", err)
continue
}
if matched {
l.Debug("Excluding path due to exclusion pattern (plain match)", "path", relPath, "pattern", pattern)
return true, nil
}
// If it is a directory, also try matching with a trailing slash.
if info.IsDir() {
matched, err = u.PathMatch(pattern, relPath+"/")
if err != nil {
l.Debug("Error matching exclusion pattern with trailing slash", "pattern", pattern, "path", relPath+"/", "error", err)
continue
}
if matched {
l.Debug("Excluding directory due to exclusion pattern (with trailing slash)", "path", relPath+"/", "pattern", pattern)
return true, nil
}
}
}

// Process inclusion patterns (only for non-directory files).
// (Directories are generally picked up by the inclusion branch in copyToTargetWithPatterns.)
if len(included) > 0 && !info.IsDir() {
matchedAny := false
for _, pattern := range included {
matched, err := u.PathMatch(pattern, relPath)
if err != nil {
l.Debug("Error matching inclusion pattern", "pattern", pattern, "path", relPath, "error", err)
continue
}
if matched {
l.Debug("Including path due to inclusion pattern", "path", relPath, "pattern", pattern)
matchedAny = true
break
}
}
if !matchedAny {
l.Debug("Excluding path because it does not match any inclusion pattern", "path", relPath)
return true, nil
}
}
return false, nil
}

// copyDirRecursive recursively copies srcDir to dstDir using shouldSkipEntry filtering.
// This function is used in cases where the entire sourceDir is the base for relative paths.
func copyDirRecursive(srcDir, dstDir, baseDir string, excluded, included []string) error {
entries, err := os.ReadDir(srcDir)
if err != nil {
return fmt.Errorf("reading directory %q: %w", srcDir, err)
}
for _, entry := range entries {
srcPath := filepath.Join(srcDir, entry.Name())
dstPath := filepath.Join(dstDir, entry.Name())

info, err := entry.Info()
if err != nil {
return fmt.Errorf("getting info for %q: %w", srcPath, err)
}

// Check if this entry should be skipped.
skip, err := shouldSkipEntry(info, srcPath, baseDir, excluded, included)
if err != nil {
return err
}
if skip {
l.Debug("Skipping entry", "srcPath", srcPath)
continue
}

// Skip symlinks.
if info.Mode()&os.ModeSymlink != 0 {
l.Debug("Skipping symlink", "path", srcPath)
continue
}

if info.IsDir() {
if err := os.MkdirAll(dstPath, info.Mode()); err != nil {
return fmt.Errorf("creating directory %q: %w", dstPath, err)
}
if err := copyDirRecursive(srcPath, dstPath, baseDir, excluded, included); err != nil {
return err
}
} else {
if err := copyFile(srcPath, dstPath); err != nil {
return err
}
}
}
return nil
}

// copyDirRecursiveWithPrefix recursively copies srcDir to dstDir while preserving the global relative path.
// Instead of using the local srcDir as the base for computing relative paths, this function uses the original
// source directory (globalBase) and an accumulated prefix that represents the relative path from globalBase.
func copyDirRecursiveWithPrefix(srcDir, dstDir, globalBase, prefix string, excluded []string) error {
entries, err := os.ReadDir(srcDir)
if err != nil {
return fmt.Errorf("reading directory %q: %w", srcDir, err)
}
for _, entry := range entries {
// Compute the full relative path from the original source.
fullRelPath := filepath.ToSlash(filepath.Join(prefix, entry.Name()))
srcPath := filepath.Join(srcDir, entry.Name())
dstPath := filepath.Join(dstDir, entry.Name())

info, err := entry.Info()
if err != nil {
return fmt.Errorf("getting info for %q: %w", srcPath, err)
}

// Skip .git directories.
if entry.Name() == ".git" {
l.Debug("Skipping .git directory", "path", fullRelPath)
continue
}

// Check exclusion patterns using the full relative path.
skip := false
for _, pattern := range excluded {
// Check plain match.
matched, err := u.PathMatch(pattern, fullRelPath)
if err != nil {
l.Debug("Error matching exclusion pattern in prefix function", "pattern", pattern, "path", fullRelPath, "error", err)
continue
}
if matched {
l.Debug("Excluding (prefix) due to exclusion pattern (plain match)", "path", fullRelPath, "pattern", pattern)
skip = true
break
}
// For directories, also try with a trailing slash.
if info.IsDir() {
matched, err = u.PathMatch(pattern, fullRelPath+"/")
if err != nil {
l.Debug("Error matching exclusion pattern with trailing slash in prefix function", "pattern", pattern, "path", fullRelPath+"/", "error", err)
continue
}
if matched {
l.Debug("Excluding (prefix) due to exclusion pattern (with trailing slash)", "path", fullRelPath+"/", "pattern", pattern)
skip = true
break
}
}
}
if skip {
continue
}

if info.IsDir() {
if err := os.MkdirAll(dstPath, info.Mode()); err != nil {
return fmt.Errorf("creating directory %q: %w", dstPath, err)
}
// Recurse with updated prefix.
if err := copyDirRecursiveWithPrefix(srcPath, dstPath, globalBase, fullRelPath, excluded); err != nil {
return err
}
} else {
if err := copyFile(srcPath, dstPath); err != nil {
return err
}
}
}
return nil
}

// getMatchesForPattern returns files/directories matching a pattern relative to sourceDir.
// If no matches are found, it logs a debug message and returns an empty slice.
// When the pattern ends with "/*", it retries with a recursive "/**" variant.
func getMatchesForPattern(sourceDir, pattern string) ([]string, error) {
fullPattern := filepath.Join(sourceDir, pattern)
matches, err := u.GetGlobMatches(fullPattern)
if err != nil {
return nil, fmt.Errorf("error getting glob matches for %q: %w", fullPattern, err)
}
if len(matches) == 0 {
if strings.HasSuffix(pattern, "/*") {
recursivePattern := strings.TrimSuffix(pattern, "/*") + "/**"
fullRecursivePattern := filepath.Join(sourceDir, recursivePattern)
matches, err = u.GetGlobMatches(fullRecursivePattern)
if err != nil {
return nil, fmt.Errorf("error getting glob matches for recursive pattern %q: %w", fullRecursivePattern, err)
}
if len(matches) == 0 {
l.Debug("No matches found for recursive pattern; target directory will be empty", "pattern", fullRecursivePattern)
return []string{}, nil
}
return matches, nil
}
l.Debug("No matches found for pattern; target directory will be empty", "pattern", fullPattern)
return []string{}, nil
}
return matches, nil
}

// copyToTargetWithPatterns copies the contents from sourceDir to targetPath,
// applying inclusion and exclusion patterns from the vendor source configuration.
// If sourceIsLocalFile is true and targetPath lacks an extension, the sanitized URI is appended.
// If no included paths are defined, all files (except those matching excluded paths) are copied.
// In the special case where neither inclusion nor exclusion patterns are defined,
// the optimized cp library (github.com/otiai10/copy) is used.
func copyToTargetWithPatterns(
sourceDir, targetPath string,
s *schema.AtmosVendorSource,
sourceIsLocalFile bool,
uri string,
) error {
if sourceIsLocalFile && filepath.Ext(targetPath) == "" {
targetPath = filepath.Join(targetPath, SanitizeFileName(uri))
}
l.Debug("Copying files", "source", sourceDir, "target", targetPath)
if err := os.MkdirAll(targetPath, os.ModePerm); err != nil {
return fmt.Errorf("creating target directory %q: %w", targetPath, err)
}
Comment on lines +280 to +282
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Handle potential race condition in directory creation.

The MkdirAll followed by operations on the directory could be subject to a race condition if multiple processes are involved.

Consider adding file locking or checking for concurrent access issues. Let's check if there are any existing synchronization mechanisms in the codebase:


🏁 Script executed:

#!/bin/bash
# Search for file locking mechanisms
rg -A 5 'flock|Lock|Mutex|Synchronize'

Length of output: 28258


Handle race condition in directory creation in internal/exec/copy_glob.go

After reviewing the codebase, it’s clear that although other parts of the repository (for example, in pkg/config/cache.go) leverage file locks (using gofrs/flock) or internal mutexes to coordinate concurrent file operations, the directory creation in copy_glob.go is done solely via os.MkdirAll without any explicit locking mechanism. This could lead to potential race conditions in environments where multiple processes or goroutines attempt to create or operate on the same directory concurrently.

• The os.MkdirAll call is inherently idempotent, but subsequent operations on the newly created directory might be affected by concurrent access.
• Unlike other modules that incorporate file locking, no synchronization guards the MkdirAll in copy_glob.go.

Consider introducing a locking mechanism (such as using a file-based lock or an internal mutex) around directory creation to ensure safe concurrent access.


// Optimization: if no inclusion and no exclusion patterns are defined, use the cp library for fast copying.
if len(s.IncludedPaths) == 0 && len(s.ExcludedPaths) == 0 {
l.Debug("No inclusion or exclusion patterns defined; using cp library for fast copy")
return cp.Copy(sourceDir, targetPath)
}

// If inclusion patterns are provided, use them to determine which files to copy.
if len(s.IncludedPaths) > 0 {
filesToCopy := make(map[string]struct{})
for _, pattern := range s.IncludedPaths {
matches, err := getMatchesForPattern(sourceDir, pattern)
if err != nil {
l.Debug("Warning: error getting matches for pattern", "pattern", pattern, "error", err)
continue
}
for _, match := range matches {
filesToCopy[match] = struct{}{}
}
}
if len(filesToCopy) == 0 {
l.Debug("No files matched the inclusion patterns - target directory will be empty")
return nil
}
for file := range filesToCopy {
// Retrieve file information early so that we can adjust exclusion checks if this is a directory.
info, err := os.Stat(file)
if err != nil {
return fmt.Errorf("stating file %q: %w", file, err)
}
relPath, err := filepath.Rel(sourceDir, file)
if err != nil {
return fmt.Errorf("computing relative path for %q: %w", file, err)
}
relPath = filepath.ToSlash(relPath)
skip := false
// For directories, check both the plain relative path and with a trailing slash.
for _, ex := range s.ExcludedPaths {
if info.IsDir() {
matched, err := u.PathMatch(ex, relPath)
if err != nil {
l.Debug("Error matching exclusion pattern", "pattern", ex, "path", relPath, "error", err)
} else if matched {
l.Debug("Excluding directory due to exclusion pattern (plain match)", "directory", relPath, "pattern", ex)
skip = true
break
}
// Also try matching with a trailing slash.
matched, err = u.PathMatch(ex, relPath+"/")
if err != nil {
l.Debug("Error matching exclusion pattern with trailing slash", "pattern", ex, "path", relPath+"/", "error", err)
} else if matched {
l.Debug("Excluding directory due to exclusion pattern (with trailing slash)", "directory", relPath+"/", "pattern", ex)
skip = true
break
}
} else {
// For files, just check the plain relative path.
matched, err := u.PathMatch(ex, relPath)
if err != nil {
l.Debug("Error matching exclusion pattern", "pattern", ex, "path", relPath, "error", err)
} else if matched {
l.Debug("Excluding file due to exclusion pattern", "file", relPath, "pattern", ex)
skip = true
break
}
}
}
if skip {
continue
}

// Build the destination path.
dstPath := filepath.Join(targetPath, relPath)
if info.IsDir() {
// Instead of resetting the base for relative paths,
// use the new recursive function that preserves the global relative path.
if err := copyDirRecursiveWithPrefix(file, dstPath, sourceDir, relPath, s.ExcludedPaths); err != nil {
return err
}
} else {
if err := copyFile(file, dstPath); err != nil {
return err
}
}
}
} else {
// No inclusion patterns defined; copy everything except those matching excluded items.
if err := copyDirRecursive(sourceDir, targetPath, sourceDir, s.ExcludedPaths, s.IncludedPaths); err != nil {
return fmt.Errorf("error copying from %q to %q: %w", sourceDir, targetPath, err)
}
}
return nil
}
Loading