diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 211ebd140..18aa4eaf9 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -63,3 +63,8 @@ jobs: - name: Check Go toolchain declarations agree run: tools/ci/check-go-version + + - name: Check nested Bazel module inventory + run: | + bash tools/ci/test-check-nested-modules + bash tools/ci/check-nested-modules diff --git a/tools/ci/check-nested-modules b/tools/ci/check-nested-modules new file mode 100755 index 000000000..da3369292 --- /dev/null +++ b/tools/ci/check-nested-modules @@ -0,0 +1,136 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Guard the nested-module inventory during the Bazel consolidation. +# +# Every nested MODULE.bazel must be classified in tools/ci/nested-modules.txt. +# The check fails in both directions: +# +# unlisted module A nested MODULE.bazel exists that the ledger does not +# mention. Usually this means a new service arrived with its +# own module, which is the thing consolidation is trying to +# stop. The fix is normally to build from the root module, +# not to add a ledger line. +# +# stale entry The ledger names a module that no longer exists. This is +# what a completed migration looks like, and the entry has +# to go, or the file drifts into describing a repository +# that no longer exists. +# +# Why a ledger and not one allowlist: the plan separates vendored paths, +# not-yet-migrated modules, and permanent exceptions, because only the last +# carries an exception contract. Collapsing them would make every outstanding +# migration look like an accepted exception. +# +# Usage: check-nested-modules [--root DIR] [--ledger FILE] +set -euo pipefail + +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +ledger="" + +while [ "$#" -gt 0 ]; do + case "$1" in + --root) root="$2"; shift 2 ;; + --ledger) ledger="$2"; shift 2 ;; + *) echo "unknown argument: $1" >&2; exit 2 ;; + esac +done +[ -n "${ledger}" ] || ledger="${root}/tools/ci/nested-modules.txt" + +if [ ! -f "${ledger}" ]; then + echo "ledger not found: ${ledger}" >&2 + exit 2 +fi + +# Declared paths, by category. +declared_all="$(mktemp)"; declared_ledger="$(mktemp)" +declared_exception="$(mktemp)"; found="$(mktemp)" +trap 'rm -f "${declared_all}" "${declared_ledger}" "${declared_exception}" "${found}" "${found}.raw"' EXIT + +lineno=0 +while read -r category path _rest; do + lineno=$((lineno + 1)) + case "${category}" in + ''|\#*) continue ;; + esac + # A row naming a category but no path is a malformed ledger, not an empty + # one. Skipping it would let a typo silently shrink the guard's coverage. + if [ -z "${path}" ]; then + echo "${ledger}:${lineno}: category '${category}' with no path" >&2 + exit 2 + fi + case "${category}" in + vendored|scaffolding) ;; + ledger) printf '%s\n' "${path}" >> "${declared_ledger}" ;; + exception) printf '%s\n' "${path}" >> "${declared_exception}" ;; + *) + echo "${ledger}:${lineno}: unknown category '${category}' for ${path}" >&2 + exit 2 + ;; + esac + printf '%s\n' "${path}" >> "${declared_all}" +done < "${ledger}" + +# One path, one category. Without this a repeated `ledger svc/a` inflates the +# backlog count, and the same path classified both `ledger` and `exception` +# inflates both while still passing, which would misreport how much of the +# migration is actually outstanding. +dupes="$(sort "${declared_all}" | uniq -d)" +if [ -n "${dupes}" ]; then + { + echo "error: ${ledger#"${root}"/} classifies the same path more than once:" + printf ' %s\n' ${dupes} + } >&2 + exit 2 +fi + +sort -u -o "${declared_all}" "${declared_all}" + +# Actual nested modules. The root module is the destination, not a nested one. +# Discovery failures must terminate: an unreadable tree that yields a partial +# inventory would quietly narrow what the guard can see, so `.git` is pruned +# explicitly rather than suppressing find's errors. +if ! ( cd "${root}" && find . -name .git -prune -o \ + -name MODULE.bazel ! -path './MODULE.bazel' -printf '%h\n' ) > "${found}.raw"; then + echo "error: failed to enumerate nested modules under ${root}" >&2 + exit 2 +fi +sed 's|^\./||' "${found}.raw" | sort -u > "${found}" + +fail=0 + +unlisted="$(comm -23 "${found}" "${declared_all}")" +if [ -n "${unlisted}" ]; then + fail=1 + { + echo "error: nested MODULE.bazel not classified in ${ledger#"${root}"/}:" + printf ' %s\n' ${unlisted} + echo + echo "A new nested module works against the consolidation. Prefer building" + echo "from the root module. If it genuinely cannot, add it under the" + echo "category that describes it, and for 'exception' record the owner," + echo "the rationale, and the residual risk." + } >&2 +fi + +stale="$(comm -13 "${found}" "${declared_all}")" +if [ -n "${stale}" ]; then + fail=1 + { + echo "error: ${ledger#"${root}"/} lists modules that no longer exist:" + printf ' %s\n' ${stale} + echo + echo "If these migrated, delete their lines. The ledger is the migration" + echo "backlog, so a stale entry overstates how much work is left." + } >&2 +fi + +[ "${fail}" -eq 0 ] || exit 1 + +remaining=$(wc -l < "${declared_ledger}" 2>/dev/null || echo 0) +exceptions=$(wc -l < "${declared_exception}" 2>/dev/null || echo 0) +total=$(wc -l < "${found}") +echo "nested modules: ${total} classified" +echo " awaiting migration: ${remaining}" +echo " permanent exceptions: ${exceptions}" diff --git a/tools/ci/nested-modules.txt b/tools/ci/nested-modules.txt new file mode 100644 index 000000000..63bffd2fc --- /dev/null +++ b/tools/ci/nested-modules.txt @@ -0,0 +1,60 @@ +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Every nested MODULE.bazel in the repository, classified. tools/ci/check-nested-modules +# fails if a nested module exists that is not listed here, and fails if an entry +# here no longer exists. Both directions matter: the first stops new nested +# modules appearing by accident, the second stops this file drifting into +# fiction as migration proceeds. +# +# Format: +# +# Categories, in the sense docs/dev/bazel-consolidation.md gives them: +# +# vendored Third-party source vendored into the tree. Never in scope for +# consolidation, excluded from the count outright, and carries no +# exception contract because it is not ours to converge. +# +# scaffolding Exists to support the migration and is deleted when its +# consumers no longer need it. Not a service, so it is not +# counted as a module that must converge. +# +# ledger A first-party service module that has not migrated yet. This is +# the migration backlog. Entries leave this file entirely when the +# service moves to the root module. An entry here is a statement +# that the work is outstanding, not that it is excused. +# +# exception A first-party service that will keep its own module +# permanently. This is the only category that carries the +# exception contract in the plan: a recorded owner, a rationale, +# and the residual correctness risk in writing. Adding an entry +# here reduces the number of modules the end state converges, so +# it is a deliberate scope change, not a way to silence this +# check. + +vendored src/compute-plane-services/nvca/vendor/cel.dev/expr + +scaffolding rules/oci-destinations + +ledger src/compute-plane-services/byoo-otel-collector +ledger src/compute-plane-services/ess-agent +ledger src/compute-plane-services/image-credential-helper +ledger src/compute-plane-services/nvca +ledger src/compute-plane-services/nvcf-unbound +ledger src/compute-plane-services/worker-init +ledger src/compute-plane-services/worker-llm-credentials +ledger src/compute-plane-services/worker-task +ledger src/compute-plane-services/worker-utils +ledger src/control-plane-services/function-autoscaler +ledger src/control-plane-services/helm-reval +ledger src/control-plane-services/nats-auth-callout +ledger src/invocation-plane-services/grpc-proxy +ledger src/invocation-plane-services/http-invocation +ledger src/invocation-plane-services/llm-api-gateway +ledger src/invocation-plane-services/ratelimiter +ledger src/invocation-plane-services/vanity-gateway +ledger src/libraries/rust/stargate + +# No permanent exceptions today. nvsnap was the open question in Phase 1 and it +# migrated in NVIDIA/nvcf#471, so it is absent from this file entirely, which is +# what a completed migration looks like here. diff --git a/tools/ci/test-check-nested-modules b/tools/ci/test-check-nested-modules new file mode 100755 index 000000000..b89b8c754 --- /dev/null +++ b/tools/ci/test-check-nested-modules @@ -0,0 +1,158 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Behavioral test for tools/ci/check-nested-modules. +# +# A guard that only ever passes is indistinguishable from no guard, so assert +# the failures directly: an unlisted nested module must fail, and so must a +# ledger entry whose module is gone. Each case builds a throwaway tree rather +# than depending on the real repository, so these stay true as the real ledger +# shrinks. +set -euo pipefail + +script="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/check-nested-modules" +fail=0 + +# Build a fake repo: root MODULE.bazel plus a nested one per argument. +make_tree() { + local dir; dir="$(mktemp -d)" + touch "${dir}/MODULE.bazel" + local p + for p in "$@"; do + mkdir -p "${dir}/${p}" + touch "${dir}/${p}/MODULE.bazel" + done + printf '%s' "${dir}" +} + +run() { # run -> sets $out and $rc + local root="$1" contents="$2" ledger + ledger="$(mktemp)" + printf '%s\n' "${contents}" > "${ledger}" + set +e + out="$(bash "${script}" --root "${root}" --ledger "${ledger}" 2>&1)" + rc=$? + set -e + rm -f "${ledger}" +} + +expect() { # expect [substring] + local desc="$1" want="$2" substr="${3:-}" + if [ "${rc}" != "${want}" ]; then + printf 'FAIL %s: want exit %s, got %s\n%s\n' "${desc}" "${want}" "${rc}" "${out}" + fail=1 + return + fi + if [ -n "${substr}" ] && ! printf '%s' "${out}" | grep -q -- "${substr}"; then + printf 'FAIL %s: output missing %q\n%s\n' "${desc}" "${substr}" "${out}" + fail=1 + return + fi + printf 'ok %s\n' "${desc}" +} + +# Fully classified tree passes, and the root module is not counted as nested. +t="$(make_tree svc/a svc/b vendor/x)" +run "${t}" 'ledger svc/a +ledger svc/b +vendored vendor/x' +expect "classified tree passes" 0 "nested modules: 3 classified" +expect "counts only unmigrated as remaining" 0 "awaiting migration: 2" +rm -rf "${t}" + +# A new nested module that nobody declared must fail and be named. +t="$(make_tree svc/a svc/sneaky)" +run "${t}" 'ledger svc/a' +expect "unlisted nested module fails" 1 "svc/sneaky" +rm -rf "${t}" + +# A ledger entry whose module migrated must fail, so the backlog stays honest. +t="$(make_tree svc/a)" +run "${t}" 'ledger svc/a +ledger svc/already-migrated' +expect "stale ledger entry fails" 1 "svc/already-migrated" +rm -rf "${t}" + +# Both directions at once are both reported, not just the first. +t="$(make_tree svc/a svc/new)" +run "${t}" 'ledger svc/a +ledger svc/gone' +expect "reports unlisted and stale together" 1 "svc/new" +expect " ... and names the stale one too" 1 "svc/gone" +rm -rf "${t}" + +# Exceptions are classified but counted separately from the backlog. +t="$(make_tree svc/a svc/keeps-its-own)" +run "${t}" 'ledger svc/a +exception svc/keeps-its-own' +expect "exception passes the guard" 0 "permanent exceptions: 1" +expect "exception is not counted as backlog" 0 "awaiting migration: 1" +rm -rf "${t}" + +# Comments and blank lines are ignored rather than parsed as entries. +t="$(make_tree svc/a)" +run "${t}" '# a comment + +ledger svc/a +' +expect "comments and blanks ignored" 0 "nested modules: 1 classified" +rm -rf "${t}" + +# A typo in the category must be loud, not silently treated as classified. +t="$(make_tree svc/a)" +run "${t}" 'ledgar svc/a' +expect "unknown category is an error" 2 "unknown category" +rm -rf "${t}" + +# A category with no path is a malformed ledger, not an empty one. Skipping it +# would silently narrow what the guard covers. +t="$(make_tree svc/a)" +run "${t}" 'ledger svc/a +ledger' +expect "category with no path is an error" 2 "with no path" +rm -rf "${t}" + +# One path, one category. A repeat inflates the backlog count. +t="$(make_tree svc/a)" +run "${t}" 'ledger svc/a +ledger svc/a' +expect "duplicate path is an error" 2 "more than once" +rm -rf "${t}" + +# The same path in two categories would inflate both counts while passing. +t="$(make_tree svc/a)" +run "${t}" 'ledger svc/a +exception svc/a' +expect "conflicting categories are an error" 2 "more than once" +rm -rf "${t}" + +# Discovery must fail loudly. A root that does not exist cannot yield a +# trustworthy inventory, so it must not be reported as an empty one. +empty_ledger="$(mktemp)" +set +e +out="$(bash "${script}" --root /nonexistent-root-xyz --ledger "${empty_ledger}" 2>&1)"; rc=$? +set -e +rm -f "${empty_ledger}" +expect "unreadable root is an error" 2 "failed to enumerate" + +# A nested module inside .git must not be discovered (pruned, not suppressed). +t="$(make_tree svc/a)" +mkdir -p "${t}/.git/weird" && touch "${t}/.git/weird/MODULE.bazel" +run "${t}" 'ledger svc/a' +expect ".git contents are pruned" 0 "nested modules: 1 classified" +rm -rf "${t}" + +# A missing ledger is a setup error, distinct from a policy violation. +t="$(make_tree svc/a)" +set +e +out="$(bash "${script}" --root "${t}" --ledger "${t}/nope.txt" 2>&1)"; rc=$? +set -e +expect "missing ledger is a setup error" 2 "ledger not found" +rm -rf "${t}" + +if [ "${fail}" -ne 0 ]; then + echo "check-nested-modules: FAILED" >&2 + exit 1 +fi +echo "check-nested-modules: all checks passed"