network_verifier: plugin New Network Verifier plugin type #11155
Conversation
Signed-off-by: Craig Robb <[email protected]>
add flb_network_verifier.c and include openssl header directories Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
pass through the network_verifier instance to TLS context creation and onto openssl, if non NULL will retrieve the verify callback function from the plugin and assign to the SSL_CTX_verify_callback. Also adds the network_verifier instance as a X509_STORE data variable so it is accessible for the callback function to use. Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
…ate function Signed-off-by: Craig Robb <[email protected]>
…te function Signed-off-by: Craig Robb <[email protected]>
…e function Signed-off-by: Craig Robb <[email protected]>
…nction Signed-off-by: Craig Robb <[email protected]>
…function Signed-off-by: Craig Robb <[email protected]>
… function Signed-off-by: Craig Robb <[email protected]>
…te function Signed-off-by: Craig Robb <[email protected]>
…function Signed-off-by: Craig Robb <[email protected]>
…function Signed-off-by: Craig Robb <[email protected]>
…create function Signed-off-by: Craig Robb <[email protected]>
…unction Signed-off-by: Craig Robb <[email protected]>
start / shutdown Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
…tify of network errors Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
…gin_type New Network Verifier plugin type
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR adds a new "network verifier" plugin subsystem and threads verifier instances through config parsing, plugin registration, upstream creation, TLS context creation, and connection error notifications. It introduces new public types, plugin lifecycle management, YAML section support, call‑site updates, and build system wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant CFG as Config Loader
participant PLG as Network Verifier Plugin
participant INS as Input/Output Instance
participant TLS as TLS subsystem
participant CONN as Connection
CFG->>PLG: flb_network_verifier_init_all(config)
loop per verifier instance
PLG->>PLG: flb_network_verifier_new(...)
PLG->>PLG: set properties & cb_init()
end
INS->>PLG: find_network_verifier_instance(alias)
PLG-->>INS: verifier_ins
INS->>TLS: flb_tls_create(..., verifier_ins)
TLS->>PLG: cb_verify_tls(...) (during cert verification)
CONN->>CONN: connection fails / DNS fail / handshake fail
CONN->>PLG: flb_connection_notify_error(dest, port, code, msg)
PLG->>PLG: cb_connection_failure(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
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 |
Craig-Robb-GGL
changed the title
Craig-Robb-GGL
requested review from
a team,
PettitWesley,
celalettin1286,
cosmo0920,
edsiper,
fujimotos,
niedbalski and
patrick-stephens
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| mk_list_foreach(head, &config->network_verifiers) { | ||
| verifier = mk_list_entry(head, struct flb_network_verifier_instance, _head); | ||
| if (strcmp(verifier->alias, alias) == 0) { | ||
| return verifier; |
There was a problem hiding this comment.
The lookup helper iterates over all configured network verifiers and calls strcmp(verifier->alias, alias) for each entry. When a plugin instance was created without setting an alias (e.g. a misconfigured or custom verifier that does not register a config map), verifier->alias remains NULL and this call dereferences it, leading to a crash during startup whenever find_network_verifier_instance is invoked. The code should skip instances whose alias is unset instead of dereferencing a null pointer so that misconfigured verifiers result in a clear error rather than a segmentation fault.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
it is not possible to configure a verifier to have no alias, if you do, during loading configuration phase you recieve
[error] [network_verifier] NO alias property for serverhash.0 network_verifier instance. an error indicating its an invalid config
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/flb_network.c (1)
1513-1525: Preserve the real connect error before formatting the address.Line 1518 calls
flb_net_address_to_strand stores its return value back intoret, so when we notify on Line 1524 theerror_codewe hand toflb_connection_notify_erroris whateverflb_net_address_to_strreturned (typically0), not the actual connect failure. That drops the real errno for verifiers and makes downstream handling impossible. Capture the connection error before formatting the address and pass that through instead.- ret = flb_net_address_to_str(rp->ai_family, rp->ai_addr, - address, sizeof(address)); + int connection_error = ret; + ret = flb_net_address_to_str(rp->ai_family, rp->ai_addr, + address, sizeof(address)); @@ - flb_connection_notify_error(u_conn, address, port, ret, + flb_connection_notify_error(u_conn, address, port, connection_error, "Couldn't connect to end point");src/flb_output.c (1)
1380-1399: Need the verifier header to compile.
find_network_verifier_instanceis introduced on Line 1380, but this file never includes<fluent-bit/flb_network_verifier.h>, so we’ll hit an implicit declaration/const-qualifier mismatch during the build. Please include the header with the otherfluent-bitincludes.#include <fluent-bit/flb_plugin_proxy.h> #include <fluent-bit/flb_http_client_debug.h> #include <fluent-bit/flb_output_thread.h> #include <fluent-bit/flb_mp.h> #include <fluent-bit/flb_pack.h> +#include <fluent-bit/flb_network_verifier.h>
🧹 Nitpick comments (2)
src/flb_reload.c (1)
271-277: LGTM! Network verifier check properly integrated into reload flow.The network verifier property validation is correctly integrated into the reload sequence. The error handling and return flow match the existing pattern.
Note: The comment on line 271 says "tls verifier plugins" while the error message says "network verifier plugins" - consider using consistent terminology, though both are understandable.
src/config_format/flb_config_format.c (1)
200-203: Section detection supports singular and plural.Looks good. Optional: consider accepting “network-verifier(s)” too for human-friendly YAML variants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
CMakeLists.txt(1 hunks)include/fluent-bit/config_format/flb_cf.h(2 hunks)include/fluent-bit/flb_config.h(2 hunks)include/fluent-bit/flb_connection.h(1 hunks)include/fluent-bit/flb_input.h(1 hunks)include/fluent-bit/flb_network_verifier.h(1 hunks)include/fluent-bit/flb_network_verifier_plugin.h(1 hunks)include/fluent-bit/flb_output.h(1 hunks)include/fluent-bit/flb_plugin.h(2 hunks)include/fluent-bit/flb_plugins.h.in(5 hunks)include/fluent-bit/flb_stream.h(1 hunks)include/fluent-bit/flb_upstream_node.h(2 hunks)include/fluent-bit/tls/flb_tls.h(3 hunks)plugins/CMakeLists.txt(1 hunks)plugins/filter_kubernetes/kube_meta.c(3 hunks)plugins/filter_nightfall/nightfall.c(1 hunks)plugins/in_kubernetes_events/kubernetes_events_conf.c(1 hunks)plugins/out_azure_blob/azure_blob_conf.c(1 hunks)plugins/out_azure_kusto/azure_kusto_conf.c(1 hunks)plugins/out_bigquery/bigquery.c(4 hunks)plugins/out_cloudwatch_logs/cloudwatch_logs.c(3 hunks)plugins/out_es/es_conf.c(2 hunks)plugins/out_kinesis_firehose/firehose.c(3 hunks)plugins/out_kinesis_streams/kinesis.c(3 hunks)plugins/out_opensearch/os_conf.c(2 hunks)plugins/out_s3/s3.c(5 hunks)src/CMakeLists.txt(1 hunks)src/aws/flb_aws_credentials.c(2 hunks)src/config_format/flb_cf_yaml.c(8 hunks)src/config_format/flb_config_format.c(4 hunks)src/flb_config.c(6 hunks)src/flb_connection.c(2 hunks)src/flb_engine.c(2 hunks)src/flb_input.c(7 hunks)src/flb_io.c(2 hunks)src/flb_network.c(8 hunks)src/flb_network_verifier.c(1 hunks)src/flb_oauth2.c(1 hunks)src/flb_output.c(5 hunks)src/flb_plugin.c(7 hunks)src/flb_reload.c(3 hunks)src/flb_upstream_ha.c(6 hunks)src/flb_upstream_node.c(6 hunks)src/tls/flb_tls.c(3 hunks)src/tls/openssl.c(11 hunks)tests/runtime/in_tcp.c(1 hunks)tests/runtime/out_tcp.c(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
CMakeLists.txtinclude/fluent-bit/flb_plugins.h.insrc/flb_reload.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
CMakeLists.txtinclude/fluent-bit/flb_plugins.h.insrc/flb_reload.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
include/fluent-bit/flb_plugins.h.insrc/flb_reload.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
include/fluent-bit/flb_plugins.h.insrc/flb_reload.c
🧬 Code graph analysis (14)
src/flb_network.c (1)
src/flb_connection.c (1)
flb_connection_notify_error(260-274)
include/fluent-bit/flb_connection.h (1)
src/flb_connection.c (1)
flb_connection_notify_error(260-274)
src/flb_engine.c (1)
src/flb_network_verifier.c (2)
flb_network_verifier_init_all(61-101)flb_network_verifier_exit(247-263)
src/flb_input.c (3)
src/flb_utils.c (1)
flb_utils_set_plugin_string_property(2200-2224)src/flb_sds.c (1)
flb_sds_destroy(389-399)src/flb_network_verifier.c (1)
find_network_verifier_instance(289-308)
src/flb_upstream_ha.c (2)
src/config_format/flb_config_format.c (1)
flb_cf_section_property_get_string(420-460)src/flb_sds.c (1)
flb_sds_destroy(389-399)
src/flb_io.c (1)
src/flb_connection.c (1)
flb_connection_notify_error(260-274)
src/flb_plugin.c (1)
include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
src/flb_output.c (3)
src/flb_config.c (1)
prop_key_check(681-690)src/flb_utils.c (1)
flb_utils_set_plugin_string_property(2200-2224)src/flb_network_verifier.c (1)
find_network_verifier_instance(289-308)
src/config_format/flb_cf_yaml.c (1)
src/config_format/flb_config_format.c (1)
flb_cf_section_create(647-732)
src/flb_upstream_node.c (2)
src/flb_sds.c (2)
flb_sds_create(78-90)flb_sds_destroy(389-399)src/flb_network_verifier.c (1)
find_network_verifier_instance(289-308)
src/flb_reload.c (2)
src/flb_network_verifier.c (1)
flb_network_verifier_plugin_property_check(200-235)src/flb_config_map.c (1)
flb_config_map_destroy(377-409)
src/flb_config.c (1)
src/flb_network_verifier.c (2)
flb_network_verifier_new(103-150)flb_network_verifier_set_property(153-198)
include/fluent-bit/flb_network_verifier.h (1)
src/flb_network_verifier.c (8)
flb_network_verifier_new(103-150)flb_network_verifier_get_alias(40-48)flb_network_verifier_set_property(153-198)flb_network_verifier_plugin_property_check(200-235)flb_network_verifier_init_all(61-101)flb_network_verifier_exit(247-263)flb_network_verifier_instance_destroy(266-287)find_network_verifier_instance(289-308)
src/flb_network_verifier.c (7)
include/fluent-bit/flb_mem.h (2)
flb_calloc(84-96)flb_free(126-128)src/flb_env.c (1)
flb_env_var_translate(205-318)src/flb_sds.c (1)
flb_sds_destroy(389-399)src/flb_utils.c (1)
flb_utils_set_plugin_string_property(2200-2224)src/flb_log.c (1)
flb_log_get_level_str(420-442)src/flb_kv.c (2)
flb_kv_item_create(61-77)flb_kv_release(123-133)src/flb_config_map.c (3)
flb_config_map_create(252-361)flb_config_map_properties_check(448-535)flb_config_map_destroy(377-409)
🔇 Additional comments (42)
plugins/filter_kubernetes/kube_meta.c (3)
2022-2029: LGTM - TLS API signature correctly updated.The additional NULL arguments properly adapt this call to the expanded
flb_tls_createAPI that now accepts network_verifier parameters. Passing NULL is appropriate since the Kubernetes filter's pod association functionality doesn't require custom network verification.
2068-2074: LGTM - Consistent API adaptation.The signature update matches the pattern used in other call sites. NULL values for the network_verifier parameters are correct for kubelet TLS connections.
2123-2129: LGTM - All TLS creation sites properly updated.This completes the consistent adaptation of all three
flb_tls_createcalls in the file to the expanded API signature. The NULL values correctly indicate that custom network verification is not needed for the Kubernetes API connection.src/aws/flb_aws_credentials.c (2)
438-439: LGTM - Consistent with first TLS creation.The second
flb_tls_createcall correctly includes theins->verifier_insparameter, maintaining consistency with the first TLS creation.
386-387: Code changes verified and consistent across codebase.The
ins->verifier_insparameter has been properly added to bothflb_tls_createcalls in this file (lines 378-387 and 430-439). Spot checks across core infrastructure files (flb_output.c,flb_input.c,flb_oauth2.c,flb_upstream_node.c) and AWS plugins (S3, Kinesis, CloudWatch) confirm the API update is consistently applied throughout the codebase. All invocations now pass the 10th parameter to support network verifier threading.plugins/out_azure_blob/azure_blob_conf.c (1)
370-379: LGTM!The additional NULL parameter correctly adapts the TLS creation call to the extended API signature. Using NULL is appropriate for this configuration endpoint context where no custom network verifier is needed.
src/CMakeLists.txt (1)
162-162: LGTM!The addition of
flb_network_verifier.cto the TLS source set is correctly placed within theFLB_TLSconditional block and follows the existing build structure.plugins/filter_nightfall/nightfall.c (1)
94-100: LGTM!The TLS creation call has been correctly updated with the additional NULL parameters to match the extended API signature. Using NULL is appropriate for this filter plugin context.
plugins/out_kinesis_streams/kinesis.c (3)
163-172: LGTM!The cred_tls initialization correctly propagates
ins->verifier_insas the final parameter, aligning with the extended TLS API.
179-188: LGTM!The client_tls initialization correctly propagates
ins->verifier_ins, maintaining consistency with the cred_tls initialization.
223-232: LGTM!The sts_tls initialization correctly propagates
ins->verifier_ins, completing the consistent pattern across all TLS contexts in this plugin.CMakeLists.txt (1)
744-747: LGTM!The addition of OpenSSL include directory is properly placed within the
OPENSSL_FOUNDconditional block and ensures headers are available for the network verifier subsystem.plugins/out_opensearch/os_conf.c (2)
252-261: LGTM!The aws_tls initialization correctly propagates
ins->verifier_insas the final parameter, consistent with the AWS TLS integration pattern.
314-323: LGTM!The aws_sts_tls initialization correctly propagates
ins->verifier_ins, maintaining consistency with the aws_tls initialization.plugins/out_bigquery/bigquery.c (4)
690-699: LGTM!The aws_tls initialization correctly propagates
ins->verifier_insas the final parameter for the identity federation flow.
730-739: LGTM!The aws_sts_tls initialization correctly propagates
ins->verifier_ins, consistent with the aws_tls initialization pattern.
762-771: LGTM!The google_sts_tls initialization correctly propagates
ins->verifier_ins, maintaining consistency across the identity federation TLS contexts.
791-800: LGTM!The google_iam_tls initialization correctly propagates
ins->verifier_ins, completing the consistent pattern across all identity federation TLS contexts.include/fluent-bit/flb_stream.h (1)
52-52: Review comment is incorrect.The
verifier_insfield is properly initialized before use. While it's not set inflb_stream_setup(), this is intentional design: the generic setup function handles standard stream initialization, while instance-specific fields likeverifier_insare set by the caller afterward (e.g., insrc/flb_input.c:2263: stream->base.verifier_ins = ins->verifier_ins). The field is only accessed after this assignment in connection code, so there is no uninitialized field access.Likely an incorrect or invalid review comment.
tests/runtime/in_tcp.c (1)
376-376: LGTM! TLS API signature correctly updated.The additional NULL parameter aligns with the TLS API extension to support network verifier instances. Passing NULL is appropriate for this test scenario.
plugins/out_kinesis_firehose/firehose.c (1)
184-185: LGTM! Verifier instance correctly propagated to TLS contexts.All three TLS context creations (cred_tls, client_tls, sts_tls) consistently pass
ins->verifier_insas the new parameter. The implementation properly threads the network verifier through the TLS layer.Also applies to: 200-201, 237-238
src/flb_reload.c (1)
29-29: LGTM! Network verifier property validation follows established patterns.The new
flb_network_verifier_property_check_allfunction correctly mirrors the implementation pattern used for other plugin types (input, output, filter, custom). The property validation and config map cleanup flow is consistent with the existing reload infrastructure.Also applies to: 209-233
src/flb_io.c (2)
147-149: LGTM! Error notification appropriately added for proxy connection failures.The
flb_connection_notify_errorcall is correctly placed to notify the network verifier of proxy connection failures before cleanup. The error message is descriptive and the parameters are appropriate.
170-172: LGTM! Error notification appropriately added for keepalive failures.The error notification for TCP keepalive failures is correctly integrated. The call provides the necessary context to the network verifier callback when keepalive setup fails.
include/fluent-bit/flb_connection.h (1)
184-185: LGTM! Clean API for connection error notification.The function signature is well-designed for notifying network verifiers of connection failures. The const correctness on the connection parameter is appropriate, and the parameters provide sufficient context for error handling.
include/fluent-bit/flb_config.h (1)
139-139: LGTM! Configuration structure properly extended for network verifiers.The addition of
network_verifier_pluginsandnetwork_verifierslists follows the established pattern for plugin management in Fluent Bit. The placement is logical and consistent with other plugin types.Also applies to: 160-161
src/flb_engine.c (2)
927-931: LGTM! Network verifier initialization properly integrated into engine startup.The initialization of network verifiers is correctly placed in the engine startup sequence and includes appropriate error handling to abort startup on failure. The placement after custom plugins and before storage creation is reasonable.
1326-1326: LGTM! Network verifier cleanup properly integrated into engine shutdown.The network verifier exit call is appropriately placed in the shutdown sequence, ensuring proper cleanup of verifier resources before input plugin cleanup.
src/flb_connection.c (1)
260-274: LGTM! Robust error notification implementation with proper NULL safety.The implementation of
flb_connection_notify_errorcorrectly traverses the connection structure with appropriate NULL checks at each level (connection → stream → verifier_ins → plugin → callback). The function safely handles all edge cases where the verifier chain might not be fully configured.src/config_format/flb_config_format.c (3)
137-139: List initialization for network verifiers is correctly added.No issues; matches existing init pattern for other section lists.
715-717: Section linking to cf->network_verifiers is correct.Consistent with other section lists.
817-819: String mapping updated for NETWORK_VERIFIER.Matches enum; dump output will be coherent.
include/fluent-bit/config_format/flb_cf.h (2)
63-64: Enum value FLB_CF_NETWORK_VERIFIER added in correct spot.Switches in config_format were updated accordingly.
116-118: New list member network_verifiers added.Initialization and usage present in flb_config_format.c.
plugins/out_cloudwatch_logs/cloudwatch_logs.c (1)
255-256: CloudWatch TLS contexts thread verifier_ins correctly.No behavior changes beyond enabling optional verifier callback.
Also applies to: 271-272, 308-309
plugins/out_s3/s3.c (1)
869-870: All S3 TLS contexts correctly receive verifier_ins.Verified across all four flb_tls_create callsites:
- Lines 860, 877, 912: Client, provider, and STS TLS instances pass
ins->verifier_ins- Line 1984: Blob auth TLS passes
NULL(verify=FLB_FALSE)tests/runtime/out_tcp.c (1)
334-334: All test call sites properly updated for new flb_tls_create signature.Both identified test calls conform to the new 10-parameter API:
tests/runtime/in_tcp.c:368– 10 arguments with trailing NULLtests/runtime/out_tcp.c:325– 10 arguments with trailing NULLThe trailing NULL correctly represents the new
conn_insverifier pointer parameter.plugins/out_es/es_conf.c (1)
386-387: Pass-through of verifier_ins to TLS contexts is correctly implemented across the codebase.All 35 flb_tls_create call sites have been updated to pass the 10th parameter (verifier_ins). AWS-authenticated plugins pass
ins->verifier_insfrom the plugin instance; test and non-credential contexts passNULL. The es_conf.c changes at lines 377/386-387 and 439/448-449 are consistent with this pattern and correct.plugins/out_azure_kusto/azure_kusto_conf.c (1)
88-93: TLS arg alignment looks good.The additional
NULLcleanly fills the newnetwork_verifierslot inflb_upstream_node_create, so this caller stays in sync with the updated signature.include/fluent-bit/flb_plugins.h.in (1)
28-114: Network verifier wiring looks consistent.Including the verifier header and mirroring the add/unregister flow keeps the plugin registry coherent. Looks good.
src/flb_upstream_ha.c (1)
156-358: Property plumbing checks out.Adding
network_verifierto the known keys and translating it alongside the other TLS fields keeps the node factory consistent.plugins/CMakeLists.txt (1)
41-75: Macro mirrors existing patterns.The new registration macro follows the established add/copy/register pattern, so external verifier plugins can hook in cleanly.
|
Looks like we need the openssl development headers added to a few of the builds it seems |
…er_instance reference Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
Signed-off-by: Craig Robb <[email protected]>
…-updates updates to network verifier after upstream PR comments
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/flb_reload.c (1)
271-277: Minor inconsistency in error message grammar.The error message uses "has failed" while all other error messages in this function use "is failed". For consistency, consider this adjustment:
- flb_error("[reload] check properties for network verifier plugins has failed"); + flb_error("[reload] check properties for network verifier plugins is failed");src/flb_network.c (2)
1385-1391: Consider a more descriptive error message.The generic "Connection error" message could be more informative. Since this path detects timeout scenarios (as indicated by the warning log), consider providing more context:
flb_connection_notify_error(u_conn, host, port, u_conn->net_error, - "Connection error"); + "Connection timeout after DNS resolution");
1523-1524: Consolidate inconsistent error messages.Three nearly identical but slightly different messages are used for connection failures:
- "Couldn't connect to end point" (per-attempt, line 1524)
- "Couldn't connect to end point file descriptor" (fd == -1, line 1544)
- "Couldn't connect to remote endpoint" (rp == NULL, line 1556)
These are confusing due to minor wording variations ("end point" vs "endpoint") and unclear distinctions. Consider standardizing to a single message or making them more meaningfully different:
flb_connection_notify_error(u_conn, address, port, -1, - "Couldn't connect to end point"); + "Connection attempt failed"); if (fd == -1) { flb_debug("[net] could not connect to %s:%s", host, _port); flb_connection_notify_error(u_conn, host, port, -1, - "Couldn't connect to end point file descriptor"); + "All connection attempts failed"); } if (rp == NULL) { flb_connection_notify_error(u_conn, host, port, -1, - "Couldn't connect to remote endpoint"); + "No valid address for connection"); return -1; }Also applies to: 1540-1545, 1554-1557
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
include/fluent-bit/flb_output.h(2 hunks)src/flb_engine.c(2 hunks)src/flb_network.c(8 hunks)src/flb_network_verifier.c(1 hunks)src/flb_reload.c(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- include/fluent-bit/flb_output.h
- src/flb_engine.c
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
src/flb_reload.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
src/flb_reload.c
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.
Applied to files:
src/flb_reload.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
src/flb_reload.c
🧬 Code graph analysis (3)
src/flb_network.c (1)
src/flb_connection.c (1)
flb_connection_notify_error(260-274)
src/flb_reload.c (2)
src/flb_network_verifier.c (1)
flb_network_verifier_plugin_property_check(200-235)src/flb_config_map.c (1)
flb_config_map_destroy(377-409)
src/flb_network_verifier.c (7)
include/fluent-bit/flb_mem.h (2)
flb_calloc(84-96)flb_free(126-128)src/flb_env.c (1)
flb_env_var_translate(205-318)src/flb_sds.c (1)
flb_sds_destroy(389-399)src/flb_utils.c (1)
flb_utils_set_plugin_string_property(2200-2224)src/flb_log.c (1)
flb_log_get_level_str(420-442)src/flb_kv.c (2)
flb_kv_item_create(61-77)flb_kv_release(123-133)src/flb_config_map.c (3)
flb_config_map_create(252-361)flb_config_map_properties_check(448-535)flb_config_map_destroy(377-409)
🔇 Additional comments (17)
src/flb_reload.c (2)
29-29: LGTM!The include is correctly placed and necessary for the network verifier property checking integration.
209-233: LGTM!The function correctly follows the established pattern for plugin property validation during reload. It iterates safely over network verifier instances, validates properties, and cleans up config maps that will be recreated at startup.
src/flb_network.c (5)
1333-1333: LGTM!The error variable is properly declared and initialized for capturing DNS resolution error messages.
1370-1380: LGTM!DNS resolution failure handling correctly differentiates between async (c-ares) and sync (getaddrinfo) paths, capturing both error codes and textual descriptions for downstream notification.
1405-1421: LGTM!IPv4 sorting failure path correctly notifies with a descriptive message when address list sorting fails.
1423-1440: LGTM!IPv6 sorting failure path correctly notifies with a descriptive message when address list sorting fails.
1513-1525: Verify per-attempt notification behavior.This notification occurs inside the address-attempt loop (line 1446), which means a single connection request can trigger multiple notifications if several addresses are tried. Please confirm this is the intended behavior—verifier plugins will receive a callback for each failed address, not just the final outcome.
src/flb_network_verifier.c (10)
26-38: LGTM!The ID generation correctly handles both empty lists and sequential ID assignment.
40-48: LGTM!The alias accessor provides appropriate fallback to the instance name when no alias is set.
60-101: LGTM!The initialization correctly validates properties before invoking plugin callbacks and properly cleans up on failure.
103-150: LGTM!Instance creation correctly handles plugin lookup, memory allocation, initialization, and list management.
152-198: LGTM!Property handling correctly differentiates special properties from generic ones, with proper environment variable translation and memory management.
200-235: LGTM!Property validation correctly enforces the alias requirement and validates against the plugin's config map.
237-244: LGTM!Instance exit correctly invokes the plugin callback when available.
246-263: LGTM!Global exit correctly handles all instances with safe iteration and proper cleanup.
266-287: LGTM!Instance destruction properly releases all resources in the correct order.
289-308: LGTM!The lookup function correctly guards against NULL aliases (line 302) before calling
strcmp, addressing the concern raised in previous reviews. The implementation safely handles misconfigured verifiers that may lack an alias.Based on learnings
2 participants
Added the ability to allow for more custom TLS verification by creating a new network_verifier plugin, with the ability to define the OpenSSL callback for the SSL_CTX_set_verify function. If a plugin is configured to use TLS, and has an assigned network_verifier, during the creation of the TLS connection the assigned network_verifier callback function will be set to the OpenSSL's SSL_CTX.
During the TLS connection handshake, OpenSSL will call on the callback function, allowing Fluent Bit to have more custom control for verification with access to this callback. The network_verifier callback will only be called if tls.verify is on
This allows for external network_verifier plugins to be loaded in the plugins section of a yaml conf following the existing format for external plugins, for a network_verifier it it expected to be in the file name format flb-network_verifier_.so
Added the ability to inform a Network Verifier plugin of connection issues when attempting to connect to an Endpoint. This will allow custom external handling of these error conditions.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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
New Features
Improvements