Skip to content

Commit dc7634c

Browse files
TiagoJMartinspoiana
TiagoJMartins
authored and
poiana
committed
fix(follow): file handling of artifacts with directories
Signed-off-by: Tiago Martins <[email protected]>
1 parent e71898c commit dc7634c

File tree

3 files changed

+219
-20
lines changed

3 files changed

+219
-20
lines changed

‎internal/follower/follower.go

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// SPDX-License-Identifier: Apache-2.0
2-
// Copyright (C) 2023 The Falco Authors
2+
// Copyright (C) 2025 The Falco Authors
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -203,52 +203,83 @@ func (f *Follower) follow(ctx context.Context) {
203203
return
204204
}
205205

206+
// Move files to their destination
207+
if err := f.moveFiles(filePaths, dstDir); err != nil {
208+
return
209+
}
210+
211+
f.logger.Info("Artifact correctly installed",
212+
f.logger.Args("followerName", f.ref, "artifactName", f.ref, "type", res.Type, "digest", res.Digest, "directory", dstDir))
213+
f.currentDigest = desc.Digest.String()
214+
}
215+
216+
// moveFiles moves files from their temporary location to the destination directory.
217+
// It preserves the directory structure relative to the temporary directory.
218+
// For example, if a file is at "tmpDir/subdir/file.yaml", it will be moved to
219+
// "dstDir/subdir/file.yaml". This ensures that files in subdirectories are moved
220+
// correctly as individual files, not as entire directories.
221+
func (f *Follower) moveFiles(filePaths []string, dstDir string) error {
206222
// Install the artifacts if necessary.
207223
for _, path := range filePaths {
208-
baseName := filepath.Base(path)
209-
f.logger.Debug("Installing file", f.logger.Args("followerName", f.ref, "fileName", baseName))
210-
dstPath := filepath.Join(dstDir, baseName)
224+
// Get the relative path from the temporary directory to preserve directory structure
225+
relPath, err := filepath.Rel(f.tmpDir, path)
226+
if err != nil {
227+
f.logger.Error("Unable to get relative path", f.logger.Args("followerName", f.ref, "path", path, "reason", err.Error()))
228+
return err
229+
}
230+
231+
dstPath := filepath.Join(dstDir, relPath)
232+
// Ensure the parent directory exists
233+
if err := os.MkdirAll(filepath.Dir(dstPath), 0o750); err != nil {
234+
f.logger.Error("Unable to create destination directory", f.logger.Args(
235+
"followerName", f.ref,
236+
"directory", filepath.Dir(dstPath),
237+
"reason", err.Error(),
238+
))
239+
return err
240+
}
241+
242+
f.logger.Debug("Installing file", f.logger.Args("followerName", f.ref, "path", relPath))
211243
// Check if the file exists.
212-
f.logger.Debug("Checking if file already exists", f.logger.Args("followerName", f.ref, "fileName", baseName, "directory", dstDir))
244+
f.logger.Debug("Checking if file already exists", f.logger.Args("followerName", f.ref, "path", relPath, "directory", dstDir))
213245
exists, err := utils.FileExists(dstPath)
214246
if err != nil {
215-
f.logger.Error("Unable to check existence for file", f.logger.Args("followerName", f.ref, "fileName", baseName, "reason", err.Error()))
216-
return
247+
f.logger.Error("Unable to check existence for file", f.logger.Args("followerName", f.ref, "path", relPath, "reason", err.Error()))
248+
return err
217249
}
218250

219251
if !exists {
220-
f.logger.Debug("Moving file", f.logger.Args("followerName", f.ref, "fileName", baseName, "destDirectory", dstDir))
252+
f.logger.Debug("Moving file", f.logger.Args("followerName", f.ref, "path", relPath, "destDirectory", dstDir))
221253
if err = utils.Move(path, dstPath); err != nil {
222-
f.logger.Error("Unable to move file", f.logger.Args("followerName", f.ref, "fileName", baseName, "destDirectory", dstDir, "reason", err.Error()))
223-
return
254+
f.logger.Error("Unable to move file", f.logger.Args("followerName", f.ref, "path", relPath, "destDirectory", dstDir, "reason", err.Error()))
255+
return err
224256
}
225257
f.logger.Debug("File correctly installed", f.logger.Args("followerName", f.ref, "path", path))
226258
// It's done, move to the next file.
227259
continue
228260
}
229-
f.logger.Debug(fmt.Sprintf("file %q already exists in %q, checking if it is equal to the existing one", baseName, dstDir),
261+
262+
f.logger.Debug(fmt.Sprintf("file %q already exists in %q, checking if it is equal to the existing one", relPath, dstDir),
230263
f.logger.Args("followerName", f.ref))
264+
231265
// Check if the files are equal.
232266
eq, err := equal([]string{path, dstPath})
233267
if err != nil {
234-
f.logger.Error("Unable to compare files", f.logger.Args("followerName", f.ref, "newFile", path, "existingFile", dstPath, "reason", err.Error()))
235-
return
268+
f.logger.Error("Unable to compare files", f.logger.Args("followerName", f.ref, "existingFile", dstPath, "reason", err.Error()))
269+
return err
236270
}
237271

238272
if !eq {
239273
f.logger.Debug(fmt.Sprintf("Overwriting file %q with file %q", dstPath, path), f.logger.Args("followerName", f.ref))
240274
if err = utils.Move(path, dstPath); err != nil {
241275
f.logger.Error("Unable to overwrite file", f.logger.Args("followerName", f.ref, "existingFile", dstPath, "reason", err.Error()))
242-
return
276+
return err
243277
}
244278
} else {
245279
f.logger.Debug("The two file are equal, nothing to be done")
246280
}
247281
}
248-
249-
f.logger.Info("Artifact correctly installed",
250-
f.logger.Args("followerName", f.ref, "artifactName", f.ref, "type", res.Type, "digest", res.Digest, "directory", dstDir))
251-
f.currentDigest = desc.Digest.String()
282+
return nil
252283
}
253284

254285
// pull downloads, extracts, and installs the artifact.

‎internal/follower/follower_test.go

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// SPDX-License-Identifier: Apache-2.0
2-
// Copyright (C) 2024 The Falco Authors
2+
// Copyright (C) 2025 The Falco Authors
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@ package follower
1717

1818
import (
1919
"os"
20+
"path/filepath"
2021
"testing"
2122

2223
"github.com/pterm/pterm"
@@ -135,3 +136,170 @@ func TestCheckRequirements(t *testing.T) {
135136
})
136137
}
137138
}
139+
140+
func TestMoveFiles(t *testing.T) {
141+
type testFile struct {
142+
path string
143+
content string
144+
replace bool
145+
}
146+
147+
tests := []struct {
148+
name string
149+
files []testFile
150+
existing []testFile
151+
}{
152+
{
153+
name: "basic file at root",
154+
files: []testFile{
155+
{
156+
path: "file1.yaml",
157+
content: "content1",
158+
},
159+
},
160+
},
161+
{
162+
name: "file in subdirectory",
163+
files: []testFile{
164+
{
165+
path: "subdir/file2.yaml",
166+
content: "content2",
167+
},
168+
},
169+
},
170+
{
171+
name: "multiple files in different directories",
172+
files: []testFile{
173+
{
174+
path: "file1.yaml",
175+
content: "content1",
176+
},
177+
{
178+
path: "subdir/file2.yaml",
179+
content: "content2",
180+
},
181+
{
182+
path: "subdir/nested/file3.yaml",
183+
content: "content3",
184+
},
185+
},
186+
},
187+
{
188+
name: "existing file with identical content",
189+
files: []testFile{
190+
{
191+
path: "file1.yaml",
192+
content: "content1",
193+
replace: false,
194+
},
195+
},
196+
existing: []testFile{
197+
{
198+
path: "file1.yaml",
199+
content: "content1",
200+
},
201+
},
202+
},
203+
{
204+
name: "existing file with different content",
205+
files: []testFile{
206+
{
207+
path: "file1.yaml",
208+
content: "new content",
209+
replace: true,
210+
},
211+
},
212+
existing: []testFile{
213+
{
214+
path: "file1.yaml",
215+
content: "old content",
216+
},
217+
},
218+
},
219+
{
220+
name: "mix of new and existing files",
221+
files: []testFile{
222+
{
223+
path: "file1.yaml",
224+
content: "content1",
225+
replace: false,
226+
},
227+
{
228+
path: "subdir/file2.yaml",
229+
content: "new content2",
230+
replace: true,
231+
},
232+
},
233+
existing: []testFile{
234+
{
235+
path: "file1.yaml",
236+
content: "content1",
237+
},
238+
{
239+
path: "subdir/file2.yaml",
240+
content: "old content2",
241+
},
242+
},
243+
},
244+
}
245+
246+
for _, tt := range tests {
247+
t.Run(tt.name, func(t *testing.T) {
248+
tmpDir, err := os.MkdirTemp("", "falcoctl-test-*")
249+
assert.NoError(t, err)
250+
defer os.RemoveAll(tmpDir)
251+
252+
dstDir, err := os.MkdirTemp("", "falcoctl-dst-*")
253+
assert.NoError(t, err)
254+
defer os.RemoveAll(dstDir)
255+
256+
// Setup existing files
257+
for _, ef := range tt.existing {
258+
dstPath := filepath.Join(dstDir, ef.path)
259+
err = os.MkdirAll(filepath.Dir(dstPath), 0o755)
260+
assert.NoError(t, err)
261+
err = os.WriteFile(dstPath, []byte(ef.content), 0o644)
262+
assert.NoError(t, err)
263+
}
264+
265+
f, err := New("test-registry/test-ref", output.NewPrinter(pterm.LogLevelDebug, pterm.LogFormatterJSON, os.Stdout), &Config{
266+
RulesfilesDir: dstDir,
267+
TmpDir: tmpDir,
268+
})
269+
assert.NoError(t, err)
270+
271+
var paths []string
272+
for _, tf := range tt.files {
273+
fullPath := filepath.Join(f.tmpDir, tf.path)
274+
err = os.MkdirAll(filepath.Dir(fullPath), 0o755)
275+
assert.NoError(t, err)
276+
err = os.WriteFile(fullPath, []byte(tf.content), 0o644)
277+
assert.NoError(t, err)
278+
paths = append(paths, fullPath)
279+
}
280+
281+
f.currentDigest = "test-digest"
282+
err = f.moveFiles(paths, dstDir)
283+
assert.NoError(t, err)
284+
285+
for _, tf := range tt.files {
286+
dstPath := filepath.Join(dstDir, tf.path)
287+
_, err = os.Stat(dstPath)
288+
assert.NoError(t, err, "file should exist at %s", dstPath)
289+
290+
content, err := os.ReadFile(dstPath)
291+
assert.NoError(t, err)
292+
assert.Equal(t, tf.content, string(content), "file content should match at %s", dstPath)
293+
294+
// For files marked as replace=false, verify they have identical content with existing files
295+
if !tf.replace {
296+
for _, ef := range tt.existing {
297+
if ef.path == tf.path {
298+
assert.Equal(t, ef.content, string(content), "file content should not change when replace=false: %s", dstPath)
299+
}
300+
}
301+
}
302+
}
303+
})
304+
}
305+
}

‎internal/utils/extract.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ func ExtractTarGz(ctx context.Context, gzipStream io.Reader, destDir string, str
8686
continue
8787
}
8888
info := header.FileInfo()
89-
files = append(files, path)
9089

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

0 commit comments

Comments
 (0)