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

fix(userspace/falco): applies FALCO_INSTALL_CONF_FILE as the default … #1900

Merged

Conversation

andreabonanno
Copy link
Contributor

…config.

Signed-off-by: Andrea Bonanno [email protected]

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

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

/area build

/area engine

/area rules

/area tests

/area proposals

What this PR does / why we need it:

Makes Falco defaults to the config file in /etc/falco/falco.yaml when the -c option is not used, instead of evaluating /source/falco/falco.yaml as the config source with highest priority.

Which issue(s) this PR fixes:

Fixes #1897

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

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.

Thank you!

Your solution works, but I'd rather keep the old behavior when -DCMAKE_BUILD_TYPE=Debug and use only the installation path when -DCMAKE_BUILD_TYPE=Release.

Wdyt? :)

@andreabonanno andreabonanno force-pushed the change_default_config_file branch from 945901b to b1f994e Compare February 16, 2022 15:30
@leogr
Copy link
Member

leogr commented Feb 18, 2022

/milestone 0.32.0

@poiana poiana added this to the 0.32.0 milestone Feb 18, 2022
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.

LGTM

Thank you!

/approve

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.

See @jasondellaluce comment.

userspace/falco/falco.cpp Outdated Show resolved Hide resolved
@poiana poiana removed the lgtm label Feb 18, 2022
@poiana poiana requested a review from leogr February 18, 2022 12:49
@andreabonanno andreabonanno force-pushed the change_default_config_file branch from b1f994e to e626668 Compare February 18, 2022 14:03
@andreabonanno andreabonanno force-pushed the change_default_config_file branch from e626668 to 4c21a68 Compare February 18, 2022 15:56
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.

/approve

@poiana poiana added the lgtm label Feb 18, 2022
@poiana
Copy link
Contributor

poiana commented Feb 18, 2022

LGTM label has been added.

Git tree hash: af480455ac3cfd10a884e025c506125f0142382f

@poiana
Copy link
Contributor

poiana commented Feb 18, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreabonanno, leogr

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 eedb794 into falcosecurity:master Feb 18, 2022
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.

Unnecessary default value for -c
4 participants