Compare commits

..
Author SHA1 Message Date
claude-botandClaude Opus 5 d62c5da5a0 fix(multus): include dhcp in the cni-plugins build assertion
ci/woodpecker/push/build-ci-images Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
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) <noreply@anthropic.com>
2026-07-30 14:30:20 +02:00
claude-botandClaude Opus 5 8ac30b0ef7 fix(multus): bake CNI plugins into an image, drop the node-side fetch
ci/woodpecker/push/build-ci-images Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
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) <noreply@anthropic.com>
2026-07-30 14:15:53 +02:00
3 changed files with 143 additions and 43 deletions
+29
View File
@@ -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
+68
View File
@@ -0,0 +1,68 @@
# 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-<version>.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.
#
# 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
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/
@@ -38,54 +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}"
# busybox wget + tar, NOT `apk add --no-cache curl tar`.
# Installing tools at container start is not acceptable here: it
# makes the install path depend on package-index availability at
# exactly the moment a node is being (re)built, and this is a
# DaemonSet — it runs on every node.
#
# Nothing needed to be added in the first place. Verified in
# alpine:3.21: `tar` is already busybox (v1.37.0) and handles
# -xzf, and busybox `wget` follows GitHub's release redirect to
# objects.githubusercontent.com over TLS using the base image's
# /etc/ssl/certs/ca-certificates.crt — fetched the real 48MB
# tarball and extracted an executable macvlan from it.
# Only `curl` is genuinely absent from the base, and wget covers
# what it was used for here.
wget -q -O /tmp/plugins.tgz "${URL}"
# 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