Skip to content

Commit

Permalink
cleanup: rename some error messages
Browse files Browse the repository at this point in the history
Signed-off-by: Andrea Terzolo <[email protected]>
  • Loading branch information
Andreagit97 authored and poiana committed Jan 11, 2024
1 parent ee78c86 commit c308f5c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 16 deletions.
10 changes: 5 additions & 5 deletions unit_tests/engine/test_rule_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ TEST_F(engine_loader_test, rule_override_append)
ASSERT_TRUE(load_rules(rules_content, "legit_rules.yaml")) << m_load_result_string;

// Here we don't use the deprecated `append` flag, so we don't expect the warning.
ASSERT_FALSE(check_warning_message(WARNING_APPEND_MESSAGE));
ASSERT_FALSE(check_warning_message(WARNING_APPEND));

auto rule_description = m_engine->describe_rule(&rule_name, {});
ASSERT_EQ(rule_description["rules"][0]["info"]["condition"].template get<std::string>(),
Expand Down Expand Up @@ -221,7 +221,7 @@ TEST_F(engine_loader_test, rule_append)
ASSERT_TRUE(load_rules(rules_content, "legit_rules.yaml")) << m_load_result_string;

// We should have at least one warning because the 'append' flag is deprecated.
ASSERT_TRUE(check_warning_message(WARNING_APPEND_MESSAGE));
ASSERT_TRUE(check_warning_message(WARNING_APPEND));

auto rule_description = m_engine->describe_rule(&rule_name, {});
ASSERT_EQ(rule_description["rules"][0]["details"]["condition_compiled"].template get<std::string>(),
Expand Down Expand Up @@ -344,9 +344,9 @@ TEST_F(engine_loader_test, rule_incorrect_append_override)
ASSERT_FALSE(load_rules(rules_content, "rules.yaml"));

// We should have at least one warning because the 'append' flag is deprecated.
ASSERT_TRUE(check_warning_message(WARNING_APPEND_MESSAGE));
ASSERT_TRUE(check_warning_message(WARNING_APPEND));

ASSERT_TRUE(check_error_message(OVERRIDE_APPEND_ERROR_MESSAGE));
ASSERT_TRUE(check_error_message(ERROR_OVERRIDE_APPEND));
}

TEST_F(engine_loader_test, macro_override_append_before_macro_definition)
Expand Down Expand Up @@ -872,7 +872,7 @@ TEST_F(engine_loader_test, rule_enabled_warning)
)END";

ASSERT_TRUE(load_rules(rules_content, "rules.yaml"));
ASSERT_TRUE(check_warning_message(WARNING_ENABLED_MESSAGE));
ASSERT_TRUE(check_warning_message(WARNING_ENABLED));
// The rule should be enabled at the end.
EXPECT_EQ(num_rules_for_ruleset(), 1);
}
Expand Down
14 changes: 7 additions & 7 deletions userspace/engine/rule_loader_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,15 +442,15 @@ static void read_item(
decode_optional_val(item, "append", has_append_flag, ctx);
if(has_append_flag)
{
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_APPEND_MESSAGE, ctx);
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_APPEND, ctx);
}

std::set<std::string> override_append, override_replace;
std::set<std::string> overridable {"items"};
decode_overrides(item, overridable, overridable, override_append, override_replace, ctx);
bool has_overrides = !override_append.empty() || !override_replace.empty();

THROW(has_append_flag && has_overrides, OVERRIDE_APPEND_ERROR_MESSAGE, ctx);
THROW(has_append_flag && has_overrides, ERROR_OVERRIDE_APPEND, ctx);

// Since a list only has items, if we have chosen to append them we can append the entire object
// otherwise we just want to redefine the list.
Expand Down Expand Up @@ -485,15 +485,15 @@ static void read_item(
decode_optional_val(item, "append", has_append_flag, ctx);
if(has_append_flag)
{
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_APPEND_MESSAGE, ctx);
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_APPEND, ctx);
}

std::set<std::string> override_append, override_replace;
std::set<std::string> overridable {"condition"};
decode_overrides(item, overridable, overridable, override_append, override_replace, ctx);
bool has_overrides = !override_append.empty() || !override_replace.empty();

THROW((has_append_flag && has_overrides), OVERRIDE_APPEND_ERROR_MESSAGE, ctx);
THROW((has_append_flag && has_overrides), ERROR_OVERRIDE_APPEND, ctx);

// Since a macro only has a condition, if we have chosen to append to it we can append the entire object
// otherwise we just want to redefine the macro.
Expand Down Expand Up @@ -522,7 +522,7 @@ static void read_item(
decode_optional_val(item, "append", has_append_flag, ctx);
if(has_append_flag)
{
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_APPEND_MESSAGE, ctx);
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_APPEND, ctx);
}

std::set<std::string> override_append, override_replace;
Expand All @@ -534,7 +534,7 @@ static void read_item(
bool has_overrides_replace = !override_replace.empty();
bool has_overrides = has_overrides_append || has_overrides_replace;

THROW((has_append_flag && has_overrides), OVERRIDE_APPEND_ERROR_MESSAGE, ctx);
THROW((has_append_flag && has_overrides), ERROR_OVERRIDE_APPEND, ctx);

if(has_overrides)
{
Expand Down Expand Up @@ -694,7 +694,7 @@ static void read_item(
!item["priority"].IsDefined())
{
decode_val(item, "enabled", v.enabled, ctx);
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_ENABLED_MESSAGE, ctx);
cfg.res->add_warning(falco::load_result::LOAD_DEPRECATED_ITEM, WARNING_ENABLED, ctx);
collector.enable(cfg, v);
}
else
Expand Down
15 changes: 11 additions & 4 deletions userspace/engine/rule_loading_messages.h
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
#pragma once

// todo: rename putting error at the beginning
#define OVERRIDE_APPEND_ERROR_MESSAGE "Keys 'override' and 'append: true' cannot be used together. Add an 'append' entry (e.g. 'condition: append') under 'override' instead."
////////////////
// Warnings
////////////////

#define WARNING_APPEND_MESSAGE "'append' key is deprecated. Add an 'append' entry (e.g. 'condition: append') under 'override' instead."
#define WARNING_APPEND "'append' key is deprecated. Add an 'append' entry (e.g. 'condition: append') under 'override' instead."

#define WARNING_ENABLED_MESSAGE "The standalone 'enabled' key usage is deprecated. The correct approach requires also a 'replace' entry under the 'override' key (i.e. 'enabled: replace')."
#define WARNING_ENABLED "The standalone 'enabled' key usage is deprecated. The correct approach requires also a 'replace' entry under the 'override' key (i.e. 'enabled: replace')."

////////////////
// Errors
////////////////

#define ERROR_OVERRIDE_APPEND "Keys 'override' and 'append: true' cannot be used together. Add an 'append' entry (e.g. 'condition: append') under 'override' instead."

#define ERROR_NO_PREVIOUS_MACRO "Macro uses 'append' or 'override.condition: append' but no macro by that name already exists"

Expand Down

0 comments on commit c308f5c

Please sign in to comment.