Skip to content

Commit

Permalink
fix(follow): file handling of artifacts with directories
Browse files Browse the repository at this point in the history
Signed-off-by: Tiago Martins <[email protected]>
  • Loading branch information
TiagoJMartins authored and poiana committed Jan 27, 2025
1 parent e71898c commit dc7634c
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 20 deletions.
67 changes: 49 additions & 18 deletions internal/follower/follower.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright (C) 2023 The Falco Authors
// Copyright (C) 2025 The Falco Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -203,52 +203,83 @@ func (f *Follower) follow(ctx context.Context) {
return
}

// Move files to their destination
if err := f.moveFiles(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(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)
// 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 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), 0o750); 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, "fileName", baseName, "directory", dstDir))
f.logger.Debug("Checking if file already exists", f.logger.Args("followerName", f.ref, "path", relPath, "directory", dstDir))
exists, err := utils.FileExists(dstPath)
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 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 compare files", 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
170 changes: 169 additions & 1 deletion internal/follower/follower_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright (C) 2024 The Falco Authors
// Copyright (C) 2025 The Falco Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@ package follower

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

"github.com/pterm/pterm"
Expand Down Expand Up @@ -135,3 +136,170 @@ func TestCheckRequirements(t *testing.T) {
})
}
}

func TestMoveFiles(t *testing.T) {
type testFile struct {
path string
content string
replace bool
}

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: "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), 0o755)
assert.NoError(t, err)
err = os.WriteFile(dstPath, []byte(ef.content), 0o644)
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), 0o755)
assert.NoError(t, err)
err = os.WriteFile(fullPath, []byte(tf.content), 0o644)
assert.NoError(t, err)
paths = append(paths, fullPath)
}

f.currentDigest = "test-digest"
err = f.moveFiles(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), "file content should match at %s", dstPath)

// For files marked as replace=false, verify they have identical content with existing files
if !tf.replace {
for _, ef := range tt.existing {
if ef.path == tf.path {
assert.Equal(t, ef.content, string(content), "file content should not change when replace=false: %s", dstPath)
}
}
}
}
})
}
}
2 changes: 1 addition & 1 deletion internal/utils/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ func ExtractTarGz(ctx context.Context, gzipStream io.Reader, destDir string, str
continue
}
info := header.FileInfo()
files = append(files, path)

switch header.Typeflag {
case tar.TypeDir:
Expand All @@ -106,6 +105,7 @@ func ExtractTarGz(ctx context.Context, gzipStream io.Reader, destDir string, str
if err = outFile.Close(); err != nil {
return nil, err
}
files = append(files, path)
case tar.TypeLink:
name := header.Linkname
if stripPathComponents > 0 {
Expand Down

0 comments on commit dc7634c

Please sign in to comment.