Skip to content

Conversation

@patrick-stephens
Copy link
Contributor

@patrick-stephens patrick-stephens commented Nov 7, 2025

Resolves #73 by ensuring we have the missing development dependencies for SASL support with AWS IAM and Kafka: confluentinc/librdkafka#1090

As part of this discovered that Kafka is disabled for CentOS 7 in upstream and here so re-enabled now we support it properly.

Resolves #96 by including the upstream changes for that.

Update the checks for these to be fatal rather than implicitly disabling it as well.

Upstream PR: fluent/fluent-bit#11127

Greptile Overview

Updated On: 2025-11-07 15:59:18 UTC

Greptile Summary

This PR enables comprehensive SASL support across all build targets to support AWS MSK IAM authentication with Kafka. The changes add cyrus-sasl-devel dependencies to all Linux distributions and create a dedicated CentOS 7 build that compiles OpenSSL 3.5.2 from source to meet modern dependency requirements.

Key changes:

  • Added cyrus-sasl-devel to AlmaLinux, Rocky Linux, SUSE, and created new CentOS 7 build with OpenSSL 3.5.2
  • Fixed librdkafka CURL version detection by replacing CURL_AT_LEAST_VERSION macro with direct LIBCURL_VERSION_NUM comparison for CentOS 7 compatibility
  • Enhanced CMake to make SASL dependencies fatal errors (except Windows) preventing silent feature degradation
  • Improved kafka.cmake with fallback SASL detection for systems without working pkg-config
  • Added validation function to verify required options are enabled at build time
  • Fixed BATS test script file detection bug that failed when .deb files were present during .rpm search

Confidence Score: 4/5

  • This PR is generally safe to merge with one configuration improvement recommended
  • The changes systematically add SASL support across all targets following established patterns. The OpenSSL build in source/packaging/distros/centos/7/Dockerfile:98 lacks explicit --prefix specification which could cause path inconsistencies, but this is already flagged. All other changes properly add dependencies, improve error handling, and fix actual bugs
  • Pay attention to source/packaging/distros/centos/7/Dockerfile - verify the OpenSSL Configure command includes --prefix=/usr/local to match the PKG_CONFIG_PATH on line 112

Important Files Changed

File Analysis

Filename Score Overview
source/packaging/distros/centos/7/Dockerfile 4/5 New CentOS 7 build with OpenSSL 3.5.2 from source and cyrus-sasl-devel for SASL support; OpenSSL Configure missing --prefix flag
testing/bats/run-package-functional-tests.sh 5/5 Fixed file detection logic to properly handle mixed .rpm/.deb files and improved code formatting

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CI as CI Build System
    participant Docker as Docker Build
    participant YUM as Package Manager
    participant OpenSSL as OpenSSL 3.5.2
    participant CMake as CMake Config
    participant Kafka as librdkafka

    Dev->>CI: Push commit with SASL deps
    
    Note over CI,Docker: CentOS 7 Build Process
    
    CI->>Docker: Start centos:7 container
    Docker->>YUM: Install build dependencies
    Note over YUM: Installs: gcc, cmake,<br/>cyrus-sasl-lib, cyrus-sasl-devel,<br/>openssl-devel, libcurl-devel
    
    YUM-->>Docker: Dependencies installed
    
    Note over Docker,OpenSSL: Build OpenSSL from source
    
    Docker->>OpenSSL: Download openssl-3.5.2.tar.gz
    Docker->>OpenSSL: ./Configure && make && make install
    Note over OpenSSL: Installs to default location<br/>(no --prefix specified)
    
    OpenSSL-->>Docker: OpenSSL 3.5.2 installed
    
    Docker->>CMake: Configure with custom paths
    Note over CMake: Sets paths to /usr/local/lib64/pkgconfig
    
    CMake->>CMake: Check for libsasl2 via pkg-config
    
    alt pkg-config finds libsasl2
        CMake->>CMake: Enable SASL support
    else pkg-config fails
        CMake->>CMake: Fallback: find_library(sasl2)
        CMake->>CMake: Enable SASL if found
    end
    
    CMake->>Kafka: Configure librdkafka options
    Note over Kafka: Enable SASL, SASL_CYRUS,<br/>and SASL_OAUTHBEARER
    
    Kafka->>Kafka: Check LIBCURL_VERSION_NUM
    
    alt LIBCURL_VERSION_NUM >= 0x075500
        Kafka->>Kafka: Use CURLOPT_PROTOCOLS_STR
    else Legacy CURL (CentOS 7)
        Kafka->>Kafka: Use CURLOPT_PROTOCOLS
    end
    
    CMake->>CMake: Verify AWS MSK IAM support
    
    alt SASL enabled
        CMake->>CMake: Enable AWS MSK IAM auth
    else Not Windows
        CMake->>CMake: FATAL_ERROR - SASL required
    end
    
    CMake->>CMake: validate_required_options()
    
    alt All required options enabled
        CMake-->>Docker: Build configured successfully
    else Required option missing
        CMake->>CMake: FATAL_ERROR
    end
    
    Docker->>Docker: make && cpack -G RPM
    Docker-->>CI: Copy .rpm to /output
Loading

@patrick-stephens patrick-stephens requested a review from a team as a code owner November 7, 2025 11:38
@patrick-stephens patrick-stephens added the build-packages Option to enable all package builds for a PR to test label Nov 7, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. source/packaging/distros/suse/Dockerfile, line 36-37 (link)

    logic: Missing cyrus-sasl-devel package for SASL support. All SUSE targets need this for AWS MSK IAM authentication to work.

10 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@patrick-stephens
Copy link
Contributor Author

  1. source/packaging/distros/suse/Dockerfile, line 36-37 (link)
    logic: Missing cyrus-sasl-devel package for SASL support. All SUSE targets need this for AWS MSK IAM authentication to work.

@greptile-ai we will tackle this as part of the work for #96 as the upstream PR there includes it.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! That makes sense to address the SUSE Dockerfile changes as part of issue #96 since the upstream PR already includes those updates. This avoids duplicating work and keeps the changes organized by their logical groupings.

Just to confirm - this means the SUSE builds will temporarily fail with the new fatal error validation until #96 is merged, but that's an acceptable trade-off to keep the PRs focused and avoid conflicts with the upstream changes?

Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
@patrick-stephens patrick-stephens added build-linux Option to enable building of Linux packages only for a PR to test build-windows Option to enable building of Windows packages only for a PR to test and removed build-packages Option to enable all package builds for a PR to test labels Nov 7, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Signed-off-by: Patrick Stephens <[email protected]>
@patrick-stephens patrick-stephens removed the build-windows Option to enable building of Windows packages only for a PR to test label Nov 7, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Signed-off-by: Patrick Stephens <[email protected]>
Signed-off-by: Patrick Stephens <[email protected]>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@patrick-stephens patrick-stephens merged commit c8b8328 into main Nov 7, 2025
76 of 83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build-linux Option to enable building of Linux packages only for a PR to test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add improved SLES builds Resolve standard configuration for all targets

2 participants