routing: extend input name resolution for safer direct route paths #11141
Conversation
- Extended struct flb_input_routes to retain the parsed plugin name, track whether an alias was provided, and cache the resolved input instance for cleanup and reuse. - Updated router configuration parsing and application to capture plugin names by default, resolve inputs by alias, internal instance name, or plugin type while avoiding reuse, and consume the enhanced resolver inside flb_router_apply_config. Signed-off-by: Eduardo Silva <[email protected]>
Refreshed router tests to populate the new metadata, added coverage proving distinct routing for duplicate plugin inputs without aliases, registered the test case, and kept conditional routing fixtures in sync. Signed-off-by: Eduardo Silva <[email protected]>
WalkthroughThe PR adds alias support to input routing in fluent-bit by introducing three new metadata fields to Changes
Sequence DiagramsequenceDiagram
actor Config
participant find_input_instance
participant resolve_logic
Config->>find_input_instance: find_input_instance(config, routes)
rect rgb(200, 220, 255)
Note over find_input_instance: Resolution Priority Order
alt routes->instance already set
find_input_instance->>resolve_logic: return existing instance
else has_alias
find_input_instance->>resolve_logic: match by alias + check_conflicts
else has input_name
find_input_instance->>resolve_logic: exact match, then prefix match
else has plugin_name
find_input_instance->>resolve_logic: match by plugin_name
else
find_input_instance->>resolve_logic: return NULL
end
end
resolve_logic-->>find_input_instance: flb_input_instance or NULL
find_input_instance-->>Config: resolved instance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
include/fluent-bit/flb_router.h(1 hunks)src/flb_router_config.c(6 hunks)tests/internal/conditional_routing.c(2 hunks)tests/internal/router_config.c(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/internal/router_config.c (3)
src/flb_sds.c (2)
flb_sds_create(78-90)flb_sds_destroy(389-399)src/flb_router_config.c (1)
flb_router_apply_config(1435-1526)src/flb_router.c (1)
flb_router_exit(274-301)
src/flb_router_config.c (4)
src/flb_sds.c (1)
flb_sds_destroy(389-399)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)lib/cfl/src/cfl_kvlist.c (1)
cfl_kvlist_fetch(418-421)lib/cfl/src/cfl_sds.c (1)
cfl_sds_len(88-91)
tests/internal/conditional_routing.c (1)
src/flb_sds.c (2)
flb_sds_create(78-90)flb_sds_destroy(389-399)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (11)
include/fluent-bit/flb_router.h (1)
153-155: LGTM! Clean struct extension for alias-aware routing.The three new fields properly extend
struct flb_input_routesto support alias-based input resolution with conflict detection. The additions are minimal and focused.tests/internal/conditional_routing.c (1)
976-976: LGTM! Proper cleanup of plugin_name.The cleanup correctly pairs with the initialization at line 886, ensuring no memory leaks.
tests/internal/router_config.c (3)
1166-1167: LGTM! Consistent initialization and cleanup across existing tests.The updates properly initialize the new
plugin_nameandhas_aliasfields and clean them up. The pattern is consistent across all modified test functions.Also applies to: 1193-1193, 1216-1217, 1242-1242
1247-1388: LGTM! Comprehensive test for alias-based input routing.This test properly validates that when
has_alias = FLB_TRUE, the router matches input instances by their alias field. The test setup is thorough and includes proper verification of route-to-output mappings.
1390-1530: LGTM! Excellent test for conflict detection with multiple instances.This test validates a crucial scenario: when multiple input instances share the same plugin type without aliases, the conflict detection mechanism (via
input_instance_already_selected) ensures each route is assigned to a distinct instance. Lines 1515-1516 properly verify the instance assignments.src/flb_router_config.c (6)
391-393: LGTM! Proper cleanup for the new plugin_name field.The cleanup correctly mirrors the pattern used for
input_nameand prevents memory leaks.
1138-1159: LGTM! Alias handling logic is well-structured.The code properly initializes the new fields and handles alias resolution:
plugin_nameis always set to the actual plugin nameinput_nameis set to alias if present and non-empty, otherwise to plugin namehas_aliasflag correctly tracks which scenario applies- Error handling at line 1157 properly calls
input_routes_destroy(input)which cleans up both allocated fieldsThe non-empty check at line 1149 (
cfl_sds_len(alias_var->data.as_string) > 0) appropriately prevents empty aliases.
1247-1271: LGTM! Well-designed conflict detection helper.The
input_instance_already_selectedfunction correctly prevents multiple routes from selecting the same input instance. The logic properly skips the current route and checks all others for conflicts.
1336-1352: LGTM! Partial matching enables plugin-type-based routing.The prefix matching logic correctly supports routing to plugin instances by their type (e.g., "dummy" matching "dummy.0"). The conflict detection ensures unique instance assignment when multiple instances share the same prefix.
1456-1456: LGTM! Call site properly updated for new signature.The function call correctly passes the entire
input_routesstructure, enabling access toplugin_name,has_alias, and cachedinstancefields.
1288-1334: Clarify alias fallback semantics with intentional early return.When
has_aliasis TRUE, if alias matching fails to find an input instance, the function falls through to try other matching strategies (exact name, plugin name). This means an input with name "foo" could be matched when searching for alias "foo", which may be unintended.The tests do not exercise the
has_alias=TRUEpath and no code comments explain this fallback behavior, making the design intent unclear.Suggested fix:
Add an early return after the alias matching block whenhas_aliasis TRUE to enforce stricter alias-based routing:if (routes->has_alias && routes->input_name) { mk_list_foreach(head, &config->inputs) { ins = mk_list_entry(head, struct flb_input_instance, _head); if (!ins->p || !ins->alias) { continue; } if (strcmp(ins->alias, routes->input_name) == 0 && input_instance_already_selected(config, routes, ins) == FLB_FALSE) { routes->instance = ins; return ins; } } + return NULL; }Verify whether the current fallback behavior is intentional or if alias-based routing should fail fast when the alias is not found.
| input_routes->plugin_name = flb_sds_create("tail"); | ||
| input_routes->has_alias = FLB_FALSE; |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Test setup properly initializes new routing fields.
The initialization correctly sets plugin_name to "tail" and explicitly disables alias usage with has_alias = FLB_FALSE, which aligns with the test scenario.
However, verify that if flb_sds_create("tail") fails (returns NULL), subsequent code handles it gracefully. The test doesn't check for NULL return from flb_sds_create.
🏁 Script executed:
#!/bin/bash
# Check if there's error handling for plugin_name allocation failures in tests
rg -n -A5 -B2 'plugin_name = flb_sds_create' tests/internal/conditional_routing.cLength of output: 339
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full setup_conditional_routes function to see error handling pattern
rg -n -B10 'input_routes->plugin_name = flb_sds_create' tests/internal/conditional_routing.c | head -50
Length of output: 672
🏁 Script executed:
#!/bin/bash
# Check the complete function definition and any error handling
rg -n -A30 'setup_conditional_routes' tests/internal/conditional_routing.c | head -80Length of output: 3586
🏁 Script executed:
#!/bin/bash
# Look for NULL checks on input_routes fields after initialization
rg -n -A15 'input_routes->has_alias = FLB_FALSE' tests/internal/conditional_routing.cLength of output: 677
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the exact function signature/definition
rg -n -B5 'setup_conditional_routes\(' tests/internal/conditional_routing.c | grep -A5 'setup_conditional_routes'
Length of output: 1144
🏁 Script executed:
#!/bin/bash
# Check if other test setup helpers validate allocations
rg -n -A10 'flb_sds_create' tests/internal/conditional_routing.c | grep -B2 -A3 'NULL'Length of output: 267
🏁 Script executed:
#!/bin/bash
# Look for the actual setup_conditional_routes function definition (not calls)
ast-grep --pattern 'void setup_conditional_routes($_) { $$$ }'Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the complete setup_conditional_routes function body
rg -n -A60 'static void setup_conditional_routes' tests/internal/conditional_routing.c | head -100
Length of output: 2982
Add NULL validation for allocation failures in setup_conditional_routes.
The function makes 7 flb_sds_create calls (lines 885–886, 893, 900, 907, 914, 921, 928) without checking for NULL returns. This is inconsistent with the validation pattern used elsewhere in the test file (line 229: TEST_CHECK(rule->field != NULL && rule->op != NULL && rule->value != NULL);). If any allocation fails, the function will pass NULL pointers to subsequent operations like cfl_list_add, causing undefined behavior. Add TEST_CHECK assertions to validate all allocations before using them.
🤖 Prompt for AI Agents
In tests/internal/conditional_routing.c around lines 885–928, multiple
flb_sds_create calls (lines 885–886, 893, 900, 907, 914, 921, 928) are used
without NULL checks; add TEST_CHECK assertions immediately after each
flb_sds_create to verify the returned pointer is not NULL before it is assigned
or used (e.g., TEST_CHECK(input_routes->plugin_name != NULL)); do this for every
allocation site so subsequent calls like cfl_list_add never receive NULL
pointers.
Fixes an issue when resolving input instance names for direct routing context
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes