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.

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

feat(falco): Provide a parameter for loading lua files from an alternate path #1419

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

admiral0
Copy link

@admiral0 admiral0 commented Sep 29, 2020

This will be used by the static build to load lua files from
alternate directories that are not tied to the compile flags

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:
This allows us to provide tarballs of static builds of falco and use --alternate-lua-dir $(pwd)/lua to specify where to look for lua
files.

Which issue(s) this PR fixes:

Special notes for your reviewer:
This will improve user experience for users of the static build, because currently that path is hardcoded at compile time.

Does this PR introduce a user-facing change?:

new: CLI flag `--alternate-lua-dir` to load Lua files from arbitrary paths

Copy link
Contributor

@fntlnz fntlnz left a comment

Choose a reason for hiding this comment

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

Thanks for bringing this up @admiral0 - I also really needed this for some deployments I was doing this week - I like the general approach but had some comments.

userspace/falco/falco.cpp Outdated Show resolved Hide resolved
userspace/falco/falco.cpp Outdated Show resolved Hide resolved
userspace/falco/falco.cpp Outdated Show resolved Hide resolved
@fntlnz
Copy link
Contributor

fntlnz commented Sep 29, 2020

We probably need to add the trailing slash if the user didn't provide it:

This does not work

sudo ./userspace/falco/falco -r ../rules/falco_rules.yaml -u --alternate-lua-dir /tmp/lua 
alternate lua dir: /tmp/lua
Tue Sep 29 14:55:27 2020: Runtime error: Could not find Falco Lua entrypoint (tried /usr/share/falco/lua/rule_loader.lua, /tmp/luarule_loader.lua). Exiting.

This works

sudo ./userspace/falco/falco -r ../rules/falco_rules.yaml -u --alternate-lua-dir /tmp/lua/
alternate lua dir: /tmp/lua/
Tue Sep 29 14:56:19 2020: Falco version 0.26.0-2+ac84e3e (driver version 2aa88dcf6243982697811df4c1b484bcbe9488a2)
Tue Sep 29 14:56:19 2020: Falco initialized with configuration file /home/fntlnz/Projects/falcosecurity/falco/falco.yaml
Tue Sep 29 14:56:19 2020: Loading rules from file ../rules/falco_rules.yaml:

Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

Thanks Radu for sending this PR!

This PR is in the right direction. Anyways, it is not completed yet IMHO.

For example in falco_outputs.cpp there is this line that needs to be updated too.

falco_common::init(m_lua_main_filename.c_str(), FALCO_SOURCE_LUA_DIR);

Note that the falco_common::init() is also called by the ctor of falco_engine that you updated.

@leodido
Copy link
Member

leodido commented Sep 29, 2020

/milestone 0.27.0

@poiana poiana added this to the 0.27.0 milestone Sep 29, 2020
@leodido
Copy link
Member

leodido commented Sep 29, 2020

@admiral0 I've updated the release-note line, waiting for your new commits :)

@admiral0
Copy link
Author

/assign @leodido

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

I'd personally prefer to completely remove the need for external files (by embedding them into the binary, as proposed in #1367).

However, I can agree with this approach for now 👍

Just my 2 cents :)

userspace/falco/falco.cpp Outdated Show resolved Hide resolved
…ate path

This will be used by the static build to load lua files from
alternate directories that are not tied to the compile flags

Signed-off-by: Radu Andries <[email protected]>
@poiana poiana added the lgtm label Sep 29, 2020
@poiana
Copy link
Contributor

poiana commented Sep 29, 2020

LGTM label has been added.

Git tree hash: a8605020170861c3cef418aee3d381290d6bcf97

@poiana
Copy link
Contributor

poiana commented Sep 29, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leodido

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit bc1aeac into falcosecurity:master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants