fix(function-autoscaler): restore multi-arch OpenSSL paths - #674
Conversation
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Bazel module adds an ChangesOpenSSL crate configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@MODULE.bazel`:
- Around line 232-239: Update the OpenSSL environment configuration in the
supported-targets section to define target-specific include and library
directory variables for both x86_64-apple-darwin and aarch64-apple-darwin,
rather than allowing them to inherit Linux paths; alternatively remove those
macOS triples from SUPPORTED_TRIPLES if they are not supported.
- Around line 232-239: The OpenSSL configuration in the Bazel environment uses
the x86_64 library path for aarch64 builds, making openssl-sys non-hermetic.
Update the aarch64-specific settings in MODULE.bazel to reference Bazel-managed
arm64 OpenSSL headers and libraries, including the required per-platform inputs;
alternatively remove aarch64-unknown-linux-gnu from SUPPORTED_TRIPLES until that
toolchain support exists.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dcb43f31-9d69-4bad-8f1a-43e8ac6a5138
📒 Files selected for processing (1)
MODULE.bazel
| "AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR": "/usr/include", | ||
| "AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR": "/usr/lib/aarch64-linux-gnu", | ||
| "CFLAGS_aarch64_unknown_linux_gnu": "-I/usr/include/aarch64-linux-gnu", | ||
| "OPENSSL_INCLUDE_DIR": "/usr/include", | ||
| "OPENSSL_LIB_DIR": "/usr/lib/x86_64-linux-gnu", | ||
| "OPENSSL_NO_VENDOR": "1", | ||
| "X86_64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR": "/usr/include", | ||
| "X86_64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR": "/usr/lib/x86_64-linux-gnu", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect every supported target to have matching OpenSSL configuration.
rg -n -C 8 \
'SUPPORTED_TRIPLES|supported_platform_triples|unknown-linux|OPENSSL_(INCLUDE|LIB)_DIR|CFLAGS_' \
MODULE.bazelRepository: NVIDIA/nvcf
Length of output: 3455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- guidance files ---'
find .. -name AGENTS.md -o -name .oss-allowlist
printf '%s\n' '--- relevant MODULE.bazel section ---'
sed -n '200,245p' MODULE.bazel
printf '%s\n' '--- openssl-sys declarations and lock versions ---'
rg -n -C 3 'openssl-sys|openssl' \
src/control-plane-services/function-autoscaler/Cargo.lock \
src/control-plane-services/function-autoscaler/Cargo.toml \
MODULE.bazel 2>/dev/null || true
printf '%s\n' '--- target/environment coverage check ---'
python3 - <<'PY'
import re
from pathlib import Path
text = Path("MODULE.bazel").read_text()
triples = re.search(r"SUPPORTED_TRIPLES\s*=\s*\[(.*?)\]", text, re.S).group(1)
targets = re.findall(r'"([^"]+)"', triples)
env_block = re.search(r'build_script_env\s*=\s*\{(.*?)\n\s*\}', text, re.S).group(1)
keys = set(re.findall(r'"([^"]+)"\s*:', env_block))
def target_key(triple, suffix):
return triple.replace("-", "_").upper() + suffix
for triple in targets:
target = triple.replace("-", "_").upper()
matches = sorted(k for k in keys if k.startswith(target + "_OPENSSL_") or k.startswith("CFLAGS_" + triple))
print(f"{triple}: {matches}")
print("generic:", sorted(k for k in keys if k in {"OPENSSL_INCLUDE_DIR", "OPENSSL_LIB_DIR", "OPENSSL_NO_VENDOR"}))
PYRepository: NVIDIA/nvcf
Length of output: 12099
🌐 Web query:
openssl-sys build main.rs target-specific OPENSSL_LIB_DIR environment variable precedence generic OPENSSL_LIB_DIR
💡 Result:
In the openssl-sys build process, environment variables are used to point the build script toward an OpenSSL installation, overriding automatic detection mechanisms like pkg-config, Homebrew, or vcpkg [1][2][3]. Precedence and Behavior When configuring paths, the build script prioritizes specific manual overrides. If both OPENSSL_LIB_DIR and OPENSSL_INCLUDE_DIR are provided, they take precedence and are used directly to locate the libraries and headers [4][5]. If these specific variables are not provided, the build script looks for OPENSSL_DIR [4][5]. If OPENSSL_DIR is set, the build script assumes a standard layout, looking for headers in the include subdirectory and libraries in either the lib or lib64 subdirectories of the specified path [4][5]. Target-Specific Precedence Environment variables for openssl-sys can be prefixed with an upper-cased target architecture (e.g., X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR) [1][2][3]. These target-specific variables take precedence over the generic, non-prefixed versions of the same variable [2]. For example, if both X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR and OPENSSL_DIR are defined, the build script will prefer the target-specific version for that specific build target [2]. Summary of Variable Roles - OPENSSL_LIB_DIR: Specifies the exact directory containing OpenSSL library files (e.g.,.so,.a,.lib). - OPENSSL_INCLUDE_DIR: Specifies the exact directory containing OpenSSL header files. - OPENSSL_DIR: Specifies the root directory of an OpenSSL installation, expecting standard lib/lib64 and include subdirectories. - Target-Prefixed Variables: (e.g., [TARGET]_OPENSSL_DIR) Overrides generic versions to allow for precise cross-compilation configuration [2].
Citations:
- 1: https://docs.rs/openssl/latest/openssl/index.html
- 2: https://docs.rs/crate/openssl-sys/0.9.25
- 3: https://docs.rs/openssl/latest/openssl/
- 4: https://docs.rs/crate/openssl-sys/latest/source/build/find_normal.rs
- 5: https://github.com/sfackler/rust-openssl/blob/openssl-v0.10.66/openssl-sys/build/find_normal.rs
Define OpenSSL variables for the supported macOS targets.
x86_64-apple-darwin and aarch64-apple-darwin have no target-specific variables and can inherit the Linux OPENSSL_LIB_DIR. Add matching macOS configuration or remove those targets from SUPPORTED_TRIPLES.
🤖 Prompt for 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.
In `@MODULE.bazel` around lines 232 - 239, Update the OpenSSL environment
configuration in the supported-targets section to define target-specific include
and library directory variables for both x86_64-apple-darwin and
aarch64-apple-darwin, rather than allowing them to inherit Linux paths;
alternatively remove those macOS triples from SUPPORTED_TRIPLES if they are not
supported.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI -t f \
'Dockerfile.*|.*\.bazelrc|.*\.bzl|BUILD.*|.*\.ya?ml|.*\.sh' . -0 |
xargs -0 -r rg -n -C 3 \
'aarch64-linux-gnu|x86_64-linux-gnu|libssl|libcrypto|openssl|remote|sandbox'Repository: NVIDIA/nvcf
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- guidance files ---'
fd -HI -t f 'AGENTS.md|.oss-allowlist' . -0 | xargs -0 -r -n1 printf '%s\n'
printf '%s\n' '--- MODULE.bazel context ---'
cat -n MODULE.bazel | sed -n '190,255p'
printf '%s\n' '--- relevant declarations and uses ---'
rg -n -C 8 \
'build_script_env|build_script_data|OPENSSL|openssl|aarch64-linux-gnu|x86_64-linux-gnu|rules_rust' \
MODULE.bazel MODULE.bazel.lock .bazelrc .github tools rules src \
-g 'MODULE.bazel' -g '*.bzl' -g '*.bazel' -g '.bazelrc' -g '*.yml' -g '*.yaml' -g '*.sh' \
| head -n 1200
printf '%s\n' '--- repository layout relevant to Bazel ---'
git ls-files '*MODULE.bazel' '*BUILD.bazel' '*BUILD' '*WORKSPACE*' '.bazelrc' | head -n 300Repository: NVIDIA/nvcf
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- guidance files ---'
while IFS= read -r -d '' f; do
printf '%s\n' "$f"
done < <(fd -HI -t f 'AGENTS.md|.oss-allowlist' . -0)
printf '%s\n' '--- MODULE.bazel context ---'
cat -n MODULE.bazel | sed -n '190,255p'
printf '%s\n' '--- relevant declarations and uses ---'
rg -n -C 8 \
'build_script_env|build_script_data|OPENSSL|openssl|aarch64-linux-gnu|x86_64-linux-gnu|rules_rust' \
MODULE.bazel MODULE.bazel.lock .bazelrc .github tools rules src \
-g 'MODULE.bazel' -g '*.bzl' -g '*.bazel' -g '.bazelrc' -g '*.yml' -g '*.yaml' -g '*.sh' \
| head -n 1200
printf '%s\n' '--- repository layout relevant to Bazel ---'
git ls-files '*MODULE.bazel' '*BUILD.bazel' '*BUILD' '*WORKSPACE*' '.bazelrc' | head -n 300Repository: NVIDIA/nvcf
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- guidance files ---'
fd -HI -t f 'AGENTS.md|.oss-allowlist' . || true
printf '%s\n' '--- MODULE.bazel context ---'
cat -n MODULE.bazel | sed -n '190,255p'
printf '%s\n' '--- relevant declarations and uses ---'
rg -n -C 8 \
'build_script_env|build_script_data|OPENSSL|openssl|aarch64-linux-gnu|x86_64-linux-gnu|rules_rust' \
MODULE.bazel MODULE.bazel.lock .bazelrc .github tools rules src \
-g 'MODULE.bazel' -g '*.bzl' -g '*.bazel' -g '.bazelrc' -g '*.yml' -g '*.yaml' -g '*.sh' \
| head -n 1200 || true
printf '%s\n' '--- repository layout relevant to Bazel ---'
git ls-files '*MODULE.bazel' '*BUILD.bazel' '*BUILD' '*WORKSPACE*' '.bazelrc' | head -n 300Repository: NVIDIA/nvcf
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- guidance files ---'
fd -HI -t f 'AGENTS.md|.oss-allowlist' . || true
printf '%s\n' '--- MODULE.bazel lines 210-250 ---'
cat -n MODULE.bazel | sed -n '210,250p'
printf '%s\n' '--- MODULE.bazel relevant identifiers ---'
rg -n -C 12 \
'build_script_env|build_script_data|OPENSSL|openssl|aarch64-linux-gnu|x86_64-linux-gnu|crate_universe|rules_rust' \
MODULE.bazel || true
printf '%s\n' '--- relevant source files outside generated lock data ---'
rg -l \
'build_script_env|build_script_data|OPENSSL|openssl|aarch64-linux-gnu|x86_64-linux-gnu' \
--glob '!MODULE.bazel.lock' \
--glob '!**/bazel-*' \
--glob '!**/external/**' \
. | head -n 200 || true
printf '%s\n' '--- Bazel configuration and CI references ---'
rg -n -C 5 \
'remote_cache|sandbox|build_script_env|OPENSSL|openssl|aarch64-linux-gnu|x86_64-linux-gnu' \
.bazelrc .github tools \
--glob '!**/bazel-*' | head -n 800 || trueRepository: NVIDIA/nvcf
Length of output: 46599
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- root guidance ---'
cat -n AGENTS.md | sed -n '1,240p'
printf '%s\n' '--- generated openssl-sys rule from MODULE.bazel.lock ---'
python3 - <<'PY'
import json
from pathlib import Path
lock = json.loads(Path("MODULE.bazel.lock").read_text())
found = 0
def walk(value):
global found
if isinstance(value, dict):
text = value.get("build_file_content")
if isinstance(text, str) and "crate_name = \"openssl-sys\"" in text:
found += 1
lines = text.splitlines()
for i, line in enumerate(lines):
if (
"crate_name = \"openssl-sys\"" in line
or "cargo_build_script(" in line
or "build_script_env" in line
or "data =" in line
or "compile_data =" in line
or "openssl" in line.lower()
):
lo = max(0, i - 8)
hi = min(len(lines), i + 25)
print(f"--- match {found}, lines {lo + 1}-{hi} ---")
print("\n".join(f"{j + 1:4}: {lines[j]}" for j in range(lo, hi)))
print()
for child in value.values():
walk(child)
elif isinstance(value, list):
for child in value:
walk(child)
walk(lock)
print(f"matches={found}")
PY
printf '%s\n' '--- server BUILD and build script references ---'
cat -n src/control-plane-services/function-autoscaler/crates/server/BUILD.bazel | sed -n '1,240p'
rg -n -C 8 'openssl|openssl-sys|cargo_build_script|build_script_env|build_script_data' \
src/control-plane-services/function-autoscaler \
--glob '!Cargo.lock' --glob '!**/target/**' | head -n 800 || true
printf '%s\n' '--- CI image and host OpenSSL setup references ---'
fd -HI -t f 'Dockerfile|Dockerfile.*|.*\.yml|.*\.yaml|.*\.sh' .github tools . \
| rg 'bazel|ci|runner|Dockerfile' \
| head -n 300
rg -n -C 8 'bazel-ci|apt-get.*openssl|libssl-dev|aarch64.*libssl|x86_64.*libssl|/usr/lib/aarch64-linux-gnu|/usr/include/aarch64-linux-gnu' \
.github tools . \
--glob '!MODULE.bazel.lock' \
--glob '!**/vendor/**' | head -n 1000 || trueRepository: NVIDIA/nvcf
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- openssl-sys generated metadata ---'
rg -n -m 40 -C 8 'openssl_sys|openssl-sys|OPENSSL_LIB_DIR|AARCH64_UNKNOWN_LINUX_GNU' \
MODULE.bazel.lock \
--glob 'MODULE.bazel.lock' || true
printf '%s\n' '--- function-autoscaler Bazel documentation and targets ---'
cat -n src/control-plane-services/function-autoscaler/README.md | sed -n '1,75p'
rg -n -C 10 \
'function-autoscaler|autoscaler|aarch64|arm64|linux/amd64|linux/arm64|bazel build|bazel test' \
.github tools src/control-plane-services/function-autoscaler \
--glob '!Cargo.lock' --glob '!MODULE.bazel.lock' --glob '!**/vendor/**' \
| head -n 1000 || true
printf '%s\n' '--- all root-level Rust build configuration ---'
rg -n -C 8 \
'rust.toolchain|crate.annotation|supported_platform_triples|build_script_env|OPENSSL' \
MODULE.bazel .bazelrc BAZEL.md \
--glob '!MODULE.bazel.lock' || true
printf '%s\n' '--- current host paths, if present ---'
for p in \
/usr/include \
/usr/include/aarch64-linux-gnu \
/usr/lib/x86_64-linux-gnu \
/usr/lib/aarch64-linux-gnu \
/usr/lib/x86_64-linux-gnu/libssl.so \
/usr/lib/aarch64-linux-gnu/libssl.so
do
if [ -e "$p" ]; then
printf 'present %s\n' "$p"
else
printf 'missing %s\n' "$p"
fi
doneRepository: NVIDIA/nvcf
Length of output: 50368
Make the openssl-sys dependency hermetic for aarch64-unknown-linux-gnu. The repository documents that the arm64 cross-compile fails because the annotation uses the amd64 OpenSSL path. Add Bazel-managed per-platform OpenSSL inputs, or remove aarch64 from SUPPORTED_TRIPLES until the toolchain provides the required sysroot.
🤖 Prompt for 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.
In `@MODULE.bazel` around lines 232 - 239, The OpenSSL configuration in the Bazel
environment uses the x86_64 library path for aarch64 builds, making openssl-sys
non-hermetic. Update the aarch64-specific settings in MODULE.bazel to reference
Bazel-managed arm64 OpenSSL headers and libraries, including the required
per-platform inputs; alternatively remove aarch64-unknown-linux-gnu from
SUPPORTED_TRIPLES until that toolchain support exists.
There was a problem hiding this comment.
First: Resolved by removing "OPENSSL_LIB_DIR": "/usr/lib/x86_64-linux-gnu".
Second: We’ve broadened the compatibility to the practical point without overcomplicating.
There was a problem hiding this comment.
@borao, confirmed. Removing the generic OPENSSL_LIB_DIR prevents macOS targets from inheriting the x86_64 Linux library path.
I acknowledge the practical compatibility scope for the second finding.
You are interacting with an AI system.
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
TL;DR
Restore the target-specific OpenSSL paths dropped during the root Bazel module
consolidation, unblocking the function-autoscaler ARM64 image build.
Additional Details
#533 configured
openssl-sysfor both AMD64 and ARM64. When #593 moved theautoscaler into the root Bazel module, that
crate.annotationwas nottransferred.
The omission caused the manual image push to fail because
AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIRwas unset:https://github.com/NVIDIA/nvcf/actions/runs/30954727158
This restores the previous configuration without changing service code,
dependencies, or lockfiles.
For the Reviewer
Please verify the restored annotation in
MODULE.bazelmatches the Bazel CIimage’s AMD64 and ARM64 OpenSSL paths.
For QA
git diff --checkpassed.ARM64 OpenSSL sysroot.
Issues
Relates to #527
Checklist
Summary by CodeRabbit