From c1829332f8bce094e91a09642f1c187c4eb8812b Mon Sep 17 00:00:00 2001 From: Wynne Pirini Date: Wed, 14 Oct 2026 07:41:14 +1300 Subject: [PATCH] packaging(ns8): address code review - multi-arch module image and workflow hardening Code review of the NS8 module packaging found seven issues. Fixed here: 1. Module image was amd64-only while the app image is multi-arch. NS8 runs modules as rootless containers on the node, so an arm64 node could not install Calnode at all. build-images.sh now honours TARGETARCH and publishes only per-arch :build- tags; push-manifest.sh assembles the public tags, so an interrupted release cannot publish a single-arch image under a version tag. CI registers qemu and builds both arches. 2. The exec-bit guard added in #135 failed open: the awk exited 0 on empty input, so a pathspec that stopped matching would pass vacuously and silently reintroduce the non-executable-script bug. It now asserts the file list is non-empty first. Verified: passes on the real tree, fails loudly on an empty pathspec, fails on a 100644 entry. 3. Neither job declared timeout-minutes. 4. The validate job declared no permissions block. 5. secrets.GITHUB_TOKEN, github.actor and github.event_name were interpolated directly into run: blocks; they now go through env:. 6. No dependency caching for the UI build, so every branch and PR run re-downloaded the whole Vue/Carbon tree. Now cached like upstream NS8. 7. Stale step-4 comment in 20configure that no longer matched the code. Also aligned actions/checkout and setup-qemu versions with the rest of the repo (v7 / v4) instead of diverging. Not verified: no buildah on the dev machine, so the multi-arch manifest path could not be smoke-tested locally. It needs one CI run (or a real buildah host) before release. --- .github/workflows/ns8-module.yml | 72 ++++++++++++++++--- packaging/ns8/README.md | 20 ++++-- packaging/ns8/build-images.sh | 35 +++++---- .../actions/configure-module/20configure | 5 +- packaging/ns8/push-manifest.sh | 60 ++++++++++++++++ 5 files changed, 163 insertions(+), 29 deletions(-) mode change 100644 => 100755 packaging/ns8/build-images.sh create mode 100755 packaging/ns8/push-manifest.sh diff --git a/.github/workflows/ns8-module.yml b/.github/workflows/ns8-module.yml index a649e84..b53baf7 100644 --- a/.github/workflows/ns8-module.yml +++ b/.github/workflows/ns8-module.yml @@ -23,6 +23,9 @@ concurrency: jobs: validate: runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read steps: - uses: actions/checkout@v7 @@ -33,36 +36,63 @@ jobs: python3 -c "import json, glob; [json.load(open(f)) for f in glob.glob('imageroot/actions/*/*.json') + glob.glob('ui/public/*.json') + ['ui/public/i18n/en/translation.json']]" bash -n imageroot/actions/configure-module/80start_services bash -n build-images.sh + bash -n push-manifest.sh # No build-time placeholders may remain: the unit and configure read # the app ref from ${CALNODE_IMAGE} at install time. ! grep -rn '__CALNODE_APP_IMAGE__' imageroot/ # Action scripts must be committed executable: the agent silently # skips non-executable files, and fileMode=false checkouts do not # record chmod, so assert the indexed modes, not just the worktree. - git ls-files -s -- imageroot/actions | awk '$1=="100644" && $4 !~ /validate-(input|output)\.json$/ {print "not executable in index:", $4; bad=1} END{exit bad}' + # The non-empty assertion matters: the awk below exits 0 on empty + # input, so a pathspec that stops matching would pass vacuously. + scripts=$(git ls-files -s -- imageroot/actions | grep -v 'validate-.*\.json$') + if [ -z "$scripts" ]; then + echo "::error::no action scripts matched, the exec-bit check below would pass vacuously" + exit 1 + fi + echo "$scripts" | awk '$1!="100755" {print "not executable in index:", $4; bad=1} END{exit bad}' build: needs: validate runs-on: ubuntu-latest + timeout-minutes: 45 permissions: contents: read packages: write steps: - uses: actions/checkout@v7 + # Required to build the arm64 variant of the module image on an amd64 + # runner. Without it the module image would be amd64-only and podman + # could not run it on an arm64 NS8 node. + - uses: docker/setup-qemu-action@v4 + with: + platforms: all + + # The UI build is the slow part; cache the dependency tree the way the + # upstream NS8 publish workflow does, so branch and PR runs do not + # re-download the whole Vue/Carbon tree every time. + - uses: actions/cache@v4 + with: + path: packaging/ns8/ui/node_modules + key: "yarn-${{ hashFiles('packaging/ns8/ui/yarn.lock') }}" + # Mirror the app tagging in docker-publish.yml: release tags pin both # images to the same version, branches track the moving app tags. - name: Resolve tags id: tags + env: + REF: ${{ github.ref }} + REF_NAME: ${{ github.ref_name }} run: | - if [[ "${GITHUB_REF}" == refs/tags/v* ]]; then - version="${GITHUB_REF_NAME#v}" + if [[ "${REF}" == refs/tags/v* ]]; then + version="${REF_NAME#v}" echo "module_tags=${version} ${version%.*} latest" >> "$GITHUB_OUTPUT" echo "app_image=ghcr.io/calnode/calnode:${version}" >> "$GITHUB_OUTPUT" - elif [[ "${GITHUB_REF}" == refs/heads/main ]]; then + elif [[ "${REF}" == refs/heads/main ]]; then echo "module_tags=edge" >> "$GITHUB_OUTPUT" echo "app_image=ghcr.io/calnode/calnode:edge" >> "$GITHUB_OUTPUT" - elif [[ "${GITHUB_REF}" == refs/heads/dev ]]; then + elif [[ "${REF}" == refs/heads/dev ]]; then echo "module_tags=dev" >> "$GITHUB_OUTPUT" echo "app_image=ghcr.io/calnode/calnode:dev" >> "$GITHUB_OUTPUT" else @@ -70,18 +100,42 @@ jobs: echo "app_image=ghcr.io/calnode/calnode:edge" >> "$GITHUB_OUTPUT" fi + - name: Log in to GHCR + if: github.event_name != 'pull_request' + uses: docker/login-action@v3 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + # buildah is preinstalled on the ubuntu runners, same as the upstream # NS8 publish workflow assumes. - - name: Build (push on branch pushes and on tags) + - name: Build each architecture working-directory: packaging/ns8 env: + EVENT_NAME: ${{ github.event_name }} + ACTOR: ${{ github.actor }} + GHCR_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPOBASE: ghcr.io/${{ github.repository_owner }} APP_IMAGE: ${{ steps.tags.outputs.app_image }} IMAGETAGS: ${{ steps.tags.outputs.module_tags }} run: | push=0 - if [ "${{ github.event_name }}" != "pull_request" ]; then + if [ "$EVENT_NAME" != "pull_request" ]; then push=1 - buildah login -u "${{ github.actor }}" --password-stdin ghcr.io <<<"${{ secrets.GITHUB_TOKEN }}" + buildah login -u "$ACTOR" --password-stdin ghcr.io <<<"$GHCR_TOKEN" fi - PUSH="$push" bash build-images.sh + # PUSH=1 publishes only the per-arch :build- tags. The public + # tags are assembled from them by the manifest step below. + for arch in amd64 arm64; do + PUSH="$push" TARGETARCH="$arch" bash build-images.sh + done + + - name: Assemble and push the multi-arch manifest + if: github.event_name != 'pull_request' + working-directory: packaging/ns8 + env: + REPOBASE: ghcr.io/${{ github.repository_owner }} + TARGETARCHES: "amd64 arm64" + IMAGETAGS: ${{ steps.tags.outputs.module_tags }} + run: bash push-manifest.sh diff --git a/packaging/ns8/README.md b/packaging/ns8/README.md index 68e1de9..ef3aedc 100644 --- a/packaging/ns8/README.md +++ b/packaging/ns8/README.md @@ -116,16 +116,26 @@ tag pushes, mirroring the app tags in `docker-publish.yml`: - push a `vX.Y.Z` tag pushes module `:X.Y.Z`, `:X.Y` and `:latest`, running app `:X.Y.Z`, so a module release always runs the matching app release +Because `:latest` only moves on a release tag, use `:edge` to try the newest +module on a test cluster. + +Every published module tag is a **multi-arch** manifest (amd64 + arm64). NS8 +runs modules as rootless containers on the node, so a single-arch image would +install on amd64 and fail on arm64. CI builds each arch to a private +`:build-` tag and only then assembles the public tags, so an interrupted +release cannot publish a single-arch image under a version tag. + To build locally on a machine with podman and buildah, from this directory: ``` -bash build-images.sh +PUSH=1 TARGETARCH=amd64 bash build-images.sh +PUSH=1 TARGETARCH=arm64 bash build-images.sh +TARGETARCHES="amd64 arm64" IMAGETAGS="latest" bash push-manifest.sh ``` -It prints the `buildah push` commands for your registry. Override the app -reference per build with e.g. `APP_IMAGE=ghcr.io/calnode/calnode:1.2.3 -IMAGETAGS="1.2.3 latest" PUSH=1 bash build-images.sh`. `APP_IMAGE` only sets -the `org.nethserver.images` label: at install the cluster writes that same +Override the app reference per build with e.g. `APP_IMAGE=ghcr.io/calnode/calnode:1.2.3 +IMAGETAGS="1.2.3 1.2 latest"`. `APP_IMAGE` only sets the +`org.nethserver.images` label: at install the cluster writes that same reference into the module environment as `CALNODE_IMAGE`, which is what the systemd unit and configure-module actually run. The label is the single source of truth; nothing is stamped at build time. diff --git a/packaging/ns8/build-images.sh b/packaging/ns8/build-images.sh old mode 100644 new mode 100755 index 5abd02d..a674d90 --- a/packaging/ns8/build-images.sh +++ b/packaging/ns8/build-images.sh @@ -17,6 +17,10 @@ # reads the exact reference from ${CALNODE_IMAGE}, which the # cluster writes into the module environment at install time # from that same label. One source of truth, no stamping. +# TARGETARCH architecture to build for, default amd64. NS8 runs modules +# as rootless containers on the node, so an amd64-only module +# image cannot be installed on an arm64 node: build each arch +# and assemble them into one manifest tag (see push-manifest.sh). # IMAGETAGS space-separated module tags to push, default "latest" # PUSH=1 push after building. Default 0 prints the push commands. # @@ -39,9 +43,13 @@ repobase="${repobase,,}" reponame="ns8-calnode" app_image="${APP_IMAGE:-ghcr.io/calnode/calnode:latest}" read -r -a imagetags <<< "${IMAGETAGS:-latest}" +targetarch="${TARGETARCH:-amd64}" -# Add imageroot directory to the container image -container=$(buildah from scratch) +# Per-arch build tag. push-manifest.sh stitches these into the public tags. +buildtag="build-${targetarch}" + +# Add imageroot directory to the container image, pinned to this arch. +container=$(buildah from --arch "${targetarch}" scratch) buildah add "${container}" imageroot /imageroot # Reuse existing nodebuilder-ns8-calnode container, to speed up builds @@ -68,18 +76,19 @@ buildah config --entrypoint=/ \ --label="org.nethserver.images=${app_image}" \ "${container}" # Commit the image (repobase already lowercased at variable prep above). -buildah commit "${container}" "${repobase}/${reponame}:build" +buildah commit "${container}" "${repobase}/${reponame}:${buildtag}" if [ "${PUSH:-0}" = "1" ]; then - for imagetag in "${imagetags[@]}"; do - buildah tag "${repobase}/${reponame}:build" "${repobase}/${reponame}:${imagetag}" - buildah push "${repobase}/${reponame}:${imagetag}" "docker://${repobase}/${reponame}:${imagetag}" - done + # Push only the per-arch tag. The public tags are assembled from these by + # push-manifest.sh, so a half-finished release can never publish a + # single-arch image under a version tag. + buildah push "${repobase}/${reponame}:${buildtag}" \ + "docker://${repobase}/${reponame}:${buildtag}" else - printf "Built %s:%s (app %s).\nPublish with:\n\n" "${repobase}/${reponame}" "build" "${app_image}" - for imagetag in "${imagetags[@]}"; do - printf " buildah tag %s:build %s:%s\n" "${repobase}/${reponame}" "${repobase}/${reponame}" "${imagetag}" - printf " buildah push %s:%s docker://%s:%s\n" "${repobase}/${reponame}" "${imagetag}" "${repobase}/${reponame}" "${imagetag}" - done - printf "\nOr rerun with PUSH=1 IMAGETAGS=\"...\" to push directly.\n" + printf "Built %s:%s (arch %s, app %s).\n\n" "${repobase}/${reponame}" "${buildtag}" "${targetarch}" "${app_image}" + printf "To publish this arch and assemble the multi-arch tags:\n\n" + printf " PUSH=1 TARGETARCH=%s IMAGETAGS=\"%s\" bash build-images.sh\n" \ + "${targetarch}" "${imagetags[*]}" + printf " TARGETARCHES=\"%s\" IMAGETAGS=\"%s\" bash push-manifest.sh\n" \ + "${targetarch}" "${imagetags[*]}" fi diff --git a/packaging/ns8/imageroot/actions/configure-module/20configure b/packaging/ns8/imageroot/actions/configure-module/20configure index a66fa72..ad77228 100755 --- a/packaging/ns8/imageroot/actions/configure-module/20configure +++ b/packaging/ns8/imageroot/actions/configure-module/20configure @@ -11,8 +11,9 @@ # 2. write the container env file state/calnode.env (BASE_URL follows # the hostname; the encryption key is generated once and preserved # across reconfigures, since losing it locks the SQLite data key) -# 3. register a host-based Traefik route (TLS is handled by the cluster) -# 4. pull the Calnode image so a first install works offline of updates +# 3. ensure the app image is present (tolerating a registry outage when +# the image is already local) +# 4. register a host-based Traefik route (TLS is handled by the cluster) # import json import os diff --git a/packaging/ns8/push-manifest.sh b/packaging/ns8/push-manifest.sh new file mode 100755 index 0000000..ea12679 --- /dev/null +++ b/packaging/ns8/push-manifest.sh @@ -0,0 +1,60 @@ +#!/bin/bash + +# +# Copyright (C) 2023 Nethesis S.r.l. +# SPDX-License-Identifier: GPL-3.0-or-later +# + +# Assemble the per-arch module images built by build-images.sh into one +# multi-arch tag. +# +# NS8 runs each module as a rootless container on the node, so the module +# image must exist for every node architecture. A single-arch image would +# install fine on amd64 and fail on arm64, so the per-arch tags are never +# published as-is: only the assembled tags are public. +# +# Run from this directory (packaging/ns8), after pushing each arch with +# build-images.sh. Environment: +# REPOBASE module image namespace, default ghcr.io/calnode +# TARGETARCHES space-separated arches to assemble, required +# IMAGETAGS space-separated tags to publish, default "latest" +# +# Example: +# TARGETARCHES="amd64 arm64" IMAGETAGS="1.2.3 1.2 latest" bash push-manifest.sh + +set -e + +repobase="${REPOBASE:-ghcr.io/calnode}" +repobase="${repobase,,}" +reponame="ns8-calnode" +read -r -a arches <<< "${TARGETARCHES:-}" +read -r -a imagetags <<< "${IMAGETAGS:-latest}" + +if [ "${#arches[@]}" -eq 0 ]; then + echo "TARGETARCHES is required, e.g. TARGETARCHES=\"amd64 arm64\"" >&2 + exit 1 +fi + +for arch in "${arches[@]}"; do + if ! buildah image exists "${repobase}/${reponame}:build-${arch}"; then + echo "Missing local image for ${arch}. Run build-images.sh with" \ + "TARGETARCH=${arch} PUSH=1 first." >&2 + exit 1 + fi +done + +for imagetag in "${imagetags[@]}"; do + manifest="${repobase}/${reponame}:${imagetag}" + # Drop any stale manifest so a retry cannot inherit old entries. + buildah manifest rm --all "${manifest}" 2>/dev/null || true + buildah manifest create "${manifest}" + for arch in "${arches[@]}"; do + buildah manifest add \ + "${manifest}" \ + "${repobase}/${reponame}:build-${arch}" \ + "docker.io/calnode/ns8-calnode:${imagetag}-${arch}" + done + buildah manifest push --all "${manifest}" "docker://${manifest}" + buildah manifest rm "${manifest}" + echo "Published ${manifest} (${arches[*]})" +done \ No newline at end of file