From 53817cd326e54bdc7ab6034afa7f8f6fb80a16b2 Mon Sep 17 00:00:00 2001 From: Takahiro Yamashita Date: Sun, 19 Jun 2022 13:51:58 +0900 Subject: [PATCH 1/6] tests: internal: mp: add test case for unsorted keys(#5546) Signed-off-by: Takahiro Yamashita --- tests/internal/mp.c | 200 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) diff --git a/tests/internal/mp.c b/tests/internal/mp.c index e41a7a92a75..3d04d0fa663 100644 --- a/tests/internal/mp.c +++ b/tests/internal/mp.c @@ -145,9 +145,209 @@ void test_accessor_keys_remove() msgpack_unpacked_destroy(&result); } +/* https://github.com/fluent/fluent-bit/issues/5546 */ +void test_keys_remove_subkey_key() +{ + int len; + int ret; + int type; + size_t off = 0; + char *buf; + size_t size; + char *out_buf; + size_t out_size; + char *json; + char final_json[2048] = {0}; + msgpack_unpacked result; + msgpack_unpacked result_final; + msgpack_object map; + struct flb_mp_accessor *mpa; + struct mk_list patterns; + + /* Sample JSON message */ + json = + "{\"key1\": \"something\", " + "\"kubernetes\": " + " [true, " + " false, " + " {\"a\": false, " + " \"annotations\": { " + " \"fluentbit.io/tag\": \"thetag\"," + " \"extra\": false\"" + "}}]}"; + + /* Convert to msgpack */ + len = strlen(json); + ret = flb_pack_json(json, len, &buf, &size, &type); + TEST_CHECK(ret == 0); + if (ret == -1) { + exit(EXIT_FAILURE); + } + + /* Unpack the content */ + msgpack_unpacked_init(&result); + msgpack_unpack_next(&result, buf, size, &off); + map = result.data; + + /* Create list of patterns */ + flb_slist_create(&patterns); + + /* sub key -> key */ + flb_slist_add(&patterns, "$kubernetes[2]['annotations']['fluentbit.io/tag']"); + flb_slist_add(&patterns, "$kubernetes"); + + + /* Create mp accessor */ + mpa = flb_mp_accessor_create(&patterns); + TEST_CHECK(mpa != NULL); + + /* Remove the entry that matches the pattern(s) */ + ret = flb_mp_accessor_keys_remove(mpa, &map, (void *) &out_buf, &out_size); + TEST_CHECK(ret == FLB_TRUE); + + printf("\n=== ORIGINAL ===\n"); + flb_pack_print(buf, size); + flb_free(buf); + + printf("=== FINAL MAP ===\n"); + if (ret == FLB_TRUE) { + flb_pack_print(out_buf, out_size); + } + msgpack_unpacked_destroy(&result); + + off = 0; + msgpack_unpacked_init(&result_final); + msgpack_unpack_next(&result_final, out_buf, out_size, &off); + flb_msgpack_to_json(&final_json[0], sizeof(final_json), &result_final.data); + + if (!TEST_CHECK(strstr(&final_json[0] ,"kubernetes") == NULL)) { + TEST_MSG("kubernetes field should be removed"); + } + + msgpack_unpacked_destroy(&result_final); + + flb_free(out_buf); + flb_mp_accessor_destroy(mpa); + flb_slist_destroy(&patterns); + +} + +void remove_subkey_keys(char *list[], int list_size, int index_start) +{ + int len; + int ret; + int type; + size_t off = 0; + char *buf; + size_t size; + char *out_buf; + size_t out_size; + char *json; + char final_json[2048] = {0}; + msgpack_unpacked result; + msgpack_unpacked result_final; + msgpack_object map; + struct flb_mp_accessor *mpa; + struct mk_list patterns; + int i; + int count = 0; + + /* Sample JSON message */ + json = + "{\"key1\": \"something\", " + "\"kubernetes\": " + " [true, " + " false, " + " {\"a\": false, " + " \"annotations\": { " + " \"fluentbit.io/tag\": \"thetag\"," + " \"extra\": false\"" + "}}]}"; + + /* Convert to msgpack */ + len = strlen(json); + ret = flb_pack_json(json, len, &buf, &size, &type); + TEST_CHECK(ret == 0); + if (ret == -1) { + exit(EXIT_FAILURE); + } + + /* Unpack the content */ + msgpack_unpacked_init(&result); + msgpack_unpack_next(&result, buf, size, &off); + map = result.data; + + /* Create list of patterns */ + flb_slist_create(&patterns); + + /* sub keys */ + for (i=index_start; count=list_size) { + i = 0; + } + flb_slist_add(&patterns, list[i]); + count++; + } + + /* Create mp accessor */ + mpa = flb_mp_accessor_create(&patterns); + TEST_CHECK(mpa != NULL); + + /* Remove the entry that matches the pattern(s) */ + ret = flb_mp_accessor_keys_remove(mpa, &map, (void *) &out_buf, &out_size); + TEST_CHECK(ret == FLB_TRUE); + + printf("\n=== ORIGINAL ===\n"); + flb_pack_print(buf, size); + flb_free(buf); + + printf("=== FINAL MAP ===\n"); + if (ret == FLB_TRUE) { + flb_pack_print(out_buf, out_size); + } + msgpack_unpacked_destroy(&result); + + off = 0; + msgpack_unpacked_init(&result_final); + msgpack_unpack_next(&result_final, out_buf, out_size, &off); + flb_msgpack_to_json(&final_json[0], sizeof(final_json), &result_final.data); + + if (!TEST_CHECK(strstr(&final_json[0] ,"kubernetes") == NULL)) { + TEST_MSG("kubernetes field should be removed"); + } + + msgpack_unpacked_destroy(&result_final); + + flb_free(out_buf); + flb_mp_accessor_destroy(mpa); + flb_slist_destroy(&patterns); +} + +void test_keys_remove_subkey_keys() +{ + char *list[] = {"$kubernetes[2]['annotations']['fluentbit.io/tag']", + "$kubernetes[2]['a']", + "$kubernetes"}; + char *list2[] = {"$kubernetes[2]['annotations']['fluentbit.io/tag']", + "$kubernetes", + "$kubernetes[2]['a']"}; + + int size = sizeof(list)/sizeof(char*); + int i; + + for (i=0; i Date: Sun, 19 Jun 2022 15:47:29 +0900 Subject: [PATCH 2/6] ra: add API to get number of subkeys Signed-off-by: Takahiro Yamashita --- include/fluent-bit/flb_record_accessor.h | 1 + .../record_accessor/flb_ra_parser.h | 1 + src/flb_record_accessor.c | 21 +++++++++++++++++++ src/record_accessor/flb_ra_parser.c | 15 +++++++++++++ 4 files changed, 38 insertions(+) diff --git a/include/fluent-bit/flb_record_accessor.h b/include/fluent-bit/flb_record_accessor.h index e35923e52c7..2b82583173c 100644 --- a/include/fluent-bit/flb_record_accessor.h +++ b/include/fluent-bit/flb_record_accessor.h @@ -34,6 +34,7 @@ struct flb_record_accessor { struct mk_list _head; /* Head to custom list (only used by flb_mp.h) */ }; +int flb_ra_subkey_num(struct flb_record_accessor *ra); struct flb_record_accessor *flb_ra_create(char *str, int translate_env); void flb_ra_destroy(struct flb_record_accessor *ra); void flb_ra_dump(struct flb_record_accessor *ra); diff --git a/include/fluent-bit/record_accessor/flb_ra_parser.h b/include/fluent-bit/record_accessor/flb_ra_parser.h index d54c4ef4501..f111a409dd8 100644 --- a/include/fluent-bit/record_accessor/flb_ra_parser.h +++ b/include/fluent-bit/record_accessor/flb_ra_parser.h @@ -64,6 +64,7 @@ struct flb_ra_key *flb_ra_parser_key_add(struct flb_ra_parser *ra, char *key); int flb_ra_parser_subentry_add_string(struct flb_ra_parser *rp, char *key); int flb_ra_parser_subentry_add_array_id(struct flb_ra_parser *rp, int id); +int flb_ra_parser_subkey_num(struct flb_ra_parser *rp); void flb_ra_parser_dump(struct flb_ra_parser *rp); struct flb_ra_parser *flb_ra_parser_string_create(char *str, int len); struct flb_ra_parser *flb_ra_parser_regex_id_create(int id); diff --git a/src/flb_record_accessor.c b/src/flb_record_accessor.c index dcf9e2c6906..3402dcdef54 100644 --- a/src/flb_record_accessor.c +++ b/src/flb_record_accessor.c @@ -247,6 +247,27 @@ void flb_ra_destroy(struct flb_record_accessor *ra) flb_free(ra); } +int flb_ra_subkey_num(struct flb_record_accessor *ra) +{ + struct mk_list *head; + struct flb_ra_parser *rp; + int ret = -1; + int tmp; + + if (ra == NULL) { + return -1; + } + mk_list_foreach(head, &ra->list) { + rp = mk_list_entry(head, struct flb_ra_parser, _head); + tmp = flb_ra_parser_subkey_num(rp); + if (tmp > ret) { + ret = tmp; + } + } + + return ret; +} + struct flb_record_accessor *flb_ra_create(char *str, int translate_env) { int ret; diff --git a/src/record_accessor/flb_ra_parser.c b/src/record_accessor/flb_ra_parser.c index 947b89e705b..3cb1967f2cf 100644 --- a/src/record_accessor/flb_ra_parser.c +++ b/src/record_accessor/flb_ra_parser.c @@ -27,6 +27,21 @@ #include "ra_parser.h" #include "ra_lex.h" +int flb_ra_parser_subkey_num(struct flb_ra_parser *rp) +{ + if (rp == NULL || rp->key == NULL) { + return -1; + } + else if (rp->type != FLB_RA_PARSER_KEYMAP) { + return 0; + } + else if(rp->key->subkeys == NULL) { + return -1; + } + + return mk_list_size(rp->key->subkeys); +} + void flb_ra_parser_dump(struct flb_ra_parser *rp) { struct mk_list *head; From 8a4ded6f6672afb4d64a30b60a74d3b0da503adc Mon Sep 17 00:00:00 2001 From: Takahiro Yamashita Date: Sun, 19 Jun 2022 21:05:31 +0900 Subject: [PATCH 3/6] mp: sort flb_record_accessor by a number of subkeys flb_mp_accessor_keys_remove expects incoming lists are sorted. Since it seeks shallow keys first. If it seeks nested key at first, it may fail to seek shallow key. Signed-off-by: Takahiro Yamashita --- src/flb_mp.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/src/flb_mp.c b/src/flb_mp.c index e7e0ec6fa47..6bdaf6e5a77 100644 --- a/src/flb_mp.c +++ b/src/flb_mp.c @@ -350,6 +350,61 @@ void flb_mp_array_header_end(struct flb_mp_map_header *mh) flb_mp_set_array_header_size(ptr, mh->entries); } +/* TODO: move to mk_list.h ? */ +static inline void list_add_before(struct mk_list *_new, + struct mk_list *next, + struct mk_list *head) +{ + struct mk_list *prev; + + if (_new == NULL || next == NULL || head == NULL) { + return; + } + + if ((head->prev == head && head->next == head) /*empty*/|| + next == head) { + mk_list_add(_new, head); + return; + } + + prev = next->prev; + _new->next = next; + _new->prev = prev; + prev->next = _new; + next->prev = _new; +} + +static int insert_by_subkey_num(struct flb_record_accessor *ra, struct flb_mp_accessor *mpa) +{ + int subkey_num; + int num; + struct mk_list *h; + struct flb_record_accessor *val_ra; + + /* + * sort flb_record_accessor by number of subkey + * + * e.g. + * $kubernetes + * $kubernetes[2]['a'] + * $kubernetes[2]['annotations']['fluentbit.io/tag'] + */ + subkey_num = flb_ra_subkey_num(ra); + mk_list_foreach(h, &mpa->ra_list) { + val_ra = mk_list_entry(h, struct flb_record_accessor, _head); + num = flb_ra_subkey_num(val_ra); + if (num >= subkey_num) { + list_add_before(&ra->_head, &val_ra->_head, &mpa->ra_list); + return 0; + } + } + + /* add to tail of list */ + mk_list_add(&ra->_head, &mpa->ra_list); + return 0; +} + + /* * Create an 'mp accessor' context: this context allows to create a list of * record accessor patterns based on a 'slist' context, where every slist string @@ -382,7 +437,13 @@ struct flb_mp_accessor *flb_mp_accessor_create(struct mk_list *slist_patterns) flb_mp_accessor_destroy(mpa); return NULL; } - mk_list_add(&ra->_head, &mpa->ra_list); + + /* first time */ + if (mk_list_size(&mpa->ra_list) == 0) { + mk_list_add(&ra->_head, &mpa->ra_list); + continue; + } + insert_by_subkey_num(ra, mpa); } if (mk_list_size(&mpa->ra_list) == 0) { From 3696447326358f5374f8ab6f1a45d921d5ec7a6d Mon Sep 17 00:00:00 2001 From: Takahiro Yamashita Date: Wed, 5 Oct 2022 20:36:31 +0900 Subject: [PATCH 4/6] mp: use mk_list_add_before Signed-off-by: Takahiro Yamashita --- src/flb_mp.c | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/src/flb_mp.c b/src/flb_mp.c index 6bdaf6e5a77..8f11cef6171 100644 --- a/src/flb_mp.c +++ b/src/flb_mp.c @@ -350,30 +350,6 @@ void flb_mp_array_header_end(struct flb_mp_map_header *mh) flb_mp_set_array_header_size(ptr, mh->entries); } -/* TODO: move to mk_list.h ? */ -static inline void list_add_before(struct mk_list *_new, - struct mk_list *next, - struct mk_list *head) -{ - struct mk_list *prev; - - if (_new == NULL || next == NULL || head == NULL) { - return; - } - - if ((head->prev == head && head->next == head) /*empty*/|| - next == head) { - mk_list_add(_new, head); - return; - } - - prev = next->prev; - _new->next = next; - _new->prev = prev; - prev->next = _new; - next->prev = _new; -} - static int insert_by_subkey_num(struct flb_record_accessor *ra, struct flb_mp_accessor *mpa) { int subkey_num; @@ -394,7 +370,7 @@ static int insert_by_subkey_num(struct flb_record_accessor *ra, struct flb_mp_ac val_ra = mk_list_entry(h, struct flb_record_accessor, _head); num = flb_ra_subkey_num(val_ra); if (num >= subkey_num) { - list_add_before(&ra->_head, &val_ra->_head, &mpa->ra_list); + mk_list_add_before(&ra->_head, &val_ra->_head, &mpa->ra_list); return 0; } } From c9ece8302b5c3a72f3c1743a820a7b0470651292 Mon Sep 17 00:00:00 2001 From: Takahiro Yamashita Date: Sat, 11 Feb 2023 08:52:17 +0900 Subject: [PATCH 5/6] record_accessor: replace 'num' for 'count' Signed-off-by: Takahiro Yamashita --- include/fluent-bit/flb_record_accessor.h | 2 +- include/fluent-bit/record_accessor/flb_ra_parser.h | 2 +- src/flb_mp.c | 14 +++++++------- src/flb_record_accessor.c | 4 ++-- src/record_accessor/flb_ra_parser.c | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/fluent-bit/flb_record_accessor.h b/include/fluent-bit/flb_record_accessor.h index 2b82583173c..8dbcce3ea88 100644 --- a/include/fluent-bit/flb_record_accessor.h +++ b/include/fluent-bit/flb_record_accessor.h @@ -34,7 +34,7 @@ struct flb_record_accessor { struct mk_list _head; /* Head to custom list (only used by flb_mp.h) */ }; -int flb_ra_subkey_num(struct flb_record_accessor *ra); +int flb_ra_subkey_count(struct flb_record_accessor *ra); struct flb_record_accessor *flb_ra_create(char *str, int translate_env); void flb_ra_destroy(struct flb_record_accessor *ra); void flb_ra_dump(struct flb_record_accessor *ra); diff --git a/include/fluent-bit/record_accessor/flb_ra_parser.h b/include/fluent-bit/record_accessor/flb_ra_parser.h index f111a409dd8..e646c33a1e7 100644 --- a/include/fluent-bit/record_accessor/flb_ra_parser.h +++ b/include/fluent-bit/record_accessor/flb_ra_parser.h @@ -64,7 +64,7 @@ struct flb_ra_key *flb_ra_parser_key_add(struct flb_ra_parser *ra, char *key); int flb_ra_parser_subentry_add_string(struct flb_ra_parser *rp, char *key); int flb_ra_parser_subentry_add_array_id(struct flb_ra_parser *rp, int id); -int flb_ra_parser_subkey_num(struct flb_ra_parser *rp); +int flb_ra_parser_subkey_count(struct flb_ra_parser *rp); void flb_ra_parser_dump(struct flb_ra_parser *rp); struct flb_ra_parser *flb_ra_parser_string_create(char *str, int len); struct flb_ra_parser *flb_ra_parser_regex_id_create(int id); diff --git a/src/flb_mp.c b/src/flb_mp.c index 8f11cef6171..74260227e5d 100644 --- a/src/flb_mp.c +++ b/src/flb_mp.c @@ -350,10 +350,10 @@ void flb_mp_array_header_end(struct flb_mp_map_header *mh) flb_mp_set_array_header_size(ptr, mh->entries); } -static int insert_by_subkey_num(struct flb_record_accessor *ra, struct flb_mp_accessor *mpa) +static int insert_by_subkey_count(struct flb_record_accessor *ra, struct flb_mp_accessor *mpa) { - int subkey_num; - int num; + int subkey_count; + int count; struct mk_list *h; struct flb_record_accessor *val_ra; @@ -365,11 +365,11 @@ static int insert_by_subkey_num(struct flb_record_accessor *ra, struct flb_mp_ac * $kubernetes[2]['a'] * $kubernetes[2]['annotations']['fluentbit.io/tag'] */ - subkey_num = flb_ra_subkey_num(ra); + subkey_count = flb_ra_subkey_count(ra); mk_list_foreach(h, &mpa->ra_list) { val_ra = mk_list_entry(h, struct flb_record_accessor, _head); - num = flb_ra_subkey_num(val_ra); - if (num >= subkey_num) { + count = flb_ra_subkey_count(val_ra); + if (count >= subkey_count) { mk_list_add_before(&ra->_head, &val_ra->_head, &mpa->ra_list); return 0; } @@ -419,7 +419,7 @@ struct flb_mp_accessor *flb_mp_accessor_create(struct mk_list *slist_patterns) mk_list_add(&ra->_head, &mpa->ra_list); continue; } - insert_by_subkey_num(ra, mpa); + insert_by_subkey_count(ra, mpa); } if (mk_list_size(&mpa->ra_list) == 0) { diff --git a/src/flb_record_accessor.c b/src/flb_record_accessor.c index 3402dcdef54..bd245c5b36c 100644 --- a/src/flb_record_accessor.c +++ b/src/flb_record_accessor.c @@ -247,7 +247,7 @@ void flb_ra_destroy(struct flb_record_accessor *ra) flb_free(ra); } -int flb_ra_subkey_num(struct flb_record_accessor *ra) +int flb_ra_subkey_count(struct flb_record_accessor *ra) { struct mk_list *head; struct flb_ra_parser *rp; @@ -259,7 +259,7 @@ int flb_ra_subkey_num(struct flb_record_accessor *ra) } mk_list_foreach(head, &ra->list) { rp = mk_list_entry(head, struct flb_ra_parser, _head); - tmp = flb_ra_parser_subkey_num(rp); + tmp = flb_ra_parser_subkey_count(rp); if (tmp > ret) { ret = tmp; } diff --git a/src/record_accessor/flb_ra_parser.c b/src/record_accessor/flb_ra_parser.c index 3cb1967f2cf..9f714295005 100644 --- a/src/record_accessor/flb_ra_parser.c +++ b/src/record_accessor/flb_ra_parser.c @@ -27,7 +27,7 @@ #include "ra_parser.h" #include "ra_lex.h" -int flb_ra_parser_subkey_num(struct flb_ra_parser *rp) +int flb_ra_parser_subkey_count(struct flb_ra_parser *rp) { if (rp == NULL || rp->key == NULL) { return -1; From b98c0633054cb8381de007b4acdcbcfd0c4e3e28 Mon Sep 17 00:00:00 2001 From: Takahiro Yamashita Date: Sat, 11 Feb 2023 09:06:12 +0900 Subject: [PATCH 6/6] mp: remove redundant initializing Review comment: the mk_list_foreach in insert_by_subkey_num would not satisfy the condition in the for loop when the list is empty so it would skip straight to the mk_list_add fallback Signed-off-by: Takahiro Yamashita --- src/flb_mp.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/flb_mp.c b/src/flb_mp.c index 74260227e5d..c1425e7e348 100644 --- a/src/flb_mp.c +++ b/src/flb_mp.c @@ -413,12 +413,6 @@ struct flb_mp_accessor *flb_mp_accessor_create(struct mk_list *slist_patterns) flb_mp_accessor_destroy(mpa); return NULL; } - - /* first time */ - if (mk_list_size(&mpa->ra_list) == 0) { - mk_list_add(&ra->_head, &mpa->ra_list); - continue; - } insert_by_subkey_count(ra, mpa); }