From dd0ef49e020b5a9e8de8316bc6113bfd44584153 Mon Sep 17 00:00:00 2001 From: Kokan Date: Wed, 18 Dec 2019 08:55:34 +0100 Subject: [PATCH 1/6] geoip2: reorder grammar rules Signed-off-by: Kokan --- modules/geoip2/geoip-parser-grammar.ym | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/modules/geoip2/geoip-parser-grammar.ym b/modules/geoip2/geoip-parser-grammar.ym index f356cf66aa9..c59ca22883b 100644 --- a/modules/geoip2/geoip-parser-grammar.ym +++ b/modules/geoip2/geoip-parser-grammar.ym @@ -59,11 +59,6 @@ start : LL_CONTEXT_PARSER parser_expr_maxminddb { YYACCEPT; } ; -parser_geoip_opts - : parser_geoip_opt parser_geoip_opts - | - ; - parser_expr_maxminddb : KW_GEOIP2 '(' { @@ -80,13 +75,16 @@ parser_expr_maxminddb } parser_geoip_opts ')' { $$ = last_parser; free($4); } + ; + +parser_geoip_opts + : parser_geoip_opt parser_geoip_opts + | ; parser_geoip_opt - : KW_PREFIX '(' string ')' - { geoip_parser_set_prefix(last_parser, $3); free($3); } - | KW_DATABASE '(' path_check ')' - { geoip_parser_set_database_path(last_parser, $3); free($3); } + : KW_PREFIX '(' string ')' { geoip_parser_set_prefix(last_parser, $3); free($3); } + | KW_DATABASE '(' path_check ')' { geoip_parser_set_database_path(last_parser, $3); free($3); } ; /* INCLUDE_RULES */ From 27bfc6bc25f0e9abffd5916c1c5218fc84c2b726 Mon Sep 17 00:00:00 2001 From: Kokan Date: Wed, 18 Dec 2019 09:06:17 +0100 Subject: [PATCH 2/6] geoip2: extend with general parser_opt Right now only `template()` is included with the parser_opt rule, but this also allows any other future enhancements. Signed-off-by: Kokan --- modules/geoip2/geoip-parser-grammar.ym | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/geoip2/geoip-parser-grammar.ym b/modules/geoip2/geoip-parser-grammar.ym index c59ca22883b..ff20fb37b8f 100644 --- a/modules/geoip2/geoip-parser-grammar.ym +++ b/modules/geoip2/geoip-parser-grammar.ym @@ -85,6 +85,7 @@ parser_geoip_opts parser_geoip_opt : KW_PREFIX '(' string ')' { geoip_parser_set_prefix(last_parser, $3); free($3); } | KW_DATABASE '(' path_check ')' { geoip_parser_set_database_path(last_parser, $3); free($3); } + | parser_opt ; /* INCLUDE_RULES */ From f8a3c4ccd94bfa55e4c0977f797d9612290d42ca Mon Sep 17 00:00:00 2001 From: Kokan Date: Wed, 18 Dec 2019 11:20:35 +0100 Subject: [PATCH 3/6] geoip2: unit test should always use template Signed-off-by: Kokan --- modules/geoip2/tests/test_geoip_parser.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/modules/geoip2/tests/test_geoip_parser.c b/modules/geoip2/tests/test_geoip_parser.c index 951b912b217..42c10ebc265 100644 --- a/modules/geoip2/tests/test_geoip_parser.c +++ b/modules/geoip2/tests/test_geoip_parser.c @@ -52,7 +52,7 @@ teardown(void) } static LogMessage * -parse_geoip_into_log_message_no_check(const gchar *input) +parse_geoip_into_log_message_no_check(const gchar *template_format) { LogMessage *msg; LogPathOptions path_options = LOG_PATH_OPTIONS_INIT; @@ -60,9 +60,14 @@ parse_geoip_into_log_message_no_check(const gchar *input) gboolean success; cloned_parser = (LogParser *) log_pipe_clone(&geoip_parser->super); + + LogTemplate *template = log_template_new(NULL, NULL); + log_template_compile(template, template_format, NULL); + log_parser_set_template(cloned_parser, template); + log_pipe_init(&cloned_parser->super); msg = log_msg_new_empty(); - log_msg_set_value(msg, LM_V_MESSAGE, input, -1); + success = log_parser_process_message(cloned_parser, &msg, &path_options); if (!success) { @@ -75,12 +80,12 @@ parse_geoip_into_log_message_no_check(const gchar *input) } static LogMessage * -parse_geoip_into_log_message(const gchar *input) +parse_geoip_into_log_message(const gchar *template_format) { LogMessage *msg; - msg = parse_geoip_into_log_message_no_check(input); - cr_assert_not_null(msg, "expected geoip-parser success and it returned failure, input=%s", input); + msg = parse_geoip_into_log_message_no_check(template_format); + cr_assert_not_null(msg, "expected geoip-parser success and it returned failure, template_format=%s", template_format); return msg; } @@ -106,12 +111,8 @@ Test(geoip2, test_basics) Test(geoip2, test_using_template_to_parse_input) { LogMessage *msg; - LogTemplate *template; - template = log_template_new(NULL, NULL); - log_template_compile(template, "2.125.160.216", NULL); - log_parser_set_template(geoip_parser, template); - msg = parse_geoip_into_log_message("8.8.8.8"); + msg = parse_geoip_into_log_message("2.125.160.216"); assert_log_message_value(msg, log_msg_get_value_handle(".geoip2.country.iso_code"), "GB"); assert_log_message_value(msg, log_msg_get_value_handle(".geoip2.location.latitude"), "51.750000"); From a7fb36c39ca1746fb4e15f68b43f466a10b5ac1d Mon Sep 17 00:00:00 2001 From: Kokan Date: Wed, 18 Dec 2019 11:13:01 +0100 Subject: [PATCH 4/6] geoip2: template is mandatory option Signed-off-by: Kokan --- modules/geoip2/geoip-parser.c | 6 ++++++ modules/geoip2/tests/test_geoip_parser.c | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/modules/geoip2/geoip-parser.c b/modules/geoip2/geoip-parser.c index 833786b46a0..f06409e5149 100644 --- a/modules/geoip2/geoip-parser.c +++ b/modules/geoip2/geoip-parser.c @@ -159,6 +159,12 @@ maxminddb_parser_init(LogPipe *s) { GeoIPParser *self = (GeoIPParser *) s; + if (!self->super.template) + { + msg_error("geoip2(): template is a mandatory parameter", log_pipe_location_tag(s)); + return FALSE; + } + if (!self->database_path) self->database_path = mmdb_default_database(); diff --git a/modules/geoip2/tests/test_geoip_parser.c b/modules/geoip2/tests/test_geoip_parser.c index 42c10ebc265..89950bf847f 100644 --- a/modules/geoip2/tests/test_geoip_parser.c +++ b/modules/geoip2/tests/test_geoip_parser.c @@ -89,6 +89,15 @@ parse_geoip_into_log_message(const gchar *template_format) return msg; } +Test(geoip2, template_is_mandatory) +{ + LogParser *geoip2_parser = maxminddb_parser_new(configuration); + + cr_assert_not(log_pipe_init(&geoip2_parser->super)); + + log_pipe_unref(&geoip2_parser->super); +} + Test(geoip2, test_basics) { LogMessage *msg; From 499685d666a22130ee81accc8dc4114cf476474b Mon Sep 17 00:00:00 2001 From: Kokan Date: Wed, 18 Dec 2019 11:21:08 +0100 Subject: [PATCH 5/6] geoip2: rename, cleanup unit tests Signed-off-by: Kokan --- modules/geoip2/tests/test_geoip_parser.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/geoip2/tests/test_geoip_parser.c b/modules/geoip2/tests/test_geoip_parser.c index 89950bf847f..15b31fb81cc 100644 --- a/modules/geoip2/tests/test_geoip_parser.c +++ b/modules/geoip2/tests/test_geoip_parser.c @@ -98,18 +98,19 @@ Test(geoip2, template_is_mandatory) log_pipe_unref(&geoip2_parser->super); } -Test(geoip2, test_basics) +Test(geoip2, set_prefix) { LogMessage *msg; - msg = parse_geoip_into_log_message("2.125.160.216"); - assert_log_message_value(msg, log_msg_get_value_handle(".geoip2.country.iso_code"), "GB"); - log_msg_unref(msg); - geoip_parser_set_prefix(geoip_parser, ".prefix."); msg = parse_geoip_into_log_message("2.125.160.216"); assert_log_message_value(msg, log_msg_get_value_handle(".prefix.country.iso_code"), "GB"); log_msg_unref(msg); +} + +Test(geoip2, empty_prefix) +{ + LogMessage *msg; geoip_parser_set_prefix(geoip_parser, ""); msg = parse_geoip_into_log_message("2.125.160.216"); @@ -117,7 +118,7 @@ Test(geoip2, test_basics) log_msg_unref(msg); } -Test(geoip2, test_using_template_to_parse_input) +Test(geoip2, test_basic) { LogMessage *msg; From 158fe70b2711b1244107569492e93ea6de830b2f Mon Sep 17 00:00:00 2001 From: Kokan Date: Wed, 18 Dec 2019 09:48:06 +0100 Subject: [PATCH 6/6] geoip2: make geoip2("format string") optional After this patch providing a format string after opening bracket became optional, and the following are equal: ``` geoip2("format-string"); geoip2(format-string); geoip2(template("format-string")); ``` Signed-off-by: Kokan --- modules/geoip2/geoip-parser-grammar.ym | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/geoip2/geoip-parser-grammar.ym b/modules/geoip2/geoip-parser-grammar.ym index ff20fb37b8f..08bd37314fc 100644 --- a/modules/geoip2/geoip-parser-grammar.ym +++ b/modules/geoip2/geoip-parser-grammar.ym @@ -64,17 +64,23 @@ parser_expr_maxminddb { last_parser = *instance = (LogParser *) maxminddb_parser_new(configuration); } - string + optional_direct_template + parser_geoip_opts + ')' { $$ = last_parser; } + ; + +optional_direct_template + : string { LogTemplate *template; GError *error = NULL; - template = cfg_tree_check_inline_template(&configuration->tree, $4, &error); - CHECK_ERROR_GERROR(template != NULL, @4, error, "Error compiling template"); + template = cfg_tree_check_inline_template(&configuration->tree, $1, &error); + CHECK_ERROR_GERROR(template != NULL, @1, error, "Error compiling template"); log_parser_set_template(last_parser, template); + free($1); } - parser_geoip_opts - ')' { $$ = last_parser; free($4); } + | ; parser_geoip_opts