From 6d09d885d792ea3ee630cd6503d80cba1a3f445b Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Sun, 24 May 2026 21:45:00 -0700 Subject: [PATCH] fix(dind): implement HEAD /containers/{id}/archive so docker cp works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported failure mode on stopped containers in nested containerd: $ docker run --name spc-build alpine:3.20 sh -c '...long build...' ...BUILD SUCCESSFUL... $ docker cp spc-build:/build/output.tar output.tar unable to get resource stat from response: unable to decode container path stat header: EOF Root cause was a routing gap, not a stopped-container bug. Docker CLI's `docker cp` issues a HEAD /containers/{id}/archive?path=… first to read the X-Docker-Container-Path-Stat header (base64 JSON of name/size/ mode/mtime/linkTarget) before falling through to GET for the tar bytes. routeContainer only matched MethodPut/MethodGet for the archive action; HEAD fell through to handleNotImplemented (501). The CLI's header decoder then read the missing header and produced the EOF above. Changes: * containers.go: add `case action == "archive" && r.Method == HEAD` routing to s.handleContainerStatPath. * exec.go: new handleContainerStatPath plus three small helpers shared with the existing GET path — rootfsSearchDirs overlay upperdir/lowerdir lookup, test-stubbable via rootfsSearchDirsFn resolveContainerPath first-match in upperdir-first order writeContainerPathStatHeader emit the base64 JSON header handleContainerCopyFrom is refactored to share the same helpers and now also sets the stat header on the GET response (some clients skip the HEAD pre-flight and rely on the GET header). * dind.go: optional rootfsSearchDirsFn on Server. Nil in production — falls through to the real containerd snapshotter path; tests inject to avoid standing up containerd. Tests in pkg/dind/archive_head_test.go: TestArchiveHEAD_NotFound proves the routing gap; before the fix this returned 501 (run on main without the routing change to confirm) TestArchiveHEAD_ReturnsStatHeader plants a fake container + rootfs, decodes the header, asserts the Docker-shaped struct populates Name/Size/Mode correctly TestArchiveGET_ReturnsStatHeader regression guard for the shared header-emit on GET responses Verified locally against the project Go 1.26.1 in WSL (the cgo preprocessing trap on Windows hosts noted in AGENTS.md still applies); GOOS=linux golangci-lint clean. --- pkg/dind/archive_head_test.go | 147 ++++++++++++++++++++++++++ pkg/dind/containers.go | 25 ++--- pkg/dind/dind.go | 9 +- pkg/dind/exec.go | 190 ++++++++++++++++++++++++++++------ 4 files changed, 324 insertions(+), 47 deletions(-) create mode 100644 pkg/dind/archive_head_test.go diff --git a/pkg/dind/archive_head_test.go b/pkg/dind/archive_head_test.go new file mode 100644 index 0000000..1e1a3b9 --- /dev/null +++ b/pkg/dind/archive_head_test.go @@ -0,0 +1,147 @@ +package dind + +import ( + "context" + "encoding/base64" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" +) + +// TestArchiveHEAD_NotFound is the lightweight routing test. A HEAD against +// /containers/{id}/archive for a container the daemon doesn't know about +// must respond 404, matching the existing PUT and GET handlers in +// TestCopyToNotFound / TestCopyFromNotFound. Before this fix the route +// fell through to handleNotImplemented (501) because no MethodHead case +// existed in routeContainer — that's exactly the routing gap that makes +// the Docker CLI's `docker cp` produce "unable to decode container path +// stat header: EOF" on stopped containers. +func TestArchiveHEAD_NotFound(t *testing.T) { + s := newTestServer(t) + client := dialServer(s) + + req, err := http.NewRequest("HEAD", "http://docker/containers/nonexistent/archive?path=/tmp", nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + resp, err := client.Do(req) + if err != nil { + t.Fatalf("HEAD /containers/nonexistent/archive: %v", err) + } + defer func() { + if err := resp.Body.Close(); err != nil { + t.Logf("closing response body: %v", err) + } + }() + + if resp.StatusCode != http.StatusNotFound { + t.Errorf("status = %d, want 404 (matching PUT and GET archive handlers)", resp.StatusCode) + } +} + +// TestArchiveHEAD_ReturnsStatHeader plants a fake containerEntry pointing at +// a tempdir with a known file, then issues a HEAD and asserts the +// X-Docker-Container-Path-Stat header decodes into a Docker-shaped struct +// with the expected name/size/mode. This is the precise contract the Docker +// CLI's StatPath uses — anything less than this and `docker cp` returns +// "unable to decode container path stat header: EOF" even on a 200 response. +func TestArchiveHEAD_ReturnsStatHeader(t *testing.T) { + rootfs := t.TempDir() + const wantName = "output.tar" + const wantBody = "hello-from-stopped-container\n" + if err := os.WriteFile(filepath.Join(rootfs, wantName), []byte(wantBody), 0o644); err != nil { + t.Fatalf("seeding rootfs file: %v", err) + } + + s := &Server{ + log: slog.New(slog.NewTextHandler(io.Discard, nil)), + containers: map[string]*containerEntry{}, + } + const cid = "abc123" + s.containers[cid] = &containerEntry{ID: cid, Status: "exited"} + // Test override: hand the HEAD handler our planted rootfs without + // going through containerd's snapshotter. + s.rootfsSearchDirsFn = func(_ context.Context, _ string) ([]string, error) { + return []string{rootfs}, nil + } + + req, err := http.NewRequest("HEAD", "http://docker/containers/"+cid+"/archive?path=/"+wantName, nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + rec := httptest.NewRecorder() + s.routeContainer(rec, req, "/containers/"+cid+"/archive") + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + + hdr := rec.Header().Get("X-Docker-Container-Path-Stat") + if hdr == "" { + t.Fatal("X-Docker-Container-Path-Stat header missing — CLI will fail with 'unable to decode container path stat header: EOF'") + } + raw, err := base64.StdEncoding.DecodeString(hdr) + if err != nil { + t.Fatalf("header not valid base64: %v", err) + } + // Docker's containerPathStat shape (engine-api types/container/file.go). + var got struct { + Name string `json:"name"` + Size int64 `json:"size"` + Mode os.FileMode `json:"mode"` + Mtime time.Time `json:"mtime"` + LinkTarget string `json:"linkTarget"` + } + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("header JSON: %v", err) + } + if got.Name != wantName { + t.Errorf("Name = %q, want %q", got.Name, wantName) + } + if got.Size != int64(len(wantBody)) { + t.Errorf("Size = %d, want %d", got.Size, len(wantBody)) + } + if got.Mode.IsDir() { + t.Errorf("Mode = %v, want regular file", got.Mode) + } +} + +// TestArchiveGET_ReturnsStatHeader asserts the same X-Docker-Container-Path-Stat +// header is set on the GET response too. Some Docker clients skip the HEAD +// pre-flight and rely on the header on the GET. Regression guard for the +// shared header-emit code path. +func TestArchiveGET_ReturnsStatHeader(t *testing.T) { + rootfs := t.TempDir() + const wantName = "data.bin" + if err := os.WriteFile(filepath.Join(rootfs, wantName), []byte("xyz"), 0o644); err != nil { + t.Fatalf("seeding rootfs file: %v", err) + } + s := &Server{ + log: slog.New(slog.NewTextHandler(io.Discard, nil)), + containers: map[string]*containerEntry{}, + } + const cid = "def456" + s.containers[cid] = &containerEntry{ID: cid, Status: "exited"} + s.rootfsSearchDirsFn = func(_ context.Context, _ string) ([]string, error) { + return []string{rootfs}, nil + } + + req, err := http.NewRequest("GET", "http://docker/containers/"+cid+"/archive?path=/"+wantName, nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + rec := httptest.NewRecorder() + s.routeContainer(rec, req, "/containers/"+cid+"/archive") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + if rec.Header().Get("X-Docker-Container-Path-Stat") == "" { + t.Error("GET response missing X-Docker-Container-Path-Stat header") + } +} diff --git a/pkg/dind/containers.go b/pkg/dind/containers.go index db84e09..c739c86 100644 --- a/pkg/dind/containers.go +++ b/pkg/dind/containers.go @@ -111,17 +111,17 @@ type createRequest struct { } type hostConfig struct { - Binds []string `json:"Binds"` - NetworkMode string `json:"NetworkMode"` - Privileged bool `json:"Privileged"` - SecurityOpt []string `json:"SecurityOpt"` - CapAdd []string `json:"CapAdd"` - Tmpfs map[string]string `json:"Tmpfs"` - PortBindings map[string][]portBinding `json:"PortBindings"` - RestartPolicy *restartPolicy `json:"RestartPolicy"` - Init *bool `json:"Init"` - CgroupnsMode string `json:"CgroupnsMode"` - ExtraHosts []string `json:"ExtraHosts"` + Binds []string `json:"Binds"` + NetworkMode string `json:"NetworkMode"` + Privileged bool `json:"Privileged"` + SecurityOpt []string `json:"SecurityOpt"` + CapAdd []string `json:"CapAdd"` + Tmpfs map[string]string `json:"Tmpfs"` + PortBindings map[string][]portBinding `json:"PortBindings"` + RestartPolicy *restartPolicy `json:"RestartPolicy"` + Init *bool `json:"Init"` + CgroupnsMode string `json:"CgroupnsMode"` + ExtraHosts []string `json:"ExtraHosts"` } type portBinding struct { @@ -181,6 +181,8 @@ func (s *Server) routeContainer(w http.ResponseWriter, r *http.Request, path str s.handleContainerCopyTo(w, r, id) case action == "archive" && r.Method == http.MethodGet: s.handleContainerCopyFrom(w, r, id) + case action == "archive" && r.Method == http.MethodHead: + s.handleContainerStatPath(w, r, id) default: s.handleNotImplemented(w, r) } @@ -1563,4 +1565,3 @@ func writeContainerHosts(entry *containerEntry) error { return os.WriteFile(entry.HostsPath, []byte(b.String()), 0o644) } - diff --git a/pkg/dind/dind.go b/pkg/dind/dind.go index aab80c7..241b6eb 100644 --- a/pkg/dind/dind.go +++ b/pkg/dind/dind.go @@ -47,11 +47,18 @@ type Server struct { log *slog.Logger mu sync.Mutex - images map[string]*imageEntry // in-memory image store scoped to this job + images map[string]*imageEntry // in-memory image store scoped to this job containers map[string]*containerEntry // containers created through this socket execs map[string]*execEntry // exec processes inside containers networks map[string]*networkEntry // Docker networks (logical, in-memory) auth authCache // per-job docker login cache (registry host → creds) + + // rootfsSearchDirsFn resolves the host-filesystem directories that + // together form the merged rootfs view for a container snapshot. + // Nil in production — handleContainerStatPath / handleContainerCopyFrom + // fall through to the real containerd snapshotter path. Tests stub + // this to avoid standing up containerd. + rootfsSearchDirsFn func(ctx context.Context, snapshotID string) ([]string, error) } type imageEntry struct { diff --git a/pkg/dind/exec.go b/pkg/dind/exec.go index 7a5ada6..8e7c488 100644 --- a/pkg/dind/exec.go +++ b/pkg/dind/exec.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "context" + "encoding/base64" "encoding/binary" "encoding/json" "errors" @@ -731,9 +732,101 @@ func (s *Server) copyToViaExec(w http.ResponseWriter, r *http.Request, entry *co w.WriteHeader(http.StatusOK) } -// handleContainerCopyFrom handles GET /containers/{id}/archive. -// Tars up files from the container's rootfs and streams them back. -func (s *Server) handleContainerCopyFrom(w http.ResponseWriter, r *http.Request, id string) { +// rootfsSearchDirs returns the host-filesystem directories whose union forms +// the container's merged rootfs view — upperdir first (container-written +// files take precedence), then each lowerdir. Used by both HEAD and GET +// /containers/{id}/archive to look up a file inside a container's rootfs +// without execing into a (possibly stopped) container. +// +// In production it reads the overlayfs mount options from containerd's +// snapshotter. Tests can substitute s.rootfsSearchDirsFn to point at a +// scratch tempdir. +func (s *Server) rootfsSearchDirs(ctx context.Context, snapshotID string) ([]string, error) { + if s.rootfsSearchDirsFn != nil { + return s.rootfsSearchDirsFn(ctx, snapshotID) + } + if s.client == nil { + return nil, fmt.Errorf("containerd client not available") + } + snapshotter := s.client.SnapshotService("overlayfs") + if snapshotter == nil { + return nil, fmt.Errorf("snapshotter not available") + } + mounts, err := snapshotter.Mounts(ctx, snapshotID) + if err != nil { + return nil, fmt.Errorf("getting snapshot mounts: %w", err) + } + dirs := []string{} + for _, m := range mounts { + for _, opt := range m.Options { + for _, part := range strings.Split(opt, ",") { + if strings.HasPrefix(part, "upperdir=") { + dirs = append(dirs, strings.TrimPrefix(part, "upperdir=")) + } + if strings.HasPrefix(part, "lowerdir=") { + dirs = append(dirs, strings.Split(strings.TrimPrefix(part, "lowerdir="), ":")...) + } + } + } + } + return dirs, nil +} + +// resolveContainerPath walks the rootfs search dirs in upperdir-first order +// and returns the absolute host path of the first match for srcPath, or +// empty string if it's not present in any layer. +func resolveContainerPath(searchDirs []string, srcPath string) string { + for _, dir := range searchDirs { + candidate := filepath.Join(dir, filepath.Clean(srcPath)) + if _, err := os.Stat(candidate); err == nil { + return candidate + } + } + return "" +} + +// dockerPathStat mirrors Docker's engine-api container/file.go PathStat, +// which is what the CLI's StatPath unmarshals from the +// X-Docker-Container-Path-Stat response header. +type dockerPathStat struct { + Name string `json:"name"` + Size int64 `json:"size"` + Mode os.FileMode `json:"mode"` + Mtime time.Time `json:"mtime"` + LinkTarget string `json:"linkTarget"` +} + +// writeContainerPathStatHeader serialises fi into the header Docker CLI +// expects on HEAD/GET /containers/{id}/archive responses. Without this +// header set, `docker cp` fails with "unable to decode container path +// stat header: EOF" even when the GET body is well-formed. +func writeContainerPathStatHeader(h http.Header, fullPath string, fi os.FileInfo) error { + stat := dockerPathStat{ + Name: filepath.Base(fullPath), + Size: fi.Size(), + Mode: fi.Mode(), + Mtime: fi.ModTime(), + } + if fi.Mode()&os.ModeSymlink != 0 { + if tgt, err := os.Readlink(fullPath); err == nil { + stat.LinkTarget = tgt + } + } + raw, err := json.Marshal(&stat) + if err != nil { + return err + } + h.Set("X-Docker-Container-Path-Stat", base64.StdEncoding.EncodeToString(raw)) + return nil +} + +// handleContainerStatPath handles HEAD /containers/{id}/archive. Docker CLI +// calls this before GET to discover whether the source is a file or a +// directory and to validate it exists, then parses the +// X-Docker-Container-Path-Stat header out of the response. Without a HEAD +// handler the route fell through to handleNotImplemented (501), making +// the CLI fail on the header decode with EOF — see PR description. +func (s *Server) handleContainerStatPath(w http.ResponseWriter, r *http.Request, id string) { s.mu.Lock() entry, ok := s.containers[id] s.mu.Unlock() @@ -752,52 +845,72 @@ func (s *Server) handleContainerCopyFrom(w http.ResponseWriter, r *http.Request, return } - // For running containers, exec tar inside to stream files out. - // For stopped containers, read from the overlay upperdir. ctx := namespaces.WithNamespace(r.Context(), s.jobNamespace) - snapshotID := entry.ID + "-snapshot" - snapshotter := s.client.SnapshotService("overlayfs") - if snapshotter == nil { + searchDirs, err := s.rootfsSearchDirs(ctx, entry.ID+"-snapshot") + if err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{ - "message": "snapshotter not available", + "message": err.Error(), }) return } - mounts, err := snapshotter.Mounts(ctx, snapshotID) + fullPath := resolveContainerPath(searchDirs, srcPath) + if fullPath == "" { + writeJSON(w, http.StatusNotFound, map[string]string{ + "message": fmt.Sprintf("path %s not found in container", srcPath), + }) + return + } + fi, err := os.Lstat(fullPath) if err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{ - "message": fmt.Sprintf("getting snapshot mounts: %v", err), + "message": fmt.Sprintf("statting %s: %v", srcPath, err), }) return } + if err := writeContainerPathStatHeader(w.Header(), fullPath, fi); err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{ + "message": fmt.Sprintf("encoding stat header: %v", err), + }) + return + } + w.WriteHeader(http.StatusOK) +} - // Find a directory where we can read the merged view. For overlayfs, - // the lowerdir+upperdir together form the container's rootfs. We look - // in the upperdir first (container-written files), then fall back to - // searching lowerdirs. - searchDirs := []string{} - for _, m := range mounts { - for _, opt := range m.Options { - for _, part := range strings.Split(opt, ",") { - if strings.HasPrefix(part, "upperdir=") { - searchDirs = append(searchDirs, strings.TrimPrefix(part, "upperdir=")) - } - if strings.HasPrefix(part, "lowerdir=") { - searchDirs = append(searchDirs, strings.Split(strings.TrimPrefix(part, "lowerdir="), ":")...) - } - } - } +// handleContainerCopyFrom handles GET /containers/{id}/archive. +// Tars up files from the container's rootfs and streams them back. For +// running containers the merged overlay view is read via the snapshotter; +// for stopped containers the same view is still valid because the upper +// and lower dirs survive the task exit until the container is removed. +func (s *Server) handleContainerCopyFrom(w http.ResponseWriter, r *http.Request, id string) { + s.mu.Lock() + entry, ok := s.containers[id] + s.mu.Unlock() + if !ok { + writeJSON(w, http.StatusNotFound, map[string]string{ + "message": fmt.Sprintf("container %s not found", id), + }) + return } - var fullPath string - for _, dir := range searchDirs { - candidate := filepath.Join(dir, filepath.Clean(srcPath)) - if _, err := os.Stat(candidate); err == nil { - fullPath = candidate - break - } + srcPath := r.URL.Query().Get("path") + if srcPath == "" { + writeJSON(w, http.StatusBadRequest, map[string]string{ + "message": "path query parameter is required", + }) + return } + + ctx := namespaces.WithNamespace(r.Context(), s.jobNamespace) + searchDirs, err := s.rootfsSearchDirs(ctx, entry.ID+"-snapshot") + if err != nil { + writeJSON(w, http.StatusInternalServerError, map[string]string{ + "message": err.Error(), + }) + return + } + + fullPath := resolveContainerPath(searchDirs, srcPath) if fullPath == "" { writeJSON(w, http.StatusNotFound, map[string]string{ "message": fmt.Sprintf("path %s not found in container", srcPath), @@ -805,6 +918,15 @@ func (s *Server) handleContainerCopyFrom(w http.ResponseWriter, r *http.Request, return } + // Set X-Docker-Container-Path-Stat on the GET response too — the + // Docker CLI reads it from both HEAD and GET, and some clients skip + // HEAD entirely and rely on the GET header. + if fi, err := os.Lstat(fullPath); err == nil { + if hdrErr := writeContainerPathStatHeader(w.Header(), fullPath, fi); hdrErr != nil { + s.log.Debug("encoding stat header for GET", "error", hdrErr) + } + } + w.Header().Set("Content-Type", "application/x-tar") w.WriteHeader(http.StatusOK)