diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 2dc0edfa5..8585173a5 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -345,7 +345,7 @@ jobs: # Bazelisk then selects the root .bazelversion release. Update this tag # only after the corresponding internal image has been published and # mirrored. - image: ${{ vars.BAZEL_CI_IMAGE || 'ghcr.io/nvidia/nvcf/bazel-ci:0.13.0' }} + image: ${{ vars.BAZEL_CI_IMAGE || 'ghcr.io/nvidia/nvcf/bazel-ci:0.14.0' }} credentials: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/chart-push-manual.yml b/.github/workflows/chart-push-manual.yml index f1b7f01cd..d77b32281 100644 --- a/.github/workflows/chart-push-manual.yml +++ b/.github/workflows/chart-push-manual.yml @@ -34,7 +34,7 @@ jobs: name: package + push chart to ncp-dev runs-on: ubuntu-latest container: - image: ${{ vars.BAZEL_CI_IMAGE || 'ghcr.io/nvidia/nvcf/bazel-ci:0.13.0' }} + image: ${{ vars.BAZEL_CI_IMAGE || 'ghcr.io/nvidia/nvcf/bazel-ci:0.14.0' }} defaults: run: # In container jobs Actions falls back to plain `sh` (dash), which diff --git a/.github/workflows/image-push-manual.yml b/.github/workflows/image-push-manual.yml index 6555c5d09..f0b710a47 100644 --- a/.github/workflows/image-push-manual.yml +++ b/.github/workflows/image-push-manual.yml @@ -39,7 +39,7 @@ jobs: name: push to ncp-dev runs-on: ubuntu-latest container: - image: ${{ vars.BAZEL_CI_IMAGE || 'ghcr.io/nvidia/nvcf/bazel-ci:0.13.0' }} + image: ${{ vars.BAZEL_CI_IMAGE || 'ghcr.io/nvidia/nvcf/bazel-ci:0.14.0' }} defaults: run: # In container jobs Actions falls back to plain `sh` (dash), which diff --git a/src/control-plane-services/function-autoscaler/MODULE.bazel b/src/control-plane-services/function-autoscaler/MODULE.bazel index 933283019..f349d4c8a 100644 --- a/src/control-plane-services/function-autoscaler/MODULE.bazel +++ b/src/control-plane-services/function-autoscaler/MODULE.bazel @@ -24,9 +24,8 @@ No AWS / S3 deps -- skips the aws-lc-sys crate_universe issue that invocation-service had to work around. openssl-sys is built against the system libssl via `crate.annotation` (see below); the `vendored` cargo feature is OFF because the OpenSSL source-tree build under the -crate_universe sandbox fails to locate its own tarball. Trade-off: -single-arch (linux/amd64) only until per-platform libssl paths are -wired in. +crate_universe sandbox fails to locate its own tarball. Per-platform +libssl paths are wired in, so this builds linux/amd64 and linux/arm64. The umbrella nvcf/nvcf consumes this subtree under src/control-plane-services/function-autoscaler; this file is also picked up @@ -44,6 +43,11 @@ module( bazel_dep(name = "platforms", version = "1.0.0") bazel_dep(name = "rules_oci", version = "2.2.7") bazel_dep(name = "rules_pkg", version = "1.2.0") + +# sh_test stopped being a native global in Bazel 9; the image platform-coverage +# test in rules/oci/private/common.bzl loads it from here. Matches the root +# module's version. +bazel_dep(name = "rules_shell", version = "0.8.0") bazel_dep(name = "aspect_bazel_lib", version = "2.19.3") bazel_dep(name = "hermetic_cc_toolchain", version = "4.1.0") bazel_dep(name = "rules_python", version = "1.4.1") @@ -111,11 +115,46 @@ crate.from_cargo( # machines need Homebrew openssl@3 -- if anyone hits this, point # OPENSSL_LIB_DIR/OPENSSL_INCLUDE_DIR at /opt/homebrew/opt/openssl@3 # via user.bazelrc or a wrapper. +# openssl-sys resolves its library paths from environment variables because +# neither pkg-config nor env-based discovery works inside the Bazel sandbox. +# +# The target-prefixed forms matter for cross-compilation. openssl-sys checks +# _OPENSSL_LIB_DIR before the unprefixed name, so without the aarch64 +# entries an arm64 build silently falls back to the x86_64 paths below and +# links host libraries into an aarch64 artifact. That is what previously made +# this service amd64-only: +# +# AARCH64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR unset +# OPENSSL_LIB_DIR = /usr/lib/x86_64-linux-gnu +# +# Vendoring is not an option here: openssl-src's bundled source tree is not +# materialised in the crate_universe sandbox, so the build fails with +# "openssl-src-.../openssl: No such file or directory" with or without a +# build_script_data annotation. Hence explicit per-target system paths. +# +# Both architectures' libssl must exist in the build image. bazel-ci installs +# libssl-dev and libssl-dev:arm64 for exactly this reason; a bump to that image +# is required alongside this change. +# +# CFLAGS_aarch64_unknown_linux_gnu covers a second, subtler split. libssl-dev is +# Multi-Arch: same, so its 142 arch-independent headers are shared at +# /usr/include/openssl/ while opensslconf.h and configuration.h are per-arch +# under /usr/include//openssl/. Host gcc searches its own triplet +# directory implicitly, which is why amd64 never needed this; the zig toolchain +# used for arm64 is hermetic and searches neither. OPENSSL_INCLUDE_DIR takes a +# single directory and cannot express the pair, so the triplet path is added +# through the cc crate's target-suffixed CFLAGS instead. Without it the build +# fails on `#include `. crate.annotation( build_script_env = { - "OPENSSL_LIB_DIR": "/usr/lib/x86_64-linux-gnu", + "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", }, crate = "openssl-sys", ) @@ -152,12 +191,16 @@ oci.pull( image = "nvcr.io/nvidia/distroless/cc", platforms = [ "linux/amd64", - "linux/arm64", + # arm64/v8, not arm64: nvcr.io/nvidia/distroless/cc publishes its + # aarch64 manifest under the v8 variant and rules_oci matches the + # platform string exactly, so "linux/arm64" fails the pull outright. + # http-invocation pins the same image and digest with arm64/v8. + "linux/arm64/v8", ], ) use_repo( oci, "distroless_cc", "distroless_cc_linux_amd64", - "distroless_cc_linux_arm64", + "distroless_cc_linux_arm64_v8", ) diff --git a/src/control-plane-services/function-autoscaler/rules/oci/private/BUILD.bazel b/src/control-plane-services/function-autoscaler/rules/oci/private/BUILD.bazel index ad5f27491..c96ec60f6 100644 --- a/src/control-plane-services/function-autoscaler/rules/oci/private/BUILD.bazel +++ b/src/control-plane-services/function-autoscaler/rules/oci/private/BUILD.bazel @@ -1,4 +1,24 @@ # SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +load("@rules_shell//shell:sh_test.bzl", "sh_test") + package(default_visibility = ["//rules/oci:__subpackages__"]) + +# Referenced by the platform-coverage test that create_oci_image generates for +# every image, so it must be visible wherever an image is defined, not just +# under //rules/oci. +exports_files( + ["image_index_platforms_test.sh"], + visibility = ["//visibility:public"], +) + +# Tests the guard itself against synthetic OCI layouts. The platform-coverage +# test only runs against a real image index, which is always well-formed when +# the build is healthy, so nothing there would notice the guard silently +# passing on a broken index. This asserts it actually fails. +sh_test( + name = "image_index_platforms_selftest", + srcs = ["test_image_index_platforms.sh"], + data = ["image_index_platforms_test.sh"], +) diff --git a/src/control-plane-services/function-autoscaler/rules/oci/private/common.bzl b/src/control-plane-services/function-autoscaler/rules/oci/private/common.bzl index 70923b1d5..13f1ba16a 100644 --- a/src/control-plane-services/function-autoscaler/rules/oci/private/common.bzl +++ b/src/control-plane-services/function-autoscaler/rules/oci/private/common.bzl @@ -6,19 +6,28 @@ load("@aspect_bazel_lib//lib:expand_template.bzl", "expand_template") load("@aspect_bazel_lib//lib:transitions.bzl", "platform_transition_filegroup") load("@rules_oci//oci:defs.bzl", "oci_image", "oci_image_index", "oci_load", "oci_push") +load("@rules_shell//shell:sh_test.bzl", "sh_test") load("//rules/oci:transition.bzl", "multi_arch") DEFAULT_BASE = "@distroless_cc" -# linux_x86_64 only for now. linux_arm64 builds fail at openssl-sys's -# build script because the crate.annotation in MODULE.bazel points at -# /usr/lib/x86_64-linux-gnu/libssl.so; under the zig cc cross-compile -# to aarch64 that path is wrong. Re-add linux_arm64 once we either -# vendor libssl via http_archive + multi-arch crate.annotation, or -# switch the scylla driver's TLS feature to rustls. DEFAULT_PLATFORMS = [ + "//platforms:linux_arm64", "//platforms:linux_x86_64", ] +# Architectures every published image index must carry. +# +# Deliberately a separate constant rather than something derived from +# DEFAULT_PLATFORMS. Derived expectations cannot catch a platform being dropped: +# removing arm64 from DEFAULT_PLATFORMS would remove it from the expectation +# too, the index would shrink, and the test would still pass. That is exactly +# how this service shipped an amd64-only image for months without CI noticing. +# Changing this list is a policy decision and should be visible in review. +REQUIRED_ARCHES = [ + "amd64", + "arm64", +] + COMMON_LAYERS = [] def create_oci_image( @@ -85,6 +94,16 @@ def create_oci_image( tags = all_tags, ) + # Guards the published manifest, not just the build. Note this test is NOT + # tagged "manual": the image targets are, so `bazel test //...` would skip + # this too and the guard would be inert. + sh_test( + name = name + "_platforms_test", + srcs = ["//rules/oci/private:image_index_platforms_test.sh"], + args = ["$(location {}_index)".format(name)] + REQUIRED_ARCHES, + data = [name + "_index"], + ) + load_name = name + "_load" oci_load( name = load_name, diff --git a/src/control-plane-services/function-autoscaler/rules/oci/private/image_index_platforms_test.sh b/src/control-plane-services/function-autoscaler/rules/oci/private/image_index_platforms_test.sh new file mode 100755 index 000000000..81e942f64 --- /dev/null +++ b/src/control-plane-services/function-autoscaler/rules/oci/private/image_index_platforms_test.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Assert that an OCI image index carries exactly the expected architectures. +# +# Why this exists: this service shipped an amd64-only image index for months and +# nothing noticed. The build stayed green the whole time because dropping a +# platform is a configuration edit, not a compile error -- DEFAULT_PLATFORMS lost +# an entry and every downstream target happily built the smaller index. The +# arm64 half was missing from the published manifest and only a human reading +# the registry would have caught it. +# +# The expected architectures are passed in as policy, deliberately NOT derived +# from DEFAULT_PLATFORMS. Deriving them would make this test restate whatever +# the build already decided: removing arm64 from DEFAULT_PLATFORMS would also +# remove it from the expectation and the test would pass, which is precisely the +# regression it is here to catch. +# +# Both directions are checked. A missing architecture is the known failure, and +# an unexpected extra one means the index gained a platform nobody declared. +# +# Parsed with grep and sed rather than jq so the test stays hermetic, matching +# java_image_contract_test.sh. +set -euo pipefail + +if [[ "$#" -lt 2 ]]; then + echo "usage: $0 [...]" >&2 + exit 1 +fi + +index_dir="$1" +shift +expected=("$@") + +if [[ ! -f "${index_dir}/index.json" ]]; then + echo "not an OCI layout (no index.json): ${index_dir}" >&2 + ls -la "${index_dir}" >&2 || true + exit 1 +fi + +blob_for() { # blob_for + printf '%s/blobs/%s/%s' "${index_dir}" "${1%%:*}" "${1#*:}" +} + +# Resolve the manifest list, tolerating either OCI layout shape. +# +# rules_oci currently writes index.json with a SINGLE descriptor pointing at a +# nested image-index blob: oci/private/image_index.sh.tpl sets +# .manifests = [{"mediaType": "application/vnd.oci.image.index.v1+json", ...}] +# so the platform descriptors live one level down. The OCI spec also permits +# them inline in index.json, and other tooling emits that flatter form. +# +# Detect rather than assume. Hardcoding the nested walk would make this guard +# quietly stop finding anything if the ruleset ever switched shapes: with no +# architectures found it would report a missing platform, or worse, a future +# edit "fixing" that could turn it into a pass. Presence of a platform +# architecture in index.json is what distinguishes the two. +index_flat="$(tr -d ' \n' < "${index_dir}/index.json")" + +if printf '%s' "${index_flat}" | grep -q '"architecture":"'; then + list_flat="${index_flat}" +else + top_digest="$(printf '%s' "${index_flat}" \ + | grep -o '"digest":"sha256:[0-9a-f]*"' | head -1 | cut -d'"' -f4)" + if [[ -z "${top_digest}" ]]; then + echo "index.json has neither inline platforms nor a manifest descriptor" >&2 + cat "${index_dir}/index.json" >&2 + exit 1 + fi + + manifest_list="$(blob_for "${top_digest}")" + if [[ ! -f "${manifest_list}" ]]; then + echo "index.json points at a missing blob: ${top_digest}" >&2 + exit 1 + fi + list_flat="$(tr -d ' \n' < "${manifest_list}")" +fi + +# Split the manifests array into one entry per line so an architecture is only +# ever read from the entry that declares it, and a digest is only ever read from +# the same entry as its architecture. +entries="$(printf '%s' "${list_flat}" | sed 's/}, *{/}\n{/g')" + +found=() +while IFS= read -r arch; do + [[ -n "${arch}" ]] && found+=("${arch}") +done < <(printf '%s' "${entries}" \ + | grep -o '"architecture":"[a-z0-9]*"' | cut -d'"' -f4 | sort -u) + +if [[ "${#found[@]}" -eq 0 ]]; then + echo "image index declares no architectures at all" >&2 + printf '%s\n' "${list_flat}" >&2 + exit 1 +fi + +status=0 + +for want in "${expected[@]}"; do + hit=false + for got in "${found[@]}"; do + [[ "${got}" == "${want}" ]] && hit=true && break + done + if [[ "${hit}" != "true" ]]; then + echo "image index is missing the ${want} manifest" >&2 + status=1 + continue + fi + + # Present in the list is not enough. Follow the descriptor to the config + # blob and confirm the image itself declares that architecture: an entry + # can be filed under one platform while pointing at another image, which + # pushes without complaint and then runs the wrong binary on that host. + entry="$(printf '%s' "${entries}" | grep -F "\"architecture\":\"${want}\"" | head -1)" + man_digest="$(printf '%s' "${entry}" \ + | grep -o '"digest":"sha256:[0-9a-f]*"' | head -1 | cut -d'"' -f4)" + man_blob="$(blob_for "${man_digest}")" + if [[ ! -f "${man_blob}" ]]; then + echo "${want}: manifest blob is missing: ${man_digest}" >&2 + status=1 + continue + fi + + cfg_digest="$(tr -d ' \n' < "${man_blob}" \ + | grep -o '"config":{[^}]*}' \ + | grep -o '"digest":"sha256:[0-9a-f]*"' | head -1 | cut -d'"' -f4)" + cfg_blob="$(blob_for "${cfg_digest}")" + if [[ ! -f "${cfg_blob}" ]]; then + echo "${want}: config blob is missing: ${cfg_digest}" >&2 + status=1 + continue + fi + + if ! tr -d ' \n' < "${cfg_blob}" | grep -F "\"architecture\":\"${want}\"" >/dev/null; then + echo "${want}: entry points at a config declaring a different architecture" >&2 + tr -d ' \n' < "${cfg_blob}" | grep -o '"architecture":"[a-z0-9]*"' >&2 || true + status=1 + continue + fi + + echo "ok: ${want} manifest present and self-consistent" +done + +for got in "${found[@]}"; do + hit=false + for want in "${expected[@]}"; do + [[ "${got}" == "${want}" ]] && hit=true && break + done + if [[ "${hit}" != "true" ]]; then + echo "image index declares an unexpected architecture: ${got}" >&2 + status=1 + fi +done + +if [[ "${status}" -ne 0 ]]; then + echo "expected architectures: ${expected[*]}" >&2 + echo "index declares: ${found[*]}" >&2 + exit 1 +fi diff --git a/src/control-plane-services/function-autoscaler/rules/oci/private/test_image_index_platforms.sh b/src/control-plane-services/function-autoscaler/rules/oci/private/test_image_index_platforms.sh new file mode 100755 index 000000000..4b01feeaf --- /dev/null +++ b/src/control-plane-services/function-autoscaler/rules/oci/private/test_image_index_platforms.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Behavioral test for image_index_platforms_test.sh. +# +# A guard that only ever passes is indistinguishable from no guard. The bug this +# was written for -- an index silently losing its arm64 half -- is exactly the +# case that must fail, so assert the failures directly rather than only the +# happy path. Each case builds a synthetic OCI layout so these stay true +# independently of what the real image currently contains. +set -euo pipefail + +script="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/image_index_platforms_test.sh" +fail=0 + +# Digests must be real 64-char lowercase hex: the guard matches +# sha256:[0-9a-f]* and would reject a readable placeholder, making every case +# fail for the wrong reason. Derived deterministically from a label so each blob +# gets a distinct, stable name. +digest_for() { # digest_for