refactor(registryproxy): simplify the proxy store, guest timing contract and size parsing

commit: 2b9f5c9fb95d55fad182de9da4823104fb636193
author: brice · 21h ago · 2026-10-21 15:19 UTC
parent: 57617df
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
checks: 2/2

56 files · +2184 -1857

M docs/cache-and-artifacts.md +18 -16

@@ -1,6 +1,6 @@
1	1		# Cache and artifacts
2	2
3		-	The runner serves the GitHub Actions cache and artifacts v4 to jobs. Both use one channel per sandbox.
	3	+	The runner serves the GitHub Actions cache, artifacts v4 and a Docker Hub mirror to jobs. All three use one channel per sandbox.
4	4
5	5		## Channel
6	6

@@ -84,11 +84,11 @@ Works with `actions/cache`, `setup-go` and `setup-node` with `cache:`, and anyth
84	84
85	85		The runner provides an anonymous, pull-only Docker Hub mirror on the same private
86	86		channel. Each job's dockerd uses `http://127.0.0.1:9089` as a registry mirror.
87		-	Other registries are not cached; there is no HTTPS interception or guest network
88		-	policy change. Guest credentials are never forwarded upstream. Private pulls
	87	+	Other registries are not cached. There is no HTTPS interception, and the mirror
	88	+	needs no network policy rule. Guest credentials are never forwarded upstream. Private pulls
89	89		fall back to Docker Hub directly, where dockerd can use the job's credentials.
90	90
91		-	Verified sha256 blobs and manifests live under `<state>/registry/`. Tags are
	91	+	Verified sha256 blobs and manifests live under `<state dir>/registry/`. Tags are
92	92		revalidated by an upstream HEAD on each request, with the client's Accept headers
93	93		reduced to the known manifest media types. If Docker Hub is unreachable, times
94	94		out, returns a server error or rate-limits (429), a previously cached tag can be

@@ -111,24 +111,26 @@ have at most 8 registry requests in progress; one more gets 503 at once.
111	111
112	112		Downloads have no total time limit, but they fail after 30 s without progress.
113	113		Auth, manifest and HEAD requests are limited to 10 s. Auth and manifest requests
114		-	never follow redirects; blob requests follow the CDN redirect without the bearer
115		-	token. Stored blobs support range requests. Cold blob HEAD requests fetch only
116		-	upstream metadata, not the layer body.
	114	+	never follow redirects; blob requests, HEAD included, follow the CDN redirect
	115	+	without the bearer token. Stored blobs support range requests. Cold blob HEAD
	116	+	requests fetch only upstream metadata, not the layer body.
117	117
118	118		`ERMITE_RUNNER_REGISTRY_CACHE_MAX` defaults to `10G`. Blobs, manifests and tag
119		-	files (charged 4 KiB each) share one least-recently-used eviction; an entry that
120		-	is being served is never evicted. This cap is independent of
	119	+	files share one least-recently-used eviction; each is charged at least 4 KiB, so
	120	+	the number of files stays below the cap divided by 4 KiB. An entry that is being served is
	121	+	never evicted. This cap is independent of
121	122		`ERMITE_RUNNER_CACHE_MAX`. A zero size in any unit (`0`, `0M`, `0G`) disables only
122	123		the proxy. `ERMITE_RUNNER_CACHE=0` disables the whole channel and therefore also
123	124		the proxy (logged at startup). Dockerd retains its mirror setting and falls back
124		-	to Docker Hub if the mirror is unavailable. No deployment is needed to clear
125		-	entries: stop the runner before removing `<state>/registry/`.
	125	+	to Docker Hub if the mirror is unavailable. To clear the cache, stop the runner
	126	+	and remove `<state dir>/registry/`.
126	127
127		-	Jobs with neither cache permissions nor artifacts still get the loopback channel
128		-	for dockerd, but no `ACTIONS_CACHE_URL`, `ACTIONS_RESULTS_URL`, or
	128	+	Jobs with neither cache permissions nor artifacts get the loopback channel for
	129	+	dockerd when the proxy is on, but no `ACTIONS_CACHE_URL`, `ACTIONS_RESULTS_URL`, or
129	130		`ACTIONS_RUNTIME_TOKEN` is exposed to their steps. Registry routes need no token:
130	131		only public content is served over a channel exclusive to that sandbox.
131	132
132		-	The host logs one `registry manifest` line per request with `repo`, `ref`, `hit`
133		-	and `ms`. Blob misses log `registry blob` at debug level with `digest`, `bytes`
134		-	and `ms`.
	133	+	The host logs one `registry manifest` line per request with `job_id`, `repo`,
	134	+	`ref`, `hit` and `ms`. Blob misses log `registry blob` at debug level with
	135	+	`job_id`, `digest`, `bytes` and `ms`; `job_id` is the job that started the
	136	+	download.

M docs/development.md +2 -1


@@ -23,6 +23,7 @@ They are opt-in. They run the real stub executor. Output and exit codes come bac
23	23		\| `ERMITE_RUNNER_IT_E2E=1` \| End-to-end job tests in `internal/e2e` \|
24	24		\| `ERMITE_RUNNER_IT_NET=1` \| Packet-level firewall test \|
25	25		\| `ERMITE_RUNNER_IT_DOCKER_API=1` \| Docker Engine calls against any daemon, including rootless \|
	26	+	\| `ERMITE_RUNNER_IT_REGISTRY=1` \| Live Docker Hub pulls through the registry proxy with `curl` (`TestDockerHubCurl`). Needs internet access \|
26	27		\| `ERMITE_RUNNER_IT_NODE_DIR` \| Directory with `node-v24.21.0-linux-x64.tar.gz`, for offline hosts \|
27	28		\| `ERMITE_RUNNER_TEST_IMAGE` \| Image used by the container and end-to-end tests \|
28	29

@@ -70,7 +71,7 @@ Build and register the [runner container](container.md) as usual, then constrain
70	71		its CPU budget to reproduce a small host without changing the boot sequence:
71	72
72	73		```sh
73		-	docker run --name runner-timing --cpus=1 \
	74	+	docker run -d --name runner-timing --cpus=1 \
74	75		--device /dev/kvm --device /dev/net/tun --cap-add NET_ADMIN \
75	76		-v runner-timing-state:/var/lib/ermite-runner \
76	77		--env-file runner.env -e ERMITE_RUNNER_LOG_FORMAT=json ermite-runner:<tag>

M docs/images.md +1 -0


@@ -29,6 +29,7 @@ scripts/build-image.sh tmp/image # builds the OCI image and tmp/image/base.ex
29	29		- `/home/runner/work` and `/opt/hostedtoolcache`, owned by `runner`. The tool cache holds the toolchains listed in `images/ubuntu-26.04/toolcache.lock` for the image architecture.
30	30		- `bash` and `sh`. `python3` and a `python` command for `shell: python`.
31	31		- `dockerd` and passwordless `sudo` for `runner`, for jobs that use Docker.
	32	+	- No `registry-mirrors` in `/etc/docker/daemon.json`. The runner starts dockerd with `--registry-mirror` (its Docker Hub mirror), and dockerd refuses to start when an option is set both as a flag and in that file.
32	33		- Common tools: git, curl, jq, build-essential, ca-certificates, tzdata, zip and unzip.
33	34
34	35		`pwsh` is not in the image. A step that asks for it fails with `pwsh: command not found`.

M docs/service.md +4 -3


@@ -67,9 +67,10 @@ The security boundary of a job is its microVM or container, not the unit.
67	67
68	68		## Job timing
69	69
70		-	The existing `job finished` line carries integer milliseconds (all `*_ms`
71		-	fields). Phases that did not run are omitted; driver timings are available only
72		-	when sandbox start succeeds. Guest phases are omitted when they did not run.
	70	+	The `job finished` line carries integer milliseconds (all `*_ms` fields).
	71	+	Phases that did not run are omitted, and the driver (`sb_*`) timings appear only
	72	+	when the sandbox started. `resolve_ms` includes `archive_miss_ms`, and both are
	73	+	part of `run_ms`.
73	74
74	75		\| Fields \| Meaning \|
75	76		\| --- \| --- \|

M host_cmd.go +17 -20


@@ -53,9 +53,13 @@ func setupCache(ctx context.Context, scfg sandbox.Config) (cachesvc.Service, *
53	53		default:
54	54		return nil, nil, fmt.Errorf("%s: %q is not 0 or 1", envCache, os.Getenv(envCache))
55	55		}
56		-	max, err := cachesvc.ParseMax(os.Getenv(cachesvc.EnvMax))
	56	+	// The cache API cannot be turned off by its size: 0 is refused.
	57	+	max, err := cachesvc.ParseSize(os.Getenv(cachesvc.EnvMax), cachesvc.DefaultMax)
	58	+	if err == nil && max == 0 {
	59	+	err = errors.New("0 is not a size cap; " + envCache + "=0 turns the cache off")
	60	+	}
57	61		if err != nil {
58		-	return nil, nil, err
	62	+	return nil, nil, fmt.Errorf("%s: %w", cachesvc.EnvMax, err)
59	63		}
60	64		archives, err := setupArchives(ctx, scfg.StateDir)
61	65		if err != nil {

@@ -79,9 +83,9 @@ func setupCache(ctx context.Context, scfg sandbox.Config) (cachesvc.Service, *
79	83		// service, so it is only used when that is on.
80	84		func setupArchives(ctx context.Context, stateDir string) (*actionscache.Cache, error) {
81	85		log := zerowrap.FromCtx(ctx)
82		-	max, err := actionscache.ParseMax(os.Getenv(actionscache.EnvMax))
	86	+	max, err := cachesvc.ParseSize(os.Getenv(actionscache.EnvMax), actionscache.DefaultMax)
83	87		if err != nil {
84		-	return nil, err
	88	+	return nil, fmt.Errorf("%s: %w", actionscache.EnvMax, err)
85	89		}
86	90		if max == 0 {
87	91		log.Info().Msg("the action archive cache is off (" + actionscache.EnvMax + "=0)")

@@ -95,24 +99,24 @@ func setupArchives(ctx context.Context, stateDir string) (*actionscache.Cache, e
95	99		return c, nil
96	100		}
97	101
	102	+	// envRegistryMax is the size cap of the Docker Hub mirror (default
	103	+	// defaultRegistryMax); 0 turns it off.
98	104		const envRegistryMax = "ERMITE_RUNNER_REGISTRY_CACHE_MAX"
99	105
	106	+	const defaultRegistryMax = 10 << 30
	107	+
100	108		// setupRegistry opens the Docker Hub mirror. The caller only calls this when
101	109		// the cache channel is enabled; its capacity is independent of the cache API.
102	110		func setupRegistry(ctx context.Context, stateDir string) (http.Handler, error) {
103	111		log := zerowrap.FromCtx(ctx)
104		-	value := strings.TrimSpace(os.Getenv(envRegistryMax))
105		-	if zeroSize(value) {
106		-	log.Info().Msg("registry proxy off (" + envRegistryMax + "=" + value + ")")
107		-	return nil, nil
108		-	}
109		-	if value == "" {
110		-	value = "10G"
111		-	}
112		-	max, err := cachesvc.ParseMax(value)
	112	+	max, err := cachesvc.ParseSize(os.Getenv(envRegistryMax), defaultRegistryMax)
113	113		if err != nil {
114	114		return nil, fmt.Errorf("%s: %w", envRegistryMax, err)
115	115		}
	116	+	if max == 0 {
	117	+	log.Info().Msg("registry proxy off (" + envRegistryMax + "=0)")
	118	+	return nil, nil
	119	+	}
116	120		p, err := registryproxy.New(registryproxy.Config{Dir: filepath.Join(stateDir, "registry"), Max: max, Log: log})
117	121		if err != nil {
118	122		return nil, err

@@ -121,12 +125,6 @@ func setupRegistry(ctx context.Context, stateDir string) (http.Handler, error) {
121	125		return p, nil
122	126		}
123	127
124		-	// zeroSize reports a size of zero in any unit: 0, 00, 0M, 0G, 0GiB...
125		-	func zeroSize(v string) bool {
126		-	digits := strings.TrimRight(v, "kKmMgGtTiIbB")
127		-	return digits != "" && strings.Trim(digits, "0") == ""
128		-	}
129		-
130	128		// envCache=0 turns the cache service off.
131	129		const envCache = "ERMITE_RUNNER_CACHE"
132	130

@@ -345,7 +343,6 @@ func runCmd(ctx context.Context) int {
345	343		},
346	344		Features: hcfg.ServerFeaturesFunc(url),
347	345		Cache: cache,
348		-	Registry: cache != nil && cache.RegistryEnabled(),
349	346		Archives: archives,
350	347		Results: results,
351	348		Artifacts: func(tok string) ermiteconnect.JobArtifactServiceClient {

M images/ubuntu-26.04/install-toolcache.sh +17 -5

@@ -1,7 +1,10 @@
1	1		#!/bin/sh
2	2		# SPDX-License-Identifier: MIT
	3	+	# Installs the toolchains of a lock file (tool version arch sha256 url per
	4	+	# line) for the image architecture into the @actions/tool-cache layout.
3	5		set -eu
4	6
	7	+	lock=${1:?usage: install-toolcache.sh LOCK_FILE}
5	8		case "$(dpkg --print-architecture)" in
6	9		amd64) arch=x64 ;;
7	10		arm64) arch=arm64 ;;

@@ -9,22 +12,31 @@ case "$(dpkg --print-architecture)" in
9	12		esac
10	13		cache=${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}
11	14		tmp=$(mktemp -d)
12		-	trap 'rm -rf "$tmp"' EXIT HUP INT TERM
13		-	while read -r tool version entry_arch sha url; do
	15	+	trap 'rm -rf "$tmp"' EXIT
	16	+	trap 'exit 130' INT TERM
	17	+	installed=0
	18	+	# The last line is read even without a final newline.
	19	+	while read -r tool version entry_arch sha url \|\| [ -n "$tool" ]; do
14	20		case "$tool" in ''\|'#'*) continue ;; esac
15		-	[ "$entry_arch" = "$arch" ] \|\| continue
16	21		case "$tool" in go\|node) ;; *) echo "unsupported tool: $tool" >&2; exit 1 ;; esac
	22	+	case "$entry_arch" in x64\|arm64) ;; *) echo "unsupported arch for $tool $version: $entry_arch" >&2; exit 1 ;; esac
	23	+	[ "$entry_arch" = "$arch" ] \|\| continue
17	24		case "$url" in
18	25		*.tar.gz) compression=z ;;
19	26		*.tar.xz) compression=J ;;
20	27		*) echo "unsupported archive: $url" >&2; exit 1 ;;
21	28		esac
22		-	curl -fsSL "$url" -o "$tmp/archive"
	29	+	curl -fsSL --retry 3 "$url" -o "$tmp/archive"
23	30		printf '%s %s\n' "$sha" "$tmp/archive" \| sha256sum -c -
24	31		dest="$cache/$tool/$version/$arch"
25	32		mkdir -p "$dest"
26	33		tar -x"$compression"f "$tmp/archive" -C "$dest" --strip-components=1 --no-same-owner
27	34		touch "$dest.complete"
28	35		rm "$tmp/archive"
29		-	done < "${1:?usage: install-toolcache.sh LOCK_FILE}"
	36	+	installed=$((installed + 1))
	37	+	done < "$lock"
	38	+	if [ "$installed" -eq 0 ]; then
	39	+	echo "no toolchain for $arch in $lock" >&2
	40	+	exit 1
	41	+	fi
30	42		chown -R runner:runner "$cache"

M internal/actionscache/cache.go +5 -35


@@ -316,13 +316,12 @@ func (w *countWriter) Write(p []byte) (int, error) {
316	316
317	317		// verify re-hashes the archive of key against its sum. Any mismatch (or a
318	318		// missing file) drops the entry and returns ErrNotFound.
319		-	func (c *Cache) verify(key string) (duration time.Duration, err error) {
	319	+	func (c *Cache) verify(key string) (time.Duration, error) {
320	320		start := time.Now()
321		-	defer func() { duration = time.Since(start) }()
322	321		sb, err := os.ReadFile(c.sumPath(key))
323	322		if err != nil {
324	323		c.drop(key)
325		-	return 0, ErrNotFound
	324	+	return time.Since(start), ErrNotFound
326	325		}
327	326		want, sizeS, _ := strings.Cut(strings.TrimSpace(string(sb)), " ")
328	327		size, perr := strconv.ParseInt(sizeS, 10, 64)

@@ -332,7 +331,7 @@ func (c *Cache) verify(key string) (duration time.Duration, err error) {
332	331		_ = f.Close()
333	332		}
334	333		c.drop(key)
335		-	return 0, ErrNotFound
	334	+	return time.Since(start), ErrNotFound
336	335		}
337	336		defer func() { _ = f.Close() }()
338	337		h := sha256.New()

@@ -340,9 +339,9 @@ func (c *Cache) verify(key string) (duration time.Duration, err error) {
340	339		if err != nil \|\| n != size \|\| hex.EncodeToString(h.Sum(nil)) != want {
341	340		c.log.Warn().Str("key", key).Msg("action archive failed its integrity check; dropped")
342	341		c.drop(key)
343		-	return 0, ErrNotFound
	342	+	return time.Since(start), ErrNotFound
344	343		}
345		-	return 0, nil
	344	+	return time.Since(start), nil
346	345		}
347	346
348	347		func (c *Cache) drop(key string) {

@@ -419,34 +418,5 @@ func (c *Cache) Size() int64 {
419	418		return c.total
420	419		}
421	420
422		-	// ParseMax reads ERMITE_RUNNER_ACTIONS_CACHE_MAX: empty is DefaultMax, 0 is off
423		-	// (returned as 0), otherwise a number of bytes or a size with a K, M, G or T
424		-	// suffix (optionally followed by iB or B).
425		-	func ParseMax(v string) (int64, error) {
426		-	v = strings.TrimSpace(v)
427		-	if v == "" {
428		-	return DefaultMax, nil
429		-	}
430		-	s := strings.TrimRight(strings.ToUpper(v), "IB")
431		-	mult := int64(1)
432		-	if n := len(s); n > 0 {
433		-	switch s[n-1] {
434		-	case 'K':
435		-	mult, s = 1<<10, s[:n-1]
436		-	case 'M':
437		-	mult, s = 1<<20, s[:n-1]
438		-	case 'G':
439		-	mult, s = 1<<30, s[:n-1]
440		-	case 'T':
441		-	mult, s = 1<<40, s[:n-1]
442		-	}
443		-	}
444		-	n, err := strconv.ParseInt(s, 10, 64)
445		-	if err != nil \|\| n < 0 \|\| n > (1<<62)/mult {
446		-	return 0, fmt.Errorf("%s: %q is not a size (examples: 2G, 500M, 0)", EnvMax, v)
447		-	}
448		-	return n * mult, nil
449		-	}
450		-
451	421		// Dir is the directory of the cache (tests).
452	422		func (c *Cache) Dir() string { return c.dir }

M internal/actionscache/cache_test.go +0 -13


@@ -383,16 +383,3 @@ func TestKeyNeverUsesARef(t *testing.T) {
383	383		t.Error("shape")
384	384		}
385	385		}
386		-
387		-	func TestParseMax(t *testing.T) {
388		-	for in, want := range map[string]int64{"": DefaultMax, "0": 0, "500M": 500 << 20, "2G": 2 << 30, "2GiB": 2 << 30, "1024": 1024} {
389		-	if got, err := ParseMax(in); err != nil \|\| got != want {
390		-	t.Errorf("%q: %d %v", in, got, err)
391		-	}
392		-	}
393		-	for _, in := range []string{"-1", "x", "G"} {
394		-	if _, err := ParseMax(in); err == nil {
395		-	t.Errorf("%q accepted", in)
396		-	}
397		-	}
398		-	}

M internal/cachesvc/handler.go +16 -9


@@ -48,9 +48,9 @@ func (s *Service) Handler() http.Handler { return s.handler("") }
48	48		func (s *Service) handler(job string) http.Handler {
49	49		mux := http.NewServeMux()
50	50		if s.registry != nil {
51		-	registry := s.serveRegistry(make(chan struct{}, s.registryPerChannel), registryproxy.NewQuota(s.registryDownloads))
52		-	mux.HandleFunc("GET /v2/", registry)
53		-	mux.HandleFunc("HEAD /v2/", registry)
	51	+	registry := s.serveRegistry(job, make(chan struct{}, s.registryPerChannel), registryproxy.NewQuota(RegistryDownloadsPerChannel))
	52	+	mux.HandleFunc("GET "+RegistryPath, registry)
	53	+	mux.HandleFunc("HEAD "+RegistryPath, registry)
54	54		}
55	55		// The routes are those of the toolkit's v1 client (cacheHttpClient.ts):
56	56		// GET cache?keys=k1,k2&version=v :89 lookup

@@ -80,8 +80,8 @@ func (s *Service) handler(job string) http.Handler {
80	80		})
81	81		}
82	82
83		-	// channelHandler is the handler of a sandbox's channel: the cache API and the
84		-	// action archives, and when results is not nil everything else goes to it (the results service of the
	83	+	// channelHandler is the handler of a sandbox's channel: the cache API, the
	84	+	// action archives and the registry, and when results is not nil everything else goes to it (the results service of the
85	85		// job: Twirp and blob paths). The results service gets the same progress
86	86		// deadlines as the cache: its bodies and replies must keep moving.
87	87		func (s *Service) channelHandler(job string, results http.Handler) http.Handler {

@@ -90,7 +90,7 @@ func (s *Service) channelHandler(job string, results http.Handler) http.Handler
90	90		return cache
91	91		}
92	92		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
93		-	if strings.HasPrefix(r.URL.Path, APIPath) \|\| strings.HasPrefix(r.URL.Path, ArchivePath) \|\| strings.HasPrefix(r.URL.Path, "/v2/") {
	93	+	if strings.HasPrefix(r.URL.Path, APIPath) \|\| strings.HasPrefix(r.URL.Path, ArchivePath) \|\| strings.HasPrefix(r.URL.Path, RegistryPath) {
94	94		cache.ServeHTTP(w, r)
95	95		return
96	96		}

@@ -395,6 +395,8 @@ func (p *progressWriter) Write(b []byte) (int, error) {
395	395		// Unwrap lets http.ResponseController reach the real writer.
396	396		func (p *progressWriter) Unwrap() http.ResponseWriter { return p.ResponseWriter }
397	397
	398	+	var _ registryproxy.KeepAliver = (*progressWriter)(nil)
	399	+
398	400		// KeepAlive moves the deadlines without writing. The registry proxy calls it
399	401		// while the client waits for a shared upstream download that is progressing,
400	402		// before the first byte of the reply. The read deadline moves too: once it

@@ -411,8 +413,9 @@ func (p *progressWriter) KeepAlive() {
411	413		// slots caps the requests of this channel in progress at once; one over the
412	414		// cap gets 503 at once, which dockerd retries or answers by pulling from
413	415		// Docker Hub, so the channel's connections stay free for the cache. quota is
414		-	// the channel's share of the proxy's cold downloads.
415		-	func (s Service) serveRegistry(slots chan struct{}, quota registryproxy.Quota) http.HandlerFunc {
	416	+	// the channel's share of the proxy's cold downloads. job, when not empty, is
	417	+	// the channel's job, which the registry logs.
	418	+	func (s Service) serveRegistry(job string, slots chan struct{}, quota registryproxy.Quota) http.HandlerFunc {
416	419		return func(w http.ResponseWriter, r *http.Request) {
417	420		select {
418	421		case slots <- struct{}{}:

@@ -424,7 +427,11 @@ func (s Service) serveRegistry(slots chan struct{}, quota registryproxy.Quota)
424	427		}
425	428		rc := http.NewResponseController(w)
426	429		defer func() { _ = rc.SetWriteDeadline(deadline(s.progress, nil)) }()
427		-	r = r.WithContext(registryproxy.WithQuota(r.Context(), quota))
	430	+	ctx := registryproxy.WithQuota(r.Context(), quota)
	431	+	if job != "" {
	432	+	ctx = registryproxy.WithJob(ctx, job)
	433	+	}
	434	+	r = r.WithContext(ctx)
428	435		s.registry.ServeHTTP(&progressWriter{ResponseWriter: w, rc: rc, idle: s.progress}, r)
429	436		}
430	437		}

A internal/cachesvc/registry_deadline_test.go +127 -0

@@ -0,0 +1,127 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package cachesvc
	4	+
	5	+	import (
	6	+	"bufio"
	7	+	"fmt"
	8	+	"io"
	9	+	"net"
	10	+	"net/http"
	11	+	"strings"
	12	+	"testing"
	13	+	"time"
	14	+
	15	+	"bnema.dev/ermite-runner/internal/registryproxy"
	16	+	)
	17	+
	18	+	const registryProgress = 300 * time.Millisecond
	19	+
	20	+	// registryRig serves a fake registry behind the service's real HTTP server, so
	21	+	// that the connection deadlines apply.
	22	+	func registryRig(t testing.T, h http.HandlerFunc) rig {
	23	+	t.Helper()
	24	+	return newRigWith(t, Config{Max: 1 << 20, MinCharge: 1, ProgressTimeout: registryProgress, Registry: h})
	25	+	}
	26	+
	27	+	// A registry download that keeps moving may last longer than the progress
	28	+	// period set when the request started.
	29	+	func TestRegistryProgressingTransferSucceeds(t *testing.T) {
	30	+	const chunks = 14
	31	+	r := registryRig(t, func(w http.ResponseWriter, _ *http.Request) {
	32	+	w.Header().Set("Content-Length", fmt.Sprint(chunks*1024))
	33	+	for range chunks {
	34	+	if _, err := w.Write([]byte(strings.Repeat("x", 1024))); err != nil {
	35	+	return
	36	+	}
	37	+	_ = http.NewResponseController(w).Flush()
	38	+	time.Sleep(registryProgress / 6)
	39	+	}
	40	+	})
	41	+	start := time.Now()
	42	+	resp, err := http.Get(r.srv.URL + RegistryPath + "library/alpine/blobs/x")
	43	+	if err != nil {
	44	+	t.Fatal(err)
	45	+	}
	46	+	defer func() { _ = resp.Body.Close() }()
	47	+	b, err := io.ReadAll(resp.Body)
	48	+	if err != nil \|\| len(b) != chunks*1024 {
	49	+	t.Fatalf("read %d bytes: %v", len(b), err)
	50	+	}
	51	+	if d := time.Since(start); d < 2*registryProgress {
	52	+	t.Fatalf("transfer took %v, not longer than the progress period", d)
	53	+	}
	54	+	}
	55	+
	56	+	// The proxy keeps a client alive while it waits for a shared upstream
	57	+	// download, before the first byte; the read deadline moves too, so the request
	58	+	// context stays alive.
	59	+	func TestRegistryKeepAliveWhileWaiting(t *testing.T) {
	60	+	r := registryRig(t, func(w http.ResponseWriter, req *http.Request) {
	61	+	k, ok := w.(registryproxy.KeepAliver)
	62	+	if !ok {
	63	+	t.Error("the registry writer is no KeepAliver")
	64	+	return
	65	+	}
	66	+	for range 12 { // two progress periods
	67	+	time.Sleep(registryProgress / 6)
	68	+	k.KeepAlive()
	69	+	if req.Context().Err() != nil {
	70	+	t.Error("request context ended while kept alive")
	71	+	return
	72	+	}
	73	+	}
	74	+	_, _ = io.WriteString(w, "verified")
	75	+	})
	76	+	resp, err := http.Get(r.srv.URL + RegistryPath + "library/alpine/blobs/x")
	77	+	if err != nil {
	78	+	t.Fatal(err)
	79	+	}
	80	+	defer func() { _ = resp.Body.Close() }()
	81	+	if b, err := io.ReadAll(resp.Body); err != nil \|\| string(b) != "verified" {
	82	+	t.Fatalf("%q %v", b, err)
	83	+	}
	84	+	}
	85	+
	86	+	// A client that stops reading is cut after one progress period.
	87	+	func TestRegistryStalledReaderIsCut(t *testing.T) {
	88	+	result := make(chan error, 1)
	89	+	r := registryRig(t, func(w http.ResponseWriter, _ *http.Request) {
	90	+	chunk := []byte(strings.Repeat("x", 64<<10))
	91	+	deadline := time.Now().Add(5 * time.Second)
	92	+	for time.Now().Before(deadline) {
	93	+	if _, err := w.Write(chunk); err != nil {
	94	+	result <- err
	95	+	return
	96	+	}
	97	+	}
	98	+	result <- nil
	99	+	})
	100	+	conn, err := net.Dial("tcp", strings.TrimPrefix(r.srv.URL, "http://"))
	101	+	if err != nil {
	102	+	t.Fatal(err)
	103	+	}
	104	+	defer func() { _ = conn.Close() }()
	105	+	if tc, ok := conn.(*net.TCPConn); ok {
	106	+	_ = tc.SetReadBuffer(4096)
	107	+	}
	108	+	if _, err := fmt.Fprint(conn, "GET "+RegistryPath+"library/alpine/blobs/x HTTP/1.1\r\nHost: x\r\n\r\n"); err != nil {
	109	+	t.Fatal(err)
	110	+	}
	111	+	resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
	112	+	if err != nil {
	113	+	t.Fatal(err)
	114	+	}
	115	+	if resp.StatusCode != http.StatusOK {
	116	+	t.Fatal(resp.StatusCode)
	117	+	}
	118	+	// Read nothing more.
	119	+	select {
	120	+	case err := <-result:
	121	+	if err == nil {
	122	+	t.Fatal("a stalled reader was not cut")
	123	+	}
	124	+	case <-time.After(10 * time.Second):
	125	+	t.Fatal("handler still blocked")
	126	+	}
	127	+	}

A internal/cachesvc/registry_limits_test.go +195 -0

@@ -0,0 +1,195 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package cachesvc
	4	+
	5	+	import (
	6	+	"bytes"
	7	+	"context"
	8	+	"crypto/sha256"
	9	+	"fmt"
	10	+	"io"
	11	+	"net/http"
	12	+	"net/http/httptest"
	13	+	"strings"
	14	+	"sync"
	15	+	"sync/atomic"
	16	+	"testing"
	17	+	"time"
	18	+
	19	+	"github.com/bnema/zerowrap"
	20	+
	21	+	"bnema.dev/ermite-runner/internal/registryproxy"
	22	+	)
	23	+
	24	+	// The registry requests of one channel in progress at once are capped: one
	25	+	// over the cap gets 503 at once, the cache API of that channel still answers,
	26	+	// and another channel has its own slots.
	27	+	func TestRegistryPerChannelCap(t *testing.T) {
	28	+	const limit = 2
	29	+	var inside atomic.Int32
	30	+	release := make(chan struct{})
	31	+	svc, err := New(Config{Dir: t.TempDir(), RegistryPerChannel: limit, Registry: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
	32	+	inside.Add(1)
	33	+	<-release
	34	+	_, _ = io.WriteString(w, "ok")
	35	+	})})
	36	+	if err != nil {
	37	+	t.Fatal(err)
	38	+	}
	39	+	t.Cleanup(func() { _ = svc.CloseAll() })
	40	+	var once sync.Once
	41	+	unblock := func() { once.Do(func() { close(release) }) }
	42	+	t.Cleanup(unblock)
	43	+	a, b := channel(t, svc, "sb-a", "job-a"), channel(t, svc, "sb-b", "job-b")
	44	+	get := func(c *http.Client) int {
	45	+	resp, err := c.Get("http://cache" + RegistryPath + "library/alpine/blobs/x")
	46	+	if err != nil {
	47	+	t.Error(err)
	48	+	return 0
	49	+	}
	50	+	_, _ = io.Copy(io.Discard, resp.Body)
	51	+	_ = resp.Body.Close()
	52	+	return resp.StatusCode
	53	+	}
	54	+	var wg sync.WaitGroup
	55	+	codes := make(chan int, limit)
	56	+	for range limit {
	57	+	wg.Go(func() { codes <- get(a) })
	58	+	}
	59	+	waitUntil(t, func() bool { return inside.Load() == limit })
	60	+	if code := get(a); code != http.StatusServiceUnavailable {
	61	+	t.Fatalf("over the cap: %d", code)
	62	+	}
	63	+	if code := lookupStatus(t, a, ""); code != http.StatusForbidden {
	64	+	t.Fatalf("cache API behind the registry cap: %d", code)
	65	+	}
	66	+	other := make(chan int, 1)
	67	+	go func() { other <- get(b) }()
	68	+	waitUntil(t, func() bool { return inside.Load() == limit+1 })
	69	+	unblock()
	70	+	wg.Wait()
	71	+	close(codes)
	72	+	for code := range codes {
	73	+	if code != http.StatusOK {
	74	+	t.Fatalf("within the cap: %d", code)
	75	+	}
	76	+	}
	77	+	if code := <-other; code != http.StatusOK {
	78	+	t.Fatalf("other channel: %d", code)
	79	+	}
	80	+	if code := get(a); code != http.StatusOK {
	81	+	t.Fatalf("slots not released: %d", code)
	82	+	}
	83	+	}
	84	+
	85	+	// stallingHub fakes Docker Hub for a real registryproxy: a manifest, and blobs
	86	+	// whose download stalls until the test ends. It counts the blob downloads
	87	+	// started.
	88	+	func stallingHub(t testing.T) (httptest.Server, *atomic.Int32, string) {
	89	+	t.Helper()
	90	+	const manifest = `{"schemaVersion":2}`
	91	+	digest := fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(manifest)))
	92	+	var started atomic.Int32
	93	+	done := make(chan struct{})
	94	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	95	+	switch {
	96	+	case r.URL.Path == "/token":
	97	+	_, _ = io.WriteString(w, `{"token":"public"}`)
	98	+	case strings.Contains(r.URL.Path, "/manifests/"):
	99	+	w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json")
	100	+	w.Header().Set("Docker-Content-Digest", digest)
	101	+	_, _ = io.WriteString(w, manifest)
	102	+	default:
	103	+	started.Add(1)
	104	+	w.Header().Set("Content-Length", "1000000")
	105	+	_, _ = io.WriteString(w, "partial")
	106	+	w.(http.Flusher).Flush()
	107	+	select {
	108	+	case <-r.Context().Done():
	109	+	case <-done:
	110	+	}
	111	+	}
	112	+	}))
	113	+	t.Cleanup(srv.Close)
	114	+	t.Cleanup(func() { close(done) }) // before srv.Close, which waits for the handlers
	115	+	return srv, &started, digest
	116	+	}
	117	+
	118	+	// Each channel has its own quota of cold downloads: a channel that has
	119	+	// started RegistryDownloadsPerChannel of them waits for its own to end, while
	120	+	// another channel starts its download at once.
	121	+	func TestRegistryQuotaPerChannel(t *testing.T) {
	122	+	hub, started, _ := stallingHub(t)
	123	+	proxy, err := registryproxy.New(registryproxy.Config{Dir: t.TempDir(), Max: 1 << 30, Upstream: hub.URL, AuthURL: hub.URL + "/token",
	124	+	SlotWait: time.Hour, AbandonGrace: 10 * time.Millisecond})
	125	+	if err != nil {
	126	+	t.Fatal(err)
	127	+	}
	128	+	svc, err := New(Config{Dir: t.TempDir(), Registry: proxy})
	129	+	if err != nil {
	130	+	t.Fatal(err)
	131	+	}
	132	+	t.Cleanup(func() { _ = svc.CloseAll() })
	133	+	ctx, cancel := context.WithCancel(context.Background())
	134	+	defer cancel()
	135	+	var wg sync.WaitGroup
	136	+	defer wg.Wait()
	137	+	defer cancel()
	138	+	pull := func(c *http.Client, i int) {
	139	+	req, _ := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://cache%slibrary/alpine/blobs/sha256:%x", RegistryPath, sha256.Sum256(fmt.Appendf(nil, "blob-%d", i))), nil)
	140	+	wg.Go(func() {
	141	+	if resp, err := c.Do(req); err == nil {
	142	+	_ = resp.Body.Close()
	143	+	}
	144	+	})
	145	+	}
	146	+	a, b := channel(t, svc, "sb-a", "job-a"), channel(t, svc, "sb-b", "job-b")
	147	+	for i := range RegistryDownloadsPerChannel + 1 {
	148	+	pull(a, i)
	149	+	}
	150	+	waitUntil(t, func() bool { return started.Load() == RegistryDownloadsPerChannel })
	151	+	time.Sleep(50 * time.Millisecond) // the one over A's quota would have started by now
	152	+	if n := started.Load(); n != RegistryDownloadsPerChannel {
	153	+	t.Fatalf("channel A started %d downloads, its quota is %d", n, RegistryDownloadsPerChannel)
	154	+	}
	155	+	pull(b, 100)
	156	+	waitUntil(t, func() bool { return started.Load() == RegistryDownloadsPerChannel+1 })
	157	+	}
	158	+
	159	+	// The registry lines of a channel carry its job.
	160	+	func TestRegistryLogsJob(t *testing.T) {
	161	+	hub, _, digest := stallingHub(t)
	162	+	var logs bytes.Buffer
	163	+	var mu sync.Mutex
	164	+	proxy, err := registryproxy.New(registryproxy.Config{Dir: t.TempDir(), Max: 1 << 20, Upstream: hub.URL, AuthURL: hub.URL + "/token",
	165	+	Log: zerowrap.New(zerowrap.Config{Level: "info", Format: "json", Output: writerFunc(func(p []byte) (int, error) {
	166	+	mu.Lock()
	167	+	defer mu.Unlock()
	168	+	return logs.Write(p)
	169	+	})})})
	170	+	if err != nil {
	171	+	t.Fatal(err)
	172	+	}
	173	+	svc, err := New(Config{Dir: t.TempDir(), Registry: proxy})
	174	+	if err != nil {
	175	+	t.Fatal(err)
	176	+	}
	177	+	t.Cleanup(func() { _ = svc.CloseAll() })
	178	+	resp, err := channel(t, svc, "sb-a", "job-a").Get("http://cache" + RegistryPath + "library/alpine/manifests/" + digest)
	179	+	if err != nil {
	180	+	t.Fatal(err)
	181	+	}
	182	+	_ = resp.Body.Close()
	183	+	if resp.StatusCode != http.StatusOK {
	184	+	t.Fatal(resp.StatusCode)
	185	+	}
	186	+	mu.Lock()
	187	+	defer mu.Unlock()
	188	+	if !strings.Contains(logs.String(), `"job_id":"job-a"`) \|\| !strings.Contains(logs.String(), `"message":"registry manifest"`) {
	189	+	t.Fatalf("logs: %s", logs.String())
	190	+	}
	191	+	}
	192	+
	193	+	type writerFunc func(p []byte) (int, error)
	194	+
	195	+	func (f writerFunc) Write(p []byte) (int, error) { return f(p) }

D internal/cachesvc/registry_progress_test.go +0 -235

@@ -1,235 +0,0 @@
1	-	// SPDX-License-Identifier: MIT
2	-	package cachesvc
3	-
4	-	import (
5	-	"bufio"
6	-	"fmt"
7	-	"io"
8	-	"net"
9	-	"net/http"
10	-	"strings"
11	-	"sync"
12	-	"sync/atomic"
13	-	"testing"
14	-	"time"
15	-
16	-	"bnema.dev/ermite-runner/internal/registryproxy"
17	-	)
18	-
19	-	const registryProgress = 150 * time.Millisecond
20	-
21	-	// registryRig serves a fake registry behind the service's real HTTP server, so
22	-	// that the connection deadlines apply.
23	-	func registryRig(t testing.T, h http.HandlerFunc) rig {
24	-	t.Helper()
25	-	return newRigWith(t, Config{Max: 1 << 20, MinCharge: 1, ProgressTimeout: registryProgress, Registry: h})
26	-	}
27	-
28	-	// A registry download that keeps moving may last longer than the
29	-	// progress period set when the request started.
30	-	func TestRegistryProgressingTransferSucceeds(t *testing.T) {
31	-	const chunks = 8
32	-	r := registryRig(t, func(w http.ResponseWriter, _ *http.Request) {
33	-	w.Header().Set("Content-Length", fmt.Sprint(chunks*1024))
34	-	for range chunks {
35	-	if _, err := w.Write([]byte(strings.Repeat("x", 1024))); err != nil {
36	-	return
37	-	}
38	-	_ = http.NewResponseController(w).Flush()
39	-	time.Sleep(registryProgress / 3)
40	-	}
41	-	})
42	-	start := time.Now()
43	-	resp, err := http.Get(r.srv.URL + "/v2/library/alpine/blobs/x")
44	-	if err != nil {
45	-	t.Fatal(err)
46	-	}
47	-	defer func() { _ = resp.Body.Close() }()
48	-	b, err := io.ReadAll(resp.Body)
49	-	if err != nil \|\| len(b) != chunks*1024 {
50	-	t.Fatalf("read %d bytes: %v", len(b), err)
51	-	}
52	-	if d := time.Since(start); d < 2*registryProgress {
53	-	t.Fatalf("transfer took %v, not longer than the progress period", d)
54	-	}
55	-	}
56	-
57	-	// The proxy keeps a client alive while it waits for a shared upstream
58	-	// download, before the first byte; the read deadline moves too, so the request
59	-	// context stays alive.
60	-	func TestRegistryKeepAliveWhileWaiting(t *testing.T) {
61	-	r := registryRig(t, func(w http.ResponseWriter, req *http.Request) {
62	-	k, ok := w.(interface{ KeepAlive() })
63	-	if !ok {
64	-	t.Error("no KeepAlive on the registry writer")
65	-	return
66	-	}
67	-	for range 6 {
68	-	time.Sleep(registryProgress / 3)
69	-	k.KeepAlive()
70	-	if req.Context().Err() != nil {
71	-	t.Error("request context ended while kept alive")
72	-	return
73	-	}
74	-	}
75	-	_, _ = io.WriteString(w, "verified")
76	-	})
77	-	resp, err := http.Get(r.srv.URL + "/v2/library/alpine/blobs/x")
78	-	if err != nil {
79	-	t.Fatal(err)
80	-	}
81	-	defer func() { _ = resp.Body.Close() }()
82	-	if b, err := io.ReadAll(resp.Body); err != nil \|\| string(b) != "verified" {
83	-	t.Fatalf("%q %v", b, err)
84	-	}
85	-	}
86	-
87	-	// The registry requests of one channel in progress at once are capped: one
88	-	// over the cap gets 503 at once, the cache API of that channel still answers,
89	-	// and another channel has its own slots.
90	-	func TestRegistryPerChannelCap(t *testing.T) {
91	-	const limit = 2
92	-	var inside atomic.Int32
93	-	release := make(chan struct{})
94	-	svc, err := New(Config{Dir: t.TempDir(), RegistryPerChannel: limit, Registry: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
95	-	inside.Add(1)
96	-	<-release
97	-	_, _ = io.WriteString(w, "ok")
98	-	})})
99	-	if err != nil {
100	-	t.Fatal(err)
101	-	}
102	-	t.Cleanup(func() { _ = svc.CloseAll() })
103	-	var once sync.Once
104	-	unblock := func() { once.Do(func() { close(release) }) }
105	-	t.Cleanup(unblock)
106	-	a, b := channel(t, svc, "sb-a", "job-a"), channel(t, svc, "sb-b", "job-b")
107	-	get := func(c *http.Client) int {
108	-	resp, err := c.Get("http://cache/v2/library/alpine/blobs/x")
109	-	if err != nil {
110	-	t.Error(err)
111	-	return 0
112	-	}
113	-	_, _ = io.Copy(io.Discard, resp.Body)
114	-	_ = resp.Body.Close()
115	-	return resp.StatusCode
116	-	}
117	-	var wg sync.WaitGroup
118	-	codes := make(chan int, limit)
119	-	for range limit {
120	-	wg.Go(func() { codes <- get(a) })
121	-	}
122	-	waitUntil(t, func() bool { return inside.Load() == limit })
123	-	if code := get(a); code != http.StatusServiceUnavailable {
124	-	t.Fatalf("over the cap: %d", code)
125	-	}
126	-	if code := lookupStatus(t, a, ""); code != http.StatusForbidden {
127	-	t.Fatalf("cache API behind the registry cap: %d", code)
128	-	}
129	-	other := make(chan int, 1)
130	-	go func() { other <- get(b) }()
131	-	waitUntil(t, func() bool { return inside.Load() == limit+1 })
132	-	unblock()
133	-	wg.Wait()
134	-	close(codes)
135	-	for code := range codes {
136	-	if code != http.StatusOK {
137	-	t.Fatalf("within the cap: %d", code)
138	-	}
139	-	}
140	-	if code := <-other; code != http.StatusOK {
141	-	t.Fatalf("other channel: %d", code)
142	-	}
143	-	if code := get(a); code != http.StatusOK {
144	-	t.Fatalf("slots not released: %d", code)
145	-	}
146	-	}
147	-
148	-	// Each channel hands the registry its own download quota, the same one for
149	-	// all of its requests.
150	-	func TestRegistryQuotaPerChannel(t *testing.T) {
151	-	var mu sync.Mutex
152	-	seen := map[string]map[*registryproxy.Quota]bool{}
153	-	svc, err := New(Config{Dir: t.TempDir(), Registry: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
154	-	q := registryproxy.QuotaOf(r.Context())
155	-	if q == nil {
156	-	t.Error("no quota on the registry request")
157	-	}
158	-	mu.Lock()
159	-	ch := r.Header.Get("X-Channel")
160	-	if seen[ch] == nil {
161	-	seen[ch] = map[*registryproxy.Quota]bool{}
162	-	}
163	-	seen[ch][q] = true
164	-	mu.Unlock()
165	-	})})
166	-	if err != nil {
167	-	t.Fatal(err)
168	-	}
169	-	t.Cleanup(func() { _ = svc.CloseAll() })
170	-	for _, ch := range []string{"a", "b"} {
171	-	c := channel(t, svc, "sb-"+ch, "job-"+ch)
172	-	for range 3 {
173	-	req, _ := http.NewRequest(http.MethodGet, "http://cache/v2/library/alpine/blobs/x", nil)
174	-	req.Header.Set("X-Channel", ch)
175	-	resp, err := c.Do(req)
176	-	if err != nil {
177	-	t.Fatal(err)
178	-	}
179	-	_ = resp.Body.Close()
180	-	}
181	-	}
182	-	mu.Lock()
183	-	defer mu.Unlock()
184	-	if len(seen["a"]) != 1 \|\| len(seen["b"]) != 1 {
185	-	t.Fatalf("quotas per channel: a=%d b=%d", len(seen["a"]), len(seen["b"]))
186	-	}
187	-	for qa := range seen["a"] {
188	-	if seen["b"][qa] {
189	-	t.Fatal("two channels share a quota")
190	-	}
191	-	}
192	-	}
193	-
194	-	// A client that stops reading is cut after one progress period.
195	-	func TestRegistryStalledReaderIsCut(t *testing.T) {
196	-	result := make(chan error, 1)
197	-	r := registryRig(t, func(w http.ResponseWriter, _ *http.Request) {
198	-	chunk := []byte(strings.Repeat("x", 64<<10))
199	-	deadline := time.Now().Add(5 * time.Second)
200	-	for time.Now().Before(deadline) {
201	-	if _, err := w.Write(chunk); err != nil {
202	-	result <- err
203	-	return
204	-	}
205	-	}
206	-	result <- nil
207	-	})
208	-	conn, err := net.Dial("tcp", strings.TrimPrefix(r.srv.URL, "http://"))
209	-	if err != nil {
210	-	t.Fatal(err)
211	-	}
212	-	defer func() { _ = conn.Close() }()
213	-	if tc, ok := conn.(*net.TCPConn); ok {
214	-	_ = tc.SetReadBuffer(4096)
215	-	}
216	-	if _, err := fmt.Fprint(conn, "GET /v2/library/alpine/blobs/x HTTP/1.1\r\nHost: x\r\n\r\n"); err != nil {
217	-	t.Fatal(err)
218	-	}
219	-	resp, err := http.ReadResponse(bufio.NewReader(conn), nil)
220	-	if err != nil {
221	-	t.Fatal(err)
222	-	}
223	-	if resp.StatusCode != http.StatusOK {
224	-	t.Fatal(resp.StatusCode)
225	-	}
226	-	// Read nothing more.
227	-	select {
228	-	case err := <-result:
229	-	if err == nil {
230	-	t.Fatal("a stalled reader was not cut")
231	-	}
232	-	case <-time.After(10 * time.Second):
233	-	t.Fatal("handler still blocked")
234	-	}
235	-	}

M internal/cachesvc/registry_test.go +4 -1

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package cachesvc
3	4
4	5		import (

@@ -8,6 +9,8 @@ import (
8	9		"testing"
9	10		)
10	11
	12	+	// The registry is served under RegistryPath on the cache handler and the
	13	+	// channel, never by the results service; without one, those paths are 404.
11	14		func TestRegistryRoute(t *testing.T) {
12	15		for _, on := range []bool{false, true} {
13	16		cfg := Config{}

@@ -21,7 +24,7 @@ func TestRegistryRoute(t *testing.T) {
21	24		})
22	25		for _, h := range []http.Handler{r.svc.Handler(), r.svc.channelHandler("job", results)} {
23	26		w := httptest.NewRecorder()
24		-	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/v2/", nil))
	27	+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, RegistryPath, nil))
25	28		if on {
26	29		if w.Code != http.StatusOK \|\| w.Body.String() != "registry" {
27	30		t.Fatalf("%d %s", w.Code, w.Body)

M internal/cachesvc/service.go +20 -21


@@ -62,11 +62,13 @@ const (
62	62		// progress at once, so that one sandbox's pulls leave room for its cache and
63	63		// artifact traffic. dockerd pulls 3 layers at once by default.
64	64		DefaultRegistryPerChannel = 8
65		-	// DefaultRegistryDownloadsPerChannel bounds the cold Docker Hub downloads
66		-	// one channel may have started at once (dockerd's default
	65	+	// RegistryDownloadsPerChannel bounds the cold Docker Hub downloads one
	66	+	// channel may have started at once (dockerd's default
67	67		// max-concurrent-downloads), so that one sandbox cannot take every slot of
68	68		// the registry proxy.
69		-	DefaultRegistryDownloadsPerChannel = 3
	69	+	RegistryDownloadsPerChannel = 3
	70	+	// RegistryPath is the prefix of the Docker registry API on the channel.
	71	+	RegistryPath = "/v2/"
70	72		)
71	73
72	74		// Config configures a Service.

@@ -94,7 +96,6 @@ type Config struct {
94	96		ProgressTimeout time.Duration
95	97		ConnsPerListener, Conns int
96	98		RegistryPerChannel int
97		-	RegistryDownloadsPerChannel int
98	99		MinCharge int64
99	100		MaxPending, MaxPendingPerJob int
100	101		// hookChunk, when set, is called by an upload once its range is admitted and

@@ -119,12 +120,10 @@ type Service struct {
119	120		registry http.Handler
120	121		// registryPerChannel caps the registry requests in progress per handler.
121	122		registryPerChannel int
122		-	// registryDownloads is the cold download quota of each channel.
123		-	registryDownloads int
124		-	hookChunk func(id int64)
125		-	progress time.Duration
126		-	perListener int
127		-	connsGlobal chan struct{}
	123	+	hookChunk func(id int64)
	124	+	progress time.Duration
	125	+	perListener int
	126	+	connsGlobal chan struct{}
128	127		}
129	128
130	129		type listener struct {

@@ -165,11 +164,8 @@ func New(cfg Config) (*Service, error) {
165	164		if cfg.RegistryPerChannel <= 0 {
166	165		cfg.RegistryPerChannel = DefaultRegistryPerChannel
167	166		}
168		-	if cfg.RegistryDownloadsPerChannel <= 0 {
169		-	cfg.RegistryDownloadsPerChannel = DefaultRegistryDownloadsPerChannel
170		-	}
171	167		s := &Service{store: st, sign: sg, now: cfg.Now, log: cfg.Log, channels: map[string]*listener{}, stop: make(chan struct{}),
172		-	archives: cfg.Archives, registry: cfg.Registry, hookChunk: cfg.hookChunk, progress: cfg.ProgressTimeout, perListener: cfg.ConnsPerListener, connsGlobal: make(chan struct{}, cfg.Conns), registryPerChannel: cfg.RegistryPerChannel, registryDownloads: cfg.RegistryDownloadsPerChannel}
	168	+	archives: cfg.Archives, registry: cfg.Registry, hookChunk: cfg.hookChunk, progress: cfg.ProgressTimeout, perListener: cfg.ConnsPerListener, connsGlobal: make(chan struct{}, cfg.Conns), registryPerChannel: cfg.RegistryPerChannel}
173	169		s.wg.Add(1)
174	170		go s.housekeeping()
175	171		return s, nil

@@ -319,14 +315,17 @@ func (s *Service) CloseAll() error {
319	315		return s.store.flush()
320	316		}
321	317
322		-	// ParseMax reads ERMITE_RUNNER_CACHE_MAX: a number of bytes or a size with a K,
323		-	// M, G or T suffix (optionally followed by iB or B). Empty means DefaultMax.
324		-	func ParseMax(v string) (int64, error) {
	318	+	// ParseSize reads a size: a number of bytes or a number with a K, M, G or T
	319	+	// suffix (powers of 1024), optionally followed by iB or B, such as 10G, 500M
	320	+	// or 2GiB. Surrounding spaces are ignored; empty means def. Zero in any unit
	321	+	// (0, 0G, 0GiB) is 0. The error names no variable: the caller prefixes it.
	322	+	func ParseSize(v string, def int64) (int64, error) {
	323	+	v = strings.TrimSpace(v)
325	324		if v == "" {
326		-	return DefaultMax, nil
	325	+	return def, nil
327	326		}
328	327		s := []byte(v)
329		-	for len(s) > 0 && (s[len(s)-1] == 'B' \|\| s[len(s)-1] == 'b' \|\| s[len(s)-1] == 'i') {
	328	+	for len(s) > 0 && strings.IndexByte("bBiI", s[len(s)-1]) >= 0 {
330	329		s = s[:len(s)-1]
331	330		}
332	331		mult := int64(1)

@@ -343,8 +342,8 @@ func ParseMax(v string) (int64, error) {
343	342		}
344	343		}
345	344		n, err := strconv.ParseInt(string(s), 10, 64)
346		-	if err != nil \|\| n <= 0 \|\| n > (1<<62)/mult {
347		-	return 0, fmt.Errorf("%s: %q is not a size (examples: 10G, 500M)", EnvMax, v)
	345	+	if err != nil \|\| n < 0 \|\| n > (1<<62)/mult {
	346	+	return 0, fmt.Errorf("%q is not a size (examples: 10G, 500M)", v)
348	347		}
349	348		return n * mult, nil
350	349		}

M internal/cachesvc/service_test.go +10 -5


@@ -600,15 +600,20 @@ func TestListDiagnostics(t *testing.T) {
600	600		}
601	601		}
602	602
603		-	func TestParseMax(t *testing.T) {
604		-	for in, want := range map[string]int64{"": DefaultMax, "10G": 10 << 30, "500m": 500 << 20, "1GiB": 1 << 30, "2048": 2048, "3TB": 3 << 40} {
605		-	if got, err := ParseMax(in); err != nil \|\| got != want {
	603	+	func TestParseSize(t *testing.T) {
	604	+	const def = 7
	605	+	for in, want := range map[string]int64{"": def, " ": def, "10G": 10 << 30, "500m": 500 << 20, "1GiB": 1 << 30, "2048": 2048, "3TB": 3 << 40, " 2G ": 2 << 30,
	606	+	"0": 0, "00": 0, "0G": 0, "0GiB": 0, "2GIB": 2 << 30} {
	607	+	if got, err := ParseSize(in, def); err != nil \|\| got != want {
606	608		t.Errorf("%q: %d %v", in, got, err)
607	609		}
608	610		}
609		-	for _, in := range []string{"0", "-1", "G", "ten", "1.5G", "99999999999T"} {
610		-	if _, err := ParseMax(in); err == nil {
	611	+	for _, in := range []string{"-1", "G", "ten", "1.5G", "99999999999T"} {
	612	+	_, err := ParseSize(in, def)
	613	+	if err == nil {
611	614		t.Errorf("%q accepted", in)
	615	+	} else if strings.Contains(err.Error(), "ERMITE_") {
	616	+	t.Errorf("%q: the error names a variable: %v", in, err)
612	617		}
613	618		}
614	619		}

M internal/e2e/registry_test.go +4 -2

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package e2e
3	4
4	5		import (

@@ -10,10 +11,11 @@ import (
10	11		"testing"
11	12		"time"
12	13
	14	+	"github.com/bnema/zerowrap"
	15	+
13	16		"bnema.dev/ermite-runner/internal/cachesvc"
14	17		"bnema.dev/ermite-runner/internal/execstream/execpb"
15	18		"bnema.dev/ermite-runner/internal/registryproxy"
16		-	"github.com/bnema/zerowrap"
17	19		)
18	20
19	21		type registryLogs struct {

@@ -71,8 +73,8 @@ jobs:
71	73		steps:
72	74		- run: echo alpine-job
73	75		`)
	76	+	// A registry-only job: the host sends the port and no token.
74	77		spec.CachePort = cachesvc.DefaultPort
75		-	spec.CacheToken = svc.Token("e2e", cachesvc.Scope{}, time.Hour)
76	78		s := runJob(t, d, spec, nil)
77	79		if s.Result == nil \|\| s.Result.Conclusion != execpb.StepConclusion_STEP_CONCLUSION_SUCCESS {
78	80		t.Fatalf("pull job %d: %v\n%s", i, s.Result, strings.Join(s.Logs, "\n"))

M internal/e2e/timing_test.go +3 -1


@@ -10,11 +10,13 @@ import (
10	10		"testing"
11	11		"time"
12	12
	13	+	"github.com/bnema/zerowrap"
	14	+
13	15		ermite "bnema.dev/ermite-api/gen/ermite"
14	16		"bnema.dev/ermite-api/gen/ermite/ermiteconnect"
	17	+
15	18		"bnema.dev/ermite-runner/internal/host"
16	19		"bnema.dev/ermite-runner/internal/host/hosttest"
17		-	"github.com/bnema/zerowrap"
18	20		)
19	21
20	22		type timingWriter struct {

M internal/guest/cacheurl_test.go +3 -3


@@ -121,11 +121,11 @@ func TestStartCacheRelay(t *testing.T) {
121	121		}
122	122		}
123	123
124		-	// No cache service, no relay and no variables; a port that is taken leaves the
125		-	// job without a cache instead of failing it.
	124	+	// No port, no relay and no variables; a port that is taken leaves the job
	125	+	// without a cache instead of failing it.
126	126		func TestStartCacheRelayWithoutServiceOrPort(t *testing.T) {
127	127		target := Target{Network: "unix", Addr: filepath.Join(t.TempDir(), "exec.sock")}
128		-	for _, spec := range []*execpb.JobSpec{{}, {CachePort: 9089}, {CacheToken: "tok"}} {
	128	+	for _, spec := range []*execpb.JobSpec{{}, {CacheToken: "tok"}, {CacheToken: "tok", CacheMode: "write"}} {
129	129		if got := startCacheRelay(context.Background(), target, spec, noBridge); got != (cacheRelay{}) {
130	130		t.Errorf("%+v: relay %+v", spec, got)
131	131		}

M internal/guest/dockerd.go +3 -1

@@ -1,9 +1,11 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package guest
3	4
4	5		import (
5		-	"bnema.dev/ermite-runner/internal/sandbox"
6	6		"fmt"
	7	+
	8	+	"bnema.dev/ermite-runner/internal/sandbox"
7	9		)
8	10
9	11		func defaultDockerdArgs() []string {

M internal/guest/init.go +12 -12


@@ -26,7 +26,6 @@ import (
26	26		// replace the fields.
27	27		type InitConfig struct {
28	28		Version string
29		-	bootMs map[string]int64
30	29		// Cmdline is the kernel command line; empty reads /proc/cmdline.
31	30		Cmdline string
32	31		// Getpid reports the process id; Init only acts as a VM's PID 1 when it is 1.

@@ -65,7 +64,9 @@ type InitConfig struct {
65	64		// powering the VM off, or exiting with the executor's code in a container.
66	65		func Init(ctx context.Context, cfg InitConfig) int {
67	66		entry := time.Now()
68		-	cfg.bootMs = make(map[string]int64)
	67	+	// bootMs is the boot phase timing handed to the executor (ERMITE_BOOT_MS),
	68	+	// which reports it in its Hello.
	69	+	bootMs := make(map[string]int64)
69	70		log := zerowrap.FromCtx(ctx)
70	71		// The kernel starts a VM's init with umask 0, which dockerd, the executor
71	72		// and every step would inherit: files and t.TempDir() directories would be

@@ -100,7 +101,7 @@ func Init(ctx context.Context, cfg InitConfig) int {
100	101		if b, err := os.ReadFile("/proc/uptime"); err == nil {
101	102		if fields := strings.Fields(string(b)); len(fields) > 0 {
102	103		if seconds, err := strconv.ParseFloat(fields[0], 64); err == nil && seconds >= 0 {
103		-	cfg.bootMs["kernel"] = max(0, int64(seconds*1000)-time.Since(entry).Milliseconds())
	104	+	bootMs["kernel"] = max(0, int64(seconds*1000)-time.Since(entry).Milliseconds())
104	105		}
105	106		}
106	107		}

@@ -110,9 +111,9 @@ func Init(ctx context.Context, cfg InitConfig) int {
110	111		powerOff()
111	112		return 1
112	113		}
113		-	cfg.bootMs["setup"] = time.Since(start).Milliseconds()
	114	+	bootMs["setup"] = time.Since(start).Milliseconds()
114	115		}
115		-	code := runPayload(ctx, cfg, mode, args)
	116	+	code := runPayload(ctx, cfg, mode, args, bootMs)
116	117		if inVM {
117	118		powerOff()
118	119		}

@@ -127,7 +128,9 @@ const StepUmask = 0o022
127	128		// with an empty environment.
128	129		const defaultPath = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
129	130
130		-	func runPayload(ctx context.Context, cfg InitConfig, mode string, args map[string]string) int {
	131	+	// runPayload starts dockerd and the executor and waits for them. It adds its
	132	+	// boot phases to bootMs, which the executor gets.
	133	+	func runPayload(ctx context.Context, cfg InitConfig, mode string, args map[string]string, bootMs map[string]int64) int {
131	134		log := zerowrap.FromCtx(ctx)
132	135		if os.Getenv("PATH") == "" {
133	136		_ = os.Setenv("PATH", defaultPath)

@@ -149,9 +152,6 @@ func runPayload(ctx context.Context, cfg InitConfig, mode string, args map[strin
149	152		return 1
150	153		}
151	154
152		-	if cfg.bootMs == nil {
153		-	cfg.bootMs = make(map[string]int64)
154		-	}
155	155		var dockerStart time.Time
156	156		var dockerPid, containerdPid int
157	157		_, statErr := os.Stat("/usr/bin/dockerd")

@@ -169,7 +169,7 @@ func runPayload(ctx context.Context, cfg InitConfig, mode string, args map[strin
169	169		// A containerd that never came up still cost its wait: keep it in the
170	170		// breakdown. A missing binary returns at once (0 ms) and is omitted.
171	171		if ms := time.Since(start).Milliseconds(); ok \|\| ms > 0 {
172		-	cfg.bootMs["containerd"] = ms
	172	+	bootMs["containerd"] = ms
173	173		}
174	174		if ok {
175	175		containerdPid = pid

@@ -210,7 +210,7 @@ func runPayload(ctx context.Context, cfg InitConfig, mode string, args map[strin
210	210		timeout = dockerReadyTimeout
211	211		}
212	212		err := waitDockerReady(ctx, log, sock, dockerPid, timeout, sigs)
213		-	cfg.bootMs["dockerd"] = time.Since(dockerStart).Milliseconds()
	213	+	bootMs["dockerd"] = time.Since(dockerStart).Milliseconds()
214	214		var exited *dockerdExitError
215	215		var abort *abortBootError
216	216		switch {

@@ -223,7 +223,7 @@ func runPayload(ctx context.Context, cfg InitConfig, mode string, args map[strin
223	223		}
224	224		}
225	225
226		-	execEnv := append(os.Environ(), "ERMITE_CONNECT="+target, "ERMITE_SANDBOX="+mode, "ERMITE_VERSION="+cfg.Version, "ERMITE_BOOT_MS="+formatBootMs(cfg.bootMs))
	226	+	execEnv := append(os.Environ(), "ERMITE_CONNECT="+target, "ERMITE_SANDBOX="+mode, "ERMITE_VERSION="+cfg.Version, "ERMITE_BOOT_MS="+formatBootMs(bootMs))
227	227		execPid, err := spawn(cfg.ExecArgs, execEnv)
228	228		if err != nil {
229	229		log.Error().Err(err).Msg("init: start executor")

M internal/guest/jobexec.go +5 -5


@@ -353,7 +353,7 @@ func (c cacheRelay) close() {
353	353		func startCacheRelay(ctx context.Context, target Target, spec *execpb.JobSpec, bridge bridgeLookup) cacheRelay {
354	354		port := spec.GetCachePort()
355	355		url := cacheURL(port)
356		-	if url == "" \|\| spec.GetCacheToken() == "" {
	356	+	if url == "" {
357	357		return cacheRelay{}
358	358		}
359	359		log := zerowrap.FromCtx(ctx)

@@ -365,10 +365,10 @@ func startCacheRelay(ctx context.Context, target Target, spec *execpb.JobSpec, b
365	365		return cacheRelay{}
366	366		}
367	367		results := spec.GetResults()
368		-	// Registry-only jobs need dockerd's loopback relay, but no action runtime
369		-	// URLs or token. Leave those fields empty so neither node nor docker
370		-	// actions receive ACTIONS_* runtime variables.
371		-	if !results && spec.GetCacheMode() == "" {
	368	+	// A registry-only job (no cache scope, no results service: the host sends
	369	+	// no token) needs dockerd's loopback relay, but no action runtime URLs or
	370	+	// token: neither node nor docker actions get ACTIONS_* variables.
	371	+	if spec.GetCacheToken() == "" \|\| (!results && spec.GetCacheMode() == "") {
372	372		return cacheRelay{relay: r}
373	373		}
374	374		c := cacheRelay{relay: r, url: url, token: spec.GetCacheToken(), mode: spec.GetCacheMode(),

M internal/guest/registry_test.go +11 -3

@@ -1,11 +1,13 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package guest
3	4
4	5		import (
5		-	"bnema.dev/ermite-runner/internal/execstream/execpb"
6	6		"context"
7	7		"strings"
8	8		"testing"
	9	+
	10	+	"bnema.dev/ermite-runner/internal/execstream/execpb"
9	11		)
10	12
11	13		func TestDefaultDockerdMirror(t *testing.T) {

@@ -14,14 +16,20 @@ func TestDefaultDockerdMirror(t *testing.T) {
14	16		t.Fatal(args)
15	17		}
16	18		}
	19	+
	20	+	// A registry-only job (a port, no token, no mode, no results) gets the
	21	+	// loopback relay for dockerd and no runtime environment for its steps.
17	22		func TestRegistryOnlyRelayHasNoRuntimeEnvironment(t *testing.T) {
18	23		target := serveCache(t, t.TempDir())
19		-	got := startCacheRelay(context.Background(), target, &execpb.JobSpec{CachePort: freePort(t), CacheToken: "unscoped"}, noBridge)
	24	+	got := startCacheRelay(context.Background(), target, &execpb.JobSpec{CachePort: freePort(t)}, noBridge)
20	25		defer got.close()
21	26		if got.relay == nil {
22	27		t.Fatal("missing loopback relay")
23	28		}
24		-	if got.url != "" \|\| got.containerURL != "" \|\| got.token != "" \|\| got.mode != "" \|\| got.results \|\| got.bridge != nil {
	29	+	if got.token != "" {
	30	+	t.Fatalf("token %q for a registry-only job", got.token)
	31	+	}
	32	+	if got.url != "" \|\| got.containerURL != "" \|\| got.mode != "" \|\| got.results \|\| got.bridge != nil {
25	33		t.Fatalf("runtime environment leaked: %+v", got)
26	34		}
27	35		}

M internal/host/archives.go +11 -7


@@ -30,7 +30,7 @@ const (
30	30		// busy runner does not trip the forge's rate limit (codeload 429).
31	31		//
32	32		// It only ever improves a resolution: an action that cannot be cached (no cache,
33		-	// no channel for this job, a ref that did not resolve to a sha, a download that
	33	+	// no cache scope for this job, a ref that did not resolve to a sha, a download that
34	34		// failed, a full disk) keeps its original URL and token, and the sandbox
35	35		// downloads it as before, with its retries and its messages. The sandbox prints
36	36		// "Download action repository" in either case, so a hit reads like a miss in the

@@ -41,7 +41,7 @@ func (j jobRun) cacheArchives(ctx context.Context, orig []ermite.ResolvedActio
41	41		return
42	42		}
43	43		if _, ok, err := cacheScope(j.job, j.contexts); err != nil \|\| !ok {
44		-	return // no channel for this job: the guest has no relay either
	44	+	return // action archives are served only to jobs with a cache scope
45	45		}
46	46		ttl := time.Duration(j.timeoutMinutes())*time.Minute + cacheTokenMargin
47	47		for i, a := range orig {

@@ -54,9 +54,13 @@ func (j jobRun) cacheArchives(ctx context.Context, orig []ermite.ResolvedActio
54	54		if !ok {
55	55		continue
56	56		}
	57	+	// fetched: this call ran the download itself. An invalid key, a wait
	58	+	// that was cancelled or a joined download that failed are no misses.
	59	+	var fetched bool
57	60		var fetchTime time.Duration
58	61		var bytes int64
59	62		hit, verify, err := rt.Archives.Ensure(ctx, key, func(ctx context.Context, w io.Writer) error {
	63	+	fetched = true
60	64		start := time.Now()
61	65		cw := &archiveCountWriter{Writer: w}
62	66		err := j.fetchArchive(ctx, a, origin, cw)

@@ -64,11 +68,11 @@ func (j jobRun) cacheArchives(ctx context.Context, orig []ermite.ResolvedActio
64	68		bytes += cw.n
65	69		return err
66	70		})
67		-	if !hit {
	71	+	if fetched {
68	72		j.mu.Lock()
69		-	j.timeline.add("archive_miss_ms", fetchTime)
70		-	j.timeline["archive_misses"]++
71		-	j.timeline["archive_miss_bytes"] += bytes
	73	+	j.timeline.addMs("archive_miss_ms", fetchTime)
	74	+	j.timeline.add("archive_misses", 1)
	75	+	j.timeline.add("archive_miss_bytes", bytes)
72	76		j.mu.Unlock()
73	77		}
74	78		log := j.log.With().Str("action", a.NameWithOwner).Str("sha", a.ResolvedSha).Logger()

@@ -81,7 +85,7 @@ func (j jobRun) cacheArchives(ctx context.Context, orig []ermite.ResolvedActio
81	85		continue
82	86		}
83	87		event := log.Info().Bool("hit", hit).Int64("verify_ms", verify.Milliseconds())
84		-	if !hit {
	88	+	if fetched {
85	89		event = event.Int64("fetch_ms", fetchTime.Milliseconds()).Int64("bytes", bytes)
86	90		}
87	91		event.Msg("action archive cache")

M internal/host/archives_test.go +46 -2


@@ -6,6 +6,7 @@ import (
6	6		"bytes"
7	7		"context"
8	8		"encoding/json"
	9	+	"io"
9	10		"net/http"
10	11		"net/http/httptest"
11	12		"path/filepath"

@@ -60,7 +61,7 @@ func newArchiveRig(t testing.T) archiveRig {
60	61		func (a archiveRig) job(id string) jobRun {
61	62		j := &jobRun{r: a.r, log: zerowrap.Logger{}, ctx: context.Background(),
62	63		job: &ermite.Job{JobId: id, Ref: "refs/heads/main"},
63		-	contexts: `{"github":{"repository":"o/r"}}`}
	64	+	contexts: `{"github":{"repository":"o/r"}}`, timeline: timeline{}}
64	65		return j
65	66		}
66	67

@@ -130,7 +131,7 @@ func TestCacheArchivesFallsBack(t *testing.T) {
130	131		}
131	132		}
132	133
133		-	// No cache channel for the job (no repository): the guest has no relay.
	134	+	// No cache scope for the job (no repository): archives are not served.
134	135		o, c = a.action(strings.Repeat("d", 40))
135	136		j := a.job("j")
136	137		j.contexts = `{}`

@@ -182,3 +183,46 @@ func TestArchiveTimingLogs(t *testing.T) {
182	183		t.Fatal(j.timeline)
183	184		}
184	185		}
	186	+
	187	+	// Only a download the job ran itself is a miss: a failed one counts, a job
	188	+	// whose wait for another job's download is cancelled does not.
	189	+	func TestArchiveMissesCountFetches(t *testing.T) {
	190	+	a := newArchiveRig(t)
	191	+	a.status.Store(http.StatusTooManyRequests)
	192	+	failed := a.job("failed")
	193	+	o, c := a.action(strings.Repeat("a", 40))
	194	+	failed.cacheArchives(context.Background(), []ermite.ResolvedAction{o}, []execpb.ResolvedAction{c})
	195	+	if failed.timeline["archive_misses"] != 1 {
	196	+	t.Fatalf("failed fetch: %v", failed.timeline)
	197	+	}
	198	+
	199	+	a.status.Store(http.StatusOK)
	200	+	// A joiner whose wait is cancelled while another job downloads.
	201	+	release := make(chan struct{})
	202	+	started, done := make(chan struct{}), make(chan struct{})
	203	+	sha := strings.Repeat("b", 40)
	204	+	key, ok := actionscache.Key(originOf(a.srv.URL), "x/y", sha)
	205	+	if !ok {
	206	+	t.Fatal("key")
	207	+	}
	208	+	go func() {
	209	+	defer close(done)
	210	+	_, _, _ = a.r.Archives.Ensure(context.Background(), key, func(_ context.Context, w io.Writer) error {
	211	+	close(started)
	212	+	<-release
	213	+	_, err := io.WriteString(w, "tarball")
	214	+	return err
	215	+	})
	216	+	}()
	217	+	<-started
	218	+	ctx, cancel := context.WithCancel(context.Background())
	219	+	cancel()
	220	+	joiner := a.job("joiner")
	221	+	o, c = a.action(sha)
	222	+	joiner.cacheArchives(ctx, []ermite.ResolvedAction{o}, []execpb.ResolvedAction{c})
	223	+	close(release)
	224	+	<-done // the download writes under t.TempDir: let it finish before cleanup
	225	+	if _, ok := joiner.timeline["archive_misses"]; ok {
	226	+	t.Fatalf("cancelled joiner: %v", joiner.timeline)
	227	+	}
	228	+	}

M internal/host/cache.go +8 -4


@@ -112,9 +112,10 @@ func githubRepository(contexts string) string {
112	112		// port it has in the guest, the job's ACTIONS_RUNTIME_TOKEN, the cache mode and
113	113		// whether the channel serves the results service. The channel exists when the
114	114		// runner serves a cache and the job has a cache scope, the results service,
115		-	// or the runner has a registry proxy. Without it, 0, "", "" and false.
116		-	// A job without a cache scope gets an empty mode and a token that the cache
117		-	// refuses. Registry-only jobs expose no runtime environment variables to steps.
	115	+	// or the cache serves a registry proxy. Without it, 0, "", "" and false.
	116	+	// A job with the results service and no cache scope gets an empty mode and a
	117	+	// token that the cache refuses. A registry-only job (neither) gets the port
	118	+	// alone: no token, and so no runtime environment variables for its steps.
118	119		func (j jobRun) runtimeFor(rt Runner) (port uint32, token, mode string, results bool) {
119	120		if rt.Cache == nil {
120	121		return 0, "", "", false

@@ -134,7 +135,10 @@ func (j jobRun) runtimeFor(rt Runner) (port uint32, token, mode string, result
134	135		mode = "write"
135	136		}
136	137		}
137		-	if !ok && !j.results && !rt.Registry {
	138	+	if !ok && !j.results {
	139	+	if rt.Cache.RegistryEnabled() {
	140	+	return cachesvc.DefaultPort, "", "", false
	141	+	}
138	142		return 0, "", "", false
139	143		}
140	144		var scopes []string

M internal/host/fake_test.go +6 -1


@@ -351,6 +351,7 @@ type fakeDriver struct {
351	351		blockStart chan struct{} // when set, Start waits on it
352	352		specs []sandbox.JobSpec // what each Start was asked for
353	353		iso sandbox.Isolation
	354	+	bootMs map[string]int64 // the guest's Hello.boot_ms; nil: dockerd=2
354	355		}
355	356
356	357		func (d *fakeDriver) Mode() sandbox.Mode { return sandbox.Container }

@@ -402,6 +403,10 @@ func (s *fakeSandbox) Start(ctx context.Context, spec sandbox.JobSpec) (sandbox.
402	403		}
403	404		d.started++
404	405		sc := d.script
	406	+	bootMs := d.bootMs
	407	+	if bootMs == nil {
	408	+	bootMs = map[string]int64{"dockerd": 2}
	409	+	}
405	410		d.mu.Unlock()
406	411		host, guest := net.Pipe()
407	412		s.mu.Lock()

@@ -410,7 +415,7 @@ func (s *fakeSandbox) Start(ctx context.Context, spec sandbox.JobSpec) (sandbox.
410	415		go func() {
411	416		defer func() { _ = guest.Close() }()
412	417		c := execstream.NewConn(guest)
413		-	_ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{Version: "test", Sandbox: "container", BootMs: map[string]int64{"dockerd": 2}}}})
	418	+	_ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{Version: "test", Sandbox: "container", BootMs: bootMs}}})
414	419		// NamedValue frames (secrets, variables) come before Run.
415	420		vals := &seenValues{secrets: map[string]string{}, vars: map[string]string{}}
416	421		var f *execpb.Frame

M internal/host/job.go +4 -10


@@ -60,9 +60,10 @@ type jobRun struct {
60	60		cancelKind execpb.CancelKind // kind of the first cancel request
61	61		stopping bool // the runner is shutting down
62	62
	63	+	mu sync.Mutex
	64	+	steps map[int32]*ermite.StepUpdate
	65	+	// timeline is the job's phase timing, guarded by mu.
63	66		timeline timeline
64		-	mu sync.Mutex
65		-	steps map[int32]*ermite.StepUpdate
66	67		// What the guest has told us, for the validation of the next frame: the
67	68		// guest is untrusted code, its step numbers and lines are checked before
68	69		// they reach the server (see violate).

@@ -724,14 +725,7 @@ func (j jobRun) drive(conn sandbox.Conn, rt Runner) (*execpb.Exit, error) {
724	725		Hello: func(h *execpb.Hello) {
725	726		j.mu.Lock()
726	727		defer j.mu.Unlock()
727		-	if j.timeline == nil {
728		-	j.timeline = make(timeline)
729		-	}
730		-	for k, v := range h.BootMs {
731		-	if v >= 0 {
732		-	j.timeline["guest_"+k+"_ms"] = v
733		-	}
734		-	}
	728	+	j.timeline.addGuestBoot(h.GetBootMs())
735	729		},
736	730		StepStarted: j.stepStarted, StepCompleted: j.stepCompleted,
737	731		Log: j.logLine,

M internal/host/registry_test.go +31 -23

@@ -1,41 +1,49 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package host
3	4
4	5		import (
	6	+	"testing"
	7	+
5	8		"bnema.dev/ermite-api/gen/ermite"
	9	+
6	10		"bnema.dev/ermite-runner/internal/cachesvc"
7		-	"net/http"
8		-	"net/http/httptest"
9		-	"strings"
10		-	"testing"
	11	+	"bnema.dev/ermite-runner/internal/registryproxy"
11	12		)
12	13
	14	+	// A job with neither a cache scope nor the results service gets the channel
	15	+	// for the registry proxy alone: its port and no token. Without the proxy, or
	16	+	// without the cache, it gets no channel.
13	17		func TestRuntimeForRegistryOnly(t *testing.T) {
14		-	svc, err := cachesvc.New(cachesvc.Config{Dir: t.TempDir()})
	18	+	proxy, err := registryproxy.New(registryproxy.Config{Dir: t.TempDir(), Max: 1 << 20})
15	19		if err != nil {
16	20		t.Fatal(err)
17	21		}
18		-	defer func() { _ = svc.CloseAll() }()
19		-	j := &jobRun{job: &ermite.Job{JobId: "j1", TimeoutMinutes: 5}}
20		-	port, tok, mode, results := j.runtimeFor(&Runner{Cache: svc, Registry: true})
21		-	if port != cachesvc.DefaultPort \|\| tok == "" \|\| mode != "" \|\| results {
22		-	t.Fatalf("%d %q %q %v", port, tok, mode, results)
	22	+	withRegistry, err := cachesvc.New(cachesvc.Config{Dir: t.TempDir(), Registry: proxy})
	23	+	if err != nil {
	24	+	t.Fatal(err)
	25	+	}
	26	+	defer func() { _ = withRegistry.CloseAll() }()
	27	+	without, err := cachesvc.New(cachesvc.Config{Dir: t.TempDir()})
	28	+	if err != nil {
	29	+	t.Fatal(err)
23	30		}
24		-	job, scopes, _, err := svc.RuntimeToken(tok)
25		-	if err != nil \|\| job != "j1" \|\| strings.Contains(scopes, "Actions.Results") {
26		-	t.Fatalf("%q %q %v", job, scopes, err)
	31	+	defer func() { _ = without.CloseAll() }()
	32	+	// No repository in the contexts: the job has no cache scope.
	33	+	j := &jobRun{job: &ermite.Job{JobId: "j1", TimeoutMinutes: 5}}
	34	+	port, tok, mode, results := j.runtimeFor(&Runner{Cache: withRegistry})
	35	+	if port != cachesvc.DefaultPort \|\| tok != "" \|\| mode != "" \|\| results {
	36	+	t.Fatalf("registry only: %d %q %q %v", port, tok, mode, results)
27	37		}
28		-	req := httptest.NewRequest(http.MethodPost, cachesvc.APIPath+"caches", strings.NewReader(`{"key":"k","version":"v","cacheSize":1}`))
29		-	req.Header.Set("Authorization", "Bearer "+tok)
30		-	w := httptest.NewRecorder()
31		-	svc.Handler().ServeHTTP(w, req)
32		-	if w.Code != http.StatusForbidden {
33		-	t.Fatalf("unscoped token allowed cache: %d", w.Code)
	38	+	if p, _, _, _ := j.runtimeFor(&Runner{Cache: without}); p != 0 {
	39	+	t.Fatal("proxy off still supplies a channel")
34	40		}
35		-	if p, _, _, _ := j.runtimeFor(&Runner{Cache: svc}); p != 0 {
36		-	t.Fatal("proxy off still supplies channel")
	41	+	if p, _, _, _ := j.runtimeFor(&Runner{}); p != 0 {
	42	+	t.Fatal("cache off still supplies a channel")
37	43		}
38		-	if p, _, _, _ := j.runtimeFor(&Runner{Registry: true}); p != 0 {
39		-	t.Fatal("cache off still supplies channel")
	44	+	// A job with a cache scope still gets its token, proxy or not.
	45	+	scoped := &jobRun{job: &ermite.Job{JobId: "j1", Ref: "refs/heads/main", TimeoutMinutes: 5}, contexts: `{"github":{"repository":"o/r"}}`}
	46	+	if _, tok, mode, _ := scoped.runtimeFor(&Runner{Cache: withRegistry}); tok == "" \|\| mode != "write" {
	47	+	t.Fatalf("scoped job: %q %q", tok, mode)
40	48		}
41	49		}

M internal/host/resolve.go +2 -2


@@ -116,8 +116,8 @@ func (j jobRun) resolveActions(ctx context.Context, req execpb.ResolveRequest)
116	116		defer func() {
117	117		j.mu.Lock()
118	118		defer j.mu.Unlock()
119		-	j.timeline.add("resolve_ms", time.Since(start))
120		-	j.timeline["resolve_calls"]++
	119	+	j.timeline.addMs("resolve_ms", time.Since(start))
	120	+	j.timeline.add("resolve_calls", 1)
121	121		}()
122	122		fail := func(format string, a ...any) *execpb.ResolveResponse {
123	123		return &execpb.ResolveResponse{Error: fmt.Sprintf(format, a...)}

M internal/host/resolve_test.go +1 -1


@@ -48,7 +48,7 @@ func (c resolveStub) ResolveActions(_ context.Context, r connect.Request[ermit
48	48		}
49	49
50	50		func resolveJob(c jobClient) *jobRun {
51		-	j := &jobRun{client: c, ctx: context.Background(), job: &ermite.Job{JobId: "j1"}, log: zerowrap.Logger{},
	51	+	j := &jobRun{client: c, ctx: context.Background(), job: &ermite.Job{JobId: "j1"}, log: zerowrap.Logger{}, timeline: timeline{},
52	52		t: Timing{CallTimeout: time.Second, BackoffMin: time.Millisecond, BackoffMax: time.Millisecond}}
53	53		j.setOrigins(`{"github":{"server_url":"https://ermite.example"}}`, "https://ermite.example", "")
54	54		return j

M internal/host/results_test.go +1 -1


@@ -485,7 +485,7 @@ func TestAnnotationsWaitingForTheServerAreBounded(t *testing.T) {
485	485		tm := fastTiming()
486	486		ctx, cancel := context.WithCancel(context.Background())
487	487		defer cancel()
488		-	j := &jobRun{client: cl, t: tm, ctx: ctx, job: &ermite.Job{JobId: "j1"}, steps: map[int32]*ermite.StepUpdate{},
	488	+	j := &jobRun{client: cl, t: tm, ctx: ctx, job: &ermite.Job{JobId: "j1"}, steps: map[int32]*ermite.StepUpdate{}, timeline: timeline{},
489	489		annSeen: map[int32]bool{}, sumSeen: map[int32]bool{}, features: jobFeatures{annotations: true},
490	490		cancelCh: make(chan *execpb.Cancel, 1), log: zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: logs})}
491	491		j.outbox = newOutbox(cl, "j1", tm, func(error) {}, j.log)

M internal/host/runner.go +1 -3


@@ -45,8 +45,6 @@ type Runner struct {
45	45		// which the guest reaches on its loopback (cachesvc.DefaultPort), and each
46	46		// job gets a token scoped to its repository and refs.
47	47		Cache *cachesvc.Service
48		-	// Registry enables the runtime channel for anonymous registry-only jobs.
49		-	Registry bool
50	48		// Archives, when not nil (and Cache is), is the host's cache of action
51	49		// archives: the host downloads them and the sandbox reads them from the cache
52	50		// channel (see cacheArchives). ArchiveHTTP is its HTTP client (tests); nil

@@ -213,7 +211,7 @@ func (r Runner) startJob(ctx context.Context, log zerowrap.Logger, job ermite.
213	211		r: r, job: job, client: r.Job(job.JobToken), t: t, ctx: jctx, cancel: cancel,
214	212		log: zerowrap.Logger{Logger: log.With().Str("job_id", job.JobId).Int64("run_id", job.RunId).Int32("attempt", job.Attempt).Logger()},
215	213		receivedAt: r.clock(), cancelCh: make(chan execpb.Cancel, 1), steps: map[int32]ermite.StepUpdate{},
216		-	annSeen: map[int32]bool{}, sumSeen: map[int32]bool{},
	214	+	annSeen: map[int32]bool{}, sumSeen: map[int32]bool{}, timeline: timeline{},
217	215		}
218	216		jr.log.Info().Str("job_key", job.JobKey).Msg("job assigned")
219	217		r.active.Add(1)

M internal/host/runner_test.go +9 -15


@@ -39,23 +39,17 @@ func TestJobRunsAndReports(t *testing.T) {
39	39		if err := stop(); err != nil {
40	40		t.Fatal(err)
41	41		}
42		-	for _, line := range strings.Split(h.logs.String(), "\n") {
43		-	var fields map[string]any
44		-	if json.Unmarshal([]byte(line), &fields) != nil \|\| fields["message"] != "job finished" {
45		-	continue
46		-	}
47		-	for _, key := range []string{"ack_ms", "sandbox_ms", "run_ms", "stop_ms", "complete_ms", "total_ms", "sb_socket_ms", "guest_dockerd_ms"} {
48		-	value, ok := fields[key].(float64)
49		-	if !ok \|\| value < 0 {
50		-	t.Errorf("%s = %v", key, fields[key])
51		-	}
52		-	}
53		-	if fields["total_ms"].(float64) < fields["sandbox_ms"].(float64)+fields["run_ms"].(float64) {
54		-	t.Error("total shorter than sandbox + run")
	42	+	fields := jobFinished(t, h.logs.String())
	43	+	ms := map[string]float64{}
	44	+	for _, key := range []string{"ack_ms", "sandbox_ms", "run_ms", "stop_ms", "complete_ms", "total_ms", "sb_socket_ms", "guest_dockerd_ms"} {
	45	+	value, ok := fields[key].(float64)
	46	+	if !ok \|\| value < 0 {
	47	+	t.Errorf("%s = %v", key, fields[key])
55	48		}
	49	+	ms[key] = value
56	50		}
57		-	if !strings.Contains(h.logs.String(), `"message":"job finished"`) {
58		-	t.Fatalf("no job finished log: %s", h.logs.String())
	51	+	if ms["total_ms"] < ms["sandbox_ms"]+ms["run_ms"] {
	52	+	t.Error("total shorter than sandbox + run")
59	53		}
60	54		s := h.srv
61	55		s.mu.Lock()

M internal/host/shutdown_test.go +1 -1


@@ -81,7 +81,7 @@ func TestReportBudgetFollowsTheShutdownChannel(t *testing.T) {
81	81		cl := &stubClient{}
82	82		ctx, cancel := context.WithCancel(context.Background())
83	83		defer cancel()
84		-	j := &jobRun{client: cl, t: tm, ctx: ctx, job: &ermite.Job{JobId: "j1"}, steps: map[int32]*ermite.StepUpdate{},
	84	+	j := &jobRun{client: cl, t: tm, ctx: ctx, job: &ermite.Job{JobId: "j1"}, steps: map[int32]*ermite.StepUpdate{}, timeline: timeline{},
85	85		cancelCh: make(chan *execpb.Cancel, 1), log: zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logBuf{}})}
86	86		j.outbox = newOutbox(cl, "j1", tm, func(error) {}, j.log)
87	87		j.outbox.start(ctx)

M internal/host/timeline.go +34 -21


@@ -4,36 +4,49 @@ package host
4	4
5	5		import "time"
6	6
7		-	// timeline is protected by jobRun.mu.
	7	+	// timeline is the per-phase timing of a job, logged on its "job finished"
	8	+	// line: durations in milliseconds (the *_ms keys) and counters. A jobRun
	9	+	// creates it once, in startJob; it is guarded by jobRun.mu.
8	10		type timeline map[string]int64
9	11
10		-	func (t *timeline) set(key string, d time.Duration) {
11		-	if *t == nil {
12		-	*t = make(timeline)
13		-	}
14		-	(*t)[key] = max(0, d.Milliseconds())
15		-	}
16		-	func (t *timeline) add(key string, d time.Duration) {
17		-	if *t == nil {
18		-	*t = make(timeline)
19		-	}
20		-	(*t)[key] += max(0, d.Milliseconds())
21		-	}
22		-	func (t timeline) fields() map[string]any {
23		-	fields := make(map[string]any, len(t))
24		-	for k, v := range t {
25		-	fields[k] = v
	12	+	// setMs records d as the duration of a phase, in whole milliseconds.
	13	+	func (t timeline) setMs(key string, d time.Duration) { t[key] = max(0, d.Milliseconds()) }
	14	+
	15	+	// addMs adds d to the summed duration of a phase that may run several times.
	16	+	func (t timeline) addMs(key string, d time.Duration) { t[key] += max(0, d.Milliseconds()) }
	17	+
	18	+	// add adds n to a counter.
	19	+	func (t timeline) add(key string, n int64) { t[key] += n }
	20	+
	21	+	// guestBootKeys are the boot phases the guest may report in its Hello: the
	22	+	// host logs these as guest_<key>_ms and ignores any other key, as it does not
	23	+	// trust the guest.
	24	+	var guestBootKeys = []string{"kernel", "setup", "containerd", "dockerd"}
	25	+
	26	+	// addGuestBoot records the guest's boot phases: known keys with a value of
	27	+	// at least 0.
	28	+	func (t timeline) addGuestBoot(ms map[string]int64) {
	29	+	for _, k := range guestBootKeys {
	30	+	if v, ok := ms[k]; ok && v >= 0 {
	31	+	t["guest_"+k+"_ms"] = v
	32	+	}
26	33		}
27		-	return fields
28	34		}
	35	+
29	36		func (j *jobRun) timeSet(key string, d time.Duration) {
30	37		j.mu.Lock()
31	38		defer j.mu.Unlock()
32		-	j.timeline.set(key, d)
	39	+	j.timeline.setMs(key, d)
33	40		}
	41	+
	42	+	// timingFields sets total_ms and returns a copy of the timeline for the log.
34	43		func (j *jobRun) timingFields() map[string]any {
35	44		j.mu.Lock()
36	45		defer j.mu.Unlock()
37		-	j.timeline.set("total_ms", time.Since(j.receivedAt))
38		-	return j.timeline.fields()
	46	+	j.timeline.setMs("total_ms", time.Since(j.receivedAt))
	47	+	fields := make(map[string]any, len(j.timeline))
	48	+	for k, v := range j.timeline {
	49	+	fields[k] = v
	50	+	}
	51	+	return fields
39	52		}

M internal/host/timeline_test.go +64 -16


@@ -3,24 +3,72 @@
3	3		package host
4	4
5	5		import (
	6	+	"encoding/json"
	7	+	"fmt"
	8	+	"strings"
6	9		"testing"
7		-	"time"
	10	+
	11	+	ermite "bnema.dev/ermite-api/gen/ermite"
8	12		)
9	13
10		-	func TestTimeline(t *testing.T) {
11		-	var timeline timeline
12		-	timeline.set("run_ms", 3*time.Millisecond)
13		-	timeline.add("run_ms", 2*time.Millisecond)
14		-	fields := timeline.fields()
15		-	if fields["run_ms"] != int64(5) {
16		-	t.Fatal(fields)
17		-	}
18		-	fields["run_ms"] = int64(99)
19		-	if timeline.fields()["run_ms"] != int64(5) {
20		-	t.Fatal("shared fields")
21		-	}
22		-	timeline.set("run_ms", time.Millisecond)
23		-	if timeline.fields()["run_ms"] != int64(1) {
24		-	t.Fatal(timeline)
	14	+	// jobFinished returns the fields of the one "job finished" line of logs.
	15	+	func jobFinished(t *testing.T, logs string) map[string]any {
	16	+	t.Helper()
	17	+	var found map[string]any
	18	+	for _, line := range strings.Split(logs, "\n") {
	19	+	var fields map[string]any
	20	+	if json.Unmarshal([]byte(line), &fields) != nil \|\| fields["message"] != "job finished" {
	21	+	continue
	22	+	}
	23	+	if found != nil {
	24	+	t.Fatal("two job finished lines")
	25	+	}
	26	+	found = fields
	27	+	}
	28	+	if found == nil {
	29	+	t.Fatalf("no job finished line: %s", logs)
	30	+	}
	31	+	return found
	32	+	}
	33	+
	34	+	// The guest is untrusted: of its Hello.boot_ms, the job finished line keeps
	35	+	// only the known phases with a value of at least 0, whatever else it sends.
	36	+	func TestHostileGuestBootTimings(t *testing.T) {
	37	+	h := newHarness(t, simpleScript("hello"))
	38	+	hostile := map[string]int64{
	39	+	"kernel": 5,
	40	+	"dockerd": -7, // negative: dropped
	41	+	"setup": 0,
	42	+	"total": 1, // would overwrite a host field as guest_total_ms
	43	+	"": 3,
	44	+	"x\"y": 4,
	45	+	"../../etc": 5,
	46	+	"containerd": 9,
	47	+	}
	48	+	for i := range 500 {
	49	+	hostile[fmt.Sprint("junk", i)] = int64(i)
	50	+	}
	51	+	h.driver.bootMs = hostile
	52	+	h.srv.jobs = []*ermite.Job{testJob("j1")}
	53	+	stop := h.run(t)
	54	+	eventually(t, "CompleteJob", func() bool { return h.srv.completeCount() == 1 })
	55	+	if err := stop(); err != nil {
	56	+	t.Fatal(err)
	57	+	}
	58	+	fields := jobFinished(t, h.logs.String())
	59	+	var guest []string
	60	+	for k := range fields {
	61	+	if strings.HasPrefix(k, "guest_") {
	62	+	guest = append(guest, k)
	63	+	}
	64	+	}
	65	+	want := map[string]float64{"guest_kernel_ms": 5, "guest_setup_ms": 0, "guest_containerd_ms": 9}
	66	+	if len(guest) != len(want) {
	67	+	t.Fatalf("guest fields %v, want %v", guest, want)
	68	+	}
	69	+	for k, v := range want {
	70	+	if got, ok := fields[k].(float64); !ok \|\| got != v {
	71	+	t.Errorf("%s = %v, want %v", k, fields[k], v)
	72	+	}
25	73		}
26	74		}

M internal/registryproxy/accept.go +1 -0

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (

A internal/registryproxy/accept_test.go +112 -0

@@ -0,0 +1,112 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package registryproxy
	4	+
	5	+	import (
	6	+	"fmt"
	7	+	"io"
	8	+	"net/http"
	9	+	"net/http/httptest"
	10	+	"strings"
	11	+	"sync"
	12	+	"testing"
	13	+	)
	14	+
	15	+	// Accept is reduced to the known manifest types, so its variants map to
	16	+	// a bounded set of keys.
	17	+	func TestNormalizeAccept(t *testing.T) {
	18	+	const index, v2 = "application/vnd.oci.image.index.v1+json", "application/vnd.docker.distribution.manifest.v2+json"
	19	+	all := normalizeAccept("")
	20	+	if strings.Count(all, ",") != len(manifestTypes)-1 {
	21	+	t.Fatalf("default key %q", all)
	22	+	}
	23	+	for _, tc := range []struct{ in, want string }{
	24	+	{index + ";q=0.9, " + strings.ToUpper(v2) + " , " + index, v2 + "," + index},
	25	+	{" " + index + " ;q=0.1", index},
	26	+	{"text/html, /", all},
	27	+	{"", all},
	28	+	} {
	29	+	if got := normalizeAccept(tc.in); got != tc.want {
	30	+	t.Errorf("normalizeAccept(%q) = %q, want %q", tc.in, got, tc.want)
	31	+	}
	32	+	}
	33	+	}
	34	+
	35	+	// The Accept variants a job sends for one tag reach upstream with every known
	36	+	// type they name, and make at most one tag file per subset of the known types.
	37	+	// dockerd's legacy client sends one Accept line per type; the 8 KiB bound
	38	+	// applies to the joined lines.
	39	+	func TestAcceptTagFiles(t *testing.T) {
	40	+	types := []string{
	41	+	"application/vnd.oci.image.index.v1+json",
	42	+	"application/vnd.docker.distribution.manifest.list.v2+json",
	43	+	"application/vnd.docker.distribution.manifest.v2+json",
	44	+	}
	45	+	for _, tc := range []struct {
	46	+	name string
	47	+	accepts func(i int) []string // the Accept lines of request i
	48	+	requests int
	49	+	maxTags int
	50	+	code int
	51	+	}{
	52	+	{"multi-line", func(int) []string { return types }, 2, 1, http.StatusOK},
	53	+	{"variants", func(i int) []string {
	54	+	switch i % 3 {
	55	+	case 1:
	56	+	return []string{fmt.Sprintf("x/only-junk-%d", i)}
	57	+	case 2:
	58	+	return []string{fmt.Sprintf("%s;q=0.%d, x/junk-%d, %s", types[0], i%10, i, types[1])}
	59	+	}
	60	+	return []string{fmt.Sprintf("%s;q=0.%d, x/junk-%d", types[0], i%10, i)}
	61	+	}, 300, 3, http.StatusOK},
	62	+	{"oversized", func(int) []string {
	63	+	lines := make([]string, 200)
	64	+	for i := range lines {
	65	+	lines[i] = strings.Repeat("x", 50)
	66	+	}
	67	+	return lines
	68	+	}, 1, 0, http.StatusNotFound},
	69	+	} {
	70	+	t.Run(tc.name, func(t *testing.T) {
	71	+	var mu sync.Mutex
	72	+	var seen []string
	73	+	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
	74	+	mu.Lock()
	75	+	seen = append(seen, strings.Join(r.Header.Values("Accept"), ","))
	76	+	mu.Unlock()
	77	+	w.Header().Set("Content-Type", types[0])
	78	+	w.Header().Set("Docker-Content-Digest", sum(manifestBody))
	79	+	if r.Method == http.MethodGet {
	80	+	_, _ = io.WriteString(w, manifestBody)
	81	+	}
	82	+	})
	83	+	p := proxyWith(t, srv, Config{})
	84	+	for i := range tc.requests {
	85	+	r := httptest.NewRequest(http.MethodGet, "/v2/library/alpine/manifests/3", nil)
	86	+	lines := tc.accepts(i)
	87	+	for _, line := range lines {
	88	+	r.Header.Add("Accept", line)
	89	+	}
	90	+	w := httptest.NewRecorder()
	91	+	p.ServeHTTP(w, r)
	92	+	if w.Code != tc.code {
	93	+	t.Fatalf("request %d: %d", i, w.Code)
	94	+	}
	95	+	if tc.code != http.StatusOK {
	96	+	continue
	97	+	}
	98	+	mu.Lock()
	99	+	got := seen[len(seen)-1]
	100	+	mu.Unlock()
	101	+	for _, typ := range types {
	102	+	if strings.Contains(strings.Join(lines, ","), typ) && !strings.Contains(got, typ) {
	103	+	t.Errorf("upstream Accept %q lacks %s", got, typ)
	104	+	}
	105	+	}
	106	+	}
	107	+	if n := accounted(t, p.s); n > tc.maxTags \|\| (tc.maxTags > 0 && n == 0) {
	108	+	t.Fatalf("%d tag files for %d requests", n, tc.requests)
	109	+	}
	110	+	})
	111	+	}
	112	+	}

M internal/registryproxy/clients.go +28 -49

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (

@@ -13,35 +14,28 @@ import (
13	14		// upstreamClients returns the metadata client (auth, manifests: a total
14	15		// timeout and no redirects) and the blob client (no total timeout, a response
15	16		// header timeout, and redirects that drop the bearer when leaving the host).
16		-	func upstreamClients(c Config) (http.Client, http.Client) {
17		-	base := http.Client{}
18		-	if c.Client != nil {
19		-	base = *c.Client
	17	+	func upstreamClients(requestTimeout time.Duration) (metadata, blobs *http.Client) {
	18	+	metadata = &http.Client{
	19	+	Timeout: requestTimeout,
	20	+	CheckRedirect: func(http.Request, []http.Request) error { return http.ErrUseLastResponse },
20	21		}
21		-	metadata := base
22		-	metadata.Timeout = c.RequestTimeout
23		-	metadata.CheckRedirect = func(http.Request, []http.Request) error { return http.ErrUseLastResponse }
24		-	blobs := base
25		-	blobs.Timeout = 0
26		-	transport := base.Transport
27		-	if transport == nil {
28		-	transport = http.DefaultTransport
	22	+	blobs = &http.Client{
	23	+	CheckRedirect: func(req http.Request, via []http.Request) error {
	24	+	if len(via) >= 10 {
	25	+	return errors.New("too many registry redirects")
	26	+	}
	27	+	if len(via) > 0 && req.URL.Host != via[0].URL.Host {
	28	+	req.Header.Del("Authorization")
	29	+	}
	30	+	return nil
	31	+	},
29	32		}
30		-	if tr, ok := transport.(*http.Transport); ok {
	33	+	if tr, ok := http.DefaultTransport.(*http.Transport); ok {
31	34		tr = tr.Clone()
32		-	tr.ResponseHeaderTimeout = c.RequestTimeout
	35	+	tr.ResponseHeaderTimeout = requestTimeout
33	36		blobs.Transport = tr
34	37		}
35		-	blobs.CheckRedirect = func(req http.Request, via []http.Request) error {
36		-	if len(via) >= 10 {
37		-	return errors.New("too many registry redirects")
38		-	}
39		-	if len(via) > 0 && req.URL.Host != via[0].URL.Host {
40		-	req.Header.Del("Authorization")
41		-	}
42		-	return nil
43		-	}
44		-	return &metadata, &blobs
	38	+	return metadata, blobs
45	39		}
46	40
47	41		// idleBody serializes timer reset/stop against expiry. Cancellation interrupts

@@ -66,6 +60,7 @@ func (b *idleBody) Read(p []byte) (int, error) {
66	60		}
67	61		return n, err
68	62		}
	63	+
69	64		func (b *idleBody) Close() error {
70	65		b.mu.Lock()
71	66		b.expired = true

@@ -74,20 +69,16 @@ func (b *idleBody) Close() error {
74	69		b.cancel()
75	70		return b.ReadCloser.Close()
76	71		}
	72	+
	73	+	// blob GETs a blob. Each attempt has only a header deadline (RequestTimeout)
	74	+	// and then a body progress deadline (BlobIdleTimeout), and no total transfer
	75	+	// deadline: the response body is an idleBody that owns the attempt's context.
77	76		func (u upstream) blob(ctx context.Context, name, digest string) (http.Response, error) {
78		-	// Token acquisition has a total timeout; the subsequent blob request has only
79		-	// a header/body progress deadline and no total transfer deadline.
80		-	authCtx, authCancel := context.WithTimeout(ctx, u.requestTimeout)
81		-	tok, err := u.token(authCtx, name, false)
82		-	authCancel()
83		-	if err != nil {
84		-	return nil, err
85		-	}
86		-	for attempt := 0; attempt < 2; attempt++ {
	77	+	return u.authorized(ctx, name, func(tok string) (*http.Response, error) {
87	78		bodyCtx, cancel := context.WithCancel(ctx)
88	79		lease := &idleBody{cancel: cancel, idle: u.blobIdle}
89	80		lease.timer = time.AfterFunc(u.requestTimeout, func() { lease.mu.Lock(); lease.expired = true; cancel(); lease.mu.Unlock() })
90		-	req, err := http.NewRequestWithContext(bodyCtx, http.MethodGet, u.base+"/v2/"+name+"/blobs/"+digest, nil)
	81	+	req, err := http.NewRequestWithContext(bodyCtx, http.MethodGet, u.base+"/v2/"+name+"/"+kindBlob.dir()+"/"+digest, nil)
91	82		if err != nil {
92	83		lease.timer.Stop()
93	84		cancel()

@@ -101,26 +92,14 @@ func (u upstream) blob(ctx context.Context, name, digest string) (http.Respons
101	92		return nil, err
102	93		}
103	94		lease.ReadCloser = resp.Body
	95	+	resp.Body = lease
104	96		if resp.StatusCode == http.StatusOK {
105	97		lease.mu.Lock()
106	98		if !lease.expired {
107	99		lease.timer.Reset(u.blobIdle)
108	100		}
109	101		lease.mu.Unlock()
110		-	resp.Body = lease
111		-	return resp, nil
112	102		}
113		-	_ = lease.Close()
114		-	if (resp.StatusCode == http.StatusUnauthorized \|\| resp.StatusCode == http.StatusForbidden) && attempt == 0 {
115		-	authCtx, authCancel := context.WithTimeout(ctx, u.requestTimeout)
116		-	tok, err = u.token(authCtx, name, true)
117		-	authCancel()
118		-	if err != nil {
119		-	return nil, err
120		-	}
121		-	continue
122		-	}
123		-	return nil, upstreamError{resp.StatusCode}
124		-	}
125		-	return nil, upstreamError{http.StatusUnauthorized}
	103	+	return resp, nil
	104	+	})
126	105		}

M internal/registryproxy/flights_test.go +123 -306

@@ -1,8 +1,8 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (
5		-	"bytes"
6	6		"context"
7	7		"fmt"
8	8		"io"

@@ -19,49 +19,37 @@ import (
19	19		"github.com/bnema/zerowrap"
20	20		)
21	21
22		-	// stallingUpstream serves a blob's first bytes and then holds the body until
23		-	// the request is cancelled; it counts started and cancelled downloads.
24		-	type stallingUpstream struct {
25		-	srv *httptest.Server
26		-	started, cancelled atomic.Int32
27		-	}
28		-
29		-	func newStallingUpstream(t testing.T) stallingUpstream {
30		-	t.Helper()
31		-	u := &stallingUpstream{}
32		-	u.srv = upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
33		-	u.started.Add(1)
34		-	w.Header().Set("Content-Length", "1000000")
35		-	_, _ = io.WriteString(w, "partial")
36		-	w.(http.Flusher).Flush()
37		-	select {
38		-	case <-r.Context().Done():
39		-	u.cancelled.Add(1)
40		-	case <-time.After(10 * time.Second):
41		-	}
42		-	})
43		-	return u
44		-	}
45		-
46		-	func blobPath(i int) string { return "/v2/library/alpine/blobs/" + sum(fmt.Sprint("blob-", i)) }
47		-
48		-	// serveCancelled starts a request and cancels it once its download has begun.
49		-	func serveCancelled(t testing.T, p Proxy, u *stallingUpstream, path string, started int32) {
50		-	t.Helper()
51		-	ctx, cancel := context.WithCancel(context.Background())
52		-	done := make(chan struct{})
53		-	go func() {
54		-	defer close(done)
55		-	p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
56		-	}()
57		-	waitFor(t, p.s, "the download to start", func() bool { return u.started.Load() >= started })
58		-	cancel()
59		-	<-done
	22	+	// Concurrent requests for one digest share a single fetch.
	23	+	func TestSingleFlight(t *testing.T) {
	24	+	s := mustOpen(t, storeCfg(t.TempDir(), 1<<20))
	25	+	var calls atomic.Int32
	26	+	var wg sync.WaitGroup
	27	+	start := make(chan struct{})
	28	+	for range 20 {
	29	+	wg.Go(func() {
	30	+	<-start
	31	+	f, _, _, err := s.ensure(context.Background(), kindBlob, sum("content"), func(_ context.Context, w io.Writer) (string, error) {
	32	+	calls.Add(1)
	33	+	_, err := io.WriteString(w, "content")
	34	+	return "", err
	35	+	}, keepalive{})
	36	+	if err != nil {
	37	+	t.Error(err)
	38	+	return
	39	+	}
	40	+	_ = f.Close()
	41	+	})
	42	+	}
	43	+	close(start)
	44	+	wg.Wait()
	45	+	if calls.Load() != 1 {
	46	+	t.Fatalf("fetches %d", calls.Load())
	47	+	}
60	48		}
61	49
62		-	// MAJOR 1a: a guest that requests many digests and disconnects leaves no
63		-	// download behind: once a flight has no waiter for the grace period, it is
64		-	// cancelled upstream and its temporary file removed.
	50	+	// A guest that requests many digests and disconnects leaves no download
	51	+	// behind: once a flight has no waiter for the grace period, it is cancelled
	52	+	// upstream and its temporary file removed.
65	53		func TestAbandonedDownloadsCancelled(t *testing.T) {
66	54		u := newStallingUpstream(t)
67	55		const n = 20

@@ -82,7 +70,7 @@ func TestAbandonedDownloadsCancelled(t *testing.T) {
82	70		})
83	71		}
84	72
85		-	// MAJOR 1a: a request that joins within the grace period keeps the download.
	73	+	// A request that joins within the grace period keeps the download.
86	74		func TestRejoinWithinGraceKeepsDownload(t *testing.T) {
87	75		body := strings.Repeat("layer", 4000)
88	76		release := make(chan struct{})

@@ -109,7 +97,7 @@ func TestRejoinWithinGraceKeepsDownload(t *testing.T) {
109	97		defer close(first)
110	98		p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
111	99		}()
112		-	key := "blobs/" + sum(body)[7:]
	100	+	key := kindBlob.key(sum(body)[7:])
113	101		waitFor(t, p.s, "the first waiter", func() bool { return waiters(p.s, key) == 1 })
114	102		cancel()
115	103		<-first

@@ -124,255 +112,109 @@ func TestRejoinWithinGraceKeepsDownload(t *testing.T) {
124	112		}
125	113		}
126	114
127		-	// MAJOR 1b: at most MaxDownloads cold blobs download at once; one more waits
128		-	// for SlotWait and then gets 503 (dockerd retries or pulls from Docker Hub),
129		-	// and a slot frees once a download ends.
130		-	func TestColdDownloadCap(t *testing.T) {
131		-	u := newStallingUpstream(t)
132		-	const limit = 3
133		-	const wait = 50 * time.Millisecond
134		-	p := proxyWith(t, u.srv, Config{AbandonGrace: 10 * time.Millisecond, MaxDownloads: limit, SlotWait: wait})
135		-	ctx, cancel := context.WithCancel(context.Background())
136		-	var wg sync.WaitGroup
137		-	for i := range limit {
138		-	wg.Go(func() {
139		-	p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, blobPath(i), nil).WithContext(ctx))
140		-	})
141		-	}
142		-	waitFor(t, p.s, "the cap to fill", func() bool { return p.s.downloads == limit })
143		-	start := time.Now()
144		-	w := request(t, p, http.MethodGet, blobPath(limit))
145		-	if w.Code != http.StatusServiceUnavailable \|\| w.Header().Get("Retry-After") == "" {
146		-	t.Fatalf("over the cap: %d", w.Code)
147		-	}
148		-	if d := time.Since(start); d < wait {
149		-	t.Fatalf("503 after %v, before the slot wait", d)
150		-	}
151		-	// Joining a running download takes no slot.
152		-	joined := make(chan int, 1)
153		-	go func() {
154		-	jctx, jcancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
155		-	defer jcancel()
156		-	jw := httptest.NewRecorder()
157		-	p.ServeHTTP(jw, httptest.NewRequest(http.MethodGet, blobPath(0), nil).WithContext(jctx))
158		-	joined <- jw.Code
159		-	}()
160		-	if code := <-joined; code == http.StatusServiceUnavailable {
161		-	t.Fatal("a joiner was refused a slot")
162		-	}
163		-	cancel()
164		-	wg.Wait()
165		-	waitFor(t, p.s, "the slots to free", func() bool { return p.s.downloads == 0 })
166		-	if u.started.Load() != limit {
167		-	t.Fatalf("%d downloads started", u.started.Load())
168		-	}
169		-	}
170		-
171		-	// MAJOR 2: dockerd's legacy client sends one Accept line per type. All of them
172		-	// reach upstream, and they make one tag file.
173		-	func TestMultiLineAccept(t *testing.T) {
174		-	types := []string{
175		-	"application/vnd.oci.image.index.v1+json",
176		-	"application/vnd.docker.distribution.manifest.list.v2+json",
177		-	"application/vnd.docker.distribution.manifest.v2+json",
178		-	}
179		-	var mu sync.Mutex
180		-	var seen []string
	115	+	// The shared download does not belong to the client that started it. That
	116	+	// client disconnects mid-download; a second client, joining within the grace
	117	+	// period, still gets the blob and the blob is committed, from one upstream
	118	+	// fetch.
	119	+	func TestBlobFlightSurvivesInitiatorDisconnect(t *testing.T) {
	120	+	body := strings.Repeat("layer", 4000)
	121	+	half, release := make(chan struct{}), make(chan struct{})
	122	+	var releaseOnce sync.Once
	123	+	unblock := func() { releaseOnce.Do(func() { close(release) }) }
	124	+	var gets atomic.Int32
181	125		srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
182		-	mu.Lock()
183		-	seen = append(seen, strings.Join(r.Header.Values("Accept"), ","))
184		-	mu.Unlock()
185		-	w.Header().Set("Content-Type", types[0])
186		-	w.Header().Set("Docker-Content-Digest", sum(manifestBody))
187		-	if r.Method == http.MethodGet {
188		-	_, _ = io.WriteString(w, manifestBody)
189		-	}
	126	+	gets.Add(1)
	127	+	w.Header().Set("Content-Length", fmt.Sprint(len(body)))
	128	+	_, _ = io.WriteString(w, body[:len(body)/2])
	129	+	w.(http.Flusher).Flush()
	130	+	close(half)
	131	+	<-release
	132	+	_, _ = io.WriteString(w, body[len(body)/2:])
190	133		})
	134	+	t.Cleanup(unblock) // before srv.Close, which waits for the handler
191	135		p := proxyWith(t, srv, Config{})
192		-	for range 2 {
193		-	r := httptest.NewRequest(http.MethodGet, "/v2/library/alpine/manifests/3", nil)
194		-	for _, typ := range types {
195		-	r.Header.Add("Accept", typ)
196		-	}
197		-	w := httptest.NewRecorder()
198		-	p.ServeHTTP(w, r)
199		-	if w.Code != http.StatusOK {
200		-	t.Fatal(w.Code)
201		-	}
202		-	}
203		-	mu.Lock()
204		-	for _, got := range seen {
205		-	for _, typ := range types {
206		-	if !strings.Contains(got, typ) {
207		-	t.Errorf("upstream Accept %q lacks %s", got, typ)
208		-	}
209		-	}
210		-	}
211		-	mu.Unlock()
212		-	if n := accounted(t, p.s); n != 1 {
213		-	t.Fatalf("%d tag files", n)
214		-	}
215		-	// The 8 KiB bound applies to the joined lines.
216		-	r := httptest.NewRequest(http.MethodGet, "/v2/library/alpine/manifests/3", nil)
217		-	for range 200 {
218		-	r.Header.Add("Accept", strings.Repeat("x", 50))
219		-	}
220		-	w := httptest.NewRecorder()
221		-	p.ServeHTTP(w, r)
222		-	if w.Code != http.StatusNotFound {
223		-	t.Fatalf("oversized Accept: %d", w.Code)
224		-	}
225		-	}
	136	+	path := "/v2/library/alpine/blobs/" + sum(body)
226	137
227		-	// MINOR: a rate-limited upstream (429) is unavailable: a known tag is served
228		-	// stale; an unknown one is an error.
229		-	func TestRateLimitedServesStaleTag(t *testing.T) {
230		-	f := fake(t)
231		-	p := proxyFor(t, f)
232		-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3"); w.Code != http.StatusOK {
233		-	t.Fatal(w.Code)
234		-	}
235		-	f.status.Store(http.StatusTooManyRequests)
236		-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3"); w.Code != http.StatusOK \|\| w.Body.String() != manifestBody {
237		-	t.Fatalf("stale on 429: %d", w.Code)
	138	+	ctx, cancel := context.WithCancel(context.Background())
	139	+	first := httptest.NewRecorder()
	140	+	done := make(chan struct{})
	141	+	go func() {
	142	+	defer close(done)
	143	+	p.ServeHTTP(first, httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
	144	+	}()
	145	+	<-half
	146	+	cancel()
	147	+	<-done
	148	+	if strings.Contains(first.Body.String(), "layer") {
	149	+	t.Fatal("unverified bytes sent to the initiator")
238	150		}
239		-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/other"); w.Code != http.StatusBadGateway {
240		-	t.Fatalf("unknown on 429: %d", w.Code)
	151	+	key := kindBlob.key(sum(body)[7:])
	152	+	p.s.mu.Lock()
	153	+	left := waiters(p.s, key)
	154	+	p.s.mu.Unlock()
	155	+	if left != 0 {
	156	+	t.Fatalf("waiters after the disconnect: %d", left)
241	157		}
242		-	}
243	158
244		-	// MINOR: eviction walks the LRU, evicts least recently used first, frees down
245		-	// to the low-water mark rather than the bare need, and skips pinned entries.
246		-	func TestEvictionLRUAndLowWater(t *testing.T) {
247		-	now := time.Unix(1_700_000_000, 0)
248		-	const max = 2000 // low water: 5% = 100 bytes
249		-	s, err := openStore(t.TempDir(), max, func() time.Time { return now })
250		-	if err != nil {
251		-	t.Fatal(err)
252		-	}
253		-	// 40 blobs of 50 bytes fill the store. Freeing only the need (1 byte) would
254		-	// evict one blob; the low-water mark (100 bytes) evicts two.
255		-	const n = 40
256		-	blob := func(i int) string { return fmt.Sprintf("%03d", i) + strings.Repeat("x", 47) }
257		-	for i := range n {
258		-	now = now.Add(time.Second)
259		-	put(t, s, blob(i))
260		-	}
261		-	// Touch 0 and 1, and keep 2 open: the least recently used are then 3, 4...
262		-	for _, i := range []int{0, 1} {
263		-	now = now.Add(time.Second)
264		-	f, _, err := s.open("blobs", sum(blob(i)))
265		-	if err != nil {
266		-	t.Fatal(err)
267		-	}
268		-	_ = f.Close()
269		-	}
270		-	pinned, _, err := s.open("blobs", sum(blob(2)))
271		-	if err != nil {
272		-	t.Fatal(err)
273		-	}
274		-	defer func() { _ = pinned.Close() }()
275		-	s.mu.Lock()
276		-	s.lru.MoveToFront(s.entries["blobs/"+sum(blob(2))[7:]].elem) // oldest of all, but open
277		-	s.mu.Unlock()
278		-	now = now.Add(time.Second)
279		-	put(t, s, "a") // 1 byte over the cap: frees max(1, 100) bytes
280		-	present := func(i int) bool {
281		-	_, err := os.Stat(filepath.Join(s.dir, "blobs", sum(blob(i))[7:]))
282		-	return err == nil
	159	+	second := httptest.NewRecorder()
	160	+	done = make(chan struct{})
	161	+	go func() {
	162	+	defer close(done)
	163	+	p.ServeHTTP(second, httptest.NewRequest(http.MethodGet, path, nil))
	164	+	}()
	165	+	waitFor(t, p.s, "the second client to join", func() bool { return waiters(p.s, key) == 1 })
	166	+	unblock()
	167	+	<-done
	168	+	if second.Code != http.StatusOK \|\| second.Body.String() != body {
	169	+	t.Fatalf("second client: %d", second.Code)
283	170		}
284		-	for i := range n {
285		-	want := i != 3 && i != 4 // 3 and 4 are the least recently used unpinned
286		-	if present(i) != want {
287		-	t.Errorf("blob %d present=%v", i, present(i))
288		-	}
	171	+	if _, err := os.Stat(filepath.Join(p.s.dir, key)); err != nil {
	172	+	t.Fatalf("blob not committed: %v", err)
289	173		}
290		-	accounted(t, s)
291		-	s.mu.Lock()
292		-	free := s.max - s.total
293		-	s.mu.Unlock()
294		-	if free < min(lowWater, int64(max)/20)-1 {
295		-	t.Fatalf("freed only to %d", free)
	174	+	if gets.Load() != 1 {
	175	+	t.Fatalf("%d upstream fetches", gets.Load())
296	176		}
297	177		}
298	178
299		-	// MINOR: the tag index follows rewrites, and removing a manifest takes exactly
300		-	// its own tags along.
301		-	func TestTagIndex(t *testing.T) {
302		-	s, err := openStore(t.TempDir(), 1<<20, time.Now)
303		-	if err != nil {
304		-	t.Fatal(err)
305		-	}
306		-	manifests := []string{`{"schemaVersion":2,"a":1}`, `{"schemaVersion":2,"b":1}`}
307		-	for _, m := range manifests {
308		-	f, _, _, err := s.ensure(context.Background(), "manifests", sum(m), func(_ context.Context, w io.Writer) (string, error) {
309		-	_, err := io.WriteString(w, m)
310		-	return "application/vnd.oci.image.index.v1+json", err
311		-	}, keepalive{})
312		-	if err != nil {
313		-	t.Fatal(err)
314		-	}
315		-	_ = f.Close()
316		-	}
317		-	for _, tag := range []string{"x", "y"} {
318		-	if err := s.setTag("library/alpine", tag, "", sum(manifests[0])); err != nil {
319		-	t.Fatal(err)
320		-	}
321		-	}
322		-	// y moves to the second manifest.
323		-	if err := s.setTag("library/alpine", "y", "", sum(manifests[1])); err != nil {
324		-	t.Fatal(err)
325		-	}
326		-	s.mu.Lock()
327		-	if !s.removeLocked("manifests/" + sum(manifests[0])[7:]) {
328		-	t.Fatal("remove failed")
329		-	}
330		-	n0, n1 := len(s.tagsOf[sum(manifests[0])]), len(s.tagsOf[sum(manifests[1])])
331		-	s.mu.Unlock()
332		-	if s.tag("library/alpine", "x", "") != "" \|\| s.tag("library/alpine", "y", "") != sum(manifests[1]) \|\| n0 != 0 \|\| n1 != 1 {
333		-	t.Fatalf("index: %d %d", n0, n1)
	179	+	// A client waiting for a progressing download is kept alive, so the cache
	180	+	// channel's progress deadline follows the download.
	181	+	func TestWaiterKeepAlive(t *testing.T) {
	182	+	body := strings.Repeat("layer", 4000)
	183	+	srv := upstreamWith(t, slowBlob(body, 10, 30*time.Millisecond))
	184	+	p := proxyWith(t, srv, Config{KeepAliveInterval: 10 * time.Millisecond})
	185	+	w := &keepAliveRecorder{ResponseRecorder: httptest.NewRecorder()}
	186	+	p.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/v2/library/alpine/blobs/"+sum(body), nil))
	187	+	if w.Code != http.StatusOK \|\| w.calls.Load() == 0 {
	188	+	t.Fatalf("code %d, %d keepalives", w.Code, w.calls.Load())
334	189		}
335		-	accounted(t, s)
336	190		}
337	191
338		-	// NIT: the committed entry is pinned for its waiters, so an eviction between
339		-	// the commit and their open cannot remove it.
340		-	func TestCommittedEntryPinnedForWaiters(t *testing.T) {
341		-	s, err := openStore(t.TempDir(), 8, time.Now)
342		-	if err != nil {
343		-	t.Fatal(err)
344		-	}
345		-	s.hookCommitted = func(string) {
346		-	// Ask for the whole store before the waiters are woken.
347		-	s.mu.Lock()
348		-	s.evictLocked(s.max)
349		-	s.mu.Unlock()
350		-	}
351		-	f, _, _, err := s.ensure(context.Background(), "blobs", sum("aaaa"), func(_ context.Context, w io.Writer) (string, error) {
352		-	_, err := io.WriteString(w, "aaaa")
353		-	return "", err
	192	+	// A panic in a download fails its flight and frees its slots; it does not
	193	+	// leave the waiters hanging.
	194	+	func TestPanickingFetchFailsFlight(t *testing.T) {
	195	+	c := storeCfg(t.TempDir(), 1<<20)
	196	+	c.maxDownloads = 1
	197	+	s := mustOpen(t, c)
	198	+	q := NewQuota(1)
	199	+	ctx, cancel := context.WithTimeout(WithQuota(context.Background(), q), 5*time.Second)
	200	+	defer cancel()
	201	+	_, _, _, err := s.ensure(ctx, kindBlob, sum("x"), func(context.Context, io.Writer) (string, error) {
	202	+	panic("boom")
354	203		}, keepalive{})
355		-	if err != nil {
356		-	t.Fatalf("waiter lost its entry: %v", err)
	204	+	if err == nil \|\| !strings.Contains(err.Error(), "panicked") {
	205	+	t.Fatalf("err %v", err)
357	206		}
358	207		s.mu.Lock()
359		-	pins := s.entries["blobs/"+sum("aaaa")[7:]].pins
360		-	s.mu.Unlock()
361		-	if pins != 1 {
362		-	t.Fatalf("pins %d: the commit pin was not released to the reader's", pins)
	208	+	defer s.mu.Unlock()
	209	+	if s.downloads != 0 \|\| len(s.flights) != 0 \|\| q.used.Load() != 0 \|\| s.pending != 0 {
	210	+	t.Fatalf("leaked: downloads=%d flights=%d quota=%d pending=%d", s.downloads, len(s.flights), q.used.Load(), s.pending)
363	211		}
364		-	_ = f.Close()
365		-	s.hookCommitted = nil
366		-	s.mu.Lock()
367		-	s.evictLocked(s.max)
368		-	_, kept := s.entries["blobs/"+sum("aaaa")[7:]]
369		-	s.mu.Unlock()
370		-	if kept {
371		-	t.Fatal("unpinned entry not evictable")
	212	+	if ds, _ := os.ReadDir(filepath.Join(s.dir, "tmp")); len(ds) != 0 {
	213	+	t.Fatal("temp file kept")
372	214		}
373	215		}
374	216
375		-	// NIT: a request that joins a manifest flight logs hit=false.
	217	+	// A request that joins a manifest flight logs hit=false, with its job.
376	218		func TestJoinerLogsMiss(t *testing.T) {
377	219		release := make(chan struct{})
378	220		srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {

@@ -386,46 +228,21 @@ func TestJoinerLogsMiss(t *testing.T) {
386	228		var logs syncBuffer
387	229		p := proxyWith(t, srv, Config{Log: zerowrap.New(zerowrap.Config{Level: "info", Format: "json", Output: &logs})})
388	230		var wg sync.WaitGroup
389		-	for range 2 {
390		-	wg.Go(func() { request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3") })
	231	+	for i := range 2 {
	232	+	wg.Go(func() {
	233	+	ctx := WithJob(context.Background(), fmt.Sprint("job-", i))
	234	+	p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/v2/library/alpine/manifests/3", nil).WithContext(ctx))
	235	+	})
391	236		}
392		-	key := "manifests/" + sum(manifestBody)[7:]
	237	+	key := kindManifest.key(sum(manifestBody)[7:])
393	238		waitFor(t, p.s, "two waiters", func() bool { return waiters(p.s, key) == 2 })
394	239		close(release)
395	240		wg.Wait()
396		-	if out := logs.String(); strings.Count(out, `"hit":false`) != 2 \|\| strings.Contains(out, `"hit":true`) {
	241	+	out := logs.String()
	242	+	if strings.Count(out, `"hit":false`) != 2 \|\| strings.Contains(out, `"hit":true`) {
397	243		t.Fatalf("logs: %s", out)
398	244		}
399		-	}
400		-
401		-	type syncBuffer struct {
402		-	mu sync.Mutex
403		-	b bytes.Buffer
404		-	}
405		-
406		-	func (s *syncBuffer) Write(p []byte) (int, error) {
407		-	s.mu.Lock()
408		-	defer s.mu.Unlock()
409		-	return s.b.Write(p)
410		-	}
411		-	func (s *syncBuffer) String() string { s.mu.Lock(); defer s.mu.Unlock(); return s.b.String() }
412		-
413		-	// NIT: a .type file without its manifest is removed at startup.
414		-	func TestOrphanTypeRemoved(t *testing.T) {
415		-	dir := t.TempDir()
416		-	if _, err := openStore(dir, 1<<20, time.Now); err != nil {
417		-	t.Fatal(err)
418		-	}
419		-	orphan := filepath.Join(dir, "manifests", sum("gone")[7:]+".type")
420		-	if err := os.WriteFile(orphan, []byte("application/json"), 0600); err != nil {
421		-	t.Fatal(err)
422		-	}
423		-	s, err := openStore(dir, 1<<20, time.Now)
424		-	if err != nil {
425		-	t.Fatal(err)
426		-	}
427		-	if _, err := os.Stat(orphan); !os.IsNotExist(err) {
428		-	t.Fatal("orphan .type kept")
	245	+	if !strings.Contains(out, `"job_id":"job-0"`) \|\| !strings.Contains(out, `"job_id":"job-1"`) {
	246	+	t.Fatalf("no job_id on the registry lines: %s", out)
429	247		}
430		-	accounted(t, s)
431	248		}

A internal/registryproxy/helpers_test.go +375 -0

@@ -0,0 +1,375 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package registryproxy
	4	+
	5	+	import (
	6	+	"bytes"
	7	+	"context"
	8	+	"crypto/sha256"
	9	+	"fmt"
	10	+	"io"
	11	+	"net/http"
	12	+	"net/http/httptest"
	13	+	"os"
	14	+	"path/filepath"
	15	+	"strings"
	16	+	"sync"
	17	+	"sync/atomic"
	18	+	"testing"
	19	+	"time"
	20	+	)
	21	+
	22	+	// TestMain fails the run if any store hit an accounting bug (unpinLocked of
	23	+	// an unpinned entry), whichever test caused it.
	24	+	func TestMain(m *testing.M) {
	25	+	var bugs atomic.Int32
	26	+	defaultBugHook = func(msg string) {
	27	+	bugs.Add(1)
	28	+	fmt.Fprintln(os.Stderr, "registry cache bug:", msg)
	29	+	}
	30	+	code := m.Run()
	31	+	if n := bugs.Load(); n > 0 && code == 0 {
	32	+	fmt.Fprintf(os.Stderr, "FAIL: %d registry cache accounting bugs\n", n)
	33	+	code = 1
	34	+	}
	35	+	os.Exit(code)
	36	+	}
	37	+
	38	+	const manifestBody = `{"schemaVersion":2}`
	39	+	const blobBody = "a layer of bytes"
	40	+
	41	+	func sum(b string) string { return fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(b))) }
	42	+
	43	+	// storeCfg is a complete configuration for a test that opens a store directly.
	44	+	func storeCfg(dir string, max int64) storeConfig {
	45	+	return storeConfig{dir: dir, max: max, now: time.Now, grace: time.Second, maxDownloads: 16, slotWait: time.Second}
	46	+	}
	47	+
	48	+	func mustOpen(t testing.T, c storeConfig) store {
	49	+	t.Helper()
	50	+	s, err := openStore(c)
	51	+	if err != nil {
	52	+	t.Fatal(err)
	53	+	}
	54	+	return s
	55	+	}
	56	+
	57	+	// put stores b as a blob.
	58	+	func put(t testing.T, s store, b string) {
	59	+	t.Helper()
	60	+	f, _, _, err := s.ensure(context.Background(), kindBlob, sum(b), func(_ context.Context, w io.Writer) (string, error) { _, err := io.WriteString(w, b); return "", err }, keepalive{})
	61	+	if err != nil {
	62	+	t.Fatal(err)
	63	+	}
	64	+	_ = f.Close()
	65	+	}
	66	+
	67	+	// putManifest stores m as a manifest of media type media.
	68	+	func putManifest(t testing.T, s store, m, media string) {
	69	+	t.Helper()
	70	+	f, _, _, err := s.ensure(context.Background(), kindManifest, sum(m), func(_ context.Context, w io.Writer) (string, error) {
	71	+	_, err := io.WriteString(w, m)
	72	+	return media, err
	73	+	}, keepalive{})
	74	+	if err != nil {
	75	+	t.Fatal(err)
	76	+	}
	77	+	_ = f.Close()
	78	+	}
	79	+
	80	+	// accounted checks that the store's total is exactly the sum of its entries and
	81	+	// within Max, and returns the number of tag files on disk.
	82	+	func accounted(t testing.T, s store) int {
	83	+	t.Helper()
	84	+	s.mu.Lock()
	85	+	defer s.mu.Unlock()
	86	+	var sum int64
	87	+	tags := 0
	88	+	for k, e := range s.entries {
	89	+	sum += e.size
	90	+	if e.size < minCharge {
	91	+	t.Errorf("%s charged %d, under the minimum", k, e.size)
	92	+	}
	93	+	if e.kind == kindTag {
	94	+	tags++
	95	+	if e.size != minCharge {
	96	+	t.Errorf("tag %s charged %d", k, e.size)
	97	+	}
	98	+	}
	99	+	}
	100	+	if sum != s.total \|\| s.total+s.pending > s.max {
	101	+	t.Fatalf("accounting: sum=%d total=%d pending=%d max=%d", sum, s.total, s.pending, s.max)
	102	+	}
	103	+	files, err := os.ReadDir(filepath.Join(s.dir, kindTag.dir()))
	104	+	if err != nil {
	105	+	t.Fatal(err)
	106	+	}
	107	+	if len(files) != tags {
	108	+	t.Fatalf("%d tag files, %d tag entries", len(files), tags)
	109	+	}
	110	+	return tags
	111	+	}
	112	+
	113	+	// waitFor polls cond (under the store lock) until it holds.
	114	+	func waitFor(t testing.T, s store, what string, cond func() bool) {
	115	+	t.Helper()
	116	+	for end := time.Now().Add(5 * time.Second); ; {
	117	+	s.mu.Lock()
	118	+	ok := cond()
	119	+	s.mu.Unlock()
	120	+	if ok {
	121	+	return
	122	+	}
	123	+	if time.Now().After(end) {
	124	+	t.Fatalf("timed out waiting for %s", what)
	125	+	}
	126	+	time.Sleep(time.Millisecond)
	127	+	}
	128	+	}
	129	+
	130	+	// waiters is the number of requests waiting for the flight of key (-1: none).
	131	+	// The caller holds the store lock.
	132	+	func waiters(s *store, key string) int {
	133	+	if fl, ok := s.flights[key]; ok {
	134	+	return fl.waiters
	135	+	}
	136	+	return -1
	137	+	}
	138	+
	139	+	// upstreamWith fakes Docker Hub: /token answers an anonymous token and h serves
	140	+	// everything else.
	141	+	func upstreamWith(t testing.T, h http.HandlerFunc) httptest.Server {
	142	+	t.Helper()
	143	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	144	+	if r.URL.Path == "/token" {
	145	+	_, _ = io.WriteString(w, `{"token":"public","expires_in":300}`)
	146	+	return
	147	+	}
	148	+	h(w, r)
	149	+	}))
	150	+	t.Cleanup(srv.Close)
	151	+	return srv
	152	+	}
	153	+
	154	+	func proxyWith(t testing.T, srv httptest.Server, c Config) *Proxy {
	155	+	t.Helper()
	156	+	if c.Dir == "" {
	157	+	c.Dir = t.TempDir()
	158	+	}
	159	+	if c.Max == 0 {
	160	+	c.Max = 8 << 20
	161	+	}
	162	+	c.Upstream, c.AuthURL = srv.URL, srv.URL+"/token"
	163	+	p, err := New(c)
	164	+	if err != nil {
	165	+	t.Fatal(err)
	166	+	}
	167	+	return p
	168	+	}
	169	+
	170	+	// fakeRegistry is a Docker Hub with one manifest and one blob, served from a
	171	+	// CDN behind a redirect. It counts the requests it gets.
	172	+	type fakeRegistry struct {
	173	+	server *httptest.Server
	174	+	heads, gets, blobs, tokens atomic.Int32
	175	+	status atomic.Int32
	176	+	corrupt atomic.Bool
	177	+	}
	178	+
	179	+	func fake(t testing.T) fakeRegistry {
	180	+	t.Helper()
	181	+	f := &fakeRegistry{}
	182	+	cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	183	+	if r.Header.Get("Authorization") != "" {
	184	+	t.Error("bearer leaked to CDN")
	185	+	}
	186	+	w.Header().Set("Content-Length", fmt.Sprint(len(blobBody)))
	187	+	if r.Method != http.MethodHead {
	188	+	b := blobBody
	189	+	if f.corrupt.Load() {
	190	+	b = strings.Repeat("x", len(b))
	191	+	}
	192	+	_, _ = io.WriteString(w, b)
	193	+	}
	194	+	}))
	195	+	t.Cleanup(cdn.Close)
	196	+	f.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	197	+	if r.URL.Path == "/token" {
	198	+	f.tokens.Add(1)
	199	+	if r.Header.Get("Authorization") != "" {
	200	+	t.Error("guest credentials forwarded to auth")
	201	+	}
	202	+	if r.URL.Query().Get("scope") != "repository:library/alpine:pull" {
	203	+	t.Error("wrong scope")
	204	+	}
	205	+	_, _ = io.WriteString(w, `{"token":"public","expires_in":300}`)
	206	+	return
	207	+	}
	208	+	if r.Header.Get("Authorization") != "Bearer public" {
	209	+	t.Errorf("wrong bearer %q", r.Header.Get("Authorization"))
	210	+	}
	211	+	if s := f.status.Load(); s != 0 {
	212	+	w.WriteHeader(int(s))
	213	+	return
	214	+	}
	215	+	if strings.Contains(r.URL.Path, "/manifests/") {
	216	+	w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json")
	217	+	w.Header().Set("Docker-Content-Digest", sum(manifestBody))
	218	+	w.Header().Set("Content-Length", fmt.Sprint(len(manifestBody)))
	219	+	if r.Method == http.MethodHead {
	220	+	f.heads.Add(1)
	221	+	} else {
	222	+	f.gets.Add(1)
	223	+	_, _ = io.WriteString(w, manifestBody)
	224	+	}
	225	+	return
	226	+	}
	227	+	if strings.Contains(r.URL.Path, "/blobs/") {
	228	+	if r.Method == http.MethodGet {
	229	+	f.blobs.Add(1)
	230	+	}
	231	+	http.Redirect(w, r, cdn.URL+"/layer", http.StatusTemporaryRedirect)
	232	+	return
	233	+	}
	234	+	http.NotFound(w, r)
	235	+	}))
	236	+	t.Cleanup(f.server.Close)
	237	+	return f
	238	+	}
	239	+
	240	+	func proxyFor(t testing.T, f fakeRegistry) *Proxy {
	241	+	t.Helper()
	242	+	p, err := New(Config{Dir: t.TempDir(), Max: 8 << 20, Upstream: f.server.URL, AuthURL: f.server.URL + "/token"})
	243	+	if err != nil {
	244	+	t.Fatal(err)
	245	+	}
	246	+	return p
	247	+	}
	248	+
	249	+	// request serves one request with guest credentials that must never reach
	250	+	// upstream; headers are name, value pairs.
	251	+	func request(t testing.T, p http.Handler, method, path string, headers ...string) httptest.ResponseRecorder {
	252	+	t.Helper()
	253	+	r := httptest.NewRequest(method, path, nil)
	254	+	r.Header.Set("Authorization", "Bearer guest-secret")
	255	+	for i := 0; i < len(headers); i += 2 {
	256	+	r.Header.Set(headers[i], headers[i+1])
	257	+	}
	258	+	w := httptest.NewRecorder()
	259	+	p.ServeHTTP(w, r)
	260	+	return w
	261	+	}
	262	+
	263	+	// stallingUpstream serves a blob's first bytes and then holds the body until
	264	+	// the request is cancelled; it counts started and cancelled downloads.
	265	+	type stallingUpstream struct {
	266	+	srv *httptest.Server
	267	+	started, cancelled atomic.Int32
	268	+	}
	269	+
	270	+	func newStallingUpstream(t testing.T) stallingUpstream {
	271	+	t.Helper()
	272	+	u := &stallingUpstream{}
	273	+	u.srv = upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
	274	+	u.started.Add(1)
	275	+	w.Header().Set("Content-Length", "1000000")
	276	+	_, _ = io.WriteString(w, "partial")
	277	+	w.(http.Flusher).Flush()
	278	+	select {
	279	+	case <-r.Context().Done():
	280	+	u.cancelled.Add(1)
	281	+	case <-time.After(10 * time.Second):
	282	+	}
	283	+	})
	284	+	return u
	285	+	}
	286	+
	287	+	func blobPath(i int) string { return "/v2/library/alpine/blobs/" + sum(fmt.Sprint("blob-", i)) }
	288	+
	289	+	// serveCancelled starts a request and cancels it once its download has begun.
	290	+	func serveCancelled(t testing.T, p Proxy, u *stallingUpstream, path string, started int32) {
	291	+	t.Helper()
	292	+	ctx, cancel := context.WithCancel(context.Background())
	293	+	done := make(chan struct{})
	294	+	go func() {
	295	+	defer close(done)
	296	+	p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
	297	+	}()
	298	+	waitFor(t, p.s, "the download to start", func() bool { return u.started.Load() >= started })
	299	+	cancel()
	300	+	<-done
	301	+	}
	302	+
	303	+	// serveAsync serves path with ctx on its own goroutine, into w (a new
	304	+	// recorder when nil); the status code comes on the returned channel.
	305	+	func serveAsync(p Proxy, ctx context.Context, path string, w keepAliveRecorder) <-chan int {
	306	+	if w == nil {
	307	+	w = &keepAliveRecorder{ResponseRecorder: httptest.NewRecorder()}
	308	+	}
	309	+	code := make(chan int, 1)
	310	+	go func() {
	311	+	p.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
	312	+	code <- w.Code
	313	+	}()
	314	+	return code
	315	+	}
	316	+
	317	+	type keepAliveRecorder struct {
	318	+	*httptest.ResponseRecorder
	319	+	calls atomic.Int32
	320	+	}
	321	+
	322	+	func (k *keepAliveRecorder) KeepAlive() { k.calls.Add(1) }
	323	+
	324	+	// waitKeptAlive waits until w has been kept alive at least once.
	325	+	func waitKeptAlive(t testing.T, w keepAliveRecorder) {
	326	+	t.Helper()
	327	+	for end := time.Now().Add(5 * time.Second); w.calls.Load() == 0; time.Sleep(time.Millisecond) {
	328	+	if time.Now().After(end) {
	329	+	t.Fatal("request never kept alive")
	330	+	}
	331	+	}
	332	+	}
	333	+
	334	+	// blockingWriter holds a response in its first Write until release is closed.
	335	+	type blockingWriter struct {
	336	+	*httptest.ResponseRecorder
	337	+	started, release chan struct{}
	338	+	once sync.Once
	339	+	}
	340	+
	341	+	func (b *blockingWriter) Write(p []byte) (int, error) {
	342	+	b.once.Do(func() { close(b.started) })
	343	+	<-b.release
	344	+	return b.ResponseRecorder.Write(p)
	345	+	}
	346	+
	347	+	// slowBlob streams body in chunks, one every gap.
	348	+	func slowBlob(body string, chunks int, gap time.Duration) http.HandlerFunc {
	349	+	return func(w http.ResponseWriter, r *http.Request) {
	350	+	w.Header().Set("Content-Length", fmt.Sprint(len(body)))
	351	+	size := len(body) / chunks
	352	+	for i := 0; i < len(body); i += size {
	353	+	_, _ = io.WriteString(w, body[i:min(i+size, len(body))])
	354	+	w.(http.Flusher).Flush()
	355	+	select {
	356	+	case <-time.After(gap):
	357	+	case <-r.Context().Done():
	358	+	return
	359	+	}
	360	+	}
	361	+	}
	362	+	}
	363	+
	364	+	type syncBuffer struct {
	365	+	mu sync.Mutex
	366	+	b bytes.Buffer
	367	+	}
	368	+
	369	+	func (s *syncBuffer) Write(p []byte) (int, error) {
	370	+	s.mu.Lock()
	371	+	defer s.mu.Unlock()
	372	+	return s.b.Write(p)
	373	+	}
	374	+
	375	+	func (s *syncBuffer) String() string { s.mu.Lock(); defer s.mu.Unlock(); return s.b.String() }

M internal/registryproxy/hub_test.go +3 -1

@@ -1,15 +1,17 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (
5	6		"bytes"
6	7		"encoding/json"
7		-	"github.com/bnema/zerowrap"
8	8		"net/http/httptest"
9	9		"os"
10	10		"os/exec"
11	11		"strings"
12	12		"testing"
	13	+
	14	+	"github.com/bnema/zerowrap"
13	15		)
14	16
15	17		// An opt-in manual check using curl against a real local HTTP listener, with

D internal/registryproxy/limits_test.go +0 -494

@@ -1,494 +0,0 @@
1	-	// SPDX-License-Identifier: MIT
2	-	package registryproxy
3	-
4	-	import (
5	-	"context"
6	-	"errors"
7	-	"fmt"
8	-	"io"
9	-	"net/http"
10	-	"net/http/httptest"
11	-	"os"
12	-	"path/filepath"
13	-	"strings"
14	-	"sync"
15	-	"sync/atomic"
16	-	"testing"
17	-	"time"
18	-	)
19	-
20	-	// upstreamWith fakes Docker Hub: /token answers an anonymous token and h serves
21	-	// everything else.
22	-	func upstreamWith(t testing.T, h http.HandlerFunc) httptest.Server {
23	-	t.Helper()
24	-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
25	-	if r.URL.Path == "/token" {
26	-	_, _ = io.WriteString(w, `{"token":"public","expires_in":300}`)
27	-	return
28	-	}
29	-	h(w, r)
30	-	}))
31	-	t.Cleanup(srv.Close)
32	-	return srv
33	-	}
34	-
35	-	func proxyWith(t testing.T, srv httptest.Server, c Config) *Proxy {
36	-	t.Helper()
37	-	if c.Dir == "" {
38	-	c.Dir = t.TempDir()
39	-	}
40	-	if c.Max == 0 {
41	-	c.Max = 8 << 20
42	-	}
43	-	c.Upstream, c.AuthURL = srv.URL, srv.URL+"/token"
44	-	p, err := New(c)
45	-	if err != nil {
46	-	t.Fatal(err)
47	-	}
48	-	return p
49	-	}
50	-
51	-	// accounted checks that the store's total is exactly the sum of its entries and
52	-	// within Max, and returns the number of tag files on disk.
53	-	func accounted(t testing.T, s store) int {
54	-	t.Helper()
55	-	s.mu.Lock()
56	-	defer s.mu.Unlock()
57	-	var sum int64
58	-	tags := 0
59	-	for k, e := range s.entries {
60	-	sum += e.size
61	-	if strings.HasPrefix(k, "tags/") {
62	-	tags++
63	-	if e.size != tagCharge {
64	-	t.Errorf("tag %s charged %d", k, e.size)
65	-	}
66	-	}
67	-	}
68	-	if sum != s.total \|\| s.total+s.pending > s.max {
69	-	t.Fatalf("accounting: sum=%d total=%d pending=%d max=%d", sum, s.total, s.pending, s.max)
70	-	}
71	-	files, err := os.ReadDir(filepath.Join(s.dir, "tags"))
72	-	if err != nil {
73	-	t.Fatal(err)
74	-	}
75	-	if len(files) != tags {
76	-	t.Fatalf("%d tag files, %d tag entries", len(files), tags)
77	-	}
78	-	return tags
79	-	}
80	-
81	-	// Accept is reduced to the known manifest types, so its variants map to
82	-	// a bounded set of keys.
83	-	func TestNormalizeAccept(t *testing.T) {
84	-	const index, v2 = "application/vnd.oci.image.index.v1+json", "application/vnd.docker.distribution.manifest.v2+json"
85	-	all := normalizeAccept("")
86	-	for _, tc := range []struct{ in, want string }{
87	-	{index + ";q=0.9, " + strings.ToUpper(v2) + " , " + index, v2 + "," + index},
88	-	{" " + index + " ;q=0.1", index},
89	-	{"text/html, /", all},
90	-	{"", all},
91	-	} {
92	-	if got := normalizeAccept(tc.in); got != tc.want {
93	-	t.Errorf("normalizeAccept(%q) = %q, want %q", tc.in, got, tc.want)
94	-	}
95	-	}
96	-	if strings.Count(all, ",") != len(manifestTypes)-1 {
97	-	t.Fatalf("default key %q", all)
98	-	}
99	-	}
100	-
101	-	// A job varying Accept on one tag creates at most one tag file per
102	-	// subset of the known types, and every tag file is charged.
103	-	func TestTagFilesBoundedByAccept(t *testing.T) {
104	-	f := fake(t)
105	-	p := proxyFor(t, f)
106	-	for i := range 300 {
107	-	accept := fmt.Sprintf("application/vnd.oci.image.index.v1+json;q=0.%d, x/junk-%d", i%10, i)
108	-	switch i % 3 {
109	-	case 1:
110	-	accept = fmt.Sprintf("x/only-junk-%d", i)
111	-	case 2:
112	-	accept += ", application/vnd.docker.distribution.manifest.list.v2+json"
113	-	}
114	-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3", "Accept", accept); w.Code != http.StatusOK {
115	-	t.Fatalf("manifest: %d", w.Code)
116	-	}
117	-	}
118	-	if n := accounted(t, p.s); n == 0 \|\| n > 3 {
119	-	t.Fatalf("%d tag files for 300 Accept variants", n)
120	-	}
121	-	}
122	-
123	-	// Tag files share the LRU of the store, so they cannot grow the store
124	-	// past Max.
125	-	func TestTagsChargedAndEvicted(t *testing.T) {
126	-	now := time.Unix(1_700_000_000, 0)
127	-	const media = "application/vnd.oci.image.index.v1+json"
128	-	dir := t.TempDir()
129	-	max := int64(len(manifestBody)+len(media)) + 3*tagCharge
130	-	s, err := openStore(dir, max, func() time.Time { return now })
131	-	if err != nil {
132	-	t.Fatal(err)
133	-	}
134	-	m, _, _, err := s.ensure(context.Background(), "manifests", sum(manifestBody), func(_ context.Context, w io.Writer) (string, error) {
135	-	_, err := io.WriteString(w, manifestBody)
136	-	return media, err
137	-	}, keepalive{})
138	-	if err != nil {
139	-	t.Fatal(err)
140	-	}
141	-	_ = m.Close()
142	-	for i := range 20 {
143	-	now = now.Add(time.Second)
144	-	if err := s.setTag("library/alpine", fmt.Sprint("t", i), "", sum(manifestBody)); err != nil {
145	-	t.Fatalf("tag %d: %v", i, err)
146	-	}
147	-	if n := accounted(t, s); n > 3 {
148	-	t.Fatalf("%d tags over a cap of 3", n)
149	-	}
150	-	}
151	-	if s.tag("library/alpine", "t19", "") != sum(manifestBody) \|\| s.tag("library/alpine", "t0", "") != "" {
152	-	t.Fatal("tags not evicted least recently used first")
153	-	}
154	-	// Reopening charges the tag files again.
155	-	s, err = openStore(dir, max, time.Now)
156	-	if err != nil {
157	-	t.Fatal(err)
158	-	}
159	-	if n := accounted(t, s); n != 3 {
160	-	t.Fatalf("reopened with %d tags", n)
161	-	}
162	-	}
163	-
164	-	// An entry that is open for serving is not evicted (its bytes would
165	-	// stay allocated while uncharged); it can be evicted once closed.
166	-	func TestPinnedBlobSurvivesEviction(t *testing.T) {
167	-	dir := t.TempDir()
168	-	s, err := openStore(dir, 8, time.Now)
169	-	if err != nil {
170	-	t.Fatal(err)
171	-	}
172	-	put(t, s, "aaaa")
173	-	f, _, err := s.open("blobs", sum("aaaa"))
174	-	if err != nil {
175	-	t.Fatal(err)
176	-	}
177	-	fill := func() error {
178	-	g, _, _, err := s.ensure(context.Background(), "blobs", sum("bbbbbbbb"), func(_ context.Context, w io.Writer) (string, error) {
179	-	_, err := io.WriteString(w, "bbbbbbbb")
180	-	return "", err
181	-	}, keepalive{})
182	-	if err == nil {
183	-	_ = g.Close()
184	-	}
185	-	return err
186	-	}
187	-	if err := fill(); !errors.Is(err, errTooLarge) {
188	-	t.Fatalf("fill over a pinned entry: %v", err)
189	-	}
190	-	if _, err := os.Stat(filepath.Join(dir, "blobs", sum("aaaa")[7:])); err != nil {
191	-	t.Fatalf("pinned blob removed: %v", err)
192	-	}
193	-	accounted(t, s)
194	-	if err := f.Close(); err != nil {
195	-	t.Fatal(err)
196	-	}
197	-	if err := fill(); err != nil {
198	-	t.Fatal(err)
199	-	}
200	-	if _, err := os.Stat(filepath.Join(dir, "blobs", sum("aaaa")[7:])); !os.IsNotExist(err) {
201	-	t.Fatal("unpinned blob not evicted")
202	-	}
203	-	accounted(t, s)
204	-	}
205	-
206	-	// blockingWriter holds a response in its first Write until release is closed.
207	-	type blockingWriter struct {
208	-	*httptest.ResponseRecorder
209	-	started, release chan struct{}
210	-	once sync.Once
211	-	}
212	-
213	-	func (b *blockingWriter) Write(p []byte) (int, error) {
214	-	b.once.Do(func() { close(b.started) })
215	-	<-b.release
216	-	return b.ResponseRecorder.Write(p)
217	-	}
218	-
219	-	// A blob that a response is still sending is pinned: a fill that needs
220	-	// its room fails, and succeeds once the response has closed the file.
221	-	func TestEvictionWhileServing(t *testing.T) {
222	-	a, b := strings.Repeat("a", 1024), strings.Repeat("b", 1024)
223	-	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
224	-	switch {
225	-	case strings.HasSuffix(r.URL.Path, sum(a)):
226	-	_, _ = io.WriteString(w, a)
227	-	case strings.HasSuffix(r.URL.Path, sum(b)):
228	-	_, _ = io.WriteString(w, b)
229	-	default:
230	-	http.NotFound(w, r)
231	-	}
232	-	})
233	-	p := proxyWith(t, srv, Config{Max: 1500})
234	-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(a)); w.Code != http.StatusOK {
235	-	t.Fatal(w.Code)
236	-	}
237	-	bw := &blockingWriter{ResponseRecorder: httptest.NewRecorder(), started: make(chan struct{}), release: make(chan struct{})}
238	-	done := make(chan struct{})
239	-	go func() {
240	-	defer close(done)
241	-	p.ServeHTTP(bw, httptest.NewRequest(http.MethodGet, "/v2/library/alpine/blobs/"+sum(a), nil))
242	-	}()
243	-	<-bw.started
244	-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(b)); w.Code != http.StatusBadGateway {
245	-	t.Fatalf("fill evicted a blob being served: %d", w.Code)
246	-	}
247	-	if _, err := os.Stat(filepath.Join(p.s.dir, "blobs", sum(a)[7:])); err != nil {
248	-	t.Fatalf("served blob removed: %v", err)
249	-	}
250	-	accounted(t, p.s)
251	-	close(bw.release)
252	-	<-done
253	-	if bw.Code != http.StatusOK \|\| bw.Body.String() != a {
254	-	t.Fatalf("served: %d", bw.Code)
255	-	}
256	-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(b)); w.Code != http.StatusOK \|\| w.Body.String() != b {
257	-	t.Fatalf("after release: %d", w.Code)
258	-	}
259	-	if _, err := os.Stat(filepath.Join(p.s.dir, "blobs", sum(a)[7:])); !os.IsNotExist(err) {
260	-	t.Fatal("released blob not evicted")
261	-	}
262	-	accounted(t, p.s)
263	-	}
264	-
265	-	// slowBlob streams body in chunks, one every gap.
266	-	func slowBlob(body string, chunks int, gap time.Duration) http.HandlerFunc {
267	-	return func(w http.ResponseWriter, r *http.Request) {
268	-	w.Header().Set("Content-Length", fmt.Sprint(len(body)))
269	-	size := len(body) / chunks
270	-	for i := 0; i < len(body); i += size {
271	-	_, _ = io.WriteString(w, body[i:min(i+size, len(body))])
272	-	w.(http.Flusher).Flush()
273	-	select {
274	-	case <-time.After(gap):
275	-	case <-r.Context().Done():
276	-	return
277	-	}
278	-	}
279	-	}
280	-	}
281	-
282	-	// A blob body has no total deadline: a transfer that keeps moving may
283	-	// last far longer than RequestTimeout and BlobIdleTimeout.
284	-	func TestBlobTransferOutlivesRequestTimeout(t *testing.T) {
285	-	body := strings.Repeat("layer", 4000)
286	-	srv := upstreamWith(t, slowBlob(body, 20, 25*time.Millisecond))
287	-	p := proxyWith(t, srv, Config{RequestTimeout: 100 * time.Millisecond, BlobIdleTimeout: 150 * time.Millisecond})
288	-	start := time.Now()
289	-	w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(body))
290	-	if w.Code != http.StatusOK \|\| w.Body.String() != body {
291	-	t.Fatalf("blob: %d", w.Code)
292	-	}
293	-	if d := time.Since(start); d < 300*time.Millisecond {
294	-	t.Fatalf("transfer took %v, not longer than the timeouts", d)
295	-	}
296	-	}
297	-
298	-	// A blob body that stops moving is cut after BlobIdleTimeout, and auth,
299	-	// manifests and HEAD keep their total RequestTimeout.
300	-	func TestStalledUpstreamTimesOut(t *testing.T) {
301	-	body := strings.Repeat("layer", 4000)
302	-	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
303	-	if strings.Contains(r.URL.Path, "/blobs/") && r.Method == http.MethodGet {
304	-	w.Header().Set("Content-Length", fmt.Sprint(len(body)))
305	-	_, _ = io.WriteString(w, body[:100])
306	-	w.(http.Flusher).Flush()
307	-	}
308	-	select { // a stalled body, or headers that never come
309	-	case <-r.Context().Done():
310	-	case <-time.After(5 * time.Second):
311	-	}
312	-	})
313	-	p := proxyWith(t, srv, Config{RequestTimeout: 100 * time.Millisecond, BlobIdleTimeout: 150 * time.Millisecond})
314	-	for _, tc := range []struct{ method, path string }{
315	-	{http.MethodGet, "/v2/library/alpine/blobs/" + sum(body)},
316	-	{http.MethodHead, "/v2/library/alpine/blobs/" + sum(body)},
317	-	{http.MethodGet, "/v2/library/alpine/manifests/3"},
318	-	} {
319	-	start := time.Now()
320	-	if w := request(t, p, tc.method, tc.path); w.Code != http.StatusBadGateway {
321	-	t.Errorf("%s %s: %d", tc.method, tc.path, w.Code)
322	-	}
323	-	if d := time.Since(start); d > 2*time.Second {
324	-	t.Errorf("%s %s: cut after %v", tc.method, tc.path, d)
325	-	}
326	-	}
327	-	if _, err := os.Stat(filepath.Join(p.s.dir, "blobs", sum(body)[7:])); !os.IsNotExist(err) {
328	-	t.Fatal("partial blob stored")
329	-	}
330	-	}
331	-
332	-	// waitFor polls cond (under the store lock) until it holds.
333	-	func waitFor(t testing.T, s store, what string, cond func() bool) {
334	-	t.Helper()
335	-	for end := time.Now().Add(5 * time.Second); ; {
336	-	s.mu.Lock()
337	-	ok := cond()
338	-	s.mu.Unlock()
339	-	if ok {
340	-	return
341	-	}
342	-	if time.Now().After(end) {
343	-	t.Fatalf("timed out waiting for %s", what)
344	-	}
345	-	time.Sleep(time.Millisecond)
346	-	}
347	-	}
348	-
349	-	// waiters is the number of requests waiting for the flight of key (-1: none).
350	-	func waiters(s *store, key string) int {
351	-	if fl, ok := s.flights[key]; ok {
352	-	return fl.waiters
353	-	}
354	-	return -1
355	-	}
356	-
357	-	// The shared download does not belong to the client that started it.
358	-	// That client disconnects mid-download; a second client, joining within the
359	-	// grace period, still gets the blob and the blob is committed, from one
360	-	// upstream fetch.
361	-	func TestBlobFlightSurvivesInitiatorDisconnect(t *testing.T) {
362	-	body := strings.Repeat("layer", 4000)
363	-	half, release := make(chan struct{}), make(chan struct{})
364	-	var releaseOnce sync.Once
365	-	unblock := func() { releaseOnce.Do(func() { close(release) }) }
366	-	var gets atomic.Int32
367	-	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
368	-	gets.Add(1)
369	-	w.Header().Set("Content-Length", fmt.Sprint(len(body)))
370	-	_, _ = io.WriteString(w, body[:len(body)/2])
371	-	w.(http.Flusher).Flush()
372	-	close(half)
373	-	<-release
374	-	_, _ = io.WriteString(w, body[len(body)/2:])
375	-	})
376	-	t.Cleanup(unblock) // before srv.Close, which waits for the handler
377	-	p := proxyWith(t, srv, Config{})
378	-	path := "/v2/library/alpine/blobs/" + sum(body)
379	-
380	-	ctx, cancel := context.WithCancel(context.Background())
381	-	first := httptest.NewRecorder()
382	-	done := make(chan struct{})
383	-	go func() {
384	-	defer close(done)
385	-	p.ServeHTTP(first, httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
386	-	}()
387	-	<-half
388	-	cancel()
389	-	<-done
390	-	if strings.Contains(first.Body.String(), "layer") {
391	-	t.Fatal("unverified bytes sent to the initiator")
392	-	}
393	-	key := "blobs/" + sum(body)[7:]
394	-	p.s.mu.Lock()
395	-	left := waiters(p.s, key)
396	-	p.s.mu.Unlock()
397	-	if left != 0 {
398	-	t.Fatalf("waiters after the disconnect: %d", left)
399	-	}
400	-
401	-	second := httptest.NewRecorder()
402	-	done = make(chan struct{})
403	-	go func() {
404	-	defer close(done)
405	-	p.ServeHTTP(second, httptest.NewRequest(http.MethodGet, path, nil))
406	-	}()
407	-	waitFor(t, p.s, "the second client to join", func() bool { return waiters(p.s, key) == 1 })
408	-	unblock()
409	-	<-done
410	-	if second.Code != http.StatusOK \|\| second.Body.String() != body {
411	-	t.Fatalf("second client: %d", second.Code)
412	-	}
413	-	if _, err := os.Stat(filepath.Join(p.s.dir, "blobs", sum(body)[7:])); err != nil {
414	-	t.Fatalf("blob not committed: %v", err)
415	-	}
416	-	if gets.Load() != 1 {
417	-	t.Fatalf("%d upstream fetches", gets.Load())
418	-	}
419	-	}
420	-
421	-	type keepAliveRecorder struct {
422	-	*httptest.ResponseRecorder
423	-	calls atomic.Int32
424	-	}
425	-
426	-	func (k *keepAliveRecorder) KeepAlive() { k.calls.Add(1) }
427	-
428	-	// A client waiting for a progressing download is kept alive, so
429	-	// the cache channel's progress deadline follows the download.
430	-	func TestWaiterKeepAlive(t *testing.T) {
431	-	body := strings.Repeat("layer", 4000)
432	-	srv := upstreamWith(t, slowBlob(body, 10, 30*time.Millisecond))
433	-	p := proxyWith(t, srv, Config{KeepAliveInterval: 10 * time.Millisecond})
434	-	w := &keepAliveRecorder{ResponseRecorder: httptest.NewRecorder()}
435	-	p.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/v2/library/alpine/blobs/"+sum(body), nil))
436	-	if w.Code != http.StatusOK \|\| w.calls.Load() == 0 {
437	-	t.Fatalf("code %d, %d keepalives", w.Code, w.calls.Load())
438	-	}
439	-	}
440	-
441	-	// Auth and manifest requests never follow a redirect.
442	-	func TestMetadataRedirectsNotFollowed(t *testing.T) {
443	-	var elsewhere atomic.Int32
444	-	other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
445	-	elsewhere.Add(1)
446	-	_, _ = io.WriteString(w, `{"token":"public"}`)
447	-	}))
448	-	t.Cleanup(other.Close)
449	-	for _, redirectAuth := range []bool{true, false} {
450	-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
451	-	if r.URL.Path == "/token" && !redirectAuth {
452	-	_, _ = io.WriteString(w, `{"token":"public"}`)
453	-	return
454	-	}
455	-	http.Redirect(w, r, other.URL+r.URL.Path, http.StatusTemporaryRedirect)
456	-	}))
457	-	p := proxyWith(t, srv, Config{})
458	-	for _, ref := range []string{"3", sum(manifestBody)} {
459	-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/"+ref); w.Code != http.StatusBadGateway {
460	-	t.Errorf("auth redirect %v, ref %s: %d", redirectAuth, ref, w.Code)
461	-	}
462	-	}
463	-	srv.Close()
464	-	}
465	-	if elsewhere.Load() != 0 {
466	-	t.Fatalf("%d redirects followed", elsewhere.Load())
467	-	}
468	-	}
469	-
470	-	// Blob requests follow the CDN redirect, without the bearer token.
471	-	func TestBlobRedirectStripsBearer(t *testing.T) {
472	-	var cdnHits atomic.Int32
473	-	cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
474	-	cdnHits.Add(1)
475	-	if r.Header.Get("Authorization") != "" {
476	-	t.Error("bearer sent to the CDN")
477	-	}
478	-	_, _ = io.WriteString(w, blobBody)
479	-	}))
480	-	t.Cleanup(cdn.Close)
481	-	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
482	-	if r.Header.Get("Authorization") != "Bearer public" {
483	-	t.Error("no bearer on the registry")
484	-	}
485	-	http.Redirect(w, r, cdn.URL+"/layer", http.StatusTemporaryRedirect)
486	-	})
487	-	p := proxyWith(t, srv, Config{})
488	-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(blobBody)); w.Code != http.StatusOK \|\| w.Body.String() != blobBody {
489	-	t.Fatalf("blob: %d", w.Code)
490	-	}
491	-	if cdnHits.Load() != 1 {
492	-	t.Fatalf("CDN hits %d", cdnHits.Load())
493	-	}
494	-	}

D internal/registryproxy/main_test.go +0 -25

@@ -1,25 +0,0 @@
1	-	// SPDX-License-Identifier: MIT
2	-	package registryproxy
3	-
4	-	import (
5	-	"fmt"
6	-	"os"
7	-	"sync/atomic"
8	-	"testing"
9	-	)
10	-
11	-	// TestMain fails the run if any store hit an accounting bug (unpinLocked of
12	-	// an unpinned entry), whichever test caused it.
13	-	func TestMain(m *testing.M) {
14	-	var bugs atomic.Int32
15	-	defaultBugHook = func(msg string) {
16	-	bugs.Add(1)
17	-	fmt.Fprintln(os.Stderr, "registry cache bug:", msg)
18	-	}
19	-	code := m.Run()
20	-	if n := bugs.Load(); n > 0 && code == 0 {
21	-	fmt.Fprintf(os.Stderr, "FAIL: %d registry cache accounting bugs\n", n)
22	-	code = 1
23	-	}
24	-	os.Exit(code)
25	-	}

M internal/registryproxy/proxy.go +63 -44


@@ -28,8 +28,6 @@ type Config struct {
28	28		Max int64
29	29		Upstream string // default https://registry-1.docker.io
30	30		AuthURL string // default https://auth.docker.io/token
31		-	Client *http.Client
32		-	Now func() time.Time
33	31		Log zerowrap.Logger
34	32		// RequestTimeout bounds auth, manifests and HEAD (default 10s).
35	33		RequestTimeout time.Duration

@@ -38,7 +36,7 @@ type Config struct {
38	36		BlobIdleTimeout time.Duration
39	37		// KeepAliveInterval is how often a client waiting for a cold blob is kept
40	38		// alive while the download progresses (default 1s), when its
41		-	// ResponseWriter has a KeepAlive method.
	39	+	// ResponseWriter is a KeepAliver.
42	40		KeepAliveInterval time.Duration
43	41		// AbandonGrace is how long a download nobody waits for any more keeps
44	42		// running in case another request joins it; then it is cancelled and its

@@ -52,6 +50,15 @@ type Config struct {
52	50		SlotWait time.Duration
53	51		}
54	52
	53	+	// KeepAliver is a ResponseWriter that can move its connection's deadlines
	54	+	// without writing. While a request waits for a cold blob, the Proxy calls
	55	+	// KeepAlive every KeepAliveInterval as long as the download progresses, so
	56	+	// that a slow but moving download does not time the client out before the
	57	+	// first byte of the reply.
	58	+	type KeepAliver interface {
	59	+	KeepAlive()
	60	+	}
	61	+
55	62		// Proxy implements http.Handler for GET and HEAD under /v2/.
56	63		type Proxy struct {
57	64		s *store

@@ -62,9 +69,6 @@ type Proxy struct {
62	69
63	70		// New opens the persistent cache. It starts no goroutines or listeners.
64	71		func New(c Config) (*Proxy, error) {
65		-	if c.Now == nil {
66		-	c.Now = time.Now
67		-	}
68	72		if c.Upstream == "" {
69	73		c.Upstream = "https://registry-1.docker.io"
70	74		}

@@ -95,14 +99,14 @@ func New(c Config) (*Proxy, error) {
95	99		if c.SlotWait <= 0 {
96	100		c.SlotWait = 30 * time.Second
97	101		}
98		-	client, blobs := upstreamClients(c)
99		-
100		-	st, err := openStore(c.Dir, c.Max, c.Now)
	102	+	st, err := openStore(storeConfig{dir: c.Dir, max: c.Max, now: time.Now, grace: c.AbandonGrace,
	103	+	maxDownloads: c.MaxDownloads, slotWait: c.SlotWait, log: c.Log})
101	104		if err != nil {
102	105		return nil, err
103	106		}
104		-	st.grace, st.maxDownloads, st.slotWait, st.log = c.AbandonGrace, c.MaxDownloads, c.SlotWait, c.Log
105		-	return &Proxy{s: st, u: &upstream{base: strings.TrimRight(c.Upstream, "/"), auth: c.AuthURL, client: client, blobClient: blobs, requestTimeout: c.RequestTimeout, blobIdle: c.BlobIdleTimeout, now: c.Now, tokens: map[string]bearer{}}, log: c.Log, keepEvery: c.KeepAliveInterval}, nil
	107	+	client, blobs := upstreamClients(c.RequestTimeout)
	108	+	return &Proxy{s: st, u: &upstream{base: strings.TrimRight(c.Upstream, "/"), auth: c.AuthURL, client: client, blobClient: blobs,
	109	+	requestTimeout: c.RequestTimeout, blobIdle: c.BlobIdleTimeout, tokens: map[string]bearer{}}, log: c.Log, keepEvery: c.KeepAliveInterval}, nil
106	110		}
107	111
108	112		var namePattern = regexp.MustCompile(`^[a-z0-9]+(?:[._-][a-z0-9]+)(?:/[a-z0-9]+(?:[._-][a-z0-9]+))*$`)

@@ -147,23 +151,32 @@ func (p Proxy) ServeHTTP(w http.ResponseWriter, r http.Request) {
147	151		http.NotFound(w, r)
148	152		return
149	153		}
150		-	name, kind := path[:i], path[i+1:]
	154	+	name, route := path[:i], path[i+1:]
151	155		// dockerd's legacy client sends one Accept line per media type.
152	156		accept := strings.Join(r.Header.Values("Accept"), ",")
153	157		if len(name) > 255 \|\| !namePattern.MatchString(name) \|\| len(accept) > 8192 {
154	158		http.NotFound(w, r)
155	159		return
156	160		}
157		-	if kind == "manifests" && (validDigest(ref) \|\| tagPattern.MatchString(ref)) {
	161	+	if route == kindManifest.dir() && (validDigest(ref) \|\| tagPattern.MatchString(ref)) {
158	162		p.manifest(w, r, name, ref, normalizeAccept(accept))
159	163		return
160	164		}
161		-	if kind == "blobs" && validDigest(ref) {
	165	+	if route == kindBlob.dir() && validDigest(ref) {
162	166		p.blob(w, r, name, ref)
163	167		return
164	168		}
165	169		http.NotFound(w, r)
166	170		}
	171	+
	172	+	// logFor is the Proxy's logger, with the job_id of the request (WithJob).
	173	+	func (p *Proxy) logFor(ctx context.Context) zerowrap.Logger {
	174	+	if job := jobFrom(ctx); job != "" {
	175	+	return zerowrap.Logger{Logger: p.log.With().Str("job_id", job).Logger()}
	176	+	}
	177	+	return p.log
	178	+	}
	179	+
167	180		func serveFile(w http.ResponseWriter, r http.Request, f storedFile, media, digest string) {
168	181		defer func() { _ = f.Close() }()
169	182		fi, err := f.Stat()

@@ -176,29 +189,39 @@ func serveFile(w http.ResponseWriter, r http.Request, f storedFile, media, dig
176	189		http.ServeContent(w, r, "", fi.ModTime(), f)
177	190		}
178	191
	192	+	// serveStaleTag serves the manifest a tag pointed to when upstream could not
	193	+	// answer, if that manifest is still stored. It reports whether it served one.
	194	+	func (p Proxy) serveStaleTag(w http.ResponseWriter, r http.Request, name, tag, accept string) bool {
	195	+	digest := p.s.tag(name, tag, accept)
	196	+	if !validDigest(digest) {
	197	+	return false
	198	+	}
	199	+	f, media, err := p.s.open(kindManifest, digest)
	200	+	if err != nil {
	201	+	return false
	202	+	}
	203	+	serveFile(w, r, f, media, digest)
	204	+	return true
	205	+	}
	206	+
179	207		// manifest serves a manifest by tag or digest. accept is normalized.
180	208		func (p Proxy) manifest(w http.ResponseWriter, r http.Request, name, ref, accept string) {
181	209		start := time.Now()
182	210		hit := false
	211	+	log := p.logFor(r.Context())
183	212		defer func() {
184		-	p.log.Info().Str("repo", name).Str("ref", ref).Bool("hit", hit).Int64("ms", time.Since(start).Milliseconds()).Msg("registry manifest")
	213	+	log.Info().Str("repo", name).Str("ref", ref).Bool("hit", hit).Int64("ms", time.Since(start).Milliseconds()).Msg("registry manifest")
185	214		}()
186	215		ctx, cancel := context.WithTimeout(r.Context(), p.u.requestTimeout)
187	216		defer cancel()
188	217		digest := ref
189	218		tag := !validDigest(ref)
190	219		if tag {
191		-	resp, err := p.u.request(ctx, http.MethodHead, name, "manifests", ref, accept)
	220	+	resp, err := p.u.request(ctx, http.MethodHead, name, kindManifest, ref, accept)
192	221		if err != nil {
193		-	if unavailable(err) {
194		-	digest = p.s.tag(name, ref, accept)
195		-	if validDigest(digest) {
196		-	if f, media, e := p.s.open("manifests", digest); e == nil {
197		-	hit = true
198		-	serveFile(w, r, f, media, digest)
199		-	return
200		-	}
201		-	}
	222	+	if unavailable(err) && p.serveStaleTag(w, r, name, ref, accept) {
	223	+	hit = true
	224	+	return
202	225		}
203	226		http.Error(w, "registry upstream unavailable", proxyStatus(err))
204	227		return

@@ -211,10 +234,10 @@ func (p Proxy) manifest(w http.ResponseWriter, r http.Request, name, ref, acce
211	234		}
212	235		}
213	236		// Shared flight, as for blobs: it must not end with the initiator's request.
214		-	f, media, cached, err := p.s.ensure(ctx, "manifests", digest, func(flightCtx context.Context, dst io.Writer) (string, error) {
	237	+	f, media, cached, err := p.s.ensure(ctx, kindManifest, digest, func(flightCtx context.Context, dst io.Writer) (string, error) {
215	238		fillCtx, fillCancel := context.WithTimeout(flightCtx, p.u.requestTimeout)
216	239		defer fillCancel()
217		-	resp, err := p.u.request(fillCtx, http.MethodGet, name, "manifests", digest, accept)
	240	+	resp, err := p.u.request(fillCtx, http.MethodGet, name, kindManifest, digest, accept)
218	241		if err != nil {
219	242		return "", err
220	243		}

@@ -231,15 +254,9 @@ func (p Proxy) manifest(w http.ResponseWriter, r http.Request, name, ref, acce
231	254		}, keepalive{})
232	255		hit = cached
233	256		if err != nil {
234		-	if tag && unavailable(err) {
235		-	previous := p.s.tag(name, ref, accept)
236		-	if validDigest(previous) {
237		-	if old, oldMedia, e := p.s.open("manifests", previous); e == nil {
238		-	hit = true
239		-	serveFile(w, r, old, oldMedia, previous)
240		-	return
241		-	}
242		-	}
	257	+	if tag && unavailable(err) && p.serveStaleTag(w, r, name, ref, accept) {
	258	+	hit = true
	259	+	return
243	260		}
244	261		http.Error(w, "registry fetch failed", proxyStatus(err))
245	262		return

@@ -249,8 +266,9 @@ func (p Proxy) manifest(w http.ResponseWriter, r http.Request, name, ref, acce
249	266		}
250	267		serveFile(w, r, f, media, digest)
251	268		}
	269	+
252	270		func (p Proxy) blob(w http.ResponseWriter, r http.Request, name, digest string) {
253		-	if f, media, err := p.s.open("blobs", digest); err == nil {
	271	+	if f, media, err := p.s.open(kindBlob, digest); err == nil {
254	272		serveFile(w, r, f, media, digest)
255	273		return
256	274		}

@@ -258,7 +276,7 @@ func (p Proxy) blob(w http.ResponseWriter, r http.Request, name, digest string
258	276		if r.Method == http.MethodHead {
259	277		ctx, cancel := context.WithTimeout(r.Context(), p.u.requestTimeout)
260	278		defer cancel()
261		-	resp, err := p.u.request(ctx, http.MethodHead, name, "blobs", digest, "")
	279	+	resp, err := p.u.request(ctx, http.MethodHead, name, kindBlob, digest, "")
262	280		if err != nil {
263	281		http.Error(w, "registry fetch failed", proxyStatus(err))
264	282		return

@@ -282,18 +300,19 @@ func (p Proxy) blob(w http.ResponseWriter, r http.Request, name, digest string
282	300		// when no client has waited for it for AbandonGrace. At most MaxDownloads
283	301		// run at once, and at most its Quota per client: a request over either
284	302		// waits up to SlotWait for a slot, then gets 503, which dockerd retries
285		-	// (about 50 s) before pulling from Docker Hub. While it waits, a writer
286		-	// with KeepAlive (the cache channel's progress deadline) is kept alive as
287		-	// long as the download moves.
	303	+	// (about 50 s) before pulling from Docker Hub. While it waits, a
	304	+	// KeepAliver writer (the cache channel's progress deadline) is kept alive
	305	+	// as long as the download moves.
288	306		keep := keepalive{every: p.keepEvery}
289		-	if k, ok := w.(interface{ KeepAlive() }); ok {
	307	+	if k, ok := w.(KeepAliver); ok {
290	308		keep.fn = k.KeepAlive
291	309		}
292		-	f, media, _, err := p.s.ensure(r.Context(), "blobs", digest, func(downloadCtx context.Context, dst io.Writer) (string, error) {
	310	+	log := p.logFor(r.Context())
	311	+	f, media, _, err := p.s.ensure(r.Context(), kindBlob, digest, func(downloadCtx context.Context, dst io.Writer) (string, error) {
293	312		start := time.Now()
294	313		var size int64
295	314		defer func() {
296		-	p.log.Debug().Str("digest", digest).Int64("bytes", size).Int64("ms", time.Since(start).Milliseconds()).Msg("registry blob")
	315	+	log.Debug().Str("digest", digest).Int64("bytes", size).Int64("ms", time.Since(start).Milliseconds()).Msg("registry blob")
297	316		}()
298	317		resp, err := p.u.blob(downloadCtx, name, digest)
299	318		if err != nil {

M internal/registryproxy/proxy_test.go +21 -118

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (

@@ -9,100 +10,12 @@ import (
9	10		"os"
10	11		"path/filepath"
11	12		"strings"
12		-	"sync/atomic"
13	13		"testing"
14	14		)
15	15
16		-	const manifestBody = `{"schemaVersion":2}`
17		-	const blobBody = "a layer of bytes"
18		-
19		-	type fakeRegistry struct {
20		-	server *httptest.Server
21		-	heads, gets, blobs, tokens atomic.Int32
22		-	status atomic.Int32
23		-	corrupt atomic.Bool
24		-	}
25		-
26		-	func fake(t testing.T) fakeRegistry {
27		-	t.Helper()
28		-	f := &fakeRegistry{}
29		-	cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
30		-	if r.Header.Get("Authorization") != "" {
31		-	t.Error("bearer leaked to CDN")
32		-	}
33		-	w.Header().Set("Content-Length", fmt.Sprint(len(blobBody)))
34		-	if r.Method != http.MethodHead {
35		-	b := blobBody
36		-	if f.corrupt.Load() {
37		-	b = strings.Repeat("x", len(b))
38		-	}
39		-	_, _ = io.WriteString(w, b)
40		-	}
41		-	}))
42		-	t.Cleanup(cdn.Close)
43		-	f.server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
44		-	if r.URL.Path == "/token" {
45		-	f.tokens.Add(1)
46		-	if r.Header.Get("Authorization") != "" {
47		-	t.Error("guest credentials forwarded to auth")
48		-	}
49		-	if r.URL.Query().Get("scope") != "repository:library/alpine:pull" {
50		-	t.Error("wrong scope")
51		-	}
52		-	_, _ = io.WriteString(w, `{"token":"public","expires_in":300}`)
53		-	return
54		-	}
55		-	if r.Header.Get("Authorization") != "Bearer public" {
56		-	t.Errorf("wrong bearer %q", r.Header.Get("Authorization"))
57		-	}
58		-	if s := f.status.Load(); s != 0 {
59		-	w.WriteHeader(int(s))
60		-	return
61		-	}
62		-	if strings.Contains(r.URL.Path, "/manifests/") {
63		-	w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json")
64		-	w.Header().Set("Docker-Content-Digest", sum(manifestBody))
65		-	w.Header().Set("Content-Length", fmt.Sprint(len(manifestBody)))
66		-	if r.Method == http.MethodHead {
67		-	f.heads.Add(1)
68		-	} else {
69		-	f.gets.Add(1)
70		-	_, _ = io.WriteString(w, manifestBody)
71		-	}
72		-	return
73		-	}
74		-	if strings.Contains(r.URL.Path, "/blobs/") {
75		-	if r.Method == http.MethodGet {
76		-	f.blobs.Add(1)
77		-	}
78		-	http.Redirect(w, r, cdn.URL+"/layer", http.StatusTemporaryRedirect)
79		-	return
80		-	}
81		-	http.NotFound(w, r)
82		-	}))
83		-	t.Cleanup(f.server.Close)
84		-	return f
85		-	}
86		-	func proxyFor(t testing.T, f fakeRegistry) *Proxy {
87		-	t.Helper()
88		-	p, err := New(Config{Dir: t.TempDir(), Max: 8 << 20, Upstream: f.server.URL, AuthURL: f.server.URL + "/token"})
89		-	if err != nil {
90		-	t.Fatal(err)
91		-	}
92		-	return p
93		-	}
94		-	func request(t testing.T, p http.Handler, method, path string, headers ...string) httptest.ResponseRecorder {
95		-	t.Helper()
96		-	r := httptest.NewRequest(method, path, nil)
97		-	r.Header.Set("Authorization", "Bearer guest-secret")
98		-	for i := 0; i < len(headers); i += 2 {
99		-	r.Header.Set(headers[i], headers[i+1])
100		-	}
101		-	w := httptest.NewRecorder()
102		-	p.ServeHTTP(w, r)
103		-	return w
104		-	}
105		-	func TestPullAndOfflineTag(t *testing.T) {
	16	+	// A pulled manifest and blob are served from the store the second time: a tag
	17	+	// is revalidated by HEAD, a digest is not fetched again.
	18	+	func TestPullCached(t *testing.T) {
106	19		f := fake(t)
107	20		p := proxyFor(t, f)
108	21		for range 2 {

@@ -118,19 +31,10 @@ func TestPullAndOfflineTag(t *testing.T) {
118	31		if f.heads.Load() != 2 \|\| f.gets.Load() != 1 \|\| f.blobs.Load() != 1 \|\| f.tokens.Load() != 1 {
119	32		t.Fatalf("counts HEAD=%d GET=%d blobs=%d tokens=%d", f.heads.Load(), f.gets.Load(), f.blobs.Load(), f.tokens.Load())
120	33		}
121		-	for _, down := range []int32{http.StatusServiceUnavailable, 0} {
122		-	f.status.Store(down)
123		-	if down == 0 {
124		-	f.server.Close()
125		-	}
126		-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3"); w.Code != http.StatusOK \|\| w.Body.String() != manifestBody {
127		-	t.Fatalf("stale: %d %s", w.Code, w.Body)
128		-	}
129		-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/unknown"); w.Code != http.StatusBadGateway {
130		-	t.Fatalf("unknown: %d", w.Code)
131		-	}
132		-	}
133	34		}
	35	+
	36	+	// HEAD of a blob answers its length, cold or stored, with no body; a stored
	37	+	// blob serves ranges.
134	38		func TestBlobHeadAndRange(t *testing.T) {
135	39		f := fake(t)
136	40		p := proxyFor(t, f)

@@ -162,25 +66,18 @@ func TestCorruptBlobRejected(t *testing.T) {
162	66		if w.Code != http.StatusBadGateway \|\| strings.Contains(w.Body.String(), "xxxx") {
163	67		t.Fatalf("corrupt blob: %d %q", w.Code, w.Body)
164	68		}
165		-	if _, err := os.Stat(filepath.Join(p.s.dir, "blobs", sum(blobBody)[7:])); !os.IsNotExist(err) {
	69	+	if _, err := os.Stat(filepath.Join(p.s.dir, kindBlob.key(sum(blobBody)[7:]))); !os.IsNotExist(err) {
166	70		t.Fatal("corrupt blob stored")
167	71		}
168		-	ds, _ := os.ReadDir(filepath.Join(p.s.dir, "tmp"))
169		-	if len(ds) != 0 {
	72	+	if ds, _ := os.ReadDir(filepath.Join(p.s.dir, "tmp")); len(ds) != 0 {
170	73		t.Fatal("partial temp retained")
171	74		}
172	75		}
173		-	func TestDeniedAndValidation(t *testing.T) {
174		-	f := fake(t)
175		-	p := proxyFor(t, f)
176		-	f.status.Store(http.StatusUnauthorized)
177		-	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3"); w.Code != http.StatusNotFound {
178		-	t.Fatal(w.Code)
179		-	}
180		-	if f.tokens.Load() != 2 {
181		-	t.Fatal("no fresh token retry")
182		-	}
183		-	for _, path := range []string{"/v2/../library/alpine/manifests/3", "/v2/Library/alpine/manifests/3", "/v2/library/alpine/manifests/.bad", "/v2/library/alpine/manifests/sha256:bad", "/v2/library/alpine/blobs/sha256:bad", "/v2/library/alpine/tags/list"} {
	76	+
	77	+	// Only GET and HEAD of valid names, tags and digests under /v2/ are served.
	78	+	func TestRouteValidation(t *testing.T) {
	79	+	p := proxyFor(t, fake(t))
	80	+	for _, path := range []string{"/v2/../library/alpine/manifests/3", "/v2/Library/alpine/manifests/3", "/v2/library/alpine/manifests/.bad", "/v2/library/alpine/manifests/sha256:bad", "/v2/library/alpine/blobs/sha256:bad", "/v2/library/alpine/tags/list", "/v3/library/alpine/manifests/3"} {
184	81		if w := request(t, p, http.MethodGet, path); w.Code != http.StatusNotFound {
185	82		t.Errorf("%s: %d", path, w.Code)
186	83		}

@@ -188,7 +85,13 @@ func TestDeniedAndValidation(t *testing.T) {
188	85		if w := request(t, p, http.MethodPut, "/v2/"); w.Code != http.StatusMethodNotAllowed {
189	86		t.Fatal(w.Code)
190	87		}
	88	+	if w := request(t, p, http.MethodGet, "/v2/"); w.Code != http.StatusOK \|\| w.Body.String() != "{}" {
	89	+	t.Fatalf("/v2/: %d %s", w.Code, w.Body)
	90	+	}
191	91		}
	92	+
	93	+	// A manifest whose bytes do not match its digest, or over the size limit, is
	94	+	// a 502 and is not stored.
192	95		func TestManifestLimitsAndDigest(t *testing.T) {
193	96		for _, body := range []string{"tampered", strings.Repeat("x", manifestLimit+1)} {
194	97		up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

@@ -207,7 +110,7 @@ func TestManifestLimitsAndDigest(t *testing.T) {
207	110		if w.Code != http.StatusBadGateway {
208	111		t.Fatal(w.Code)
209	112		}
210		-	if _, _, err := p.s.open("manifests", sum(manifestBody)); err == nil {
	113	+	if _, _, err := p.s.open(kindManifest, sum(manifestBody)); err == nil {
211	114		t.Fatal("invalid manifest stored")
212	115		}
213	116		}

M internal/registryproxy/quota.go +14 -3

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (

@@ -27,14 +28,24 @@ func WithQuota(ctx context.Context, q *Quota) context.Context {
27	28
28	29		type quotaKey struct{}
29	30
30		-	// QuotaOf returns the quota attached by WithQuota, or nil.
31		-	func QuotaOf(ctx context.Context) *Quota { return quotaFrom(ctx) }
32		-
33	31		func quotaFrom(ctx context.Context) *Quota {
34	32		q, _ := ctx.Value(quotaKey{}).(*Quota)
35	33		return q
36	34		}
37	35
	36	+	// WithJob attaches the id of the job a request serves to its context: the
	37	+	// Proxy logs it as job_id on its registry lines.
	38	+	func WithJob(ctx context.Context, job string) context.Context {
	39	+	return context.WithValue(ctx, jobKey{}, job)
	40	+	}
	41	+
	42	+	type jobKey struct{}
	43	+
	44	+	func jobFrom(ctx context.Context) string {
	45	+	job, _ := ctx.Value(jobKey{}).(string)
	46	+	return job
	47	+	}
	48	+
38	49		func (q *Quota) tryAcquire() bool {
39	50		for {
40	51		n := q.used.Load()

R internal/registryproxy/quota_test.go → internal/registryproxy/slots_test.go +88 -125 renamed, 51% similar

@@ -1,4 +1,5 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (

@@ -6,34 +7,61 @@ import (
6	7		"io"
7	8		"net/http"
8	9		"net/http/httptest"
9		-	"os"
10		-	"path/filepath"
11	10		"strings"
12	11		"sync"
13		-	"sync/atomic"
14	12		"testing"
15	13		"time"
16	14		)
17	15
18		-	// serveAsync serves path with ctx on its own goroutine, into w (a new
19		-	// recorder when nil); the status code comes on the returned channel.
20		-	func serveAsync(p Proxy, ctx context.Context, path string, w keepAliveRecorder) <-chan int {
21		-	if w == nil {
22		-	w = &keepAliveRecorder{ResponseRecorder: httptest.NewRecorder()}
23		-	}
24		-	code := make(chan int, 1)
25		-	go func() {
26		-	p.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil).WithContext(ctx))
27		-	code <- w.Code
28		-	}()
29		-	return code
	16	+	// At most MaxDownloads cold blobs download at once; one more waits for
	17	+	// SlotWait and then gets 503 (dockerd retries or pulls from Docker Hub), a
	18	+	// joiner of a running download takes no slot, and a slot frees once a
	19	+	// download ends.
	20	+	func TestColdDownloadCap(t *testing.T) {
	21	+	u := newStallingUpstream(t)
	22	+	const limit = 3
	23	+	const wait = 50 * time.Millisecond
	24	+	p := proxyWith(t, u.srv, Config{AbandonGrace: 10 * time.Millisecond, MaxDownloads: limit, SlotWait: wait})
	25	+	ctx, cancel := context.WithCancel(context.Background())
	26	+	var wg sync.WaitGroup
	27	+	for i := range limit {
	28	+	wg.Go(func() {
	29	+	p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, blobPath(i), nil).WithContext(ctx))
	30	+	})
	31	+	}
	32	+	waitFor(t, p.s, "the cap to fill", func() bool { return p.s.downloads == limit })
	33	+	start := time.Now()
	34	+	w := request(t, p, http.MethodGet, blobPath(limit))
	35	+	if w.Code != http.StatusServiceUnavailable \|\| w.Header().Get("Retry-After") == "" {
	36	+	t.Fatalf("over the cap: %d", w.Code)
	37	+	}
	38	+	if d := time.Since(start); d < wait {
	39	+	t.Fatalf("503 after %v, before the slot wait", d)
	40	+	}
	41	+	// Joining a running download takes no slot: the joiner is a waiter of the
	42	+	// flight, and the number of downloads does not move.
	43	+	key := kindBlob.key(sum("blob-0")[7:])
	44	+	go p.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, blobPath(0), nil).WithContext(ctx))
	45	+	waitFor(t, p.s, "the joiner", func() bool { return waiters(p.s, key) == 2 })
	46	+	p.s.mu.Lock()
	47	+	downloads := p.s.downloads
	48	+	p.s.mu.Unlock()
	49	+	if downloads != limit {
	50	+	t.Fatalf("%d downloads with a joiner, want %d", downloads, limit)
	51	+	}
	52	+	cancel()
	53	+	wg.Wait()
	54	+	waitFor(t, p.s, "the slots to free", func() bool { return p.s.downloads == 0 })
	55	+	if u.started.Load() != limit {
	56	+	t.Fatalf("%d downloads started", u.started.Load())
	57	+	}
30	58		}
31	59
32	60		// A sandbox that saturates its quota does not hold back another sandbox: its
33	61		// download starts at once while the global cap still has room.
34	62		func TestQuotaIsolatesChannels(t *testing.T) {
35	63		u := newStallingUpstream(t)
36		-	p := proxyWith(t, u.srv, Config{MaxDownloads: 16, SlotWait: time.Hour, AbandonGrace: 10 * time.Millisecond})
	64	+	p := proxyWith(t, u.srv, Config{SlotWait: time.Hour, AbandonGrace: 10 * time.Millisecond})
37	65		a, b := NewQuota(3), NewQuota(3)
38	66		ctxA, cancelA := context.WithCancel(WithQuota(context.Background(), a))
39	67		defer cancelA()

@@ -51,8 +79,15 @@ func TestQuotaIsolatesChannels(t *testing.T) {
51	79		if d := time.Since(start); d > time.Second {
52	80		t.Fatalf("B started after %v", d)
53	81		}
54		-	if got := u.started.Load(); got != 4 {
55		-	t.Fatalf("%d downloads started, want A's 3 and B's 1", got)
	82	+	// The quotas are taken before the downloads reach upstream: wait for the
	83	+	// 4 of A and B there, then check under the store lock that A's fourth
	84	+	// took no slot.
	85	+	waitFor(t, p.s, "4 downloads upstream", func() bool { return u.started.Load() == 4 })
	86	+	p.s.mu.Lock()
	87	+	downloads := p.s.downloads
	88	+	p.s.mu.Unlock()
	89	+	if downloads != 4 {
	90	+	t.Fatalf("%d downloads, want A's 3 and B's 1", downloads)
56	91		}
57	92		cancelA()
58	93		cancelB()

@@ -90,16 +125,11 @@ func TestQuotaWaiterGetsFreedSlot(t *testing.T) {
90	125		waitFor(t, p.s, "the quota to fill", func() bool { return q.used.Load() == 1 })
91	126		w := &keepAliveRecorder{ResponseRecorder: httptest.NewRecorder()}
92	127		second := serveAsync(p, ctx, "/v2/library/alpine/blobs/"+sum(body), w)
93		-	for end := time.Now().Add(5 * time.Second); w.calls.Load() == 0; { // waiting for a slot, kept alive
94		-	select {
95		-	case code := <-second:
96		-	t.Fatalf("did not wait for a slot: %d", code)
97		-	default:
98		-	}
99		-	if time.Now().After(end) {
100		-	t.Fatal("not kept alive while waiting for a slot")
101		-	}
102		-	time.Sleep(time.Millisecond)
	128	+	waitKeptAlive(t, w) // waiting for a slot
	129	+	select {
	130	+	case code := <-second:
	131	+	t.Fatalf("did not wait for a slot: %d", code)
	132	+	default:
103	133		}
104	134		unblock() // the first download ends (digest mismatch), freeing the slot
105	135		if code := <-first; code != http.StatusBadGateway {

@@ -128,16 +158,40 @@ func TestQuotaWaitTimesOut(t *testing.T) {
128	158		if d := time.Since(start); d < wait {
129	159		t.Fatalf("503 after %v", d)
130	160		}
131		-	// A request that disconnects while waiting leaves at once, with no slot.
132		-	cctx, ccancel := context.WithCancel(WithQuota(context.Background(), q))
133		-	waiting := serveAsync(p, cctx, blobPath(2), nil)
134		-	ccancel()
135		-	<-waiting
136	161		if u.started.Load() != 1 {
137	162		t.Fatalf("%d downloads started", u.started.Load())
138	163		}
139	164		}
140	165
	166	+	// A request that disconnects while it waits for a slot leaves at once, with no
	167	+	// slot and no 503.
	168	+	func TestQuotaWaiterDisconnects(t *testing.T) {
	169	+	u := newStallingUpstream(t)
	170	+	const wait = time.Hour
	171	+	p := proxyWith(t, u.srv, Config{SlotWait: wait, AbandonGrace: 10 * time.Millisecond, KeepAliveInterval: 5 * time.Millisecond})
	172	+	q := NewQuota(1)
	173	+	ctx, cancel := context.WithCancel(WithQuota(context.Background(), q))
	174	+	defer cancel()
	175	+	serveAsync(p, ctx, blobPath(0), nil)
	176	+	waitFor(t, p.s, "the quota to fill", func() bool { return q.used.Load() == 1 })
	177	+	wctx, wcancel := context.WithCancel(WithQuota(context.Background(), q))
	178	+	w := &keepAliveRecorder{ResponseRecorder: httptest.NewRecorder()}
	179	+	waiting := serveAsync(p, wctx, blobPath(1), w)
	180	+	waitKeptAlive(t, w) // really waiting for a slot
	181	+	start := time.Now()
	182	+	wcancel()
	183	+	code := <-waiting
	184	+	if d := time.Since(start); d > time.Second {
	185	+	t.Fatalf("left after %v", d)
	186	+	}
	187	+	if code == http.StatusServiceUnavailable {
	188	+	t.Fatal("a disconnected waiter got 503")
	189	+	}
	190	+	if u.started.Load() != 1 \|\| q.used.Load() != 1 {
	191	+	t.Fatalf("%d downloads started, quota %d", u.started.Load(), q.used.Load())
	192	+	}
	193	+	}
	194	+
141	195		// Joining a download in progress takes none of the joiner's quota.
142	196		func TestQuotaJoinersExempt(t *testing.T) {
143	197		u := newStallingUpstream(t)

@@ -150,7 +204,7 @@ func TestQuotaJoinersExempt(t *testing.T) {
150	204		ctxB, cancelB := context.WithCancel(WithQuota(context.Background(), b))
151	205		defer cancelB()
152	206		serveAsync(p, ctxB, blobPath(0), nil) // joins A's
153		-	key := "blobs/" + sum("blob-0")[7:]
	207	+	key := kindBlob.key(sum("blob-0")[7:])
154	208		waitFor(t, p.s, "B to join", func() bool { return waiters(p.s, key) == 2 })
155	209		if b.used.Load() != 0 {
156	210		t.Fatal("a joiner took quota")

@@ -159,94 +213,3 @@ func TestQuotaJoinersExempt(t *testing.T) {
159	213		serveAsync(p, ctxB, blobPath(1), nil)
160	214		waitFor(t, p.s, "B's own download", func() bool { return b.used.Load() == 1 })
161	215		}
162		-
163		-	// A panic in a download fails its flight and frees its slots; it does not
164		-	// leave the waiters hanging.
165		-	func TestPanickingFetchFailsFlight(t *testing.T) {
166		-	s, err := openStore(t.TempDir(), 1<<20, time.Now)
167		-	if err != nil {
168		-	t.Fatal(err)
169		-	}
170		-	s.maxDownloads = 1
171		-	q := NewQuota(1)
172		-	ctx, cancel := context.WithTimeout(WithQuota(context.Background(), q), 5*time.Second)
173		-	defer cancel()
174		-	_, _, _, err = s.ensure(ctx, "blobs", sum("x"), func(context.Context, io.Writer) (string, error) {
175		-	panic("boom")
176		-	}, keepalive{})
177		-	if err == nil \|\| !strings.Contains(err.Error(), "panicked") {
178		-	t.Fatalf("err %v", err)
179		-	}
180		-	s.mu.Lock()
181		-	defer s.mu.Unlock()
182		-	if s.downloads != 0 \|\| len(s.flights) != 0 \|\| q.used.Load() != 0 \|\| s.pending != 0 {
183		-	t.Fatalf("leaked: downloads=%d flights=%d quota=%d pending=%d", s.downloads, len(s.flights), q.used.Load(), s.pending)
184		-	}
185		-	if ds, _ := os.ReadDir(filepath.Join(s.dir, "tmp")); len(ds) != 0 {
186		-	t.Fatal("temp file kept")
187		-	}
188		-	}
189		-
190		-	// Eviction restarts from the front after a manifest removal that fails, and
191		-	// does not loop on it.
192		-	func TestEvictionAfterFailedManifestRemoval(t *testing.T) {
193		-	now := time.Unix(1_700_000_000, 0)
194		-	const media = "application/vnd.oci.image.index.v1+json"
195		-	s, err := openStore(t.TempDir(), 1<<20, func() time.Time { return now })
196		-	if err != nil {
197		-	t.Fatal(err)
198		-	}
199		-	m := `{"schemaVersion":2}`
200		-	f, _, _, err := s.ensure(context.Background(), "manifests", sum(m), func(_ context.Context, w io.Writer) (string, error) {
201		-	_, err := io.WriteString(w, m)
202		-	return media, err
203		-	}, keepalive{})
204		-	if err != nil {
205		-	t.Fatal(err)
206		-	}
207		-	_ = f.Close()
208		-	now = now.Add(time.Second)
209		-	if err := s.setTag("library/alpine", "3", "", sum(m)); err != nil {
210		-	t.Fatal(err)
211		-	}
212		-	now = now.Add(time.Second)
213		-	put(t, s, "blob")
214		-	// The manifest's .type cannot be removed: replace it with a non-empty
215		-	// directory. Its tag goes first all the same.
216		-	typ := filepath.Join(s.dir, "manifests", sum(m)[7:]+".type")
217		-	if err := os.Remove(typ); err != nil {
218		-	t.Fatal(err)
219		-	}
220		-	if err := os.MkdirAll(filepath.Join(typ, "x"), 0700); err != nil {
221		-	t.Fatal(err)
222		-	}
223		-	s.mu.Lock()
224		-	s.evictLocked(s.max) // ask for everything
225		-	_, manifestKept := s.entries["manifests/"+sum(m)[7:]]
226		-	_, blobKept := s.entries["blobs/"+sum("blob")[7:]]
227		-	tags := len(s.tagsOf)
228		-	s.mu.Unlock()
229		-	if !manifestKept \|\| blobKept \|\| tags != 0 {
230		-	t.Fatalf("manifest kept=%v blob kept=%v tags=%d", manifestKept, blobKept, tags)
231		-	}
232		-	accounted(t, s)
233		-	}
234		-
235		-	// unpinLocked of an unpinned entry is reported (and fails the test run through
236		-	// TestMain's hook); here the hook is replaced to observe it.
237		-	func TestUnpinBugReported(t *testing.T) {
238		-	s, err := openStore(t.TempDir(), 1<<20, time.Now)
239		-	if err != nil {
240		-	t.Fatal(err)
241		-	}
242		-	var bugs atomic.Int32
243		-	s.hookBug = func(string) { bugs.Add(1) }
244		-	put(t, s, "aaaa")
245		-	s.mu.Lock()
246		-	s.unpinLocked("blobs/" + sum("aaaa")[7:])
247		-	pins := s.entries["blobs/"+sum("aaaa")[7:]].pins
248		-	s.mu.Unlock()
249		-	if bugs.Load() != 1 \|\| pins != 0 {
250		-	t.Fatalf("bugs %d pins %d", bugs.Load(), pins)
251		-	}
252		-	}

M internal/registryproxy/store.go +114 -57


@@ -28,7 +28,39 @@ var errDigest = errors.New("registry digest mismatch")
28	28		// 503, and dockerd retries the layer (about 50 s) before pulling from Docker Hub.
29	29		var errBusy = errors.New("registry downloads busy")
30	30
31		-	const tagCharge int64 = 4096
	31	+	// minCharge is the least an entry is charged, whatever its size: about one
	32	+	// disk block and inode. It bounds the number of entries, and so of files, to
	33	+	// Max/minCharge. A tag file is charged exactly that.
	34	+	const minCharge int64 = 4096
	35	+
	36	+	// charge is what an entry of n bytes counts against Max.
	37	+	func charge(n int64) int64 { return max(n, minCharge) }
	38	+
	39	+	// kind is the directory of an entry: content (blobs, manifests) or a tag.
	40	+	type kind uint8
	41	+
	42	+	const (
	43	+	kindBlob kind = iota
	44	+	kindManifest
	45	+	kindTag
	46	+	)
	47	+
	48	+	var kinds = []kind{kindBlob, kindManifest, kindTag}
	49	+
	50	+	// dir is the directory of the kind in the store, and its route in /v2/.
	51	+	func (k kind) dir() string {
	52	+	switch k {
	53	+	case kindBlob:
	54	+	return "blobs"
	55	+	case kindManifest:
	56	+	return "manifests"
	57	+	default:
	58	+	return "tags"
	59	+	}
	60	+	}
	61	+
	62	+	// key is the key of an entry of kind k: its path in the store.
	63	+	func (k kind) key(name string) string { return k.dir() + "/" + name }
32	64
33	65		// lowWater bounds the extra room an eviction frees (with 5% of Max), so that a
34	66		// full store evicts once per that many bytes, not on every write.

@@ -36,6 +68,7 @@ const lowWater int64 = 64 << 20
36	68
37	69		type entry struct {
38	70		key string
	71	+	kind kind
39	72		size int64
40	73		used time.Time
41	74		pins int

@@ -55,27 +88,36 @@ type flight struct {
55	88		gen int // bumped each time waiters drops to zero
56	89		finished bool
57	90		pinned bool // the committed entry holds a pin for the waiters
58		-	blob bool // takes a download slot
	91	+	kind kind // a blob flight takes a download slot
59	92		quota *Quota // the starter's quota, held until the flight ends
60	93		cancel context.CancelFunc
61	94		}
62	95
63		-	type store struct {
64		-	dir string
65		-	max, total, pending int64
66		-	now func() time.Time
67		-	// grace is how long a flight with no waiter left keeps downloading in
	96	+	// storeConfig is a store's complete configuration: openStore applies no
	97	+	// defaults.
	98	+	type storeConfig struct {
	99	+	dir string
	100	+	max int64
	101	+	now func() time.Time
	102	+	// grace is how long a blob flight with no waiter left keeps downloading in
68	103		// case another request joins it; then it is cancelled.
69	104		grace time.Duration
70		-	// maxDownloads caps the concurrent blob downloads (0: no cap).
71		-	maxDownloads, downloads int
	105	+	// maxDownloads caps the concurrent blob downloads.
	106	+	maxDownloads int
72	107		// slotWait is how long a request waits for a download slot before errBusy.
73	108		slotWait time.Duration
	109	+	log zerowrap.Logger
	110	+	}
	111	+
	112	+	type store struct {
	113	+	storeConfig
	114	+
	115	+	mu sync.Mutex
	116	+	total, pending int64
	117	+	downloads int
74	118		// slotFreed is closed and replaced each time a download slot frees.
75	119		slotFreed chan struct{}
76		-	log zerowrap.Logger
77	120
78		-	mu sync.Mutex
79	121		entries map[string]*entry
80	122		lru list.List // entry, least recently used first
81	123		tagsOf map[string]map[string]struct{} // manifest digest -> tag keys

@@ -109,13 +151,17 @@ func (f *storedFile) Close() error {
109	151		return f.err
110	152		}
111	153
112		-	func openStore(dir string, max int64, now func() time.Time) (*store, error) {
113		-	if max <= 0 {
114		-	return nil, errors.New("registry cache cap must be positive")
	154	+	func openStore(c storeConfig) (*store, error) {
	155	+	if c.max < minCharge {
	156	+	return nil, fmt.Errorf("registry cache cap must be at least %d bytes", minCharge)
115	157		}
116		-	s := &store{dir: dir, max: max, now: now, entries: map[string]entry{}, lru: list.New(), tagsOf: map[string]map[string]struct{}{}, flights: map[string]flight{}, slotFreed: make(chan struct{}), hookBug: defaultBugHook}
117		-	for _, kind := range []string{"blobs", "manifests", "tags", "tmp"} {
118		-	if err := os.MkdirAll(filepath.Join(dir, kind), 0700); err != nil {
	158	+	if c.now == nil \|\| c.grace <= 0 \|\| c.maxDownloads <= 0 \|\| c.slotWait <= 0 {
	159	+	return nil, errors.New("registry store configuration incomplete")
	160	+	}
	161	+	dir := c.dir
	162	+	s := &store{storeConfig: c, entries: map[string]entry{}, lru: list.New(), tagsOf: map[string]map[string]struct{}{}, flights: map[string]flight{}, slotFreed: make(chan struct{}), hookBug: defaultBugHook}
	163	+	for _, sub := range []string{kindBlob.dir(), kindManifest.dir(), kindTag.dir(), "tmp"} {
	164	+	if err := os.MkdirAll(filepath.Join(dir, sub), 0700); err != nil {
119	165		return nil, err
120	166		}
121	167		}

@@ -129,13 +175,13 @@ func openStore(dir string, max int64, now func() time.Time) (*store, error) {
129	175		}
130	176		}
131	177		var types []string
132		-	for _, kind := range []string{"blobs", "manifests", "tags"} {
133		-	ds, err := os.ReadDir(filepath.Join(dir, kind))
	178	+	for _, k := range kinds {
	179	+	ds, err := os.ReadDir(filepath.Join(dir, k.dir()))
134	180		if err != nil {
135	181		return nil, err
136	182		}
137	183		for _, de := range ds {
138		-	if base, ok := strings.CutSuffix(de.Name(), ".type"); ok && kind == "manifests" {
	184	+	if base, ok := strings.CutSuffix(de.Name(), ".type"); ok && k == kindManifest {
139	185		types = append(types, base)
140	186		continue
141	187		}

@@ -149,36 +195,38 @@ func openStore(dir string, max int64, now func() time.Time) (*store, error) {
149	195		if !fi.Mode().IsRegular() {
150	196		continue
151	197		}
152		-	key := kind + "/" + de.Name()
153		-	e := &entry{key: key, size: fi.Size(), used: fi.ModTime()}
154		-	if kind == "manifests" {
	198	+	key := k.key(de.Name())
	199	+	size := fi.Size()
	200	+	e := &entry{key: key, kind: k, used: fi.ModTime()}
	201	+	switch k {
	202	+	case kindManifest:
155	203		info, err := os.Stat(filepath.Join(dir, key) + ".type")
156	204		if err != nil {
157	205		_ = os.Remove(filepath.Join(dir, key))
158	206		continue
159	207		}
160		-	e.size += info.Size()
161		-	}
162		-	if kind == "tags" {
	208	+	size += info.Size()
	209	+	case kindTag:
163	210		digest := s.readTagFile(key)
164	211		if !validDigest(digest) {
165	212		_ = os.Remove(filepath.Join(dir, key))
166	213		continue
167	214		}
168		-	if _, ok := s.entries["manifests/"+digest[7:]]; !ok {
	215	+	if _, ok := s.entries[kindManifest.key(digest[7:])]; !ok {
169	216		_ = os.Remove(filepath.Join(dir, key))
170	217		continue
171	218		}
172		-	e.size = tagCharge
	219	+	size = 0
173	220		e.digest = digest
174	221		}
	222	+	e.size = charge(size)
175	223		s.addLocked(e)
176	224		}
177	225		}
178	226		// A .type whose manifest is gone (a crash between the two writes).
179	227		for _, base := range types {
180		-	if _, ok := s.entries["manifests/"+base]; !ok {
181		-	_ = os.Remove(filepath.Join(dir, "manifests", base+".type"))
	228	+	if _, ok := s.entries[kindManifest.key(base)]; !ok {
	229	+	_ = os.Remove(filepath.Join(dir, kindManifest.key(base)+".type"))
182	230		}
183	231		}
184	232		// Order the LRU by the access times kept in the mtimes.

@@ -254,9 +302,9 @@ func (s *store) removeLocked(key string) bool {
254	302		if e.pins > 0 {
255	303		return false
256	304		}
257		-	if hexDigest, ok := strings.CutPrefix(key, "manifests/"); ok {
	305	+	if e.kind == kindManifest {
258	306		// Prune references first. If a tag cannot be removed, retain its target too.
259		-	for k := range s.tagsOf["sha256:"+hexDigest] {
	307	+	for k := range s.tagsOf["sha256:"+strings.TrimPrefix(key, kindManifest.key(""))] {
260	308		if !s.removeLocked(k) {
261	309		return false
262	310		}

@@ -291,7 +339,7 @@ func (s *store) evictLocked(need int64) {
291	339		next := el.Next()
292	340		if e.pins == 0 {
293	341		removed := s.removeLocked(e.key)
294		-	if strings.HasPrefix(e.key, "manifests/") {
	342	+	if e.kind == kindManifest {
295	343		// Removing a manifest removes its tags first, even when it then
296	344		// fails: next may be gone. Start over; only pinned or
297	345		// unremovable entries are walked again.

@@ -317,7 +365,7 @@ func (s store) openLocked(key string) (storedFile, string, error) {
317	365		return nil, "", err
318	366		}
319	367		media := "application/octet-stream"
320		-	if strings.HasPrefix(key, "manifests/") {
	368	+	if e.kind == kindManifest {
321	369		b, err := os.ReadFile(filepath.Join(s.dir, key) + ".type")
322	370		if err != nil {
323	371		_ = f.Close()

@@ -329,13 +377,13 @@ func (s store) openLocked(key string) (storedFile, string, error) {
329	377		e.pins++
330	378		return &storedFile{File: f, s: s, key: key}, media, nil
331	379		}
332		-	func (s store) open(kind, digest string) (storedFile, string, error) {
333		-	if !validDigest(digest) \|\| (kind != "blobs" && kind != "manifests") {
	380	+	func (s store) open(k kind, digest string) (storedFile, string, error) {
	381	+	if !validDigest(digest) {
334	382		return nil, "", errDigest
335	383		}
336	384		s.mu.Lock()
337	385		defer s.mu.Unlock()
338		-	return s.openLocked(kind + "/" + digest[7:])
	386	+	return s.openLocked(k.key(digest[7:]))
339	387		}
340	388
341	389		// fetch writes the upstream content to w. ctx belongs to the flight: it ends

@@ -360,12 +408,12 @@ type keepalive struct {
360	408		// carried by ctx (WithQuota), both held until the flight really ends; joining
361	409		// one takes neither. When a slot is missing, the request waits for one, kept
362	410		// alive, for at most the slot wait, and then fails with errBusy.
363		-	func (s store) ensure(ctx context.Context, kind, digest string, fetch fetch, keep keepalive) (f storedFile, media string, hit bool, err error) {
364		-	if !validDigest(digest) \|\| (kind != "blobs" && kind != "manifests") {
	411	+	func (s store) ensure(ctx context.Context, k kind, digest string, fetch fetch, keep keepalive) (f storedFile, media string, hit bool, err error) {
	412	+	if !validDigest(digest) {
365	413		return nil, "", false, errDigest
366	414		}
367		-	key := kind + "/" + digest[7:]
368		-	blob := kind == "blobs"
	415	+	key := k.key(digest[7:])
	416	+	blob := k == kindBlob
369	417		quota := quotaFrom(ctx)
370	418		var slotDeadline <-chan time.Time
371	419		var fl *flight

@@ -381,7 +429,7 @@ func (s *store) ensure(ctx context.Context, kind, digest string, fetch fetch, ke
381	429		}
382	430		if !blob \|\| s.takeSlotLocked(quota) {
383	431		dctx, cancel := context.WithCancel(context.Background())
384		-	fl = &flight{key: key, done: make(chan struct{}), cancel: cancel, blob: blob}
	432	+	fl = &flight{key: key, kind: k, done: make(chan struct{}), cancel: cancel}
385	433		if blob {
386	434		fl.quota = quota
387	435		}

@@ -433,7 +481,7 @@ func (s store) leaveLocked(fl flight) {
433	481		// once, so its flights never outnumber the requests in progress. A blob
434	482		// holds a download slot until it really ends, which bounds the abandoned
435	483		// ones to MaxDownloads.
436		-	if s.grace <= 0 \|\| !fl.blob {
	484	+	if fl.kind != kindBlob {
437	485		s.abandonLocked(fl)
438	486		return
439	487		}

@@ -458,7 +506,7 @@ func (s store) abandonLocked(fl flight) {
458	506		// takeSlotLocked takes a global download slot and one of quota (nil: none),
459	507		// both or neither.
460	508		func (s store) takeSlotLocked(quota Quota) bool {
461		-	if s.maxDownloads > 0 && s.downloads >= s.maxDownloads {
	509	+	if s.downloads >= s.maxDownloads {
462	510		return false
463	511		}
464	512		if quota != nil && !quota.tryAcquire() {

@@ -504,7 +552,7 @@ func (s store) finishLocked(fl flight, err error, committed *entry) {
504	552		if s.flights[fl.key] == fl {
505	553		delete(s.flights, fl.key)
506	554		}
507		-	if fl.blob {
	555	+	if fl.kind == kindBlob {
508	556		s.downloads--
509	557		if fl.quota != nil {
510	558		fl.quota.release()

@@ -626,34 +674,43 @@ func (s store) fill(ctx context.Context, fl flight, digest string, fetch fetch
626	674		}
627	675		path := filepath.Join(s.dir, key)
628	676		metadata := int64(0)
629		-	if strings.HasPrefix(key, "manifests/") {
	677	+	if fl.kind == kindManifest {
630	678		if len(media) > 1024 {
631	679		return errors.New("manifest media type too large")
632	680		}
633	681		metadata = int64(len(media))
634		-	s.evictLocked(metadata)
635		-	if metadata > s.max-s.total-s.pending {
	682	+	}
	683	+	// The download is charged its bytes in pending; the entry is charged at
	684	+	// least minCharge, and its media type.
	685	+	size := charge(cw.n + metadata)
	686	+	if extra := size - cw.n; extra > 0 {
	687	+	s.evictLocked(extra)
	688	+	if extra > s.max-s.total-s.pending {
636	689		return errTooLarge
637	690		}
	691	+	}
	692	+	if fl.kind == kindManifest {
638	693		if err := os.WriteFile(path+".type", []byte(media), 0600); err != nil {
639	694		return err
640	695		}
641	696		}
642	697		if err := os.Rename(f.Name(), path); err != nil {
643		-	if metadata > 0 {
	698	+	if fl.kind == kindManifest {
644	699		_ = os.Remove(path + ".type")
645	700		}
646	701		return err
647	702		}
648		-	e := &entry{key: key, size: cw.n + metadata, used: s.now()}
	703	+	e := &entry{key: key, kind: fl.kind, size: size, used: s.now()}
649	704		s.addLocked(e)
650	705		s.pending -= cw.n
651	706		cw.n = 0
652	707		s.finishLocked(fl, nil, e)
653	708		return nil
654	709		}
	710	+
	711	+	// tagKey is the key of a tag for a normalized Accept (normalizeAccept).
655	712		func tagKey(name, tag, accept string) string {
656		-	return fmt.Sprintf("%x", sha256.Sum256([]byte(name+"\x00"+tag+"\x00"+normalizeAccept(accept))))
	713	+	return kindTag.key(fmt.Sprintf("%x", sha256.Sum256([]byte(name+"\x00"+tag+"\x00"+accept))))
657	714		}
658	715		func (s *store) readTagFile(key string) string {
659	716		f, err := os.Open(filepath.Join(s.dir, key))

@@ -670,7 +727,7 @@ func (s *store) readTagFile(key string) string {
670	727		func (s *store) tag(name, tag, accept string) string {
671	728		s.mu.Lock()
672	729		defer s.mu.Unlock()
673		-	e, ok := s.entries["tags/"+tagKey(name, tag, accept)]
	730	+	e, ok := s.entries[tagKey(name, tag, accept)]
674	731		if !ok {
675	732		return ""
676	733		}

@@ -683,21 +740,21 @@ func (s *store) setTag(name, tag, accept, digest string) error {
683	740		}
684	741		s.mu.Lock()
685	742		defer s.mu.Unlock()
686		-	manifest, ok := s.entries["manifests/"+digest[7:]]
	743	+	manifest, ok := s.entries[kindManifest.key(digest[7:])]
687	744		if !ok {
688	745		return os.ErrNotExist
689	746		}
690	747		// Protect the target while making room for its metadata.
691	748		manifest.pins++
692	749		defer func() { manifest.pins-- }()
693		-	key := "tags/" + tagKey(name, tag, accept)
	750	+	key := tagKey(name, tag, accept)
694	751		old, exists := s.entries[key]
695	752		if exists {
696	753		// Already charged: rewriting it needs no room.
697	754		s.touchLocked(old)
698	755		} else {
699		-	s.evictLocked(tagCharge)
700		-	if tagCharge > s.max-s.total-s.pending {
	756	+	s.evictLocked(charge(0))
	757	+	if charge(0) > s.max-s.total-s.pending {
701	758		return errTooLarge
702	759		}
703	760		}

@@ -722,6 +779,6 @@ func (s *store) setTag(name, tag, accept, digest string) error {
722	779		s.indexTagLocked(key, digest)
723	780		return nil
724	781		}
725		-	s.addLocked(&entry{key: key, size: tagCharge, used: s.now(), digest: digest})
	782	+	s.addLocked(&entry{key: key, kind: kindTag, size: charge(0), used: s.now(), digest: digest})
726	783		return nil
727	784		}

M internal/registryproxy/store_test.go +280 -78

@@ -1,125 +1,327 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package registryproxy
3	4
4	5		import (
5	6		"context"
6		-	"crypto/sha256"
7	7		"fmt"
8	8		"io"
	9	+	"net/http"
	10	+	"net/http/httptest"
9	11		"os"
10	12		"path/filepath"
11		-	"sync"
12		-	"sync/atomic"
	13	+	"strings"
13	14		"testing"
14	15		"time"
15	16		)
16	17
17		-	func sum(b string) string { return fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(b))) }
18		-	func put(t testing.T, s store, b string) {
19		-	t.Helper()
20		-	f, _, _, err := s.ensure(context.Background(), "blobs", sum(b), func(_ context.Context, w io.Writer) (string, error) { _, err := io.WriteString(w, b); return "", err }, keepalive{})
	18	+	// Reopening a store keeps its content, charges it again and clears the
	19	+	// incomplete downloads in tmp/.
	20	+	func TestStoreReopen(t *testing.T) {
	21	+	dir := t.TempDir()
	22	+	s := mustOpen(t, storeCfg(dir, 8*minCharge))
	23	+	put(t, s, "aaaa")
	24	+	putManifest(t, s, manifestBody, "application/vnd.oci.image.index.v1+json")
	25	+	if err := s.setTag("library/alpine", "3", normalizeAccept(""), sum(manifestBody)); err != nil {
	26	+	t.Fatal(err)
	27	+	}
	28	+	if err := os.WriteFile(filepath.Join(dir, "tmp", "leftover"), []byte("partial"), 0600); err != nil {
	29	+	t.Fatal(err)
	30	+	}
	31	+	total := s.total
	32	+	s = mustOpen(t, storeCfg(dir, 8*minCharge))
	33	+	f, _, err := s.open(kindBlob, sum("aaaa"))
21	34		if err != nil {
22	35		t.Fatal(err)
23	36		}
24	37		_ = f.Close()
	38	+	if got := s.tag("library/alpine", "3", normalizeAccept("")); got != sum(manifestBody) {
	39	+	t.Fatalf("tag after reopen: %q", got)
	40	+	}
	41	+	if ds, _ := os.ReadDir(filepath.Join(dir, "tmp")); len(ds) != 0 {
	42	+	t.Fatal("tmp not cleared")
	43	+	}
	44	+	if accounted(t, s); s.total != total {
	45	+	t.Fatalf("reopened total %d, was %d", s.total, total)
	46	+	}
25	47		}
26		-	func TestStoreIntegrityReopenEviction(t *testing.T) {
27		-	now := time.Now()
	48	+
	49	+	// Every entry is charged at least minCharge, on commit and on reopen, so tiny
	50	+	// blobs cannot make more than Max/minCharge files.
	51	+	func TestEntriesChargedAtLeastMinCharge(t *testing.T) {
28	52		dir := t.TempDir()
29		-	s, err := openStore(dir, 8, func() time.Time { return now })
30		-	if err != nil {
	53	+	s := mustOpen(t, storeCfg(dir, 3*minCharge))
	54	+	for i := range 10 {
	55	+	put(t, s, fmt.Sprint("tiny-", i))
	56	+	}
	57	+	accounted(t, s)
	58	+	if n := len(s.entries); n > 3 {
	59	+	t.Fatalf("%d entries under a cap of 3 charges", n)
	60	+	}
	61	+	s = mustOpen(t, storeCfg(dir, 3*minCharge))
	62	+	accounted(t, s)
	63	+	if s.total != int64(len(s.entries))*minCharge {
	64	+	t.Fatalf("reopened total %d for %d entries", s.total, len(s.entries))
	65	+	}
	66	+	}
	67	+
	68	+	// A cap below one entry's minimum charge is refused; a blob larger than Max is
	69	+	// refused and leaves nothing charged.
	70	+	func TestStoreOversize(t *testing.T) {
	71	+	if _, err := openStore(storeCfg(t.TempDir(), minCharge-1)); err == nil {
	72	+	t.Fatal("cap below minCharge accepted")
	73	+	}
	74	+	s := mustOpen(t, storeCfg(t.TempDir(), minCharge))
	75	+	large := strings.Repeat("x", int(minCharge)+1)
	76	+	_, _, _, err := s.ensure(context.Background(), kindBlob, sum(large), func(_ context.Context, w io.Writer) (string, error) {
	77	+	_, err := io.WriteString(w, large)
	78	+	return "", err
	79	+	}, keepalive{})
	80	+	if err != errTooLarge {
31	81		t.Fatal(err)
32	82		}
33		-	put(t, s, "aaaa")
34		-	now = now.Add(time.Second)
35		-	put(t, s, "bbbb")
36		-	now = now.Add(time.Second)
37		-	f, _, err := s.open("blobs", sum("aaaa"))
	83	+	if s.pending != 0 \|\| s.total != 0 {
	84	+	t.Fatalf("accounting: %d %d", s.pending, s.total)
	85	+	}
	86	+	}
	87	+
	88	+	// Eviction walks the LRU, evicts least recently used first, frees down to the
	89	+	// low-water mark rather than the bare need, and skips pinned entries.
	90	+	func TestEvictionLRUAndLowWater(t *testing.T) {
	91	+	now := time.Unix(1_700_000_000, 0)
	92	+	const n = 40
	93	+	c := storeCfg(t.TempDir(), n*minCharge) // low water: 5%, two entries
	94	+	c.now = func() time.Time { return now }
	95	+	s := mustOpen(t, c)
	96	+	// 40 blobs fill the store. Freeing only the need would evict one blob; the
	97	+	// low-water mark evicts two.
	98	+	blob := func(i int) string { return fmt.Sprintf("%03d", i) + strings.Repeat("x", 47) }
	99	+	for i := range n {
	100	+	now = now.Add(time.Second)
	101	+	put(t, s, blob(i))
	102	+	}
	103	+	// Touch 0 and 1, and keep 2 open: the least recently used are then 3, 4...
	104	+	for _, i := range []int{0, 1} {
	105	+	now = now.Add(time.Second)
	106	+	f, _, err := s.open(kindBlob, sum(blob(i)))
	107	+	if err != nil {
	108	+	t.Fatal(err)
	109	+	}
	110	+	_ = f.Close()
	111	+	}
	112	+	pinned, _, err := s.open(kindBlob, sum(blob(2)))
38	113		if err != nil {
39	114		t.Fatal(err)
40	115		}
41		-	_ = f.Close()
	116	+	defer func() { _ = pinned.Close() }()
	117	+	s.mu.Lock()
	118	+	s.lru.MoveToFront(s.entries[kindBlob.key(sum(blob(2))[7:])].elem) // oldest of all, but open
	119	+	s.mu.Unlock()
42	120		now = now.Add(time.Second)
43		-	put(t, s, "cccc")
44		-	if f, _, err := s.open("blobs", sum("bbbb")); err == nil {
45		-	_ = f.Close()
46		-	t.Fatal("LRU survived eviction")
	121	+	put(t, s, "a") // over the cap: frees the low-water mark
	122	+	present := func(i int) bool {
	123	+	_, err := os.Stat(filepath.Join(s.dir, kindBlob.key(sum(blob(i))[7:])))
	124	+	return err == nil
47	125		}
48		-	_, _, _, err = s.ensure(context.Background(), "blobs", sum("dddd"), func(_ context.Context, w io.Writer) (string, error) {
49		-	_, err := io.WriteString(w, "evil")
50		-	return "", err
51		-	}, keepalive{})
52		-	if err != errDigest {
53		-	t.Fatalf("mismatch: %v", err)
	126	+	for i := range n {
	127	+	want := i != 3 && i != 4 // 3 and 4 are the least recently used unpinned
	128	+	if present(i) != want {
	129	+	t.Errorf("blob %d present=%v", i, present(i))
	130	+	}
54	131		}
55		-	if _, err := os.Stat(filepath.Join(dir, "blobs", sum("dddd")[7:])); !os.IsNotExist(err) {
56		-	t.Fatalf("bad commit: %v", err)
	132	+	accounted(t, s)
	133	+	s.mu.Lock()
	134	+	free := s.max - s.total
	135	+	s.mu.Unlock()
	136	+	if free < min(lowWater, s.max/20)-minCharge {
	137	+	t.Fatalf("freed only to %d", free)
57	138		}
58		-	put(t, s, "eeee")
59		-	if err := os.WriteFile(filepath.Join(dir, "tmp", "leftover"), []byte("partial"), 0600); err != nil {
	139	+	}
	140	+
	141	+	// Eviction restarts from the front after a manifest removal that fails, and
	142	+	// does not loop on it.
	143	+	func TestEvictionAfterFailedManifestRemoval(t *testing.T) {
	144	+	now := time.Unix(1_700_000_000, 0)
	145	+	c := storeCfg(t.TempDir(), 1<<20)
	146	+	c.now = func() time.Time { return now }
	147	+	s := mustOpen(t, c)
	148	+	m := `{"schemaVersion":2}`
	149	+	putManifest(t, s, m, "application/vnd.oci.image.index.v1+json")
	150	+	now = now.Add(time.Second)
	151	+	if err := s.setTag("library/alpine", "3", "", sum(m)); err != nil {
60	152		t.Fatal(err)
61	153		}
62		-	s, err = openStore(dir, 8, time.Now)
63		-	if err != nil {
	154	+	now = now.Add(time.Second)
	155	+	put(t, s, "blob")
	156	+	// The manifest's .type cannot be removed: replace it with a non-empty
	157	+	// directory. Its tag goes first all the same.
	158	+	typ := filepath.Join(s.dir, kindManifest.key(sum(m)[7:])+".type")
	159	+	if err := os.Remove(typ); err != nil {
64	160		t.Fatal(err)
65	161		}
66		-	if f, _, err := s.open("blobs", sum("eeee")); err != nil {
	162	+	if err := os.MkdirAll(filepath.Join(typ, "x"), 0700); err != nil {
67	163		t.Fatal(err)
68		-	} else {
69		-	_ = f.Close()
70		-	}
71		-	ds, _ := os.ReadDir(filepath.Join(dir, "tmp"))
72		-	if len(ds) != 0 {
73		-	t.Fatal("tmp not cleared")
74	164		}
75	165		s.mu.Lock()
76		-	defer s.mu.Unlock()
77		-	if s.total+s.pending > 8 {
78		-	t.Fatal("cap exceeded")
	166	+	s.evictLocked(s.max) // ask for everything
	167	+	_, manifestKept := s.entries[kindManifest.key(sum(m)[7:])]
	168	+	_, blobKept := s.entries[kindBlob.key(sum("blob")[7:])]
	169	+	tags := len(s.tagsOf)
	170	+	s.mu.Unlock()
	171	+	if !manifestKept \|\| blobKept \|\| tags != 0 {
	172	+	t.Fatalf("manifest kept=%v blob kept=%v tags=%d", manifestKept, blobKept, tags)
79	173		}
	174	+	accounted(t, s)
80	175		}
81		-	func TestStoreConcurrentEnsure(t *testing.T) {
82		-	s, err := openStore(t.TempDir(), 1024, time.Now)
	176	+
	177	+	// The committed entry is pinned for its waiters, so an eviction between the
	178	+	// commit and their open cannot remove it.
	179	+	func TestCommittedEntryPinnedForWaiters(t *testing.T) {
	180	+	s := mustOpen(t, storeCfg(t.TempDir(), minCharge))
	181	+	key := kindBlob.key(sum("aaaa")[7:])
	182	+	s.hookCommitted = func(string) {
	183	+	// Ask for the whole store before the waiters are woken.
	184	+	s.mu.Lock()
	185	+	s.evictLocked(s.max)
	186	+	s.mu.Unlock()
	187	+	}
	188	+	f, _, _, err := s.ensure(context.Background(), kindBlob, sum("aaaa"), func(_ context.Context, w io.Writer) (string, error) {
	189	+	_, err := io.WriteString(w, "aaaa")
	190	+	return "", err
	191	+	}, keepalive{})
83	192		if err != nil {
84		-	t.Fatal(err)
	193	+	t.Fatalf("waiter lost its entry: %v", err)
	194	+	}
	195	+	s.mu.Lock()
	196	+	pins := s.entries[key].pins
	197	+	s.mu.Unlock()
	198	+	if pins != 1 {
	199	+	t.Fatalf("pins %d: the commit pin was not released to the reader's", pins)
85	200		}
86		-	var calls atomic.Int32
87		-	var wg sync.WaitGroup
88		-	start := make(chan struct{})
89		-	for range 20 {
90		-	wg.Go(func() {
91		-	<-start
92		-	f, _, _, err := s.ensure(context.Background(), "blobs", sum("content"), func(_ context.Context, w io.Writer) (string, error) {
93		-	calls.Add(1)
94		-	_, err := io.WriteString(w, "content")
95		-	return "", err
96		-	}, keepalive{})
97		-	if err != nil {
98		-	t.Error(err)
99		-	return
100		-	}
101		-	_ = f.Close()
102		-	})
103		-	}
104		-	close(start)
105		-	wg.Wait()
106		-	if calls.Load() != 1 {
107		-	t.Fatalf("fetches %d", calls.Load())
	201	+	_ = f.Close()
	202	+	s.hookCommitted = nil
	203	+	s.mu.Lock()
	204	+	s.evictLocked(s.max)
	205	+	_, kept := s.entries[key]
	206	+	s.mu.Unlock()
	207	+	if kept {
	208	+	t.Fatal("unpinned entry not evictable")
108	209		}
109	210		}
110		-	func TestStoreOversize(t *testing.T) {
111		-	s, err := openStore(t.TempDir(), 3, time.Now)
112		-	if err != nil {
	211	+
	212	+	// A blob that a response is still sending is pinned: a fill that needs its
	213	+	// room fails, and succeeds once the response has closed the file.
	214	+	func TestEvictionWhileServing(t *testing.T) {
	215	+	a, b := strings.Repeat("a", int(minCharge)), strings.Repeat("b", int(minCharge))
	216	+	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
	217	+	switch {
	218	+	case strings.HasSuffix(r.URL.Path, sum(a)):
	219	+	_, _ = io.WriteString(w, a)
	220	+	case strings.HasSuffix(r.URL.Path, sum(b)):
	221	+	_, _ = io.WriteString(w, b)
	222	+	default:
	223	+	http.NotFound(w, r)
	224	+	}
	225	+	})
	226	+	p := proxyWith(t, srv, Config{Max: minCharge * 3 / 2})
	227	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(a)); w.Code != http.StatusOK {
	228	+	t.Fatal(w.Code)
	229	+	}
	230	+	bw := &blockingWriter{ResponseRecorder: httptest.NewRecorder(), started: make(chan struct{}), release: make(chan struct{})}
	231	+	done := make(chan struct{})
	232	+	go func() {
	233	+	defer close(done)
	234	+	p.ServeHTTP(bw, httptest.NewRequest(http.MethodGet, "/v2/library/alpine/blobs/"+sum(a), nil))
	235	+	}()
	236	+	<-bw.started
	237	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(b)); w.Code != http.StatusBadGateway {
	238	+	t.Fatalf("fill evicted a blob being served: %d", w.Code)
	239	+	}
	240	+	if _, err := os.Stat(filepath.Join(p.s.dir, kindBlob.key(sum(a)[7:]))); err != nil {
	241	+	t.Fatalf("served blob removed: %v", err)
	242	+	}
	243	+	accounted(t, p.s)
	244	+	close(bw.release)
	245	+	<-done
	246	+	if bw.Code != http.StatusOK \|\| bw.Body.String() != a {
	247	+	t.Fatalf("served: %d", bw.Code)
	248	+	}
	249	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(b)); w.Code != http.StatusOK \|\| w.Body.String() != b {
	250	+	t.Fatalf("after release: %d", w.Code)
	251	+	}
	252	+	if _, err := os.Stat(filepath.Join(p.s.dir, kindBlob.key(sum(a)[7:]))); !os.IsNotExist(err) {
	253	+	t.Fatal("released blob not evicted")
	254	+	}
	255	+	accounted(t, p.s)
	256	+	}
	257	+
	258	+	// Tag files share the LRU of the store, so they cannot grow the store past
	259	+	// Max; reopening charges them again.
	260	+	func TestTagsChargedAndEvicted(t *testing.T) {
	261	+	now := time.Unix(1_700_000_000, 0)
	262	+	dir := t.TempDir()
	263	+	c := storeCfg(dir, 4*minCharge) // the manifest and 3 tags
	264	+	c.now = func() time.Time { return now }
	265	+	s := mustOpen(t, c)
	266	+	putManifest(t, s, manifestBody, "application/vnd.oci.image.index.v1+json")
	267	+	for i := range 20 {
	268	+	now = now.Add(time.Second)
	269	+	if err := s.setTag("library/alpine", fmt.Sprint("t", i), "", sum(manifestBody)); err != nil {
	270	+	t.Fatalf("tag %d: %v", i, err)
	271	+	}
	272	+	if n := accounted(t, s); n > 3 {
	273	+	t.Fatalf("%d tags over a cap of 3", n)
	274	+	}
	275	+	}
	276	+	if s.tag("library/alpine", "t19", "") != sum(manifestBody) \|\| s.tag("library/alpine", "t0", "") != "" {
	277	+	t.Fatal("tags not evicted least recently used first")
	278	+	}
	279	+	s = mustOpen(t, storeCfg(dir, 4*minCharge))
	280	+	if n := accounted(t, s); n != 3 {
	281	+	t.Fatalf("reopened with %d tags", n)
	282	+	}
	283	+	}
	284	+
	285	+	// The tag index follows rewrites, and removing a manifest takes exactly its
	286	+	// own tags along.
	287	+	func TestTagIndex(t *testing.T) {
	288	+	s := mustOpen(t, storeCfg(t.TempDir(), 1<<20))
	289	+	manifests := []string{`{"schemaVersion":2,"a":1}`, `{"schemaVersion":2,"b":1}`}
	290	+	for _, m := range manifests {
	291	+	putManifest(t, s, m, "application/vnd.oci.image.index.v1+json")
	292	+	}
	293	+	for _, tag := range []string{"x", "y"} {
	294	+	if err := s.setTag("library/alpine", tag, "", sum(manifests[0])); err != nil {
	295	+	t.Fatal(err)
	296	+	}
	297	+	}
	298	+	// y moves to the second manifest.
	299	+	if err := s.setTag("library/alpine", "y", "", sum(manifests[1])); err != nil {
113	300		t.Fatal(err)
114	301		}
115		-	_, _, _, err = s.ensure(context.Background(), "blobs", sum("large"), func(_ context.Context, w io.Writer) (string, error) {
116		-	_, err := io.WriteString(w, "large")
117		-	return "", err
118		-	}, keepalive{})
119		-	if err != errTooLarge {
	302	+	s.mu.Lock()
	303	+	if !s.removeLocked(kindManifest.key(sum(manifests[0])[7:])) {
	304	+	t.Fatal("remove failed")
	305	+	}
	306	+	n0, n1 := len(s.tagsOf[sum(manifests[0])]), len(s.tagsOf[sum(manifests[1])])
	307	+	s.mu.Unlock()
	308	+	if s.tag("library/alpine", "x", "") != "" \|\| s.tag("library/alpine", "y", "") != sum(manifests[1]) \|\| n0 != 0 \|\| n1 != 1 {
	309	+	t.Fatalf("index: %d %d", n0, n1)
	310	+	}
	311	+	accounted(t, s)
	312	+	}
	313	+
	314	+	// A .type file without its manifest is removed at startup.
	315	+	func TestOrphanTypeRemoved(t *testing.T) {
	316	+	dir := t.TempDir()
	317	+	mustOpen(t, storeCfg(dir, 1<<20))
	318	+	orphan := filepath.Join(dir, kindManifest.key(sum("gone")[7:])+".type")
	319	+	if err := os.WriteFile(orphan, []byte("application/json"), 0600); err != nil {
120	320		t.Fatal(err)
121	321		}
122		-	if s.pending != 0 \|\| s.total != 0 {
123		-	t.Fatalf("accounting: %d %d", s.pending, s.total)
	322	+	s := mustOpen(t, storeCfg(dir, 1<<20))
	323	+	if _, err := os.Stat(orphan); !os.IsNotExist(err) {
	324	+	t.Fatal("orphan .type kept")
124	325		}
	326	+	accounted(t, s)
125	327		}

M internal/registryproxy/upstream.go +35 -22


@@ -29,6 +29,7 @@ func unavailable(err error) bool {
29	29		var network net.Error
30	30		return errors.As(err, &network) \|\| errors.Is(err, context.DeadlineExceeded)
31	31		}
	32	+
32	33		func proxyStatus(err error) int {
33	34		var e upstreamError
34	35		if errors.As(err, &e) && (e.status == http.StatusUnauthorized \|\| e.status == http.StatusForbidden \|\| e.status == http.StatusNotFound) {

@@ -41,12 +42,12 @@ type bearer struct {
41	42		value string
42	43		until time.Time
43	44		}
	45	+
44	46		type upstream struct {
45	47		base, auth string
46	48		client *http.Client
47	49		blobClient *http.Client
48	50		requestTimeout, blobIdle time.Duration
49		-	now func() time.Time
50	51		mu sync.Mutex
51	52		tokens map[string]bearer
52	53		}

@@ -58,7 +59,7 @@ func (u *upstream) token(ctx context.Context, name string, refresh bool) (string
58	59		u.mu.Lock()
59	60		t, ok := u.tokens[name]
60	61		u.mu.Unlock()
61		-	if ok && !refresh && u.now().Before(t.until) {
	62	+	if ok && !refresh && time.Now().Before(t.until) {
62	63		return t.value, nil
63	64		}
64	65		a, err := url.Parse(u.auth)

@@ -113,33 +114,23 @@ func (u *upstream) token(ctx context.Context, name string, refresh bool) (string
113	114		if len(u.tokens) >= 1024 {
114	115		clear(u.tokens)
115	116		}
116		-	u.tokens[name] = bearer{data.Token, u.now().Add(time.Duration(data.Expires) * time.Second)}
	117	+	u.tokens[name] = bearer{data.Token, time.Now().Add(time.Duration(data.Expires) * time.Second)}
117	118		return data.Token, nil
118	119		}
119	120
120		-	// request is a metadata request (manifests, blob HEAD) bounded by ctx. Manifests
121		-	// use the client that never follows redirects; a blob HEAD uses the blob client,
122		-	// because Docker Hub answers it with a redirect to its CDN.
123		-	func (u upstream) request(ctx context.Context, method, name, kind, ref, accept string) (http.Response, error) {
124		-	client := u.client
125		-	if kind == "blobs" {
126		-	client = u.blobClient
127		-	}
	121	+	// authorized calls send with a bearer token for name, and once more with a
	122	+	// fresh token after a 401 or 403. Each token request is bounded by
	123	+	// RequestTimeout (and ctx). It returns a 200 response; any other closes the
	124	+	// body and is an upstreamError.
	125	+	func (u upstream) authorized(ctx context.Context, name string, send func(tok string) (http.Response, error)) (*http.Response, error) {
128	126		for attempt := 0; attempt < 2; attempt++ {
129		-	tok, err := u.token(ctx, name, attempt > 0)
	127	+	authCtx, cancel := context.WithTimeout(ctx, u.requestTimeout)
	128	+	tok, err := u.token(authCtx, name, attempt > 0)
	129	+	cancel()
130	130		if err != nil {
131	131		return nil, err
132	132		}
133		-	req, err := http.NewRequestWithContext(ctx, method, u.base+"/v2/"+name+"/"+kind+"/"+ref, nil)
134		-	if err != nil {
135		-	return nil, err
136		-	}
137		-	req.Header.Set("Authorization", "Bearer "+tok)
138		-	if accept != "" {
139		-	req.Header.Set("Accept", accept)
140		-	}
141		-	// Do not forward guest Authorization, cookies, query parameters or Range.
142		-	resp, err := client.Do(req)
	133	+	resp, err := send(tok)
143	134		if err != nil {
144	135		return nil, err
145	136		}

@@ -154,3 +145,25 @@ func (u *upstream) request(ctx context.Context, method, name, kind, ref, accept
154	145		}
155	146		return nil, upstreamError{http.StatusUnauthorized}
156	147		}
	148	+
	149	+	// request is a metadata request (manifests, blob HEAD) bounded by ctx. Manifests
	150	+	// use the client that never follows redirects; a blob HEAD uses the blob client,
	151	+	// because Docker Hub answers it with a redirect to its CDN.
	152	+	func (u upstream) request(ctx context.Context, method, name string, k kind, ref, accept string) (http.Response, error) {
	153	+	client := u.client
	154	+	if k == kindBlob {
	155	+	client = u.blobClient
	156	+	}
	157	+	return u.authorized(ctx, name, func(tok string) (*http.Response, error) {
	158	+	req, err := http.NewRequestWithContext(ctx, method, u.base+"/v2/"+name+"/"+k.dir()+"/"+ref, nil)
	159	+	if err != nil {
	160	+	return nil, err
	161	+	}
	162	+	req.Header.Set("Authorization", "Bearer "+tok)
	163	+	if accept != "" {
	164	+	req.Header.Set("Accept", accept)
	165	+	}
	166	+	// Do not forward guest Authorization, cookies, query parameters or Range.
	167	+	return client.Do(req)
	168	+	})
	169	+	}

A internal/registryproxy/upstream_test.go +172 -0

@@ -0,0 +1,172 @@
	1	+	// SPDX-License-Identifier: MIT
	2	+
	3	+	package registryproxy
	4	+
	5	+	import (
	6	+	"fmt"
	7	+	"io"
	8	+	"net/http"
	9	+	"net/http/httptest"
	10	+	"os"
	11	+	"path/filepath"
	12	+	"strings"
	13	+	"sync/atomic"
	14	+	"testing"
	15	+	"time"
	16	+	)
	17	+
	18	+	// A blob body has no total deadline: a transfer that keeps moving may last
	19	+	// far longer than RequestTimeout and BlobIdleTimeout.
	20	+	func TestBlobTransferOutlivesRequestTimeout(t *testing.T) {
	21	+	body := strings.Repeat("layer", 4000)
	22	+	srv := upstreamWith(t, slowBlob(body, 20, 25*time.Millisecond))
	23	+	p := proxyWith(t, srv, Config{RequestTimeout: 100 * time.Millisecond, BlobIdleTimeout: 150 * time.Millisecond})
	24	+	start := time.Now()
	25	+	w := request(t, p, http.MethodGet, "/v2/library/alpine/blobs/"+sum(body))
	26	+	if w.Code != http.StatusOK \|\| w.Body.String() != body {
	27	+	t.Fatalf("blob: %d", w.Code)
	28	+	}
	29	+	if d := time.Since(start); d < 300*time.Millisecond {
	30	+	t.Fatalf("transfer took %v, not longer than the timeouts", d)
	31	+	}
	32	+	}
	33	+
	34	+	// A blob body that stops moving is cut after BlobIdleTimeout, and auth,
	35	+	// manifests and HEAD keep their total RequestTimeout.
	36	+	func TestStalledUpstreamTimesOut(t *testing.T) {
	37	+	body := strings.Repeat("layer", 4000)
	38	+	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
	39	+	if strings.Contains(r.URL.Path, "/blobs/") && r.Method == http.MethodGet {
	40	+	w.Header().Set("Content-Length", fmt.Sprint(len(body)))
	41	+	_, _ = io.WriteString(w, body[:100])
	42	+	w.(http.Flusher).Flush()
	43	+	}
	44	+	select { // a stalled body, or headers that never come
	45	+	case <-r.Context().Done():
	46	+	case <-time.After(5 * time.Second):
	47	+	}
	48	+	})
	49	+	p := proxyWith(t, srv, Config{RequestTimeout: 100 * time.Millisecond, BlobIdleTimeout: 150 * time.Millisecond})
	50	+	for _, tc := range []struct{ method, path string }{
	51	+	{http.MethodGet, "/v2/library/alpine/blobs/" + sum(body)},
	52	+	{http.MethodHead, "/v2/library/alpine/blobs/" + sum(body)},
	53	+	{http.MethodGet, "/v2/library/alpine/manifests/3"},
	54	+	} {
	55	+	start := time.Now()
	56	+	if w := request(t, p, tc.method, tc.path); w.Code != http.StatusBadGateway {
	57	+	t.Errorf("%s %s: %d", tc.method, tc.path, w.Code)
	58	+	}
	59	+	if d := time.Since(start); d > 2*time.Second {
	60	+	t.Errorf("%s %s: cut after %v", tc.method, tc.path, d)
	61	+	}
	62	+	}
	63	+	if _, err := os.Stat(filepath.Join(p.s.dir, kindBlob.key(sum(body)[7:]))); !os.IsNotExist(err) {
	64	+	t.Fatal("partial blob stored")
	65	+	}
	66	+	}
	67	+
	68	+	// Auth and manifest requests never follow a redirect.
	69	+	func TestMetadataRedirectsNotFollowed(t *testing.T) {
	70	+	var elsewhere atomic.Int32
	71	+	other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	72	+	elsewhere.Add(1)
	73	+	_, _ = io.WriteString(w, `{"token":"public"}`)
	74	+	}))
	75	+	t.Cleanup(other.Close)
	76	+	for _, redirectAuth := range []bool{true, false} {
	77	+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	78	+	if r.URL.Path == "/token" && !redirectAuth {
	79	+	_, _ = io.WriteString(w, `{"token":"public"}`)
	80	+	return
	81	+	}
	82	+	http.Redirect(w, r, other.URL+r.URL.Path, http.StatusTemporaryRedirect)
	83	+	}))
	84	+	p := proxyWith(t, srv, Config{})
	85	+	for _, ref := range []string{"3", sum(manifestBody)} {
	86	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/"+ref); w.Code != http.StatusBadGateway {
	87	+	t.Errorf("auth redirect %v, ref %s: %d", redirectAuth, ref, w.Code)
	88	+	}
	89	+	}
	90	+	srv.Close()
	91	+	}
	92	+	if elsewhere.Load() != 0 {
	93	+	t.Fatalf("%d redirects followed", elsewhere.Load())
	94	+	}
	95	+	}
	96	+
	97	+	// Blob requests, HEAD included, follow the CDN redirect without the bearer
	98	+	// token.
	99	+	func TestBlobRedirectStripsBearer(t *testing.T) {
	100	+	var cdnHits atomic.Int32
	101	+	cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	102	+	cdnHits.Add(1)
	103	+	if r.Header.Get("Authorization") != "" {
	104	+	t.Error("bearer sent to the CDN")
	105	+	}
	106	+	w.Header().Set("Content-Length", fmt.Sprint(len(blobBody)))
	107	+	if r.Method == http.MethodGet {
	108	+	_, _ = io.WriteString(w, blobBody)
	109	+	}
	110	+	}))
	111	+	t.Cleanup(cdn.Close)
	112	+	srv := upstreamWith(t, func(w http.ResponseWriter, r *http.Request) {
	113	+	if r.Header.Get("Authorization") != "Bearer public" {
	114	+	t.Error("no bearer on the registry")
	115	+	}
	116	+	http.Redirect(w, r, cdn.URL+"/layer", http.StatusTemporaryRedirect)
	117	+	})
	118	+	p := proxyWith(t, srv, Config{})
	119	+	path := "/v2/library/alpine/blobs/" + sum(blobBody)
	120	+	if w := request(t, p, http.MethodHead, path); w.Code != http.StatusOK \|\| w.Header().Get("Content-Length") != fmt.Sprint(len(blobBody)) {
	121	+	t.Fatalf("HEAD: %d %v", w.Code, w.Header())
	122	+	}
	123	+	if w := request(t, p, http.MethodGet, path); w.Code != http.StatusOK \|\| w.Body.String() != blobBody {
	124	+	t.Fatalf("blob: %d", w.Code)
	125	+	}
	126	+	if cdnHits.Load() != 2 {
	127	+	t.Fatalf("CDN hits %d", cdnHits.Load())
	128	+	}
	129	+	}
	130	+
	131	+	// A 401 or 403 is retried once with a fresh token; a second one is a 404, so
	132	+	// that dockerd falls back to Docker Hub with the job's credentials.
	133	+	func TestUnauthorizedRetriedOnce(t *testing.T) {
	134	+	for _, path := range []string{"/v2/library/alpine/manifests/3", "/v2/library/alpine/blobs/" + sum(blobBody)} {
	135	+	f := fake(t)
	136	+	p := proxyFor(t, f)
	137	+	f.status.Store(http.StatusUnauthorized)
	138	+	if w := request(t, p, http.MethodGet, path); w.Code != http.StatusNotFound {
	139	+	t.Fatalf("%s: %d", path, w.Code)
	140	+	}
	141	+	if f.tokens.Load() != 2 {
	142	+	t.Fatalf("%s: %d token requests, want a fresh one", path, f.tokens.Load())
	143	+	}
	144	+	}
	145	+	}
	146	+
	147	+	// When upstream is down, fails or rate-limits (429), a known tag is served
	148	+	// stale; an unknown one is an error.
	149	+	func TestStaleTagWhenUpstreamUnavailable(t *testing.T) {
	150	+	for _, tc := range []struct {
	151	+	name string
	152	+	status int32 // 0: the server is closed
	153	+	}{{"rate limited", http.StatusTooManyRequests}, {"failing", http.StatusServiceUnavailable}, {"down", 0}} {
	154	+	t.Run(tc.name, func(t *testing.T) {
	155	+	f := fake(t)
	156	+	p := proxyFor(t, f)
	157	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3"); w.Code != http.StatusOK {
	158	+	t.Fatal(w.Code)
	159	+	}
	160	+	if tc.status == 0 {
	161	+	f.server.Close()
	162	+	}
	163	+	f.status.Store(tc.status)
	164	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/3"); w.Code != http.StatusOK \|\| w.Body.String() != manifestBody {
	165	+	t.Fatalf("stale: %d %s", w.Code, w.Body)
	166	+	}
	167	+	if w := request(t, p, http.MethodGet, "/v2/library/alpine/manifests/unknown"); w.Code != http.StatusBadGateway {
	168	+	t.Fatalf("unknown: %d", w.Code)
	169	+	}
	170	+	})
	171	+	}
	172	+	}

M internal/sandbox/container/driver.go +4 -10


@@ -22,6 +22,7 @@ import (
22	22		"context"
23	23		"errors"
24	24		"fmt"
	25	+	"maps"
25	26		"net"
26	27		"net/netip"
27	28		"os"

@@ -306,16 +307,9 @@ type containerSandbox struct {
306	307		attached bool
307	308		}
308	309
309		-	// StartTimings returns a copy of the successful start's phase durations.
310		-	func (s *containerSandbox) StartTimings() map[string]int64 {
311		-	s.mu.Lock()
312		-	defer s.mu.Unlock()
313		-	out := make(map[string]int64, len(s.timings))
314		-	for k, v := range s.timings {
315		-	out[k] = v
316		-	}
317		-	return out
318		-	}
	310	+	// StartTimings returns a copy of the phase durations of a successful Start.
	311	+	// It is valid once Start has returned, which writes them.
	312	+	func (s *containerSandbox) StartTimings() map[string]int64 { return maps.Clone(s.timings) }
319	313
320	314		// ID implements sandbox.Sandbox.
321	315		func (s *containerSandbox) ID() string { return s.id }

M internal/sandbox/sandbox.go +2 -0


@@ -91,6 +91,8 @@ type Sandbox interface {
91	91
92	92		// Timed is implemented by sandboxes that measure their start phases.
93	93		type Timed interface {
	94	+	// StartTimings returns a copy of the phase durations (ms) of a successful
	95	+	// Start, nil after a failed one. Call it only once Start has returned.
94	96		StartTimings() map[string]int64
95	97		}
96	98

M internal/sandbox/vm/driver.go +4 -10


@@ -16,6 +16,7 @@ import (
16	16		"errors"
17	17		"fmt"
18	18		"io/fs"
	19	+	"maps"
19	20		"net"
20	21		"net/netip"
21	22		"os"

@@ -258,16 +259,9 @@ type vmSandbox struct {
258	259		attached bool
259	260		}
260	261
261		-	// StartTimings returns a copy of the successful start's phase durations.
262		-	func (s *vmSandbox) StartTimings() map[string]int64 {
263		-	s.mu.Lock()
264		-	defer s.mu.Unlock()
265		-	out := make(map[string]int64, len(s.timings))
266		-	for k, v := range s.timings {
267		-	out[k] = v
268		-	}
269		-	return out
270		-	}
	262	+	// StartTimings returns a copy of the phase durations of a successful Start.
	263	+	// It is valid once Start has returned, which writes them.
	264	+	func (s *vmSandbox) StartTimings() map[string]int64 { return maps.Clone(s.timings) }
271	265
272	266		// ID implements sandbox.Sandbox.
273	267		func (s *vmSandbox) ID() string { return s.id }

M registry_setup_test.go +49 -17

@@ -1,37 +1,69 @@
1	1		// SPDX-License-Identifier: MIT
	2	+
2	3		package main
3	4
4	5		import (
5		-	"bnema.dev/ermite-runner/internal/sandbox"
6	6		"context"
	7	+	"strings"
7	8		"testing"
	9	+
	10	+	"bnema.dev/ermite-runner/internal/actionscache"
	11	+	"bnema.dev/ermite-runner/internal/cachesvc"
	12	+	"bnema.dev/ermite-runner/internal/sandbox"
8	13		)
9	14
10		-	func TestRegistryConfiguration(t *testing.T) {
	15	+	// The three size caps share one parser: a zero size in any unit turns the
	16	+	// registry proxy and the action archive cache off, and is refused for the
	17	+	// cache API, which ERMITE_RUNNER_CACHE=0 turns off. Each error names its
	18	+	// variable.
	19	+	func TestCacheSizeConfiguration(t *testing.T) {
11	20		t.Setenv(envCache, "1")
12		-	t.Setenv("ERMITE_RUNNER_ACTIONS_CACHE_MAX", "0")
13	21		for _, tc := range []struct {
14		-	registry, cache string
15		-	on bool
16		-	}{{"", "1M", true}, {"1M", "2M", true}, {"0", "1M", false}, {"0G", "1M", false}, {" 0M ", "1M", false}, {"00", "1M", false}, {"0GiB", "1M", false}, {"10M", "1M", true}} {
	22	+	registry, actions, cache string
	23	+	registryOn, archivesOn bool
	24	+	}{
	25	+	{"", "", "1M", true, true},
	26	+	{"1M", "0", "2M", true, false},
	27	+	{"0", "1M", "1M", false, true},
	28	+	{"0G", "0GiB", "1M", false, false},
	29	+	{" 0M ", "", "1M", false, true},
	30	+	{"00", "", "1M", false, true},
	31	+	{"0GiB", "", "1M", false, true},
	32	+	{"10M", "", "1M", true, true},
	33	+	} {
17	34		t.Setenv(envRegistryMax, tc.registry)
18		-	t.Setenv("ERMITE_RUNNER_CACHE_MAX", tc.cache)
	35	+	t.Setenv(actionscache.EnvMax, tc.actions)
	36	+	t.Setenv(cachesvc.EnvMax, tc.cache)
19	37		cfg := sandbox.Config{StateDir: t.TempDir()}
20		-	svc, _, err := setupCache(context.Background(), &cfg)
	38	+	svc, archives, err := setupCache(context.Background(), &cfg)
21	39		if err != nil {
22		-	t.Fatal(err)
	40	+	t.Fatalf("%+v: %v", tc, err)
23	41		}
24		-	if svc.RegistryEnabled() != tc.on {
25		-	t.Fatalf("%+v: registry=%v", tc, svc.RegistryEnabled())
	42	+	if svc.RegistryEnabled() != tc.registryOn \|\| (archives != nil) != tc.archivesOn {
	43	+	t.Fatalf("%+v: registry=%v archives=%v", tc, svc.RegistryEnabled(), archives != nil)
26	44		}
27	45		_ = svc.CloseAll()
28	46		}
29		-	for _, bad := range []string{"invalid", "G", "-1G"} {
30		-	t.Setenv(envRegistryMax, bad)
31		-	cfg := sandbox.Config{StateDir: t.TempDir()}
32		-	if _, _, err := setupCache(context.Background(), &cfg); err == nil {
33		-	t.Fatalf("%q accepted", bad)
34		-	}
	47	+	t.Setenv(envRegistryMax, "")
	48	+	t.Setenv(actionscache.EnvMax, "")
	49	+	t.Setenv(cachesvc.EnvMax, "")
	50	+	for _, tc := range []struct{ env, value string }{
	51	+	{envRegistryMax, "invalid"}, {envRegistryMax, "G"}, {envRegistryMax, "-1G"},
	52	+	{actionscache.EnvMax, "x"},
	53	+	{cachesvc.EnvMax, "0"}, {cachesvc.EnvMax, "0G"}, {cachesvc.EnvMax, "-1"},
	54	+	} {
	55	+	t.Run(tc.env+"="+tc.value, func(t *testing.T) {
	56	+	t.Setenv(tc.env, tc.value)
	57	+	cfg := sandbox.Config{StateDir: t.TempDir()}
	58	+	svc, _, err := setupCache(context.Background(), &cfg)
	59	+	if err == nil {
	60	+	_ = svc.CloseAll()
	61	+	t.Fatal("accepted")
	62	+	}
	63	+	if !strings.HasPrefix(err.Error(), tc.env+": ") {
	64	+	t.Fatalf("error does not name %s: %v", tc.env, err)
	65	+	}
	66	+	})
35	67		}
36	68		t.Setenv(envCache, "0")
37	69		t.Setenv(envRegistryMax, "invalid")