From 90ec425fe3a7ac87201bde71b0fe8000007ebc5c Mon Sep 17 00:00:00 2001 From: Oleg Kushniriov Date: Sun, 23 Aug 2026 15:34:49 +0300 Subject: [PATCH 1/3] fix(agent): bundle static tar for restore Signed-off-by: Oleg Kushniriov --- agent/Dockerfile | 86 +++++++++++++++++++++++--- agent/compliance/README.source.txt | 2 + agent/compliance/gen-attribution.sh | 2 +- agent/internal/executor/nsrestore.go | 3 +- agent/internal/executor/restore.go | 6 +- agent/internal/runtime/overlay.go | 6 +- agent/internal/runtime/overlay_test.go | 25 ++++++-- 7 files changed, 107 insertions(+), 23 deletions(-) diff --git a/agent/Dockerfile b/agent/Dockerfile index d6fdba85..23bcb6e0 100644 --- a/agent/Dockerfile +++ b/agent/Dockerfile @@ -27,6 +27,8 @@ ARG GO_VERSION=1.26.6 # --build-arg CRIU_REF= ARG CRIU_REPO=https://github.com/checkpoint-restore/criu.git ARG CRIU_REF=criu-dev +ARG GNU_TAR_VERSION=1.35 +ARG GNU_TAR_SHA256=4d62ff37342ec7aed748535323930c7cf94acf71c3591882b26a7ea50f3edc16 ARG AGENT_BASE_IMAGE=nvcr.io/nvidia/cuda-dl-base:25.11-cuda13.0-devel-ubuntu24.04@sha256:8315e2455736c4f9b597f15c5fb4d31f834e798e0c6b66bbdbdbac491ce26bd1 # For placeholder target only - this default allows agent builds to succeed, @@ -87,6 +89,66 @@ RUN CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -ldflags="-w -s # Corresponding source for the Go modules compiled into the binaries above. RUN mkdir -p /go-src && go mod vendor -o /go-src/vendor +# ============================================================================= +# Stage: Restore tar builder +# +# Rootfs diffs are extracted inside the placeholder container. A dynamically +# linked tar would use that image's ELF loader and glibc, which may be older +# than the agent image's. Build GNU tar against musl so restore extraction does +# not depend on any userspace library in the placeholder image. +# ============================================================================= +FROM alpine:3.23 AS tar-builder + +ARG GNU_TAR_VERSION +ARG GNU_TAR_SHA256 + +RUN apk add --no-cache \ + build-base \ + ca-certificates \ + curl \ + xz + +WORKDIR /tmp + +RUN set -eu; \ + curl -fsSLo gnu-tar.tar.xz \ + "https://ftp.gnu.org/gnu/tar/tar-${GNU_TAR_VERSION}.tar.xz"; \ + echo "${GNU_TAR_SHA256} gnu-tar.tar.xz" | sha256sum -c -; \ + cp gnu-tar.tar.xz /gnu-tar-source.tar.xz; \ + xz -d gnu-tar.tar.xz; \ + tar -xf gnu-tar.tar + +WORKDIR /tmp/tar-${GNU_TAR_VERSION} + +RUN set -eu; \ + FORCE_UNSAFE_CONFIGURE=1 LDFLAGS=-static ./configure \ + --disable-nls \ + --without-posix-acls \ + --without-selinux; \ + make -j"$(nproc)"; \ + mkdir -p /out; \ + cp src/tar /out/tar; \ + cp COPYING /out/COPYING; \ + chmod 0755 /out/tar; \ + strip /out/tar; \ + if readelf -l /out/tar | grep -q 'INTERP'; then \ + echo "ERROR: restore tar has a dynamic ELF interpreter" >&2; \ + exit 1; \ + fi; \ + if readelf -d /out/tar | grep -q '(NEEDED)'; then \ + echo "ERROR: restore tar has shared-library dependencies" >&2; \ + exit 1; \ + fi; \ + mkdir -p /tmp/tar-smoke/source /tmp/tar-smoke/target; \ + echo preserved > /tmp/tar-smoke/target/existing; \ + echo replaced > /tmp/tar-smoke/source/existing; \ + echo created > /tmp/tar-smoke/source/created; \ + /out/tar -C /tmp/tar-smoke/source -cf /tmp/tar-smoke/archive.tar .; \ + /out/tar --skip-old-files --blocking-factor=2048 \ + -C /tmp/tar-smoke/target -xf /tmp/tar-smoke/archive.tar; \ + grep -qx preserved /tmp/tar-smoke/target/existing; \ + grep -qx created /tmp/tar-smoke/target/created + # ============================================================================= # Stage: CUDA checkpoint helper builder # ============================================================================= @@ -207,6 +269,9 @@ RUN sh /tmp/collect-sources.sh /tmp/base-packages.tsv /sources/dpkg # CRIU source, at the exact ref this image was built from. COPY --from=criu-builder /criu-src.tar.gz /sources/criu/criu-src.tar.gz +# Pinned upstream source for the statically linked restore-time GNU tar. +COPY --from=tar-builder /gnu-tar-source.tar.xz /sources/gnu-tar/gnu-tar-source.tar.xz + # Go module source for the statically linked binaries. COPY --from=builder /go-src/vendor /sources/go/vendor @@ -246,17 +311,19 @@ COPY --from=criu-builder /criu-install/usr/local/sbin/criu /snapshot-binaries/cr COPY --from=criu-builder /criu-install/usr/local/lib/snapshot/criu-plugins/ /snapshot-binaries/criu-plugins/ COPY --from=criu-builder /tmp/cuda-checkpoint/bin/x86_64_Linux/cuda-checkpoint /snapshot-binaries/cuda-checkpoint COPY --from=cuda-helper-builder /cuda-checkpoint-helper /snapshot-binaries/cuda-checkpoint-helper +COPY --from=tar-builder /out/tar /snapshot-binaries/tar # Network binaries needed during restore: # ip — address/route restore (criu/net.c) -# tar — used by runtime.ApplyRootfsDiff +# tar — used by runtime.ApplyRootfsDiff; statically linked and copied above # Network lock is set to SKIP (--network-lock skip) in BuildRestoreOpts, so # criu never forks iptables-restore/ip6tables-restore on this path and neither # binary needs to be in the bundle. See BuildRestoreOpts for why SKIP is safe # for restore specifically. -# All resolve through PATH inside the namespace. +# ip resolves through PATH inside the namespace. ApplyRootfsDiff invokes tar by +# its explicit bundle path so it cannot fall back to the placeholder's tar. RUN set -eu; \ - cp -L "$(command -v ip)" "$(command -v tar)" /snapshot-binaries/; \ + cp -L "$(command -v ip)" /snapshot-binaries/; \ chmod +x /snapshot-binaries/criu /snapshot-binaries/nsrestore \ /snapshot-binaries/cuda-checkpoint /snapshot-binaries/cuda-checkpoint-helper \ /snapshot-binaries/ip /snapshot-binaries/tar @@ -285,7 +352,7 @@ RUN set -eu; \ # a silent ldd failure leaves lib/ empty; asserting here catches that. RUN set -eu; \ for f in /snapshot-binaries/criu /snapshot-binaries/ip \ - /snapshot-binaries/tar /snapshot-binaries/criu-plugins/*.so; do \ + /snapshot-binaries/criu-plugins/*.so; do \ [ -f "$f" ] || continue; \ if LD_LIBRARY_PATH=/snapshot-binaries/lib ldd "$f" 2>&1 | grep -q 'not found'; then \ echo "ERROR: unresolved shared libraries in $f:" >&2; \ @@ -295,6 +362,7 @@ RUN set -eu; \ done COPY --from=sources /sources /legal/source +COPY --from=tar-builder /out/COPYING /legal/gnu-tar/COPYING # Attribution generated from the delta manifest and the vendored modules. COPY compliance/gen-attribution.sh /tmp/gen-attribution.sh @@ -322,11 +390,11 @@ ENTRYPOINT ["/usr/local/bin/snapshot-agent"] # namespace, so nothing needs pre-provisioning here. # # Three constraints follow from that and must hold for every BASE_IMAGE: -# - BASE_IMAGE must be glibc-compatible with AGENT_BASE_IMAGE, because the -# bundle's library closure is built from AGENT_BASE_IMAGE (ubuntu 24.04, -# glibc 2.39) and loaded ahead of the placeholder's own libraries via -# LD_LIBRARY_PATH. A 22.04-based BASE_IMAGE builds green but fails at -# restore with "version 'GLIBC_2.38' not found". +# - BASE_IMAGE must be glibc-compatible with AGENT_BASE_IMAGE for the +# dynamically linked CRIU, ip, and CUDA tools. Their library closure is +# built from AGENT_BASE_IMAGE (ubuntu 24.04, glibc 2.39) and loaded ahead +# of the placeholder's own libraries via LD_LIBRARY_PATH. The restore tar +# is statically linked and does not share this constraint. # - The node kernel must be >= 5.12 (mount_setattr, used by ns-bind-mount). # - The placeholder pod must NOT set readOnlyRootFilesystem: true unless a # writable volume is mounted at /tmp. The agent bind-mounts the binary diff --git a/agent/compliance/README.source.txt b/agent/compliance/README.source.txt index bee40cc3..46718c96 100644 --- a/agent/compliance/README.source.txt +++ b/agent/compliance/README.source.txt @@ -11,6 +11,8 @@ Upstream source for the third-party components redistributed in this image. a package published without source by an NVIDIA repository; any other fetch failure fails the build. criu/ CRIU source at the commit this image was built from. + gnu-tar/ Pinned GNU tar source used to build the statically linked + restore-time tar binary. go/vendor/ Source for the Go modules linked into the binaries in this image; modules.txt records the exact module set. diff --git a/agent/compliance/gen-attribution.sh b/agent/compliance/gen-attribution.sh index 263c3d29..b904c16e 100755 --- a/agent/compliance/gen-attribution.sh +++ b/agent/compliance/gen-attribution.sh @@ -79,7 +79,7 @@ HEADER # Fold in the license texts the Dockerfile stages separately, so this file # is self-contained. - for extra in /legal/CRIU/COPYING /legal/cuda-checkpoint/LICENSE; do + for extra in /legal/CRIU/COPYING /legal/cuda-checkpoint/LICENSE /legal/gnu-tar/COPYING; do [ -f "$extra" ] || continue echo echo "================================================================================" diff --git a/agent/internal/executor/nsrestore.go b/agent/internal/executor/nsrestore.go index 1aa97f23..c672f234 100644 --- a/agent/internal/executor/nsrestore.go +++ b/agent/internal/executor/nsrestore.go @@ -127,7 +127,8 @@ func executeRestore( timings = &nsrestorePhaseTimings{} overlayStart := time.Now() - if err := snapshotruntime.ApplyRootfsDiff(opts.CheckpointPath, "/", log); err != nil { + tarBinary := filepath.Join(opts.BundleDir, "tar") + if err := snapshotruntime.ApplyRootfsDiff(opts.CheckpointPath, "/", tarBinary, log); err != nil { return nil, 0, nil, fmt.Errorf("rootfs diff failed: %w", err) } if err := snapshotruntime.ApplyDeletedFiles(opts.CheckpointPath, "/", log); err != nil { diff --git a/agent/internal/executor/restore.go b/agent/internal/executor/restore.go index 8dfde222..7220d0c3 100644 --- a/agent/internal/executor/restore.go +++ b/agent/internal/executor/restore.go @@ -310,9 +310,9 @@ func inspectRestore( // 2. nsrestore binary fd: we open nsrestore from the agent host side (SnapshotBinSrc) // before entering any namespace and exec it via /proc/self/fd/N. This protects // the nsrestore binary itself against path-based substitution inside the -// container. Binaries that nsrestore subsequently loads (criu, ip, tar, .so -// files) are still resolved by PATH/LD_LIBRARY_PATH inside the container's -// mount namespace. +// container. CRIU, ip, and shared objects that nsrestore subsequently loads +// are resolved by PATH/LD_LIBRARY_PATH inside the container's mount +// namespace. The restore tar is invoked by its explicit bundle path. func execNSRestore(ctx context.Context, log logr.Logger, req RestoreRequest, snap *types.RestoreContainerSnapshot, mp nsmount.MountPoint, checkpointPath string) (*RestoreInNamespaceResult, error) { // Open nsrestore from the agent host side before entering the container diff --git a/agent/internal/runtime/overlay.go b/agent/internal/runtime/overlay.go index b1e927e3..1b252b6a 100644 --- a/agent/internal/runtime/overlay.go +++ b/agent/internal/runtime/overlay.go @@ -158,7 +158,7 @@ func CaptureDeletedFiles(upperDir, checkpointDir string) (bool, error) { // The archive is copied to local disk first. tar walks members with many small // reads; doing that directly from NFS is much slower than one sequential copy // plus a local extract. -func ApplyRootfsDiff(checkpointPath, targetRoot string, log logr.Logger) error { +func ApplyRootfsDiff(checkpointPath, targetRoot, tarBinary string, log logr.Logger) error { rootfsDiffPath := filepath.Join(checkpointPath, rootfsDiffFilename) info, err := os.Stat(rootfsDiffPath) if os.IsNotExist(err) { @@ -183,11 +183,11 @@ func ApplyRootfsDiff(checkpointPath, targetRoot string, log logr.Logger) error { // The rootfs diff only contains overlay upperdir changes (runtime-generated files // like triton caches, tmp files) — base image files should not be overwritten. log.Info("Applying rootfs diff", "target", targetRoot, "bytes", info.Size()) - cmd := exec.Command("tar", "--skip-old-files", "--blocking-factor=2048", "-C", targetRoot, "-xf", localPath) + cmd := exec.Command(tarBinary, "--skip-old-files", "--blocking-factor=2048", "-C", targetRoot, "-xf", localPath) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { - return fmt.Errorf("tar extract failed: %w", err) + return fmt.Errorf("tar extract with %s failed: %w", tarBinary, err) } return nil } diff --git a/agent/internal/runtime/overlay_test.go b/agent/internal/runtime/overlay_test.go index 4d654c6b..33a5b583 100644 --- a/agent/internal/runtime/overlay_test.go +++ b/agent/internal/runtime/overlay_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "testing" "github.com/go-logr/logr/testr" @@ -179,7 +180,7 @@ func TestCaptureRootfsDiff(t *testing.T) { } targetRoot := t.TempDir() - if err := ApplyRootfsDiff(checkpointDir, targetRoot, testr.New(t)); err != nil { + if err := ApplyRootfsDiff(checkpointDir, targetRoot, "tar", testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } data, err := os.ReadFile(filepath.Join(targetRoot, "generated.txt")) @@ -213,7 +214,7 @@ func TestCaptureRootfsDiff(t *testing.T) { func TestApplyRootfsDiff(t *testing.T) { t.Run("missing archive is no-op", func(t *testing.T) { - if err := ApplyRootfsDiff(t.TempDir(), t.TempDir(), testr.New(t)); err != nil { + if err := ApplyRootfsDiff(t.TempDir(), t.TempDir(), "tar", testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } }) @@ -223,7 +224,7 @@ func TestApplyRootfsDiff(t *testing.T) { if err := os.WriteFile(filepath.Join(checkpointDir, rootfsDiffFilename), nil, 0644); err != nil { t.Fatalf("write empty rootfs diff: %v", err) } - if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), testr.New(t)); err != nil { + if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), "tar", testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } }) @@ -233,11 +234,23 @@ func TestApplyRootfsDiff(t *testing.T) { if err := os.WriteFile(filepath.Join(checkpointDir, rootfsDiffFilename), []byte("not a tar archive"), 0644); err != nil { t.Fatalf("write invalid rootfs diff: %v", err) } - if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), testr.New(t)); err == nil { + if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), "tar", testr.New(t)); err == nil { t.Fatal("ApplyRootfsDiff should fail for invalid non-empty archive") } }) + t.Run("uses the provided tar binary", func(t *testing.T) { + checkpointDir := t.TempDir() + if err := os.WriteFile(filepath.Join(checkpointDir, rootfsDiffFilename), []byte("not a tar archive"), 0644); err != nil { + t.Fatalf("write invalid rootfs diff: %v", err) + } + tarBinary := filepath.Join(t.TempDir(), "missing-tar") + err := ApplyRootfsDiff(checkpointDir, t.TempDir(), tarBinary, testr.New(t)) + if err == nil || !strings.Contains(err.Error(), tarBinary) { + t.Fatalf("ApplyRootfsDiff error = %v, want provided tar path %q", err, tarBinary) + } + }) + t.Run("non-ENOENT stat error is propagated", func(t *testing.T) { // Pass a regular file as checkpointPath so stat of // checkpointPath/rootfs-diff.tar returns ENOTDIR, not ENOENT. @@ -246,7 +259,7 @@ func TestApplyRootfsDiff(t *testing.T) { t.Fatalf("create temp file: %v", err) } f.Close() - if err := ApplyRootfsDiff(f.Name(), t.TempDir(), testr.New(t)); err == nil { + if err := ApplyRootfsDiff(f.Name(), t.TempDir(), "tar", testr.New(t)); err == nil { t.Fatal("ApplyRootfsDiff should propagate non-ENOENT stat error") } }) @@ -265,7 +278,7 @@ func TestApplyRootfsDiff(t *testing.T) { if err != nil { t.Fatalf("glob staged copies: %v", err) } - if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), testr.New(t)); err != nil { + if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), "tar", testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } after, err := filepath.Glob(filepath.Join(os.TempDir(), rootfsDiffFilename+".*.tmp")) From bf0b6a3c96a7eb480d4b6ac6cb312f76f0165e57 Mon Sep 17 00:00:00 2001 From: Oleg Kushniriov Date: Wed, 26 Aug 2026 13:42:56 +0300 Subject: [PATCH 2/3] fix(agent): enforce absolute restore tar path and restore xattrs ApplyRootfsDiff now rejects a relative tar binary path, so an empty bundle dir can no longer degrade to PATH lookup inside the placeholder. Extraction now restores the xattrs the capture side archives, scoped to user.* and security.*: the archive is cut from the overlay upperdir and may carry trusted.overlay.* entries that must never be re-applied through the restored container's overlay mount. The tar-builder smoke test mirrors the exact restore flags and asserts a user xattr survives, failing the build if xattr support was compiled out. Signed-off-by: Oleg Kushniriov --- agent/Dockerfile | 8 +++++-- agent/internal/runtime/overlay.go | 16 ++++++++++++- agent/internal/runtime/overlay_test.go | 31 +++++++++++++++++++++----- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/agent/Dockerfile b/agent/Dockerfile index 23bcb6e0..73afdec3 100644 --- a/agent/Dockerfile +++ b/agent/Dockerfile @@ -103,6 +103,7 @@ ARG GNU_TAR_VERSION ARG GNU_TAR_SHA256 RUN apk add --no-cache \ + attr \ build-base \ ca-certificates \ curl \ @@ -143,11 +144,14 @@ RUN set -eu; \ echo preserved > /tmp/tar-smoke/target/existing; \ echo replaced > /tmp/tar-smoke/source/existing; \ echo created > /tmp/tar-smoke/source/created; \ - /out/tar -C /tmp/tar-smoke/source -cf /tmp/tar-smoke/archive.tar .; \ + setfattr -n user.snapshot-smoke -v smoke /tmp/tar-smoke/source/created; \ + /out/tar --xattrs -C /tmp/tar-smoke/source -cf /tmp/tar-smoke/archive.tar .; \ /out/tar --skip-old-files --blocking-factor=2048 \ + --xattrs --xattrs-include='user.*' --xattrs-include='security.*' \ -C /tmp/tar-smoke/target -xf /tmp/tar-smoke/archive.tar; \ grep -qx preserved /tmp/tar-smoke/target/existing; \ - grep -qx created /tmp/tar-smoke/target/created + grep -qx created /tmp/tar-smoke/target/created; \ + test "$(getfattr --only-values -n user.snapshot-smoke /tmp/tar-smoke/target/created)" = smoke # ============================================================================= # Stage: CUDA checkpoint helper builder diff --git a/agent/internal/runtime/overlay.go b/agent/internal/runtime/overlay.go index 1b252b6a..1cf3b34f 100644 --- a/agent/internal/runtime/overlay.go +++ b/agent/internal/runtime/overlay.go @@ -159,6 +159,12 @@ func CaptureDeletedFiles(upperDir, checkpointDir string) (bool, error) { // reads; doing that directly from NFS is much slower than one sequential copy // plus a local extract. func ApplyRootfsDiff(checkpointPath, targetRoot, tarBinary string, log logr.Logger) error { + // A relative path would resolve through PATH inside the placeholder — + // exactly the fallback the bundled static tar exists to prevent. + if !filepath.IsAbs(tarBinary) { + return fmt.Errorf("tar binary path %q is not absolute", tarBinary) + } + rootfsDiffPath := filepath.Join(checkpointPath, rootfsDiffFilename) info, err := os.Stat(rootfsDiffPath) if os.IsNotExist(err) { @@ -182,8 +188,16 @@ func ApplyRootfsDiff(checkpointPath, targetRoot, tarBinary string, log logr.Logg // --skip-old-files: silently skip files that already exist in the restore target. // The rootfs diff only contains overlay upperdir changes (runtime-generated files // like triton caches, tmp files) — base image files should not be overwritten. + // + // The capture side archives with --xattrs; restore them, but only the user + // and security namespaces. The archive is cut from the overlay upperdir, so + // it can carry trusted.overlay.* entries that must never be re-applied + // through the restored container's overlay mount. These flags are mirrored + // by the tar-builder smoke test in the Dockerfile — keep them in sync. log.Info("Applying rootfs diff", "target", targetRoot, "bytes", info.Size()) - cmd := exec.Command(tarBinary, "--skip-old-files", "--blocking-factor=2048", "-C", targetRoot, "-xf", localPath) + cmd := exec.Command(tarBinary, "--skip-old-files", "--blocking-factor=2048", + "--xattrs", "--xattrs-include=user.*", "--xattrs-include=security.*", + "-C", targetRoot, "-xf", localPath) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { diff --git a/agent/internal/runtime/overlay_test.go b/agent/internal/runtime/overlay_test.go index 33a5b583..ce118529 100644 --- a/agent/internal/runtime/overlay_test.go +++ b/agent/internal/runtime/overlay_test.go @@ -6,6 +6,7 @@ package runtime import ( "encoding/json" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -15,6 +16,17 @@ import ( "github.com/ai-dynamo/snapshot/agent/internal/types" ) +// systemTar resolves the host tar to an absolute path; ApplyRootfsDiff +// rejects relative tar paths so they cannot resolve through PATH. +func systemTar(t *testing.T) string { + t.Helper() + path, err := exec.LookPath("tar") + if err != nil { + t.Fatalf("resolve system tar: %v", err) + } + return path +} + func TestBuildExclusions(t *testing.T) { tests := []struct { name string @@ -180,7 +192,7 @@ func TestCaptureRootfsDiff(t *testing.T) { } targetRoot := t.TempDir() - if err := ApplyRootfsDiff(checkpointDir, targetRoot, "tar", testr.New(t)); err != nil { + if err := ApplyRootfsDiff(checkpointDir, targetRoot, systemTar(t), testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } data, err := os.ReadFile(filepath.Join(targetRoot, "generated.txt")) @@ -214,7 +226,7 @@ func TestCaptureRootfsDiff(t *testing.T) { func TestApplyRootfsDiff(t *testing.T) { t.Run("missing archive is no-op", func(t *testing.T) { - if err := ApplyRootfsDiff(t.TempDir(), t.TempDir(), "tar", testr.New(t)); err != nil { + if err := ApplyRootfsDiff(t.TempDir(), t.TempDir(), systemTar(t), testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } }) @@ -224,7 +236,7 @@ func TestApplyRootfsDiff(t *testing.T) { if err := os.WriteFile(filepath.Join(checkpointDir, rootfsDiffFilename), nil, 0644); err != nil { t.Fatalf("write empty rootfs diff: %v", err) } - if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), "tar", testr.New(t)); err != nil { + if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), systemTar(t), testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } }) @@ -234,11 +246,18 @@ func TestApplyRootfsDiff(t *testing.T) { if err := os.WriteFile(filepath.Join(checkpointDir, rootfsDiffFilename), []byte("not a tar archive"), 0644); err != nil { t.Fatalf("write invalid rootfs diff: %v", err) } - if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), "tar", testr.New(t)); err == nil { + if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), systemTar(t), testr.New(t)); err == nil { t.Fatal("ApplyRootfsDiff should fail for invalid non-empty archive") } }) + t.Run("relative tar path is rejected", func(t *testing.T) { + err := ApplyRootfsDiff(t.TempDir(), t.TempDir(), "tar", testr.New(t)) + if err == nil || !strings.Contains(err.Error(), "not absolute") { + t.Fatalf("ApplyRootfsDiff error = %v, want rejection of relative tar path", err) + } + }) + t.Run("uses the provided tar binary", func(t *testing.T) { checkpointDir := t.TempDir() if err := os.WriteFile(filepath.Join(checkpointDir, rootfsDiffFilename), []byte("not a tar archive"), 0644); err != nil { @@ -259,7 +278,7 @@ func TestApplyRootfsDiff(t *testing.T) { t.Fatalf("create temp file: %v", err) } f.Close() - if err := ApplyRootfsDiff(f.Name(), t.TempDir(), "tar", testr.New(t)); err == nil { + if err := ApplyRootfsDiff(f.Name(), t.TempDir(), systemTar(t), testr.New(t)); err == nil { t.Fatal("ApplyRootfsDiff should propagate non-ENOENT stat error") } }) @@ -278,7 +297,7 @@ func TestApplyRootfsDiff(t *testing.T) { if err != nil { t.Fatalf("glob staged copies: %v", err) } - if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), "tar", testr.New(t)); err != nil { + if err := ApplyRootfsDiff(checkpointDir, t.TempDir(), systemTar(t), testr.New(t)); err != nil { t.Fatalf("ApplyRootfsDiff: %v", err) } after, err := filepath.Glob(filepath.Join(os.TempDir(), rootfsDiffFilename+".*.tmp")) From d6ec542434df98b4a7797d48a8af7da6ceafbc9f Mon Sep 17 00:00:00 2001 From: Oleg Kushniriov Date: Wed, 26 Aug 2026 17:19:30 +0300 Subject: [PATCH 3/3] refactor(agent): address review nits on the restore tar bundle Name the bundled restore tar via runtime.RestoreTarBinaryName instead of a string literal, matching the cuda.HelperBinaryName convention, and move the tar-builder stage's two inline RUN blocks into standalone scripts (agent/scripts/fetch-gnu-tar.sh, build-gnu-tar.sh) per the repo's script-file convention. No behavior change; the smoke-test keep-in-sync note now lives in the build script and the ApplyRootfsDiff comment points there. Signed-off-by: Oleg Kushniriov --- agent/Dockerfile | 42 +++-------------------- agent/internal/executor/nsrestore.go | 2 +- agent/internal/runtime/overlay.go | 7 +++- agent/scripts/build-gnu-tar.sh | 50 ++++++++++++++++++++++++++++ agent/scripts/fetch-gnu-tar.sh | 19 +++++++++++ 5 files changed, 80 insertions(+), 40 deletions(-) create mode 100644 agent/scripts/build-gnu-tar.sh create mode 100644 agent/scripts/fetch-gnu-tar.sh diff --git a/agent/Dockerfile b/agent/Dockerfile index 73afdec3..2f75699e 100644 --- a/agent/Dockerfile +++ b/agent/Dockerfile @@ -111,47 +111,13 @@ RUN apk add --no-cache \ WORKDIR /tmp -RUN set -eu; \ - curl -fsSLo gnu-tar.tar.xz \ - "https://ftp.gnu.org/gnu/tar/tar-${GNU_TAR_VERSION}.tar.xz"; \ - echo "${GNU_TAR_SHA256} gnu-tar.tar.xz" | sha256sum -c -; \ - cp gnu-tar.tar.xz /gnu-tar-source.tar.xz; \ - xz -d gnu-tar.tar.xz; \ - tar -xf gnu-tar.tar +COPY scripts/fetch-gnu-tar.sh scripts/build-gnu-tar.sh /tmp/scripts/ + +RUN sh /tmp/scripts/fetch-gnu-tar.sh "${GNU_TAR_VERSION}" "${GNU_TAR_SHA256}" WORKDIR /tmp/tar-${GNU_TAR_VERSION} -RUN set -eu; \ - FORCE_UNSAFE_CONFIGURE=1 LDFLAGS=-static ./configure \ - --disable-nls \ - --without-posix-acls \ - --without-selinux; \ - make -j"$(nproc)"; \ - mkdir -p /out; \ - cp src/tar /out/tar; \ - cp COPYING /out/COPYING; \ - chmod 0755 /out/tar; \ - strip /out/tar; \ - if readelf -l /out/tar | grep -q 'INTERP'; then \ - echo "ERROR: restore tar has a dynamic ELF interpreter" >&2; \ - exit 1; \ - fi; \ - if readelf -d /out/tar | grep -q '(NEEDED)'; then \ - echo "ERROR: restore tar has shared-library dependencies" >&2; \ - exit 1; \ - fi; \ - mkdir -p /tmp/tar-smoke/source /tmp/tar-smoke/target; \ - echo preserved > /tmp/tar-smoke/target/existing; \ - echo replaced > /tmp/tar-smoke/source/existing; \ - echo created > /tmp/tar-smoke/source/created; \ - setfattr -n user.snapshot-smoke -v smoke /tmp/tar-smoke/source/created; \ - /out/tar --xattrs -C /tmp/tar-smoke/source -cf /tmp/tar-smoke/archive.tar .; \ - /out/tar --skip-old-files --blocking-factor=2048 \ - --xattrs --xattrs-include='user.*' --xattrs-include='security.*' \ - -C /tmp/tar-smoke/target -xf /tmp/tar-smoke/archive.tar; \ - grep -qx preserved /tmp/tar-smoke/target/existing; \ - grep -qx created /tmp/tar-smoke/target/created; \ - test "$(getfattr --only-values -n user.snapshot-smoke /tmp/tar-smoke/target/created)" = smoke +RUN sh /tmp/scripts/build-gnu-tar.sh # ============================================================================= # Stage: CUDA checkpoint helper builder diff --git a/agent/internal/executor/nsrestore.go b/agent/internal/executor/nsrestore.go index c672f234..21199c93 100644 --- a/agent/internal/executor/nsrestore.go +++ b/agent/internal/executor/nsrestore.go @@ -127,7 +127,7 @@ func executeRestore( timings = &nsrestorePhaseTimings{} overlayStart := time.Now() - tarBinary := filepath.Join(opts.BundleDir, "tar") + tarBinary := filepath.Join(opts.BundleDir, snapshotruntime.RestoreTarBinaryName) if err := snapshotruntime.ApplyRootfsDiff(opts.CheckpointPath, "/", tarBinary, log); err != nil { return nil, 0, nil, fmt.Errorf("rootfs diff failed: %w", err) } diff --git a/agent/internal/runtime/overlay.go b/agent/internal/runtime/overlay.go index 1cf3b34f..f8d849c5 100644 --- a/agent/internal/runtime/overlay.go +++ b/agent/internal/runtime/overlay.go @@ -20,6 +20,11 @@ import ( const ( rootfsDiffFilename = "rootfs-diff.tar" deletedFilesFilename = "deleted-files.json" + + // RestoreTarBinaryName is the file name of the statically linked GNU tar + // in the agent binary bundle. Must match the name the Dockerfile bundle + // stage copies into /snapshot-binaries/. + RestoreTarBinaryName = "tar" ) // GetRootFS returns the container's root filesystem path via /host/proc. @@ -193,7 +198,7 @@ func ApplyRootfsDiff(checkpointPath, targetRoot, tarBinary string, log logr.Logg // and security namespaces. The archive is cut from the overlay upperdir, so // it can carry trusted.overlay.* entries that must never be re-applied // through the restored container's overlay mount. These flags are mirrored - // by the tar-builder smoke test in the Dockerfile — keep them in sync. + // by the smoke test in scripts/build-gnu-tar.sh — keep them in sync. log.Info("Applying rootfs diff", "target", targetRoot, "bytes", info.Size()) cmd := exec.Command(tarBinary, "--skip-old-files", "--blocking-factor=2048", "--xattrs", "--xattrs-include=user.*", "--xattrs-include=security.*", diff --git a/agent/scripts/build-gnu-tar.sh b/agent/scripts/build-gnu-tar.sh new file mode 100644 index 00000000..ef85be63 --- /dev/null +++ b/agent/scripts/build-gnu-tar.sh @@ -0,0 +1,50 @@ +#!/bin/sh +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Build the statically linked GNU tar used for restore-time rootfs-diff +# extraction, assert it is fully static, and smoke-test the exact invocation +# runtime.ApplyRootfsDiff uses. Installs the binary and its license text to +# /out. Run from the unpacked GNU tar source directory. +# +# Usage: build-gnu-tar.sh +set -eu + +# FORCE_UNSAFE_CONFIGURE: GNU tar's configure refuses to run as root otherwise. +FORCE_UNSAFE_CONFIGURE=1 LDFLAGS=-static ./configure \ + --disable-nls \ + --without-posix-acls \ + --without-selinux +make -j"$(nproc)" + +mkdir -p /out +cp src/tar /out/tar +cp COPYING /out/COPYING +chmod 0755 /out/tar +strip /out/tar + +# A static binary has neither an ELF interpreter nor NEEDED entries; either +# one means the placeholder's userspace would leak back into restore. +if readelf -l /out/tar | grep -q 'INTERP'; then + echo "ERROR: restore tar has a dynamic ELF interpreter" >&2 + exit 1 +fi +if readelf -d /out/tar | grep -q '(NEEDED)'; then + echo "ERROR: restore tar has shared-library dependencies" >&2 + exit 1 +fi + +# Smoke test mirroring the restore invocation in runtime.ApplyRootfsDiff — +# keep these flags in sync with that call site. +mkdir -p /tmp/tar-smoke/source /tmp/tar-smoke/target +echo preserved > /tmp/tar-smoke/target/existing +echo replaced > /tmp/tar-smoke/source/existing +echo created > /tmp/tar-smoke/source/created +setfattr -n user.snapshot-smoke -v smoke /tmp/tar-smoke/source/created +/out/tar --xattrs -C /tmp/tar-smoke/source -cf /tmp/tar-smoke/archive.tar . +/out/tar --skip-old-files --blocking-factor=2048 \ + --xattrs --xattrs-include='user.*' --xattrs-include='security.*' \ + -C /tmp/tar-smoke/target -xf /tmp/tar-smoke/archive.tar +grep -qx preserved /tmp/tar-smoke/target/existing +grep -qx created /tmp/tar-smoke/target/created +test "$(getfattr --only-values -n user.snapshot-smoke /tmp/tar-smoke/target/created)" = smoke diff --git a/agent/scripts/fetch-gnu-tar.sh b/agent/scripts/fetch-gnu-tar.sh new file mode 100644 index 00000000..78b69640 --- /dev/null +++ b/agent/scripts/fetch-gnu-tar.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Download and verify the pinned GNU tar source tarball, keep a pristine copy +# at /gnu-tar-source.tar.xz for the image's corresponding-source staging, and +# unpack the source into the current directory for the build. +# +# Usage: fetch-gnu-tar.sh +set -eu + +VERSION="$1" +SHA256="$2" + +curl -fsSLo gnu-tar.tar.xz "https://ftp.gnu.org/gnu/tar/tar-${VERSION}.tar.xz" +echo "${SHA256} gnu-tar.tar.xz" | sha256sum -c - +cp gnu-tar.tar.xz /gnu-tar-source.tar.xz +xz -d gnu-tar.tar.xz +tar -xf gnu-tar.tar