-
Notifications
You must be signed in to change notification settings - Fork 1.8k
out_es: fix elasticsearch output api key usage #10949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughReplaces direct formatting into a NULL SDS with explicit preallocation when building the Elasticsearch HTTP Authorization header for API key auth: allocate an SDS sized for the header, then format "ApiKey " into it. No public interfaces were changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CB as cb_es_flush
participant SDS as flb_sds (allocated)
participant HTTP as http_client / headers
CB->>SDS: flb_sds_create_size(len)
CB->>SDS: flb_sds_printf(&header_line, "ApiKey %s", ctx->http_api_key)
SDS-->>CB: header_line ("ApiKey <key>")
CB->>HTTP: add header ("Authorization": header_line)
CB->>HTTP: perform request / flush
alt success
HTTP-->>CB: response
else error
HTTP-->>CB: error -> cleanup
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 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)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_es/es.c
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/out_es/es.c (1)
src/flb_sds.c (2)
flb_sds_create_size
(92-95)flb_sds_printf
(336-387)
existing docs issue: fluent/fluent-bit-docs#1727 |
e6f5b03
to
eb74bd8
Compare
I'm like be wary of code rabbit which, with my help for not reading carefully enough, introduced this bug: #10461 (comment) |
Wow yeah Coderabbit screwed this one up. I should have caught this too in the review, so my bad as well! |
Commit messages need to be prefixed with |
Signed-off-by: Gil Raphaelli <[email protected]>
5779614
to
0470788
Compare
squashed the two commits to one while force pushing anyway to update commit messages, thanks. |
fixes #10948
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-test
label 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