bazel: support select() for cache_entries in envoy_cmake #42461
Conversation
Allow envoy_cmake to accept cache_entries as either a dict or a select()
statement. This enables conditional CMake cache entries based on build
configuration, which is required for FIPS builds where different library
paths must be used for FIPS vs non-FIPS builds.
When cache_entries is a dict (the common case), the function continues
to merge default_cache_entries and wrap the result in a select() for
debug builds. When cache_entries is already a select(), it is passed
through directly to avoid nested select() statements, which Bazel does
not support.
This change enables targets like ipp-crypto to use selects.with_or() to
conditionally set OPENSSL_CRYPTO_LIBRARY based on whether the build is
FIPS-compliant (using libcrypto.a) or non-FIPS (using libcrypto_internal.a).
without this patch the build fails with:
```
-- Found OpenSSL: /build/.cache/bazel/_bazel_envoybuild/5510e63bd001cefa746eb005f1949cb5/sandbox/processwrapper-sandbox/6607/execroot/envoy/bazel-out/k8-opt/bin/contrib/cryptomb/private_key_providers/source/ipp-crypto.ext_build_deps/lib/libcrypto_internal.a (found version "")
-- Configuring done (6.3s)
CMake Error at src/CMakeLists.txt:235 (target_link_libraries):
Target "crypto_mb_s" links to:
OpenSSL::Crypto
but the target was not found. Possible reasons include:
* There is a typo in the target name.
* A find_package call is missing for an IMPORTED target.
* An ALIAS target is missing.
CMake Error at src/CMakeLists.txt:273 (target_link_libraries):
Target "crypto_mb" links to:
OpenSSL::Crypto
but the target was not found. Possible reasons include:
* There is a typo in the target name.
* A find_package call is missing for an IMPORTED target.
* An ALIAS target is missing.
-- Generating done (0.4s)
```
Signed-off-by: William Dauchy <[email protected]>
|
@phlax is it something you may review? |
| "@envoy//bazel:dbg_build": cache_entries_debug, | ||
| "//conditions:default": cache_entries, | ||
| }), | ||
| cache_entries = final_cache_entries, |
There was a problem hiding this comment.
oh wait - i see it already has a select here - so this is working around that
| # FIPS builds (both not_ppc and ppc) use libcrypto.a/libssl.a | ||
| ( | ||
| "//bazel:boringssl_fips_not_ppc", | ||
| "//bazel:boringssl_fips_ppc", |
There was a problem hiding this comment.
do we need a with_or here? porobably it would be cleaner to just have //bazel:boringssl_fips that was arch agnostic
| "BORINGSSL": "on", | ||
| "DYNAMIC_LIB": "off", | ||
| "MB_STANDALONE": "on", | ||
| "OPENSSL_CRYPTO_LIBRARY": "$$EXT_BUILD_DEPS/lib/libcrypto.a", |
There was a problem hiding this comment.
i think boringssl is always libcrypto_internal.a, no?
There was a problem hiding this comment.
not for fips I think, thus my fix
envoy/bazel/external/boringssl_fips.BUILD
Line 81 in 0f111f5
The libcrypto_internal.a naming likely comes from BoringSSL's CMake build, which uses the _internal suffix for non-FIPS builds.
However, when boringssl is used as a dependency via deps = ["//bazel:boringssl"] in a foreign_cc build, the libraries are installed into $$EXT_BUILD_DEPS/lib/. Since libsxg uses libcrypto_internal.a unconditionally and works for both FIPS and non-FIPS, both may provide libraries with that name in the dependency path, or there's a normalization step.
There was a problem hiding this comment.
im a bit confused in that case
when we use boringssl in cc_library, cc_binary as deps - its handled by bazel as to what the path is etc
for foreign_cc - a recent discovery was that a number of builds were actually using openssl - hence switching to libcrypto_internal.a - which is what you get if you build boringssl directly
so now im wondering if the fips build is wrong - but then we also no longer have openssl in the build - so i suspect this must be correct
2 participants
Commit Message:
Allow envoy_cmake to accept cache_entries as either a dict or a select() statement. This enables conditional CMake cache entries based on build configuration, which is required for FIPS builds where different library paths must be used for FIPS vs non-FIPS builds.
When cache_entries is a dict (the common case), the function continues to merge default_cache_entries and wrap the result in a select() for debug builds. When cache_entries is already a select(), it is passed through directly to avoid nested select() statements, which Bazel does not support.
This change enables targets like ipp-crypto to use selects.with_or() to
conditionally set OPENSSL_CRYPTO_LIBRARY based on whether the build is
FIPS-compliant (using libcrypto.a) or non-FIPS (using libcrypto_internal.a).
Additional Description:
without this patch the build fails with:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]