Ensure openssl 3.0 compatibility of released packages#476
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 |
b2b371d to
ad9c71a
Compare
Remove spurious double-quotes from BUILD_DIR assignment. Make variable assignments should not be quoted — the quotes become literal parts of the value, breaking concatenation in contexts like $(BUILD_DIR)/subdir where the embedded quotes produce paths the shell cannot resolve. Co-authored-by: Dmitry Kropachev <dmitry.kropachev@gmail.com>
Replace the hardcoded OpenSSL version (1.1.1u) in .package-configure with a variable so other targets (build-static-integration-test-bin) can reuse it without duplicating the version string. Co-authored-by: Dmitry Kropachev <dmitry.kropachev@gmail.com>
The static driver archive contains OpenSSL code compiled with zlib compression support (libcrypto uses zlib for TLS record compression and PKCS#7/CMS operations). On Debian/Ubuntu, libssl-dev declares a dependency on zlib1g-dev so it is always present. On Fedora, openssl-devel does not depend on zlib-devel — it only requires the shared libz.so at runtime via openssl-libs, not the static lib or development symlink. Without zlib-devel installed the linker cannot resolve deflate/inflate symbols that libcrypto.a references, failing the static smoke test link. Co-authored-by: Dmitry Kropachev <dmitry.kropachev@gmail.com>
66c63d7 to
a9298e2
Compare
If one uses the driver as a CMake dependency, they expect to link against the driver target and get all the transitive dependencies (like OpenSSL) linked in as well. This is what INTERFACE_LINK_LIBRARIES does, and it is the standard way to propagate usage requirements in CMake. Declare INTERFACE_LINK_LIBRARIES on the CMake imported target so in-tree consumers (integration tests) link correctly. Recap: _imported targets_ are a CMake mechanism to represent external dependencies; in our case, it's the Rust wrapper built by cargo. Co-authored-by: Dmitry Kropachev <dmitry.kropachev@gmail.com>
Based on my research, using `pkg-config` is a common way to handle transitive dependencies for static linking. Example invocation: `cc \ $(pkg-config --cflags scylladb_static) \ ../examples/ssl/ssl.c \ $(pkg-config --libs --static scylladb_static) \ -o ssl_example` Declare the static driver's transitive dependencies (OpenSSL, system libs, threading, macOS frameworks) in the generated scylladb_static.pc metadata via Requires.private and Libs.private. This lets installed consumers resolve all required symbols via `pkg-config --static` without hardcoding platform-specific flags. Co-authored-by: Dmitry Kropachev <dmitry.kropachev@gmail.com>
a9298e2 to
22158be
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the release/CI pipeline to ensure the shipped static driver artifacts remain link-compatible with OpenSSL 3.0.x (minimum supported), addressing the regression described in #455. It also improves how static consumers discover transitive link requirements and adds build-environment metadata to dev packages.
Changes:
- Add a Makefile-based OpenSSL 3.0.2 link verification step and run it in PR CI and DEB packaging CI.
- Publish/propagate static driver transitive link dependencies via CMake (
INTERFACE_LINK_LIBRARIES) and thescylladb_static.pcmetadata. - Generate and install a
BUILD_INFOfile capturing build environment/toolchain details into dev packages.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scylla-rust-wrapper/scylladb_static.pc.in |
Updates pkg-config metadata for the static driver, including private dependency fields. |
scylla-rust-wrapper/CMakeLists.txt |
Adds static transitive link deps export and introduces BUILD_INFO generation + install. |
Makefile |
Adds verify-openssl-3.0-compat target and related CI/packaging tweaks. |
ci/generate-build-info.sh |
New script to produce BUILD_INFO describing platform/toolchain/OpenSSL detected at build time. |
.github/workflows/build-lint-and-test.yml |
Runs the OpenSSL 3.0 link verification step in PR CI after building artifacts. |
.github/workflows/build-cpack-packages.yml |
Runs the OpenSSL 3.0 link verification step after DEB package build/test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Requires.private: @scylladb_static_requires_private@ | ||
| Libs: -L${libdir} -lscylladb_static | ||
| Libs.private: @scylladb_static_libs_private@ | ||
| Cflags: -I${includedir} |
There was a problem hiding this comment.
I had a long discussion with Claude Opus about Libs vs Libs.private for this. It kept contradicting itself and couldn't decide. Reviewers, please come for rescue.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/generate-build-info.sh`:
- Around line 89-93: The patch version extraction in the OpenSSL version parsing
is using an incorrect bit shift. The line extracting the patch component with
`patch=$(( (dec >> 12) & 0xFF ))` shifts by 12 bits, but OpenSSL 3.x encodes the
patch version in bits 4-11. Change the bit shift value from 12 to 4 in the patch
extraction statement to correctly extract the patch component and ensure
BUILD_INFO reports the accurate OpenSSL patch version.
In `@Makefile`:
- Around line 292-293: Add checksum validation for the OpenSSL 3.0 package
download to prevent security vulnerabilities from MITM attacks or compromised
sources. Define a new variable to store the SHA256 checksum of the libssl-dev
package referenced by OPENSSL_3_0_LIBSSL_DEV_URL, then modify the download and
extraction logic at line 298 to verify the downloaded .deb file's checksum
matches the pinned value before proceeding with extraction. Use a tool like
sha256sum to perform the verification and fail the build if the checksum does
not match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3adf66dd-328a-4935-92b1-f2fbd20c26d0
📒 Files selected for processing (6)
.github/workflows/build-cpack-packages.yml.github/workflows/build-lint-and-test.ymlMakefileci/generate-build-info.shscylla-rust-wrapper/CMakeLists.txtscylla-rust-wrapper/scylladb_static.pc.in
Add a Makefile target and CI steps that verify the static driver archive can link against OpenSSL 3.0.2 (our minimum supported version). The test copies the already-built static archive and pkg-config metadata into a sysroot alongside OpenSSL 3.0.2 headers and static libraries from a pinned Launchpad deb, then attempts to compile and link examples/ssl/ssl.c. If the build environment was contaminated (e.g. by Homebrew exposing a newer OpenSSL with IDEA or 3.3+ APIs), the archive will contain symbol references that don't exist in OpenSSL 3.0 and the link will fail — catching the regression from issue scylladb#455. The verification runs in both PR CI (after build-integration-test-bin) and packaging CI (after test-package-deb), since both produce the static archive in build/.
6eff933 to
28ba4d4
Compare
Add a script that captures build environment metadata (OS, toolchain, OpenSSL version and configuration, CMake options, Rust profile) into a BUILD_INFO file in the build directory. A CMake custom target runs the script after the cargo build completes, ensuring openssl-sys detection results are available. If bash is not present, the target is silently skipped. This addresses issue scylladb#455's request for transparency about what Linux version and OpenSSL configuration the static archive was built against.
Install the generated BUILD_INFO file into the dev package at share/doc/scylla-cpp-driver/BUILD_INFO. This makes the build environment metadata available to consumers who install the development package (DEB/RPM/PKG/MSI).
28ba4d4 to
3a9d388
Compare
Fixes: #455
Problem recap
#455 correctly observed that linking the static driver library included in release 1.0.0 (1.0.1 is also impacted) against an older OpenSSL fails, because the lib has undefined symbols that are only supplied by OpenSSL 3.3/3.4/3.5 or newer. Considering our OpenSSL minimum requirements is 3.0.2 (should be enough - this is present on Ubuntu 22.04, so quite an old one), this should be considered a bug in our build/release CI.
There are three problems in particular:
Problem 1: We have built packages with too new OpenSSL headers.
This led to including functions not available at 3.0.x as unresolved symbols.
Problem 2: We haven't verified that the released packages link against OpenSSL 3.0.x correctly.
Problem 3: We didn't put any build info manifest in the released package.
This @vladzcloudius noted in #455 as the minimum to address the issue.
Solution
Problem 2: OpenSSL 3.0.2 linking verification
This PR introduces an OpenSSL verification Makefile recipe. Outline:
1. copy the already-built static archive and pkg-config metadata into a sysroot;
2. alongside, put OpenSSL 3.0.2 headers and static libraries from a pinned Launchpad deb;
3. attempt to compile and link
examples/ssl/ssl.c.If the build environment was contaminated (e.g. by a newer OpenSSL with IDEA or 3.3+ APIs), the archive will contain symbol references that don't exist in OpenSSL 3.0 and the link will fail — catching the regression from issue #455.
The verification runs in both PR CI (after
build-integration-test-bin) and packaging CI (aftertest-package-deb), since both produce the static archive inbuild/.Problem 1: too new OpenSSL in the building/packaging CI workflow
The plot twist is that one week after we released 1.0.1 with no support for OpenSSL 3.0.x, once I freshly rebuilt the 1.0.1 package (in the same CI workflow) and attempted linkage against OpenSSL 3.0.2 using the new verification procedure, it succeeded. Investigation revealed that the fresh lib has much different cgus (compile gen units) - different number and sizes. Most likely explanation: LTO in the Rust toolchain improved during that week, fixing the problem in our building/packaging CI.
If the problem happens to strike back, our verification step will immediately catch it. We're thus guarded and safe.
Problem 3: lack of build info in the package
It makes sense to generate plaintext build info manifest (
Key: Valuelines) during build and put it in the built dev package. Exact contents of this build info is subject to discussion; this PR introduces what Opus came up with (and I believe it's a fine list of properties).PR contents in detail
First 3 commits
They are extracted from #438. These are small fixes.
Next 3 commits
Also extracted from #438.
scylla-rust-wrapper/CMakeLists.txt.scylladb_static.pc(pkg-configmanifest). This supports a common (according to my research) way that people configure their compiler & linker. More details in the commit message.Next commit
Introduces the OpenSSL 3.0.2 compatibility verification, described earlier.
Last 2 commits
Introduce
BUILD_INFOgeneration and include it in the generateddevpackages. This addresses the main request of #455.`BUILD_INFO` contents when built on my machine:
Build Information for scylla-cpp-driver v1.0.0-145-g541cb277
Platform
OS: Ubuntu 25.10
Architecture: x86_64
Kernel: 6.17.0-35-generic
glibc: 2.42
Toolchain
Rust: rustc 1.96.0 (ac68faa20 2026-05-25)
Cargo: cargo 1.96.0 (30a34c682 2026-05-25)
CMake: cmake version 3.31.6
CC: cc (Ubuntu 15.2.0-4ubuntu4) 15.2.0
OpenSSL (detected by openssl-sys at build time)
Version: 3.5.0
Include path: /usr/include
Disabled (osslconf): OPENSSL_NO_IDEA OPENSSL_NO_SSL3_METHOD
Build Configuration
CMAKE_BUILD_TYPE: Release
CMAKE_INSTALL_PREFIX: /usr
CASS_BUILD_SHARED: ON
CASS_BUILD_STATIC: ON
LTO: true
Panic strategy: abort
Static Library Compatibility Notes
The static archive (libscylladb_static.a) requires consumers to
provide OpenSSL >= 3.0 at link time. The minimum OpenSSL version
is determined by the build-time detection performed by the
openssl-sys crate.
Consumers linking on systems with a different OpenSSL configuration
(e.g. different osslconf flags) may encounter undefined symbol errors
for conditionally-compiled functions.
`BUILD_INFO` contents when built in the CI:
Build Information for scylla-cpp-driver 6eff933
Platform
OS: Ubuntu 22.04.5 LTS
Architecture: x86_64
Kernel: 6.8.0-1052-azure
glibc: 2.35
unknown
Toolchain
Rust: rustc 1.96.0 (ac68faa20 2026-05-25)
Cargo: cargo 1.96.0 (30a34c682 2026-05-25)
CMake: cmake version 3.31.6
CC: cc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0
OpenSSL (detected by openssl-sys at build time)
Version: 3.0.2
Include path: /usr/include
Disabled (osslconf): unknown (openssl-sys build output not found)
Build Configuration
CMAKE_BUILD_TYPE: Release
CMAKE_INSTALL_PREFIX: /usr
CASS_BUILD_SHARED: ON
CASS_BUILD_STATIC: ON
LTO: true
Panic strategy: abort
Static Library Compatibility Notes
The static archive (libscylladb_static.a) requires consumers to
provide OpenSSL >= 3.0 at link time. The minimum OpenSSL version
is determined by the build-time detection performed by the
openssl-sys crate.
Consumers linking on systems with a different OpenSSL configuration
(e.g. different osslconf flags) may encounter undefined symbol errors
for conditionally-compiled functions.
Pre-review checklist
[ ] I have implemented Rust unit tests for the features/changes introduced.[ ] I have enabled appropriate tests inMakefilein{SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.Fixes:annotations to PR description.