fix: close controller-peers-env-fallback validation gaps (bug-1909..1912)

- bug-1912: xce_parse_peers() ignores whitespace around entries and skips
  blank ones, matching xceadmin's resolve_controllers(), so "a=h:1, b=h:2"
  names the second candidate "b" (5 libxce tests + regression case).
- bug-1911: xcecd frees the -l log dir before the -h / --version returns
  (regression case: no LeakSanitizer report, exit 0).
- bug-1910: both version scripts build the apiserver with -mod=vendor
  (as the meson build does) instead of -mod=readonly, so they no longer
  need a Go module cache.
- bug-1909: test-version-startup-consistency.sh accepts exactly the
  dirty-content-fingerprint pairing ('<sha>-<8 hex>' in --version with
  '<sha>-dirty' in the startup log), with self-test cases (e)/(f).

Both version scripts now exit 0 on a dirty tree with XCE_CONTROLLER_PEERS
set; regression script 35/35; xce-build all: C 127/0, scheduler 16/0,
xceadmin 364 passed, no ASAN reports.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-03 13:06:58 +00:00
parent 66520e6b1a
commit ef6e562cde
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
7 changed files with 159 additions and 9 deletions

View file

@ -517,13 +517,15 @@ fi
yellow "=== [apiserver] go build (direct invocation, see header comment re: bug-1597 / go_build's goproxy.cn hang in this sandbox) -- commit_id=$API_COMMIT_ID build_time=$API_BUILD_TIME dirty_fingerprint=$API_DIRTY_FINGERPRINT ==="
APISERVER_BIN="$APILIB_DIR/frontend/apiserver/xce_apiserver"
mkdir -p "$(dirname "$APISERVER_BIN")"
# bug-1910: -mod=vendor builds from the committed vendor/ tree (as the
# canonical meson build does), so no Go module cache is needed.
nix develop "$ISO_SRC" --command bash -c "
set -e
cd '$ISO_SRC/frontend/apiserver'
CGO_CFLAGS=\"-g \$(pkg-config --cflags librtti c-capnproto openssl tinylog) -I$ISO_SRC/include -I$APILIB_DIR/lib\"
CGO_LDFLAGS=\"-g \$(pkg-config --libs librtti c-capnproto openssl tinylog) -lm -lrt -L$APILIB_DIR/lib -lxce\"
GOPROXY=off CGO_ENABLED=1 CGO_CFLAGS=\"\$CGO_CFLAGS\" CGO_LDFLAGS=\"\$CGO_LDFLAGS\" \
go build -mod=readonly -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME -X main.DirtyFingerprint=$API_DIRTY_FINGERPRINT\" \
go build -mod=vendor -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME -X main.DirtyFingerprint=$API_DIRTY_FINGERPRINT\" \
-o $APISERVER_BIN ./cmd/main.go
"

View file

@ -39,7 +39,10 @@
# so this script extracts just those two field values from each capture
# and asserts the extracted VALUES are byte-identical between --version
# and the startup log, for each binary. Surrounding text differing is
# expected and not a failure.
# expected and not a failure. One sanctioned exception (bug-1909): on a
# dirty tree the dirty-content-fingerprint feature shows
# '<sha>-<8-hex fingerprint>' in --version while the startup log keeps
# '<sha>-dirty'; check_consistency() accepts exactly that pairing.
#
# Build path notes:
# - xce_scheduler is built via the exact same real `/xce-build`-equivalent
@ -63,11 +66,12 @@
# `go build` directly with the same -ldflags shape go_build/Makefile
# use and the same CGO_CFLAGS/CGO_LDFLAGS pkg-config derivation
# test-backend/apiserver's meson.build already use, against the
# default (already-populated) module cache -- functionally the same
# committed vendor/ tree -- functionally the same
# real build (same source, same compiler, same ldflags injection
# mechanism), just not routed through the specific wrapper script that
# hangs in this sandbox. Neither go_build nor the Makefile is modified.
# `go build` is invoked with `-mod=readonly` explicitly (confirmed live:
# `go build` was originally invoked with `-mod=readonly` (superseded by
# `-mod=vendor`, see the bug-1910 note below; confirmed live at the time:
# without it, this build silently ADDED ~65 unrelated indirect-
# dependency /go.mod checksum lines to the committed
# src/frontend/apiserver/go.sum as a side effect of resolving the full
@ -78,6 +82,13 @@
# check" constraint requires. The build still succeeds under
# -mod=readonly against this repo's current go.sum, so nothing was
# actually missing -- the extra entries were superfluous).
# bug-1910: `-mod=readonly` made Go ignore the committed
# src/frontend/apiserver/vendor/ tree and need a populated module
# cache, so with GOPROXY=off the build failed on any machine without
# one. The build now uses `-mod=vendor`, as the canonical meson build
# does (GOFLAGS=-mod=vendor in src/frontend/apiserver/meson.build);
# vendor mode never touches go.mod or go.sum either, so the bug-1607
# no-repo-mutation property still holds.
#
# Usage:
# bash src/tools/test-version-startup-consistency.sh [--keep-artifacts]
@ -216,7 +227,17 @@ extract_zap_fields() {
# behaviour.
check_consistency() {
local label="$1" v_commit="$2" v_build="$3" l_commit="$4" l_build="$5"
local ok=1
local ok=1 v_shown="$2"
# bug-1909: dirty-content-fingerprint (requirements.md, scope note and
# 2.2) deliberately shows '<sha>-<8-hex fingerprint>' in --version on a
# dirty tree while the startup log keeps '<sha>-dirty'. Treat exactly
# that pairing -- same base commit, '-dirty' in the log -- as agreement.
if [[ "$v_commit" =~ ^(.+)-[0-9a-f]{8}$ ]] \
&& [ "$l_commit" = "${BASH_REMATCH[1]}-dirty" ]; then
yellow "[$label] --version fingerprint '$v_commit' pairs with startup log '$l_commit' (dirty-content-fingerprint)"
v_commit="$l_commit"
fi
if [ -z "$v_commit" ] || [ -z "$l_commit" ]; then
red "FAIL [$label]: commit id missing from one or both captures (--version='$v_commit' startup-log='$l_commit')"
@ -235,7 +256,7 @@ check_consistency() {
fi
if [ "$ok" = 1 ]; then
green "PASS [$label]: --version and startup log agree -- commit='$v_commit' build_time='$v_build'"
green "PASS [$label]: --version and startup log agree -- commit='$v_shown' (startup log '$l_commit') build_time='$v_build'"
return 0
fi
return 1
@ -275,6 +296,24 @@ demonstrate_detection_capability() {
fi
green "Confirmed: an empty field on one side is correctly rejected."
yellow "--- (e) dirty tree: --version '<sha>-<8-hex fingerprint>' with startup log '<sha>-dirty' is the documented dirty-content-fingerprint pairing and must PASS ---"
if ! check_consistency "DEMO-dirty-fingerprint-pairing" "a1b2c3d4-0f1e2d3c" "$real_build" "a1b2c3d4-dirty" "$real_build"; then
red "DEMONSTRATION FAILED: incorrectly FAILED the documented dirty-fingerprint pairing."
exit 1
fi
green "Confirmed: the documented dirty-fingerprint pairing is accepted."
yellow "--- (f) the pairing must not hide a real mismatch: different base commit, or a fingerprint with a clean log ---"
if check_consistency "DEMO-dirty-pairing-other-sha" "a1b2c3d4-0f1e2d3c" "$real_build" "deadbeef-dirty" "$real_build"; then
red "DEMONSTRATION FAILED: incorrectly PASSED a fingerprint against a different commit's -dirty."
exit 1
fi
if check_consistency "DEMO-fingerprint-clean-log" "a1b2c3d4-0f1e2d3c" "$real_build" "a1b2c3d4" "$real_build"; then
red "DEMONSTRATION FAILED: incorrectly PASSED a fingerprint against a clean startup-log commit."
exit 1
fi
green "Confirmed: the pairing is accepted only for the same base commit with '-dirty' in the log."
yellow "--- (d) sanity: genuinely matching real values must PASS ---"
if ! check_consistency "DEMO-sanity-real-match" "$real_commit" "$real_build" "$real_commit" "$real_build"; then
red "DEMONSTRATION FAILED: incorrectly FAILED genuinely matching real values."
@ -386,7 +425,7 @@ cd '$SRC_DIR/frontend/apiserver'
CGO_CFLAGS=\"-g \$(pkg-config --cflags librtti c-capnproto openssl tinylog) -I$SRC_DIR/include -I$APILIB_DIR/lib\"
CGO_LDFLAGS=\"-g \$(pkg-config --libs librtti c-capnproto openssl tinylog) -lm -lrt -L$APILIB_DIR/lib -lxce\"
GOPROXY=off CGO_ENABLED=1 CGO_CFLAGS=\"\$CGO_CFLAGS\" CGO_LDFLAGS=\"\$CGO_LDFLAGS\" \
go build -mod=readonly -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME\" \
go build -mod=vendor -gcflags 'all=-N -l' -ldflags \"-X main.GitCommit=$API_COMMIT_ID -X main.BuildTime=$API_BUILD_TIME\" \
-o $APISERVER_BIN ./cmd/main.go
"
@ -502,7 +541,7 @@ else
fi
if [ "$overall_pass" = 1 ]; then
green "=== ALL CHECKS PASS: xce_scheduler and xce_apiserver each report byte-identical commit id + build time between --version/-version and their startup log ==="
green "=== ALL CHECKS PASS: xce_scheduler and xce_apiserver each report the same commit id (or the sanctioned dirty-fingerprint pairing) and byte-identical build time between --version/-version and their startup log ==="
exit 0
else
red "=== INCONSISTENCY DETECTED: see FAIL line(s) above ==="

View file

@ -498,6 +498,20 @@ case_controller_env_own_name_present() {
add_case case_controller_env_own_name_present \
"controller: env list containing the own name is accepted (1.6)"
# bug-1912: spaces after the commas (natural in a shell variable) must not
# become part of a candidate's name, so --myname b still matches " b=...".
case_controller_env_spaced_list() {
local d; d="$(new_scratch controller-env-spaced)"
run_controller "$d" \
"XCE_CONTROLLER_PEERS=a=localhost:1, b=localhost:2, c=localhost:3" \
-- --myname b
_assert_stopped_early "$d" || return 1
assert_grep "$d/controller.log" 'HA mode: 3 controller candidates, identity=b' || return 1
assert_no_grep "$d/stderr" "$OWN_NAME_RE" "stderr" || return 1
}
add_case case_controller_env_spaced_list \
"controller: spaces around env entries are ignored (bug-1912)"
case_controller_flag_own_name_unchecked() {
local d; d="$(new_scratch controller-own-flag)"
run_controller "$d" -u XCE_CONTROLLER_PEERS -- --myname zz --peers "$ENV3"
@ -798,6 +812,20 @@ case_worker_help_names_variable() {
add_case case_worker_help_names_variable \
"worker: -h documents XCE_CONTROLLER_PEERS, the opt-out and that sudo drops it (1.5, 1.7)"
# bug-1911: -h and --version after -l must exit 0 without a LeakSanitizer
# report (the -l string used to leak on those early returns).
case_worker_help_version_after_logdir_no_leak() {
local d opt
for opt in -h --version; do
d="$(new_scratch "worker-early-exit${opt}")"
run_worker "$d" -u XCE_CONTROLLER_PEERS -- "$opt"
assert_rc 0 || return 1
assert_no_grep "$d/stderr" 'LeakSanitizer' "stderr ($opt after -l)" || return 1
done
}
add_case case_worker_help_version_after_logdir_no_leak \
"worker: -h / --version after -l exit 0 without a leak (bug-1911)"
# ── Task 3.3 (Req 1.1-1.5, 1.7): the harness (xce_test) takes its `masters`
# default from XCE_CONTROLLER_PEERS; an explicit `set masters` wins. Always
# run with --norc so the developer's own rc file cannot change the result,