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

chore(userspace/falco): deprecate old 'rules_file' config key. #3162

Merged
merged 3 commits into from
Apr 16, 2024
Merged
Changes from 1 commit
Commits
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
Next Next commit
chore(userspace/falco): deprecate old 'rules_file' config key.
Signed-off-by: Federico Di Pierro <[email protected]>
FedeDP committed Apr 10, 2024

Verified

This commit was signed with the committer’s verified signature.
commit 06fcc35690e424a3490d9084e89585224aee7b63
13 changes: 7 additions & 6 deletions falco.yaml
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@
# Falco config files
# configs_files
# Falco rules files
# rules_file
# rules_files
# Falco engine
# engine
# Falco plugins
@@ -128,19 +128,20 @@
# Therefore, loaded config files *can* override values from main config file.
# Also, nested include is not allowed, ie: included config files won't be able to include other config files.
#
# Like for 'rules_file', specifying a folder will load all the configs files present in it in a lexicographical order.
# Like for 'rules_files', specifying a folder will load all the configs files present in it in a lexicographical order.
configs_files:
- /etc/falco/config.d

#####################
# Falco rules files #
#####################

# [Stable] `rules_file`
# [Stable] `rules_files`
FedeDP marked this conversation as resolved.
Show resolved Hide resolved
#
# Falco rules can be specified using files or directories, which are loaded at
# startup. The name "rules_file" is maintained for backwards compatibility. If
# the entry is a file, it will be read directly. If the entry is a directory,
# startup. The old name "rules_file" is maintained for backwards compatibility.
FedeDP marked this conversation as resolved.
Show resolved Hide resolved
#
# If the entry is a file, it will be read directly. If the entry is a directory,
# all files within that directory will be read in alphabetical order.
#
# The falco_rules.yaml file ships with the Falco package and is overridden with
@@ -169,7 +170,7 @@ configs_files:
# "first match wins" principle. However, enabling the `all` matching option may result
# in a performance penalty. We recommend carefully testing this alternative setting
# before deploying it in production. Read more under the `rule_matching` configuration.
rules_file:
rules_files:
- /etc/falco/falco_rules.yaml
- /etc/falco/falco_rules.local.yaml
- /etc/falco/rules.d
45 changes: 30 additions & 15 deletions userspace/falco/configuration.cpp
Original file line number Diff line number Diff line change
@@ -167,6 +167,18 @@ void falco_configuration::merge_configs_files(const std::string& config_name, st
}
}

void falco_configuration::init_logger()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is somewhat a fix: initialize the logger before loading anything else from the config; this allows us to log at the correct level in case we need to.
NOTE: this wasn't a bug since no logging was present in the yaml loading phase in configuration.cpp; but now we have to log a warning when both new and old keys are defined.

All in all, logging is quite always the first thing that needs to be inited, thus the change.

{
m_log_level = config.get_scalar<std::string>("log_level", "info");
falco_logger::set_level(m_log_level);
falco_logger::set_sinsp_logging(
config.get_scalar<bool>("libs_logger.enabled", false),
config.get_scalar<std::string>("libs_logger.severity", "debug"),
"[libs]: ");
falco_logger::log_stderr = config.get_scalar<bool>("log_stderr", false);
falco_logger::log_syslog = config.get_scalar<bool>("log_syslog", true);
}

void falco_configuration::load_engine_config(const std::string& config_name)
{
// Set driver mode if not already set.
@@ -238,12 +250,28 @@ void falco_configuration::load_engine_config(const std::string& config_name)

void falco_configuration::load_yaml(const std::string& config_name)
{
init_logger();
load_engine_config(config_name);
m_log_level = config.get_scalar<std::string>("log_level", "info");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was done twice :)


std::list<std::string> rules_files;

config.get_sequence<std::list<std::string>>(rules_files, std::string("rules_file"));
// Small glue code to support old deprecated 'rules_file' config key.
int num_rules_files_opts = 0;
if (config.is_defined("rules_files"))
{
num_rules_files_opts++;
config.get_sequence<std::list<std::string>>(rules_files, std::string("rules_files"));
}
if (config.is_defined("rules_file"))
{
num_rules_files_opts++;
config.get_sequence<std::list<std::string>>(rules_files, std::string("rules_file"));
falco_logger::log(falco_logger::level::WARNING, "Using deprecated config key 'rules_file'. Please use new 'rules_files' config key.");
FedeDP marked this conversation as resolved.
Show resolved Hide resolved
}
if (num_rules_files_opts == 2)
{
throw std::logic_error("Error reading config file (" + config_name + "): both 'rules_files' and 'rules_file' keys set");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw std::logic_error("Error reading config file (" + config_name + "): both 'rules_files' and 'rules_file' keys set");
throw std::logic_error("Error reading config file (" + config_name + "): both 'rules_files' (plural form) and 'rules_file' (singular form) keys set. Please only use new 'rules_files' config key (plural form).");

}

m_rules_filenames.clear();
m_loaded_rules_filenames.clear();
@@ -393,19 +421,6 @@ void falco_configuration::load_yaml(const std::string& config_name)
m_outputs.push_back(grpc_output);
}

m_log_level = config.get_scalar<std::string>("log_level", "info");

falco_logger::set_level(m_log_level);


falco_logger::set_sinsp_logging(
config.get_scalar<bool>("libs_logger.enabled", false),
config.get_scalar<std::string>("libs_logger.severity", "debug"),
"[libs]: ");

falco_logger::log_stderr = config.get_scalar<bool>("log_stderr", false);
falco_logger::log_syslog = config.get_scalar<bool>("log_syslog", true);

m_output_timeout = config.get_scalar<uint32_t>("output_timeout", 2000);

std::string rule_matching = config.get_scalar<std::string>("rule_matching", "first");
5 changes: 1 addition & 4 deletions userspace/falco/configuration.h
Original file line number Diff line number Diff line change
@@ -173,13 +173,10 @@ class falco_configuration

private:
void merge_configs_files(const std::string& config_name, std::vector<std::string>& loaded_config_files);

void load_yaml(const std::string& config_name);

void init_logger();
void load_engine_config(const std::string& config_name);

void init_cmdline_options(const std::vector<std::string>& cmdline_options);

/**
* Given a <key>=<value> specifier, set the appropriate option
* in the underlying yaml config. <key> can contain '.'