Skip to content

Commit

Permalink
fix: allow directories in tar.gz artifacts
Browse files Browse the repository at this point in the history 
Signed-off-by: Tiago Martins <[email protected]>
  • Loading branch information
@TiagoJMartins
TiagoJMartins committed Jan 14, 2025
1 parent 18c4322 commit 1ef75d7
Show file tree
Hide file tree
Showing 2 changed files with 250 additions and 21 deletions.
80 changes: 59 additions & 21 deletions internal/follower/follower.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,18 @@ func New(ref string, printer *output.Printer, conf *Config) (*Follower, error) {
return nil, err
}

// Create temp dir where to put pulled artifacts.
tmpDir, err := os.MkdirTemp(conf.TmpDir, "falcoctl-")
// Always create temporary directories at the root level to avoid nesting issues.
// If TmpDir points to a subdirectory (e.g., /tmp/foo/bar), we use its parent (/tmp/foo)
// to prevent creating temporary directories inside artifact directories.
baseDir := conf.TmpDir
if baseDir != "" {
baseDir = filepath.Clean(baseDir)
if filepath.Base(baseDir) != "." {
baseDir = filepath.Dir(baseDir)
}
}

tmpDir, err := os.MkdirTemp(baseDir, "falcoctl-")
if err != nil {
return nil, fmt.Errorf("unable to create temporary directory: %w", err)
}
Expand Down Expand Up @@ -203,52 +213,80 @@ func (f *Follower) follow(ctx context.Context) {
return
}

// Move files to their destination
if err := f.moveFiles(ctx, filePaths, dstDir); err != nil {
return
}

f.logger.Info("Artifact correctly installed",
f.logger.Args("followerName", f.ref, "artifactName", f.ref, "type", res.Type, "digest", res.Digest, "directory", dstDir))
f.currentDigest = desc.Digest.String()
}

// moveFiles moves files from their temporary location to the destination directory.
// It preserves the directory structure relative to the temporary directory.
// For example, if a file is at "tmpDir/subdir/file.yaml", it will be moved to
// "dstDir/subdir/file.yaml". This ensures that files in subdirectories are moved
// correctly as individual files, not as entire directories.
func (f *Follower) moveFiles(ctx context.Context, filePaths []string, dstDir string) error {
// Install the artifacts if necessary.
for _, path := range filePaths {
baseName := filepath.Base(path)
f.logger.Debug("Installing file", f.logger.Args("followerName", f.ref, "fileName", baseName))
dstPath := filepath.Join(dstDir, baseName)
// Check if the file exists.
f.logger.Debug("Checking if file already exists", f.logger.Args("followerName", f.ref, "fileName", baseName, "directory", dstDir))
exists, err := utils.FileExists(dstPath)
// Get the relative path from the temporary directory to preserve directory structure
relPath, err := filepath.Rel(f.tmpDir, path)
if err != nil {
f.logger.Error("Unable to check existence for file", f.logger.Args("followerName", f.ref, "fileName", baseName, "reason", err.Error()))
return
f.logger.Error("Unable to get relative path", f.logger.Args("followerName", f.ref, "path", path, "reason", err.Error()))
return err
}

dstPath := filepath.Join(dstDir, relPath)
// Ensure the parent directory exists
if err := os.MkdirAll(filepath.Dir(dstPath), 0755); err != nil {
f.logger.Error("Unable to create destination directory", f.logger.Args("followerName", f.ref, "directory", filepath.Dir(dstPath), "reason", err.Error()))
return err
}

f.logger.Debug("Installing file", f.logger.Args("followerName", f.ref, "path", relPath))
// Check if the file exists.
f.logger.Debug("Checking if file already exists", f.logger.Args("followerName", f.ref, "path", relPath, "directory", dstDir))
_, err = os.Stat(dstPath)
exists := err == nil
if err != nil && !os.IsNotExist(err) {
f.logger.Error("Unable to check existence for file", f.logger.Args("followerName", f.ref, "path", relPath, "reason", err.Error()))
return err
}

if !exists {
f.logger.Debug("Moving file", f.logger.Args("followerName", f.ref, "fileName", baseName, "destDirectory", dstDir))
f.logger.Debug("Moving file", f.logger.Args("followerName", f.ref, "path", relPath, "destDirectory", dstDir))
if err = utils.Move(path, dstPath); err != nil {
f.logger.Error("Unable to move file", f.logger.Args("followerName", f.ref, "fileName", baseName, "destDirectory", dstDir, "reason", err.Error()))
return
f.logger.Error("Unable to move file", f.logger.Args("followerName", f.ref, "path", relPath, "destDirectory", dstDir, "reason", err.Error()))
return err
}
f.logger.Debug("File correctly installed", f.logger.Args("followerName", f.ref, "path", path))
// It's done, move to the next file.
continue
}
f.logger.Debug(fmt.Sprintf("file %q already exists in %q, checking if it is equal to the existing one", baseName, dstDir),

f.logger.Debug(fmt.Sprintf("file %q already exists in %q, checking if it is equal to the existing one", relPath, dstDir),
f.logger.Args("followerName", f.ref))

// Check if the files are equal.
eq, err := equal([]string{path, dstPath})
if err != nil {
f.logger.Error("Unable to compare files", f.logger.Args("followerName", f.ref, "newFile", path, "existingFile", dstPath, "reason", err.Error()))
return
f.logger.Error("Unable to check if files are equal", f.logger.Args("followerName", f.ref, "existingFile", dstPath, "reason", err.Error()))
return err
}

if !eq {
f.logger.Debug(fmt.Sprintf("Overwriting file %q with file %q", dstPath, path), f.logger.Args("followerName", f.ref))
if err = utils.Move(path, dstPath); err != nil {
f.logger.Error("Unable to overwrite file", f.logger.Args("followerName", f.ref, "existingFile", dstPath, "reason", err.Error()))
return
return err
}
} else {
f.logger.Debug("The two file are equal, nothing to be done")
}
}

f.logger.Info("Artifact correctly installed",
f.logger.Args("followerName", f.ref, "artifactName", f.ref, "type", res.Type, "digest", res.Digest, "directory", dstDir))
f.currentDigest = desc.Digest.String()
return nil
}

// pull downloads, extracts, and installs the artifact.
Expand Down
191 changes: 191 additions & 0 deletions internal/follower/follower_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@
package follower

import (
"context"
"os"
"path/filepath"
"testing"
"time"

"github.com/pterm/pterm"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -135,3 +138,191 @@ func TestCheckRequirements(t *testing.T) {
})
}
}

func TestFollowFileHandling(t *testing.T) {
type testFile struct {
path string
content string
replace bool // true if file should be replaced when it exists
}

tests := []struct {
name string
files []testFile
existing []testFile
}{
{
name: "basic file at root",
files: []testFile{
{
path: "file1.yaml",
content: "content1",
},
},
},
{
name: "file in subdirectory",
files: []testFile{
{
path: "subdir/file2.yaml",
content: "content2",
},
},
},
{
name: "file in nested subdirectory",
files: []testFile{
{
path: "subdir/nested/file3.yaml",
content: "content3",
},
},
},
{
name: "multiple files in different directories",
files: []testFile{
{
path: "file1.yaml",
content: "content1",
},
{
path: "subdir/file2.yaml",
content: "content2",
},
{
path: "subdir/nested/file3.yaml",
content: "content3",
},
},
},
{
name: "existing file with identical content",
files: []testFile{
{
path: "file1.yaml",
content: "content1",
replace: false,
},
},
existing: []testFile{
{
path: "file1.yaml",
content: "content1",
},
},
},
{
name: "existing file with different content",
files: []testFile{
{
path: "file1.yaml",
content: "new content",
replace: true,
},
},
existing: []testFile{
{
path: "file1.yaml",
content: "old content",
},
},
},
{
name: "mix of new and existing files",
files: []testFile{
{
path: "file1.yaml",
content: "content1",
replace: false,
},
{
path: "subdir/file2.yaml",
content: "new content2",
replace: true,
},
},
existing: []testFile{
{
path: "file1.yaml",
content: "content1",
},
{
path: "subdir/file2.yaml",
content: "old content2",
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "falcoctl-test-*")
assert.NoError(t, err)
defer os.RemoveAll(tmpDir)

dstDir, err := os.MkdirTemp("", "falcoctl-dst-*")
assert.NoError(t, err)
defer os.RemoveAll(dstDir)

// Setup existing files
for _, ef := range tt.existing {
dstPath := filepath.Join(dstDir, ef.path)
err = os.MkdirAll(filepath.Dir(dstPath), 0755)
assert.NoError(t, err)
err = os.WriteFile(dstPath, []byte(ef.content), 0644)
assert.NoError(t, err)
}

f, err := New("test-registry/test-ref", output.NewPrinter(pterm.LogLevelDebug, pterm.LogFormatterJSON, os.Stdout), &Config{
RulesfilesDir: dstDir,
TmpDir: tmpDir,
})
assert.NoError(t, err)

var paths []string
for _, tf := range tt.files {
fullPath := filepath.Join(f.tmpDir, tf.path)
err = os.MkdirAll(filepath.Dir(fullPath), 0755)
assert.NoError(t, err)
err = os.WriteFile(fullPath, []byte(tf.content), 0644)
assert.NoError(t, err)
paths = append(paths, fullPath)
}

// Track modification times of existing files
modTimes := make(map[string]time.Time)
for _, ef := range tt.existing {
dstPath := filepath.Join(dstDir, ef.path)
info, err := os.Stat(dstPath)
if err == nil {
modTimes[dstPath] = info.ModTime()
}
}

ctx := context.Background()
f.currentDigest = "test-digest"
err = f.moveFiles(ctx, paths, dstDir)
assert.NoError(t, err)

for _, tf := range tt.files {
dstPath := filepath.Join(dstDir, tf.path)
_, err = os.Stat(dstPath)
assert.NoError(t, err, "file should exist at %s", dstPath)

content, err := os.ReadFile(dstPath)
assert.NoError(t, err)
assert.Equal(t, tf.content, string(content))

if origTime, exists := modTimes[dstPath]; exists {
info, err := os.Stat(dstPath)
assert.NoError(t, err)
if tf.replace {
assert.NotEqual(t, origTime, info.ModTime(), "file should be replaced: %s", dstPath)
} else {
assert.Equal(t, origTime, info.ModTime(), "file should not be replaced: %s", dstPath)
}
}
}
})
}
}

0 comments on commit 1ef75d7

Please sign in to comment.