cmake: kafka: fix OAuth Bearer detection on Windows #11294
Conversation
Signed-off-by: BP Cheng <[email protected]>
bp-cheng
requested review from
celalettin1286,
fujimotos,
niedbalski and
patrick-stephens
as code owners
WalkthroughThe Kafka CMake configuration introduces platform-aware conditional logic for OAuth Bearer token authentication. Windows builds enable OAuth Bearer only when WITH_SSL is set, while non-Windows platforms use FLB_SASL_ENABLED to maintain existing Cyrus SASL compatibility. MSK IAM authentication behavior remains unchanged. Comments updated to document platform-specific behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 (1)
cmake/kafka.cmake(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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`.
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.
📚 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:
cmake/kafka.cmake
📚 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:
cmake/kafka.cmake
📚 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:
cmake/kafka.cmake
📚 Learning: 2025-08-29T06:24:55.855Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:52-56
Timestamp: 2025-08-29T06:24:55.855Z
Learning: ZSTD compression is always available in Fluent Bit and does not require conditional compilation guards. Unlike Arrow/Parquet which use #ifdef FLB_HAVE_ARROW guards, ZSTD is built unconditionally with flb_zstd.c included directly in src/CMakeLists.txt and a bundled ZSTD library at lib/zstd-1.5.7/.
Applied to files:
cmake/kafka.cmake
📚 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, ZSTD support is always available and doesn't need build-time conditionals.
Applied to files:
cmake/kafka.cmake
📚 Learning: 2025-08-29T06:24:26.170Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:39-42
Timestamp: 2025-08-29T06:24:26.170Z
Learning: In Fluent Bit, ZSTD compression support is enabled by default and does not require conditional compilation guards (like #ifdef FLB_HAVE_ZSTD) around ZSTD-related code declarations and implementations.
Applied to files:
cmake/kafka.cmake
📚 Learning: 2025-08-29T06:24:44.797Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: src/aws/flb_aws_compress.c:26-26
Timestamp: 2025-08-29T06:24:44.797Z
Learning: In Fluent Bit, ZSTD support is always available and enabled by default. The build system automatically detects and uses either the system libzstd library or builds the bundled ZSTD version. Unlike other optional dependencies like Arrow which use conditional compilation guards (e.g., FLB_HAVE_ARROW), ZSTD does not require conditional includes or build flags.
Applied to files:
cmake/kafka.cmake
🔇 Additional comments (1)
cmake/kafka.cmake (1)
31-33: LGTM! Clear documentation of platform differences.The comments effectively document the platform-specific SASL requirements and help future maintainers understand why the logic differs between Windows and other platforms.
| if(WIN32) | ||
| if(WITH_SSL) | ||
| set(FLB_SASL_OAUTHBEARER_ENABLED ON) | ||
| else() | ||
| set(FLB_SASL_OAUTHBEARER_ENABLED OFF) | ||
| endif() | ||
| else() | ||
| # Non-Windows platforms: require Cyrus SASL | ||
| set(FLB_SASL_OAUTHBEARER_ENABLED ${FLB_SASL_ENABLED}) | ||
| endif() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find kafka.cmake and examine its content
find . -name "kafka.cmake" -o -name "CMakeLists.txt" | head -5
# Check cmake/kafka.cmake
if [ -f "cmake/kafka.cmake" ]; then
echo "=== cmake/kafka.cmake ==="
wc -l cmake/kafka.cmake
echo ""
echo "=== Lines 30-55 of cmake/kafka.cmake ==="
sed -n '30,55p' cmake/kafka.cmake
fi
# Check main CMakeLists.txt for WITH_SSL and kafka.cmake inclusion order
if [ -f "CMakeLists.txt" ]; then
echo ""
echo "=== Looking for WITH_SSL and kafka.cmake in CMakeLists.txt ==="
grep -n "WITH_SSL\|kafka.cmake" CMakeLists.txt | head -20
fiRepository: fluent/fluent-bit
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Check where WITH_SSL is first defined in CMakeLists.txt
echo "=== Searching for WITH_SSL definition in CMakeLists.txt ==="
grep -n "WITH_SSL\|FLB_TLS" CMakeLists.txt | head -30
echo ""
echo "=== Lines around line 808 where kafka.cmake is included ==="
sed -n '800,815p' CMakeLists.txt
echo ""
echo "=== Full kafka.cmake to understand the context ==="
head -80 cmake/kafka.cmakeRepository: fluent/fluent-bit
Length of output: 3185
🏁 Script executed:
#!/bin/bash
# Check if WITH_SSL is defined elsewhere or if FLB_OPTION creates cache variables
echo "=== Checking for FLB_OPTION macro definition ==="
grep -r "macro.*FLB_OPTION\|function.*FLB_OPTION" cmake/ 2>/dev/null | head -5
echo ""
echo "=== Checking all references to WITH_SSL in kafka.cmake ==="
grep -n "WITH_SSL" cmake/kafka.cmake
echo ""
echo "=== Checking if FLB_TLS should be used instead ==="
grep -n "FLB_TLS" CMakeLists.txt | head -10
echo ""
echo "=== Checking the flow: is kafka.cmake included after FLB_TLS is set? ==="
sed -n '737,808p' CMakeLists.txtRepository: fluent/fluent-bit
Length of output: 2360
🏁 Script executed:
#!/bin/bash
# Check the FLB_OPTION macro definition to understand what it does
echo "=== FLB_OPTION macro definition ==="
grep -A 5 "macro(FLB_OPTION" cmake/macros.cmake
echo ""
echo "=== Check what variables are actually defined/used ==="
echo "Variables defined in CMakeLists.txt:"
grep -n "^option\|^set.*ON\|^set.*OFF" CMakeLists.txt | grep -i "ssl\|tls" | head -20
echo ""
echo "=== Verify: Does WITH_SSL exist as a CMake variable? ==="
grep -r "^option.*WITH_SSL\|set(WITH_SSL" CMakeLists.txt cmake/*.cmake 2>/dev/nullRepository: fluent/fluent-bit
Length of output: 481
Verify that FLB_TLS is used instead of WITH_SSL for OAuth Bearer logic.
The condition at line 35 checks if(WITH_SSL), but WITH_SSL is never defined before this check. The main CMakeLists.txt defines option(FLB_TLS ...) at line 175 for SSL/TLS support, and kafka.cmake is included after FLB_TLS is set (line 808 vs line 737). The code should check if(FLB_TLS) instead, or WITH_SSL must be initialized before line 35. As written, the check will evaluate to false on all platforms, causing OAuth Bearer to be incorrectly disabled on Windows even when SSL/TLS is enabled.
The claim about librdkafka OAuth Bearer support on Windows is accurate: librdkafka supports OAUTHBEARER, and Windows provides built-in SASL capabilities through SSPI, so only SSL is required for token endpoint verification.
🤖 Prompt for AI Agents
In cmake/kafka.cmake around lines 34 to 43, the Windows branch checks
if(WITH_SSL) but WITH_SSL is never defined here; update the condition to use the
actual TLS option defined in the top-level CMakeLists (if(FLB_TLS)) so OAuth
Bearer is enabled when FLB_TLS is ON on Windows; adjust the if(...) and
corresponding else branch to reference FLB_TLS (or alternatively initialize
WITH_SSL from FLB_TLS before this block) so OAuth Bearer logic behaves
correctly.
There was a problem hiding this comment.
Yeah I agree with this one, I think we use FLB_TLS
There was a problem hiding this comment.
@patrick-stephens, thanks for confirming! You're absolutely right. The code should use FLB_TLS instead of WITH_SSL. Here's the corrected version:
# OAuth Bearer support:
# - Windows: Built-in SASL, only needs SSL (no Cyrus SASL required)
# - Linux/macOS: Needs both SSL and Cyrus SASL
if(WIN32)
if(FLB_TLS)
set(FLB_SASL_OAUTHBEARER_ENABLED ON)
else()
set(FLB_SASL_OAUTHBEARER_ENABLED OFF)
endif()
else()
# Non-Windows platforms: require Cyrus SASL
set(FLB_SASL_OAUTHBEARER_ENABLED ${FLB_SASL_ENABLED})
endif()The key change is on line 35: if(FLB_TLS) instead of if(WITH_SSL). This ensures that OAuth Bearer is correctly enabled on Windows when TLS/SSL support is built into Fluent Bit.
🧠 Learnings used
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`.
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.
![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".
| if(WIN32) | ||
| if(WITH_SSL) | ||
| set(FLB_SASL_OAUTHBEARER_ENABLED ON) |
There was a problem hiding this comment.
In the new Windows branch the condition if(WITH_SSL) is evaluated before WITH_SSL is ever set (FLB_OPTION(WITH_SSL On) appears later in the file), so on a default Windows build the variable expands to empty and the branch is always false. That leaves FLB_SASL_OAUTHBEARER_ENABLED OFF even when SSL support is present, so OAuth/MSK IAM remain disabled contrary to the commit’s intent (CMake treats undefined variables as false).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@bp-cheng can you resolve the comments raised by the bot? Either it is an issue to be fixed or something that can be ignored with justification.
|
Thanks for this, was wondering what was up with it! |
| # OAuth Bearer support: | ||
| # - Windows: Built-in SASL, only needs SSL (no Cyrus SASL required) | ||
| # - Linux/macOS: Needs both SSL and Cyrus SASL | ||
| if(WIN32) |
There was a problem hiding this comment.
@cosmo0920 should this be FLB_WINDOWS or something similar?
2 participants
On Windows, OAuth Bearer authentication was incorrectly disabled because
the detection logic tied FLB_SASL_OAUTHBEARER_ENABLED to finding Cyrus
SASL (libsasl2), which is not available on Windows.
However, Windows has built-in SASL support through SSPI (Security Support
Provider Interface), and librdkafka can use OAuth Bearer on Windows when
SSL is available, without requiring Cyrus SASL.
This change adds platform-specific detection:
Signed-off-by: Gang [email protected]
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.