diff --git a/.github/onie-build/Dockerfile b/.github/onie-build/Dockerfile index 500e8aedf..3fc4420b1 100644 --- a/.github/onie-build/Dockerfile +++ b/.github/onie-build/Dockerfile @@ -87,14 +87,35 @@ RUN sed -i -e 's/# en_US.UTF-8 UTF-8/en_US.UTF-8 UTF-8/' /etc/locale.gen && \ # (https://github.com/moby/moby/issues/5419#issuecomment-41478290) ARG UID=1000 ARG GID=1000 -RUN groupadd -g $GID build && \ - useradd -l -m -u $UID -g $GID -s /bin/bash build && \ - chown -R build:build /onie +# Reuse the host's primary group when that GID is already taken in the +# image. Debian fills the low GID range with system groups (30 dip, 50 +# staff, 100 users), so insisting on creating our own group makes the image +# unbuildable for anyone whose primary group lands there -- common with +# central/NFS accounts, and the norm on macOS hosts (GID 20 = dialout here). +# When the group is reused, "build" no longer exists as a group name, so +# chown by numeric id. +RUN if getent group "$GID" >/dev/null; then \ + echo "GID $GID is $(getent group "$GID" | cut -d: -f1); reusing it"; \ + else \ + groupadd -g "$GID" build; \ + fi && \ + useradd -l -m -u "$UID" -g "$GID" -s /bin/bash build && \ + chown -R "$UID:$GID" /onie -USER build +# /sbin and /usr/sbin hold tools the build invokes. Set this in two places +# so it holds however the image is invoked: +# ENV -- non-login shells (docker run , bash -c) +# profile.d -- login shells. Debian's /etc/profile *replaces* PATH for +# non-root users with one that omits both sbin dirs, and it +# does so before sourcing profile.d, so the snippet wins. +# ~/.bashrc, which this used to use, works for neither: Debian's default +# .bashrc returns early when the shell is not interactive, so under the +# workflow's "bash -lc" the PATH line was never reached. +ENV PATH="/sbin:/usr/sbin:${PATH}" +RUN printf '%s\n' 'export PATH="/sbin:/usr/sbin:$PATH"' \ + > /etc/profile.d/00-onie-sbin.sh -# /sbin and /usr/sbin hold tools the build invokes. -RUN echo 'export PATH="/sbin:/usr/sbin:$PATH"' >> ~/.bashrc +USER build # The build runs git commands; give it a default identity. RUN git config --global user.email "build@example.com" && \ diff --git a/.github/workflows/build-onie.yml b/.github/workflows/build-onie.yml index f5b94c5e7..3e25f3d39 100644 --- a/.github/workflows/build-onie.yml +++ b/.github/workflows/build-onie.yml @@ -10,21 +10,96 @@ name: Build ONIE (kvm_x86_64) # confirm the change still builds, and the kvm_x86_64 image depends on far # more than the build files (installer/, rootconf/, patches/, ...), so path # filtering would risk skipping validation on build-affecting changes. +# +# A push to a branch that already has an open pull request IN THIS repository +# fires both events and ran the whole pipeline twice; the concurrency group +# below cannot collapse them, because the two events carry different refs +# (refs/heads/ vs refs/pull//merge). The "gate" job below drops the +# duplicate push run. See the comment there for why this is a query rather +# than a branch filter on "push:". on: workflow_dispatch: push: pull_request: -# Cancel an in-progress run when a newer commit is pushed to the same ref, -# so stacked pushes don't pile up concurrent builds. +# Cancel an in-progress run when a newer commit is pushed to the same ref, so +# stacked pushes don't pile up concurrent builds. Runs on the long-lived +# branches are never cancelled -- those results are the integration signal. concurrency: group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: >- + ${{ github.ref != 'refs/heads/master' + && github.ref != 'refs/heads/onie-modernization-2026' }} + +# Nothing here writes to the repository; without this the workflow inherits +# the repository default token scope, which is read/write in many repos. +permissions: + contents: read jobs: + # Skip a push-triggered run when an open pull request in this repository + # already covers the commit -- the pull_request run is the one that reports + # on the PR, so the push run is pure duplicate cost. + # + # Deliberately a query rather than restricting "push:" to the long-lived + # branches. A branch filter also removes CI from every topic branch in a + # FORK, and that is where this workflow does most of its pre-submit work: a + # fork's pull requests are opened against the upstream repository, so they + # raise no pull_request event in the fork, and workflow_dispatch is not + # available there either unless the workflow is also on the fork's default + # branch. A branch filter would leave such a branch with no CI at all. The + # query has no such blind spot: in a fork it simply finds no pull request and + # the push run proceeds. + # + # Only "build" needs to depend on this -- the other jobs chain from it, and a + # skipped job skips its dependents. + gate: + name: Check for a duplicate run + runs-on: ubuntu-latest + timeout-minutes: 5 + # Job-level permissions replace the workflow-level block rather than adding + # to it; this job reads pull requests and checks out nothing. + permissions: + pull-requests: read + outputs: + run: ${{ steps.check.outputs.run }} + steps: + - name: Decide whether this run is needed + id: check + # Everything from the event context is passed via env rather than + # interpolated into the script body, so a branch name can never be + # parsed as shell. + env: + GH_TOKEN: ${{ github.token }} + EVENT: ${{ github.event_name }} + REPO: ${{ github.repository }} + OWNER: ${{ github.repository_owner }} + BRANCH: ${{ github.ref_name }} + run: | + if [ "$EVENT" != push ]; then + echo "$EVENT is not a push; running." + echo "run=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + open="$(gh api --method GET "repos/$REPO/pulls" \ + -f state=open -f head="$OWNER:$BRANCH" --jq 'length')" + if [ "${open:-0}" -gt 0 ]; then + echo "An open pull request in $REPO already covers $BRANCH;" \ + "its pull_request run reports on it. Skipping the push run." + echo "run=false" >> "$GITHUB_OUTPUT" + else + echo "No open pull request in $REPO for $BRANCH; running." + echo "run=true" >> "$GITHUB_OUTPUT" + fi + build: name: Build kvm_x86_64 + needs: gate + if: needs.gate.outputs.run == 'true' runs-on: ubuntu-latest + # A cold build (toolchain + full image) is roughly 45-60 min; cap it so a + # wedged build fails here instead of burning the 6-hour GitHub default. + timeout-minutes: 180 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -71,6 +146,17 @@ jobs: # environment the toolchain is built in. No loose restore-keys: any # input change forces a clean rebuild rather than restoring a # mismatched toolchain. + # + # kernel-download.make and the arch/machine makefiles are hashed because + # xtools.make derives + # XTOOLS_VERSION = $(ONIE_ARCH)-g$(GCC_VERSION)-lnx$(LINUX_RELEASE)-... + # (and seds CT_LINUX_VERSION into the generated toolchain .config), while + # LINUX_RELEASE lives in kernel-download.make and ONIE_ARCH in + # machine/kvm_x86_64/machine.make. With those omitted a kernel bump kept + # HITTING this key while the build/x-tools/ directory name + # changed underneath it: the restored toolchain was the wrong one, "make + # xtools" rebuilt from scratch, and the save step -- gated on a cache miss + # -- skipped it, so every later run paid the same rebuild forever. - name: Restore cross-toolchain cache id: xtools-cache uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 @@ -82,7 +168,17 @@ jobs: build/x-tools/*/stamp build/x-tools/*/install build/x-tools/*/build/.config - key: onie-xtools-kvm_x86_64-${{ hashFiles('build-config/make/xtools.make', 'build-config/make/crosstool-ng.make', 'build-config/make/compiler.make', 'build-config/conf/crosstool/**', 'patches/crosstool-NG/**', '.github/onie-build/Dockerfile') }} + key: >- + onie-xtools-kvm_x86_64-${{ hashFiles( + 'build-config/make/xtools.make', + 'build-config/make/crosstool-ng.make', + 'build-config/make/compiler.make', + 'build-config/make/kernel-download.make', + 'build-config/arch/x86_64.make', + 'machine/kvm_x86_64/machine.make', + 'build-config/conf/crosstool/**', + 'patches/crosstool-NG/**', + '.github/onie-build/Dockerfile') }} # ONIE drives its build with stamp files compared by mtime. A fresh # checkout gives every repo source file a current mtime, which is newer @@ -122,15 +218,53 @@ jobs: build/x-tools/*/build/.config \ -type f -exec touch -t "$ts" {} + + # Record which toolchains the cache restored, so the check after the + # build can tell whether "make xtools" had to build one the cache did not + # contain -- the symptom of a key that is still missing an input. + - name: Record restored toolchain set + id: xtools-before + if: steps.xtools-cache.outputs.cache-hit == 'true' + run: | + echo "dirs=$(ls -d build/x-tools/*/install 2>/dev/null | sort | tr '\n' ' ')" \ + >> "$GITHUB_OUTPUT" + + # No --privileged: the Dockerfile ends with "USER build", so the build + # runs unprivileged and cannot use any capability --privileged grants. + # It was ineffective as well as an unnecessary escalation -- the + # documented flow (contrib/build-env) builds unprivileged with fakeroot, + # and the image build uses mtools/fakeroot rather than loop mounts. - name: Build cross toolchain run: | - docker run --rm --privileged \ + docker run --rm \ -v "${PWD}:/onie" \ onie-build-env \ bash -lc 'cd build-config && make -j"$(nproc)" \ MACHINE=kvm_x86_64 \ xtools' + # A cache hit that nonetheless had to build a new toolchain means the key + # did not cover something that changed XTOOLS_VERSION. The save step + # cannot rescue that run -- the primary key already exists and GitHub + # will not overwrite it -- so warn instead of rebuilding silently forever. + - name: Check whether the cached toolchain was actually reused + if: steps.xtools-cache.outputs.cache-hit == 'true' + # Passed via env rather than interpolated into the script body, so the + # step output can never be parsed as shell. + env: + RESTORED_DIRS: ${{ steps.xtools-before.outputs.dirs }} + run: | + now="$(ls -d build/x-tools/*/install 2>/dev/null | sort | tr '\n' ' ')" + if [ "$now" != "$RESTORED_DIRS" ]; then + echo "::warning title=Stale cross-toolchain cache key::" \ + "Cache key hit but 'make xtools' built a toolchain the cache" \ + "did not contain (restored: [$RESTORED_DIRS], now: [$now])." \ + "XTOOLS_VERSION changed without changing the cache key, so this" \ + "toolchain cannot be saved and every run will rebuild it." \ + "Add the changed input to the 'Restore cross-toolchain cache' key." + else + echo "Cached toolchain reused as-is: $now" + fi + # Save only on a cache miss, and only after the toolchain build above # succeeded (default if: success()), so a broken toolchain is never # cached. Running before the full build means a later-stage failure @@ -148,9 +282,10 @@ jobs: build/x-tools/*/build/.config key: ${{ steps.xtools-cache.outputs.cache-primary-key }} + # No --privileged, as above. - name: Build ONIE run: | - docker run --rm --privileged \ + docker run --rm \ -v "${PWD}:/onie" \ onie-build-env \ bash -lc 'cd build-config && \ @@ -204,6 +339,9 @@ jobs: name: Boot test kvm_x86_64 needs: build runs-on: ubuntu-latest + # Three boots at up to 300s each plus package install and artifact + # download; this catches a QEMU that outlives the harness's own TIMEOUT. + timeout-minutes: 45 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -268,6 +406,9 @@ jobs: # time, keeping CI resource use low. needs: boot-test runs-on: ubuntu-latest + # Three boot stages at up to 600s each, plus the embed and install work in + # between. + timeout-minutes: 60 steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/emulation/ci-boot-test.sh b/emulation/ci-boot-test.sh index c247e4a58..a4148b6f6 100755 --- a/emulation/ci-boot-test.sh +++ b/emulation/ci-boot-test.sh @@ -20,13 +20,20 @@ # enrolled. shim still chainloads grub->kernel, so it validates # "does it boot" independent of the SB signature chain. Boots on # the default 'pc' machine (ONIE's onie-vm.sh reference machine). +# Also asserts the guest does NOT report secure boot as active. # secureboot Secure-Boot-ENFORCED: enrolls the demonstration PK/KEK/db from # KEYS_DIR into an OVMF varstore (virt-fw-vars), boots the secboot # OVMF firmware on 'q35,smm=on' with the flash secure flag, and # asserts ONIE boots -- i.e. the shim/grub/kernel signing chain -# verifies under enforcement. Also runs a NEGATIVE control with -# the db omitted, which MUST be rejected (proves SB is enforcing, -# not merely permissive). +# verifies under enforcement. Enforcement itself is asserted from +# inside the guest ("Info: Secure Boot: Active."), so a varstore +# that failed to enroll cannot pass merely by booting unverified. +# Also runs a NEGATIVE control with the db omitted, which MUST be +# rejected (proves SB is enforcing, not merely permissive). +# +# Each boot writes three logs beside : the serial console itself, +# -firmware.log (the OVMF debug console, empty unless the host has a debug OVMF +# build) and -qemu-stderr.log. # # Usage: ci-boot-test.sh [timeout_secs] # Env: SERIAL_LOG= serial log path (default ./onie-boot-serial.log) @@ -53,6 +60,21 @@ BOOT_MODE="${BOOT_MODE:-relaxed}" READY_BOOT='Please press Enter to activate this console|discover: (ONIE|Rescue)|Starting ONIE Service Discovery' READY_NEG='Security Violation|Access Denied|verification failed|GNU GRUB|ONIE: (OS Install|Rescue) Mode' +# A milestone only ONIE userspace can produce. The strings this check used to +# match -- "ONIE: Rescue Mode ..." and "Version :" -- are echoed by GRUB +# itself, from the menuentry and onie_entry_end in +# build-config/recovery/grub-iso.cfg, so both appear before the kernel is even +# loaded: the check duplicated "GRUB reached" and proved nothing about +# userspace. "Info: BIOS mode:" is printed unconditionally during sysinit by +# rootconf/grub-arch/sysroot-lib-onie/init-arch and cannot come from GRUB. +USERSPACE_UP='Info: BIOS mode:' + +# The same init-arch reports the firmware's Secure Boot state, read from the +# SecureBoot EFI variable via efivar. Asserting it in the guest is direct +# evidence the firmware is enforcing, rather than inferring enforcement from +# the negative control's silence. +SB_ACTIVE='Info: Secure Boot: Active\.' + [ -r "$ISO" ] || { echo "ERROR: ISO not readable: $ISO" >&2; exit 2; } # A fixed owner GUID for the enrolled demo keys -- deterministic on purpose so @@ -79,6 +101,34 @@ done WORK="$(mktemp -d)" trap 'rm -rf "$WORK"' EXIT +# vCPU count: two with hardware acceleration, one on TCG. Pure emulation +# starves the guest badly enough that an SB-enforced boot with two vCPUs never +# reaches userspace -- it dies in "rcu: INFO: rcu_sched detected stalls" and +# burns the whole timeout. With identical firmware, varstore and ISO the same +# boot completes in ~31s at -smp 1. CI has /dev/kvm and still gets two; this +# only affects the accel=kvm:tcg fallback used by local runs. +if [ -w /dev/kvm ]; then SMP=2; else SMP=1; fi + +# Firmware (OVMF debug console) log that pairs with a given serial log. +fw_log_for() { echo "${1%.log}-firmware.log"; } + +# Capture the OVMF debug console as a second evidence channel for why the +# firmware stopped. A release OVMF -- what Ubuntu ships, so what CI uses -- +# emits nothing here, and its rejection line does reach ttyS0 anyway +# ("BdsDxe: failed to load Boot0001 ...: Access Denied"), so this is +# belt-and-braces for a debug OVMF build rather than the only place a rejection +# can appear. 0x402 is the I/O port edk2 debug builds log to. Probe for the +# device so this stays strictly additive: on a QEMU without it we lose the +# extra channel instead of failing every boot. Bounded, with stdin closed, so +# a QEMU that fails to exit cannot wedge the harness before any boot starts. +if timeout 10 qemu-system-x86_64 -device help /dev/null \ + | grep -q '"isa-debugcon"'; then + HAVE_DEBUGCON=yes +else + HAVE_DEBUGCON=no + echo " NOTE QEMU has no usable isa-debugcon; firmware console not captured" +fi + # --- boot QEMU headless, serial(ttyS0) -> file, -no-reboot so a panic doesn't # loop. Polls the serial log and stops QEMU as soon as the boot reaches a # terminal state (the , e.g. the console prompt) or panics -- @@ -88,16 +138,41 @@ trap 'rm -rf "$WORK"' EXIT # Args: [extra qemu args...] run_qemu() { local ready_re="$1" code="$2" vars="$3" log="$4"; shift 4 - : >"$log" + local fw_log qerr + fw_log="$(fw_log_for "$log")"; qerr="${log%.log}-qemu-stderr.log" + : >"$log"; : >"$fw_log"; : >"$qerr" + local -a DEBUGCON_ARGS=() + [ "$HAVE_DEBUGCON" = yes ] && \ + DEBUGCON_ARGS=(-debugcon "file:$fw_log" -global isa-debugcon.iobase=0x402) + # Keep QEMU's stderr rather than discarding it, so a QEMU that never ran can + # be told apart from a firmware that refused the image -- previously both + # left an empty serial log, which is what let a broken harness score as a + # successful secure-boot rejection. It also surfaces "Could not access KVM + # kernel module ... falling back to tcg", otherwise invisible. qemu-system-x86_64 \ - -m 2048 -smp 2 \ + -m 2048 -smp "$SMP" \ -drive if=pflash,format=raw,readonly=on,file="$code" \ -drive if=pflash,format=raw,file="$vars" \ -cdrom "$ISO" -boot d \ -netdev user,id=onienet -device virtio-net,netdev=onienet \ -display none -serial "file:$log" -no-reboot \ - "$@" >/dev/null 2>&1 & - local qpid=$! t=0 + "${DEBUGCON_ARGS[@]}" \ + "$@" >/dev/null 2>"$qerr" & + local qpid=$! t=0 rc=0 + # A QEMU that dies this early never ran the firmware: bad arguments, a + # malformed varstore, a missing device model. That is a harness error, not + # a verdict about the image, so fail loudly instead of letting a silent + # serial log be scored as a pass (relaxed) or a rejection (negative + # control). Any early exit counts, including a successful one: rc=0 still + # means no boot happened. + sleep 2 + if ! kill -0 "$qpid" 2>/dev/null ; then + wait "$qpid" 2>/dev/null + rc=$? + echo "ERROR: QEMU exited immediately (rc=$rc) -- harness error, not a boot result" >&2 + sed 's/^/ /' "$qerr" >&2 + exit 2 + fi while kill -0 "$qpid" 2>/dev/null; do if grep -Eaq "$ready_re" "$log" || grep -aq 'Kernel panic' "$log"; then break; fi [ "$t" -ge "$TIMEOUT" ] && break @@ -115,16 +190,38 @@ chk() { #