in_forward: Fix user authentication to align Fluentd's behavior - #10973
Merged
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
WalkthroughAdds ownership semantics for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Config Loader
participant FW as in_forward:init
participant Cfg as fw_config
participant Log as Logger
Admin->>FW: initialize()
FW->>Cfg: setup_users()
FW->>FW: validate auth preconditions
alt users configured AND no shared_key AND not empty_shared_key
FW->>Log: error("users require shared_key or empty_shared_key")
FW->>Cfg: fw_config_destroy()
FW->>FW: delete_users(ctx)
FW-->>Admin: return -1
else OK
FW-->>Admin: continue init
end
sequenceDiagram
autonumber
participant Conn as fw_conn_add
participant Cfg as fw_config
participant HS as Handshake State
Conn->>Cfg: read shared_key, empty_shared_key, users
Conn->>HS: state = ESTABLISHED (default)
alt shared_key present OR empty_shared_key enabled OR users configured
Conn->>HS: state = HELO (init secure-forward handshake)
else
Note over Conn,HS: remain established without handshake
end
Conn-->>HS: proceed with downstream flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (30)
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 (5)
plugins/in_forward/fw.c(1 hunks)plugins/in_forward/fw.h(2 hunks)plugins/in_forward/fw_config.c(2 hunks)plugins/in_forward/fw_conn.c(1 hunks)tests/runtime/in_forward.c(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
plugins/in_forward/fw.c (1)
plugins/in_forward/fw_config.c (1)
fw_config_destroy(110-143)
plugins/in_forward/fw_config.c (1)
src/flb_sds.c (2)
flb_sds_create(78-90)flb_sds_destroy(389-399)
tests/runtime/in_forward.c (1)
src/flb_lib.c (9)
flb_create(138-220)flb_service_set(647-673)flb_input(261-271)flb_destroy(223-258)flb_output(274-284)flb_output_set(515-546)flb_input_set(300-330)flb_start(914-925)flb_stop(942-985)
⏰ 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). (30)
- 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 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: PR - fuzzing test
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 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_SIMD=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_JEMALLOC=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_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=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 (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- 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, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
niedbalski
added a commit
to telemetryforge/agent
that referenced
this pull request
Oct 3, 2025
Upstream-Ref: fluent/fluent-bit@2c25200 Upstream-PR: fluent/fluent-bit#10973 Testing upstream PR #10973 in FluentDo Agent
niedbalski
added a commit
to telemetryforge/agent
that referenced
this pull request
Oct 3, 2025
Upstream-Ref: fluent/fluent-bit@4f553dd Upstream-PR: fluent/fluent-bit#10973 Testing upstream PR #10973 in FluentDo Agent
niedbalski
added a commit
to telemetryforge/agent
that referenced
this pull request
Oct 3, 2025
Upstream-Ref: fluent/fluent-bit@c5b299d Upstream-PR: fluent/fluent-bit#10973 Testing upstream PR #10973 in FluentDo Agent
This comment was marked as off-topic.
This comment was marked as off-topic.
7 tasks
niedbalski
added a commit
to telemetryforge/agent
that referenced
this pull request
Oct 3, 2025
Upstream-Ref: fluent/fluent-bit@2c25200 Upstream-PR: fluent/fluent-bit#10973 Testing upstream PR #10973 in FluentDo Agent
niedbalski
added a commit
to telemetryforge/agent
that referenced
this pull request
Oct 3, 2025
Upstream-Ref: fluent/fluent-bit@4f553dd Upstream-PR: fluent/fluent-bit#10973 Testing upstream PR #10973 in FluentDo Agent
niedbalski
added a commit
to telemetryforge/agent
that referenced
this pull request
Oct 3, 2025
Upstream-Ref: fluent/fluent-bit@c5b299d Upstream-PR: fluent/fluent-bit#10973 Testing upstream PR #10973 in FluentDo Agent
cosmo0920
force-pushed
the
cosmo0920-fix-user-authentication
branch
from
October 3, 2025 09:40
c37fb73 to
c5b299d
Compare
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
cosmo0920
force-pushed
the
cosmo0920-fix-user-authentication
branch
from
October 4, 2025 00:34
38572c9 to
e42b5ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/runtime/in_forward.c (1)
906-1046: Suggest optional test coverage forempty_shared_keywithout users.The test suite comprehensively validates the core authentication constraints. However, consider adding a test for
empty_shared_keywithoutsecurity.usersto confirm that this configuration also starts successfully (documenting the baseline behavior).Example test:
/* 5) empty_shared_key only (no users) => start OK */ void flb_test_fw_auth_empty_shared_key_only_start_ok() { flb_ctx_t *ctx; int in_ffd, out_ffd, ret; ctx = fw_make_ctx_with_forward(&in_ffd, &out_ffd); TEST_CHECK(ctx != NULL); if (!ctx) { return; } ret = flb_input_set(ctx, in_ffd, "tag", "test", "empty_shared_key", "true", NULL); TEST_CHECK(ret == 0); ret = flb_start(ctx); TEST_CHECK(ret == 0); if (ret == 0) { flb_stop(ctx); } flb_destroy(ctx); }And register it in TEST_LIST:
{"fw_auth_shared_key_only_start_ok", flb_test_fw_auth_shared_key_only_start_ok}, {"fw_auth_shared_key_plus_users_start_ok", flb_test_fw_auth_shared_key_plus_users_start_ok}, + {"fw_auth_empty_shared_key_only_start_ok", flb_test_fw_auth_empty_shared_key_only_start_ok}, {NULL, NULL}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
plugins/in_forward/fw.c(1 hunks)plugins/in_forward/fw.h(1 hunks)plugins/in_forward/fw_config.c(3 hunks)tests/runtime/in_forward.c(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/in_forward/fw.h
- plugins/in_forward/fw.c
- plugins/in_forward/fw_config.c
🧰 Additional context used
🧬 Code graph analysis (1)
tests/runtime/in_forward.c (1)
src/flb_lib.c (8)
flb_create(138-220)flb_input(261-271)flb_destroy(223-258)flb_output(274-284)flb_output_set(515-546)flb_input_set(300-330)flb_start(914-925)flb_stop(942-985)
⏰ 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). (30)
- 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 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- 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_SIMD=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 (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- 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_SIMD=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_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 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_SMALL=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 (-DFLB_JEMALLOC=Off, 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=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 (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- 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, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- 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-centos-7
- GitHub Check: PR - fuzzing test
🔇 Additional comments (6)
tests/runtime/in_forward.c (6)
906-945: LGTM!The helper function correctly initializes a Fluent Bit context with a forward input and lib output. Error handling is appropriate, with cleanup (
flb_destroy) on failure paths.
948-972: LGTM!The test correctly validates that a users-only configuration (without
shared_keyorempty_shared_key) fails to start, enforcing fail-close behavior. Cleanup is properly handled.
975-997: LGTM!The test correctly validates that combining
empty_shared_keywithsecurity.usersallows startup, with proper cleanup.
1000-1021: LGTM!The test correctly validates backward compatibility for
shared_key-only configurations (without users), ensuring startup succeeds.
1024-1046: LGTM!The test correctly validates that combining
shared_keywithsecurity.usersallows startup, ensuring both authentication mechanisms work together.
1059-1062: LGTM!The four new authentication tests are properly registered in the test list.
edsiper
requested changes
Oct 4, 2025
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
7 tasks
6 tasks
Merged
12 tasks
Merged
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, Fluent Bit does not follow the behavior of Fluentd's in_forward.
So, we need to align that behavior on in_forward plugin.
Also, I added specification confirming test cases for user authentication at startup.
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
Bug Fixes
Refactor
Tests