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)