From 8ac30b0ef767d68e14c22b78247086d6f4752fb5 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 30 Jul 2026 14:15:53 +0200 Subject: [PATCH 1/2] fix(multus): bake CNI plugins into an image, drop the node-side fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supersedes the wget-instead-of-curl approach in #446, which removed the `apk add` but left the real problems in place. The installer fetched the containernetworking/plugins release tarball from GitHub on the node and extracted it into /opt/cni/bin with NO integrity verification of any kind. That directory is executed as root in the CNI path on every node, so a tampered or MITM'd artifact would have obtained privileged execution cluster-wide with nothing to detect it. It also put github.com on the critical path of every node rebuild. Now the binaries are baked into ci/images/cni-plugins, where the download happens once and the sha256 IS verified, and the installer is a local file copy with zero network calls (verified: no wget/curl/apk/https in the rendered script). The pinned digest was cross-checked against upstream's own published cni-plugins-linux-amd64-v1.5.1.tgz.sha256 asset, not merely recorded from a local download — a download can be tampered with, so comparing against what upstream states is the point: 77baa2f669980a82255ffa2f2717de823992480271ee778aa51a9c60ae89ff9b Verified the gate actually gates: with the correct digest the layer logic downloads, verifies, extracts and asserts macvlan/static/host-local are executable; with a deliberately wrong digest `sha256sum -c` FAILS and the extract step is never reached. Install script changes beyond dropping the fetch: - Copies unconditionally instead of the old "already installed?" guard. The guard existed to avoid re-downloading; a local copy is cheap, so copying every time means a node always matches the version pinned in the manifest rather than keeping whatever it first installed. - Copies to a temp name in the same directory then rename(2)s over the target. `cp` onto a plugin the kubelet is currently executing can fail with ETXTBSY; rename never does and is atomic, so a concurrent CNI invocation sees either the old or the new binary, never a partial one. - LICENSE and README.md are no longer installed into /opt/cni/bin; the old tar extraction put them there. The chown 0:0 is kept but re-framed as a REPAIR. Its original cause is gone — extraction applied the archive's `./` uid/gid (1001/127, the github-actions runner) to the destination, and `cp` into an existing directory does not touch its ownership. It still runs because nodes provisioned by the old installer may already carry the wrong ownership, which breaks Cilium's apply-sysctl-overwrites init container. Verified end-to-end with the rendered script, including that repair path: pre-seeded owner 1001:127 -> 0:0 after run fresh install -> host-local/macvlan/static present, macvlan executes and reports "CNI macvlan plugin v1.5.1", exit 0 re-run -> idempotent, exit 0, 18 plugins, no .tmp files left Note the image carries ~80MB of extracted plugins (18 binaries), matching what the old installer put on each node. All are kept rather than trimming to the few in use, to avoid removing plugins already present on nodes as a side effect of this change. Co-Authored-By: Claude Opus 5 (1M context) --- .woodpecker/build-ci-images.yml | 29 +++++++ ci/images/cni-plugins/Containerfile | 60 +++++++++++++++ .../multus/cni-plugins-installer.yaml | 76 +++++++++++-------- 3 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 ci/images/cni-plugins/Containerfile diff --git a/.woodpecker/build-ci-images.yml b/.woodpecker/build-ci-images.yml index 44348b32..e89e9525 100644 --- a/.woodpecker/build-ci-images.yml +++ b/.woodpecker/build-ci-images.yml @@ -159,3 +159,32 @@ steps: --dockerfile Containerfile \ --destination gitea.renner.uno/gitea_admin/alpine-jq:$TAG \ --destination gitea.renner.uno/gitea_admin/alpine-jq:latest + + # Runtime image, not CI tooling: upstream containernetworking/plugins binaries + # baked in, consumed by the multus cni-plugins-installer DaemonSet. Replaces a + # node-side fetch from GitHub that had NO checksum verification and sat on the + # critical path of every node rebuild. The download now happens here, once, + # where the sha256 is checked — see the Containerfile for the full rationale. + - name: build-cni-plugins + image: gcr.io/kaniko-project/executor:v1.23.2-debug + directory: ci/images/cni-plugins + depends_on: [] + when: + - event: manual + - event: push + path: ["ci/images/cni-plugins/**", ".woodpecker/build-ci-images.yml"] + environment: + GITEA_USER: gitea_admin + GITEA_TOKEN: + from_secret: ci_images_token + commands: + - mkdir -p /kaniko/.docker + - AUTH=$(printf '%s:%s' "$GITEA_USER" "$GITEA_TOKEN" | base64 | tr -d '\n') + - printf '{"auths":{"gitea.renner.uno":{"auth":"%s"}}}' "$AUTH" > /kaniko/.docker/config.json + - TAG=$(date +%Y-%m-%d) + - | + /kaniko/executor \ + --context . \ + --dockerfile Containerfile \ + --destination gitea.renner.uno/gitea_admin/cni-plugins:$TAG \ + --destination gitea.renner.uno/gitea_admin/cni-plugins:latest diff --git a/ci/images/cni-plugins/Containerfile b/ci/images/cni-plugins/Containerfile new file mode 100644 index 00000000..7b4d8fc6 --- /dev/null +++ b/ci/images/cni-plugins/Containerfile @@ -0,0 +1,60 @@ +# Upstream containernetworking/plugins binaries, baked in at build time. +# +# WHY THIS IMAGE EXISTS +# The multus cni-plugins-installer DaemonSet used to fetch this tarball from +# GitHub on the node itself, at node-provisioning time, and extract it straight +# into /opt/cni/bin. Two problems with that: +# +# 1. Security. The download had NO integrity verification — wget, then tar, +# into a directory whose contents are executed as root in the CNI path on +# every node. A tampered or MITM'd artifact would have got privileged +# execution cluster-wide, with nothing to detect it. +# 2. Node rebuild. It put a live dependency on github.com (and previously the +# Alpine package index too) on the critical path of bringing a node up, so +# a GitHub outage or slow transfer stalled or broke CNI installation +# exactly when a node was being rebuilt. +# +# Baking the binaries in fixes both: the fetch happens once, here, where the +# checksum IS verified, and the installer becomes a local file copy with no +# network at all. +# +# The version and digest are pinned literals, so bumping them is a reviewable +# commit rather than a silent change in what runs on every node. The digest was +# cross-checked against upstream's own published +# cni-plugins-linux-amd64-.tgz.sha256 asset rather than merely recorded +# from a local download — a download can be tampered with; the point of the +# check is to compare against what upstream states. +# +# amd64 only, matching the ARCH the installer already hardcoded and the cluster's +# node architecture. Add a second stage if a different arch is ever needed. + +FROM alpine:3.21@sha256:48b0309ca019d89d40f670aa1bc06e426dc0931948452e8491e3d65087abc07d AS fetch + +ARG CNI_PLUGINS_VERSION=v1.5.1 +ARG CNI_PLUGINS_SHA256=77baa2f669980a82255ffa2f2717de823992480271ee778aa51a9c60ae89ff9b + +# busybox wget and tar are already in the base; nothing is installed here +# either. sha256sum -c fails the BUILD if the artifact does not match, which is +# the whole point — a bad download must never reach a node. +RUN set -eu; \ + F="cni-plugins-linux-amd64-${CNI_PLUGINS_VERSION}.tgz"; \ + wget -q -O "/tmp/${F}" \ + "https://github.com/containernetworking/plugins/releases/download/${CNI_PLUGINS_VERSION}/${F}"; \ + echo "${CNI_PLUGINS_SHA256} /tmp/${F}" | sha256sum -c -; \ + mkdir -p /plugins; \ + tar -xzf "/tmp/${F}" -C /plugins; \ + rm -f "/tmp/${F}"; \ + # Drop the non-executable payload: the tarball ships LICENSE and README.md, + # which the old installer extracted into /opt/cni/bin alongside the binaries. + rm -f /plugins/LICENSE /plugins/README.md; \ + # Fail loudly here rather than shipping an image that silently installs + # nothing, if upstream ever restructures the archive. + for p in macvlan static host-local; do \ + [ -x "/plugins/$p" ] || { echo "FATAL: /plugins/$p missing or not executable" >&2; exit 1; }; \ + done + +FROM alpine:3.21@sha256:48b0309ca019d89d40f670aa1bc06e426dc0931948452e8491e3d65087abc07d + +# Alpine rather than scratch: the installer needs a shell to copy the binaries +# onto the host. busybox covers cp/mv/chown/ls, so nothing is added. +COPY --from=fetch /plugins/ /opt/cni-plugins/ diff --git a/kubernetes/infrastructure/multus/cni-plugins-installer.yaml b/kubernetes/infrastructure/multus/cni-plugins-installer.yaml index 2a115887..5d7764c1 100644 --- a/kubernetes/infrastructure/multus/cni-plugins-installer.yaml +++ b/kubernetes/infrastructure/multus/cni-plugins-installer.yaml @@ -38,41 +38,57 @@ spec: effect: NoExecute initContainers: - name: install - image: alpine:3.21 + # Plugin binaries are baked into this image, checksum-verified at build + # time (ci/images/cni-plugins/Containerfile). This replaces fetching the + # release tarball from GitHub on the node, which had NO integrity check + # at all and put github.com on the critical path of every node rebuild. + # Dated tag so a version change is a reviewable diff here, matching the + # ci-ansible / alpine-jq convention. + image: gitea.renner.uno/gitea_admin/cni-plugins:2026-07-30 command: - sh - -c - | set -eu - VERSION=v1.5.1 - ARCH=amd64 - URL="https://github.com/containernetworking/plugins/releases/download/${VERSION}/cni-plugins-linux-${ARCH}-${VERSION}.tgz" - cd /host/opt/cni/bin - if [ -x ./macvlan ] && [ -x ./static ] && [ -x ./host-local ]; then - installed=$(./macvlan 2>&1 | head -1 || true) - echo "CNI plugins already installed (${installed})" - else - echo "Fetching ${URL}" - apk add --no-cache curl tar - curl -fsSL "${URL}" -o /tmp/plugins.tgz - # Extract everything: the tarball entries are stored with a - # `./` prefix, so selective extraction by bare name fails. - # All standard plugins are tiny (~5-10MB total) and harmless - # to keep alongside Cilium's CNI binaries. - tar -xzf /tmp/plugins.tgz -C . - echo "Installed: $(ls -la macvlan static host-local)" - fi - # Always restore root ownership of /opt/cni/bin. The cni-plugins - # tarball includes a `./` directory entry whose uid/gid (1001/127 - # — the github-actions runner that built the release) gets - # applied to the destination during extract. After that, Cilium's - # apply-sysctl-overwrites init container fails with "Permission - # denied" trying to `cp` cilium-sysctlfix into /hostbin, because - # the init runs root without DAC_OVERRIDE and the directory is - # no longer owned by root. The previous cluster only worked - # because the original Cilium pods initialised before the chown - # happened and were never re-rolled; on rebuild, the rolling - # daemonset replacement bites. + SRC=/opt/cni-plugins + DST=/host/opt/cni/bin + + # No network, no download, no "already installed?" guard. The old + # guard existed to avoid re-fetching; a local copy is cheap, so + # copying unconditionally is better — the node then always matches + # the version pinned in this manifest instead of keeping whatever + # it happened to install first. + for src in "$SRC"/*; do + name=${src##*/} + # Copy to a temp name in the SAME directory and rename over the + # target. A plain `cp` onto a plugin the kubelet is currently + # executing can fail with ETXTBSY; rename(2) never does, and is + # atomic, so a concurrent CNI invocation sees either the old or + # the new binary and never a partial one. + cp "$src" "$DST/.$name.tmp" + chmod 0755 "$DST/.$name.tmp" + mv -f "$DST/.$name.tmp" "$DST/$name" + done + + echo "Installed from $(basename "$SRC") image:" + ls -la "$DST/macvlan" "$DST/static" "$DST/host-local" + echo "Version: $("$DST/macvlan" 2>&1 | head -1 || true)" + # Kept, but now as a REPAIR rather than a workaround. The original + # cause is gone: extracting the tarball on the node applied the + # archive's `./` entry uid/gid (1001/127 — the github-actions runner + # that built the release) to the destination directory, and `cp` + # into an existing directory does not touch its ownership, so that + # can no longer happen. + # + # It still runs because nodes provisioned by the old installer may + # ALREADY carry the wrong ownership, and this is what fixes them. + # Why it matters: Cilium's apply-sysctl-overwrites init container + # runs as root without DAC_OVERRIDE, so with /opt/cni/bin not owned + # by root it fails with "Permission denied" copying cilium-sysctlfix + # into /hostbin. The old cluster only survived because the original + # Cilium pods initialised before the chown and were never re-rolled; + # a rolling daemonset replacement on rebuild is what exposed it. + # Safe to drop once every node has been through this version. chown 0:0 /host/opt/cni/bin securityContext: privileged: true -- 2.54.0 From d62c5da5a0fafc4e3bcc0872e5bac6541cb9339a Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 30 Jul 2026 14:30:20 +0200 Subject: [PATCH 2/2] fix(multus): include dhcp in the cni-plugins build assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review point on #447. The build-time check asserted macvlan, static and host-local were executable but omitted dhcp, which the sibling cni-dhcp-daemon DaemonSet runs and which its own comment says ships from this installer ("the dhcp binary ships from the sibling cni-plugins-installer DaemonSet ... whole tarball extracted"). Verified that dependency in kubernetes/infrastructure/multus/ cni-dhcp-daemon.yaml before changing anything, rather than taking the review's word for it. Consequence of the omission: since the whole /plugins directory is copied regardless of what the loop checks, nothing is broken today — but the "fail loudly if upstream restructures the archive" guarantee only covered three of the four binaries this repo depends on. Upstream dropping dhcp while keeping the others would have built green and surfaced later as a broken DHCP daemon on whichever node rolled first. Confirmed all four are executable in the already-published image, so this tightens the assertion without changing what ships. Co-Authored-By: Claude Opus 5 (1M context) --- ci/images/cni-plugins/Containerfile | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ci/images/cni-plugins/Containerfile b/ci/images/cni-plugins/Containerfile index 7b4d8fc6..d42686e4 100644 --- a/ci/images/cni-plugins/Containerfile +++ b/ci/images/cni-plugins/Containerfile @@ -49,7 +49,15 @@ RUN set -eu; \ rm -f /plugins/LICENSE /plugins/README.md; \ # Fail loudly here rather than shipping an image that silently installs # nothing, if upstream ever restructures the archive. - for p in macvlan static host-local; do \ + # + # This list must cover every plugin the repo actually depends on, not just + # the ones the installer happens to log. `dhcp` is included because the + # sibling cni-dhcp-daemon DaemonSet runs that binary and documents that it + # ships from here (see kubernetes/infrastructure/multus/cni-dhcp-daemon.yaml) + # — without it in the check, upstream dropping `dhcp` while keeping the + # others would build green and only surface later as a broken DHCP daemon on + # whichever node rolled first. + for p in macvlan static host-local dhcp; do \ [ -x "/plugins/$p" ] || { echo "FATAL: /plugins/$p missing or not executable" >&2; exit 1; }; \ done -- 2.54.0