fix(follow): handle directories in tar.gz artifacts #716
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: TiagoJMartins The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @TiagoJMartins! It looks like this is your first PR to falcosecurity/falcoctl 🎉 |
TiagoJMartins
force-pushed
the
fix/allow-directories-in-artifacts
branch
from
9c6f709 to
1ef75d7
Compare
|
|
TiagoJMartins
force-pushed
the
fix/allow-directories-in-artifacts
branch
from
1ef75d7 to
48f8bfb
Compare
TiagoJMartins
force-pushed
the
fix/allow-directories-in-artifacts
branch
from
48f8bfb to
d1e9252
Compare
TiagoJMartins
changed the title
TiagoJMartins
changed the title
|
Tested with this (amd64) image: |
|
Hi @TiagoJMartins, thansk for the PR. Could you please have a look at the CI? Linting and tests are failing. |
TiagoJMartins
force-pushed
the
fix/allow-directories-in-artifacts
branch
2 times, most recently
from
6424fab to
1ba880a
Compare
I think the issues should be fixed now. I'll wait until someone approves the next CI run 👍🏻 |
There was a problem hiding this comment.
Hey @TiagoJMartins, thanks for the contribution!
I left some comments.
| // 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-") |
There was a problem hiding this comment.
Why do we need this? The temporary dir is given by the user, and is the same for all the followers/artifacts.
There was a problem hiding this comment.
@TiagoJMartins, could you please provide more info about this change?
Signed-off-by: Tiago Martins <tiago.martins@hotjar.com>
TiagoJMartins
force-pushed
the
fix/allow-directories-in-artifacts
branch
from
1ba880a to
1e715bb
Compare
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area cli
What this PR does / why we need it:
A bit of background on my use-case:
I've configured Falco to load all rules from a given sub-directory within
/etc/falco, e.g.:/etc/falco/subdir/rules1.yaml, and packaged multiple rules files into separaterulesfileartifacts intar.gzformat.During packaging, I'll generate the final directory structure I want and archive it, knowing that
falcoctl artifactwill extract the contents onto/etc/falco.The issue is that while
falco artifact installworks great,falco artifact followseems to break when processing the file path as it expects base paths to always be files, so it'll always fail when the rules gets updated and then it tries to move a directory.Which issue(s) this PR fixes:
This PR addresses the issue above by modifying the file path handling in a way that preserves the artifact directory structure, and also slightly modifies the tar.gz extraction logic by only appending to
filesif it's handling an actual file and not a directory/symlink.I've added tests for these use cases to ensure that the previous behavior remains unchanged.
The error below is printed when trying to update an artifact pushed as
.tar.gz, that includes the file:/rules.csq.d/kubernetes.yaml, which gets installed at/etc/falco/rules.csq.d/kubernetes.yaml(because/rulesfilesis mounted at/etc/falcoby the Helm chart).{ "destDirectory": "/rulesfiles", "fileName": "rules.csq.d", "followerName": "foobar/falco-rules/platform:1-kubernetes", "level": "ERROR", "msg": "Unable to move file", "reason": "unable to read file /tmp/falcoctl-1880591248/rules.csq.d: read /tmp/falcoctl-1880591248/rules.csq.d: is a directory", "timestamp": "2025-01-14 17:03:32" }