-
Notifications
You must be signed in to change notification settings - Fork 1.9k
kafka: fix cmake cross compile error #9600
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
kafka: fix cmake cross compile error #9600
Conversation
b398800 to
c96d616
Compare
c96d616 to
abc06b7
Compare
|
@edsiper It has been a while. Any chance that this gets approved? I try to reduce my own patch list. |
|
@edsiper Happy New Year! Can you have a look at this PR please? |
|
@edsiper @leonardo-albertovich @fujimotos @koleini This PR is open for a while. Is it possible to have a look? |
abc06b7 to
5c51513
Compare
|
Can we rebase to see if it resolves the unit test failures? |
5c51513 to
18017d7
Compare
I rebased it. |
@patrick-stephens rebased it, and all checks are passing. |
|
@patrick-stephens what is the next step to get this merged? I also have other open PRs which are approved, but not merged yet. Can you check my other PRs also? https://github.com/fluent/fluent-bit/pulls/ThomasDevoogdt |
|
@edsiper will be merging when he is happy to put it in a release - be aware we're finalising the 4.0 release now. |
@patrick-stephens @edsiper I see that 4.0.0 has landed, this is the ideal moment to consider this PR. |
|
@patrick-stephens can you also check this one, while at it? |
|
Looks fine to me, just want to check @edsiper is happy with the commit title |
fb46e10 to
dcecf8f
Compare
|
@patrick-stephens @edsiper It's again a bit silent around this PR. Everything is approved and ready to be merged, on what do we exactly wait? Can you just merge this one? |
|
@cosmo0920 Can you put this PR back on another more milestone, I have the feeling that next means forgotten. Or just merge this one. |
dcecf8f to
3a1a13e
Compare
WalkthroughApplied conditional guards around librdkafka include directories in Kafka input and output plugin CMake configurations, executing target_include_directories only when KAFKA_INCLUDEDIR is defined. Other build settings remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
plugins/in_kafka/CMakeLists.txt(1 hunks)plugins/out_kafka/CMakeLists.txt(1 hunks)
⏰ 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). (62)
- GitHub Check: PR - container builds / Windows container images (2025)
- GitHub Check: PR - container builds / Windows container images (2022)
- GitHub Check: PR - packages build Linux / debian/buster.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/22.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / raspbian/bookworm package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/24.04 package build and stage to S3
- GitHub Check: PR - packages build Linux / ubuntu/22.04.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bullseye.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/buster package build and stage to S3
- GitHub Check: PR - packages build Linux / debian/bookworm.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / almalinux/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/8.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2023 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7 package build and stage to S3
- GitHub Check: PR - packages build Linux / amazonlinux/2.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / rockylinux/9.arm64v8 package build and stage to S3
- GitHub Check: PR - packages build Linux / centos/7.arm64v8 package build and stage to S3
- GitHub Check: PR - container builds / amd64/production container image build
- GitHub Check: PR - container builds / arm/v7/production container image build
- GitHub Check: PR - container builds / arm/v7/debug container image build
- GitHub Check: PR - container builds / arm64/production container image build
- GitHub Check: PR - container builds / arm64/debug container image build
- GitHub Check: PR - container builds / amd64/debug container image build
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Intel macOS runner, macos-14-large, 3.31.6)
- GitHub Check: PR - packages build MacOS / call-build-macos-package (Apple Silicon macOS runner, macos-14, 3.31.6)
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: PR - packages build Windows / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=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=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_SMALL=On, 3.31.6, clang, clang++)
- 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, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=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 (-DSANITIZE_ADDRESS=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 (-DSANITIZE_ADDRESS=On, 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, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=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_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
|
@cosmo0920 This PR starts to age, can you pick this up? I would like to have it merged. |
3a1a13e to
f042b7e
Compare
|
It looks like we're hitting some older compiler failures: |
|
We need to rebase off the current master. |
KAFKA_INCLUDEDIR is not set if FLB_PREFER_SYSTEM_LIB_KAFKA is not used, when cross-compiling, it just translates to -I/librdkafka, which is not allowed. Fix this by only including KAFKA_INCLUDEDIR if really set. x86_64-linux-gcc: ERROR: unsafe header/library path used in cross-compilation: '-I/librdkafka' Signed-off-by: Thomas Devoogdt <[email protected]>
f042b7e to
2ea0c01
Compare
Done. Please also merge it now, to avoid having to rebase it over and over again. |
|
@edsiper I know it's your birthday but are we ok to merge? :) |
|
@patrick-stephens @edsiper Upstream buildroot ships now with 4.0.9 (buildroot/buildroot@39afca7) 🎉 🎉. It would be nice to get this merged, to further reduce the custom patch set in buildroot. Perhaps in 4.0.11? |
|
@edsiper can you merge this one? |
|
@patrick-stephens do you mind to just merge this PR? Otherwise it will just be open forever. |
|
@edsiper Couldn't we include this change in Fluent Bit 4.2? |
|
I'll merge as seems low risk and we can always revert |
|
Thanks! |
This is a major release that introduces new features, including one highlighted in the release notes: "The v4.2 release introduces a powerful new Direct Routing capability that allows inputs to specify routes directly to outputs, bypassing the traditional routing mechanism." But it also brings some security fixes. Not all of them are relevant, but some piece of lecture can be found here [1]. It fixes the following CVEs: CVE-2025-12977 CVE-2025-12978 CVE-2025-12972 CVE-2025-12970 CVE-2025-12969 News: - https://fluentbit.io/announcements/v4.1.0/ - https://fluentbit.io/announcements/v4.1.1/ - https://fluentbit.io/announcements/v4.2.0/ The patch 0001-plugins-kafka-fix-cmake-cross-compile-error.patch can be dropped as it has been merged upstream [2]. [1] https://www.theregister.com/2025/11/24/fluent_bit_cves/ [2] fluent/fluent-bit#9600 Signed-off-by: Thomas Devoogdt <[email protected]> Signed-off-by: Thomas Petazzoni <[email protected]>
This is a major release that introduces new features, including one highlighted in the release notes: "The v4.2 release introduces a powerful new Direct Routing capability that allows inputs to specify routes directly to outputs, bypassing the traditional routing mechanism." But it also brings some security fixes. Not all of them are relevant, but some piece of lecture can be found here [1]. It fixes the following CVEs: CVE-2025-12977 CVE-2025-12978 CVE-2025-12972 CVE-2025-12970 CVE-2025-12969 News: - https://fluentbit.io/announcements/v4.1.0/ - https://fluentbit.io/announcements/v4.1.1/ - https://fluentbit.io/announcements/v4.2.0/ The patch 0001-plugins-kafka-fix-cmake-cross-compile-error.patch can be dropped as it has been merged upstream [2]. [1] https://www.theregister.com/2025/11/24/fluent_bit_cves/ [2] fluent/fluent-bit#9600 Signed-off-by: Thomas Devoogdt <[email protected]> Signed-off-by: Thomas Petazzoni <[email protected]> (cherry picked from commit 7a037d0) Signed-off-by: Thomas Perale <[email protected]>
KAFKA_INCLUDEDIR is not set if FLB_PREFER_SYSTEM_LIB_KAFKA is not used, when cross-compiling, it just translates to -I/librdkafka, which is not allowed. Fix this by only including KAFKA_INCLUDEDIR if really set.
x86_64-linux-gcc: ERROR: unsafe header/library path used in cross-compilation: '-I/librdkafka'
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