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.

Sign up for GitHub

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(follow): handle directories in tar.gz artifacts #716

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(follow): handle directories in tar.gz artifacts #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
85 changes: 62 additions & 23 deletions internal/follower/follower.go
View file
Open in desktop
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 @@ -86,9 +86,7 @@ type Config struct {
Signature *index.Signature
}

var (
isInt = regexp.MustCompile(`^(0|([1-9]\d*))$`)
)
var isInt = regexp.MustCompile(`^(0|([1-9]\d*))$`)

// New creates a Follower configured with the passed parameters and ready to be used.
// It does not check the correctness of the parameters, make sure everything is initialized.
Expand All @@ -114,8 +112,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-")
Comment on lines +115 to +126
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this? The temporary dir is given by the user, and is the same for all the followers/artifacts.

Copy link
Member

Choose a reason for hiding this comment

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

@TiagoJMartins, could you please provide more info about this change?

if err != nil {
return nil, fmt.Errorf("unable to create temporary directory: %w", err)
}
Expand Down Expand Up @@ -203,52 +211,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
View file
Open in desktop
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
View file
Open in desktop
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
Loading