fix(buildkit): heal + retry builds hitting a missing cache-record lease; grace-window the dead-record sweep

pkg/buildkit/heal.go

@@ -62,7 +62,12 @@ import (
 //
 // The ID character class allows the ':' and digest hex of a chain ID as well
 // as BuildKit's base-36 record IDs.
-var danglingSnapshotRe = regexp.MustCompile(`(?:parent )?snapshot ([A-Za-z0-9][A-Za-z0-9._:+\-]*) does not exist`)
+// Tail-anchored for the same reason as danglingLeaseRe below: the genuine
+// containerd error is the wrap chain's suffix ("... does not exist: not
+// found" ends the string), while a RUN command that merely PRINTS a matching
+// phrase gets further wrapping appended ("did not complete successfully:
+// exit code ..."), so job-controlled text cannot reach the repair ladder.
+var danglingSnapshotRe = regexp.MustCompile(`(?:parent )?snapshot ([A-Za-z0-9][A-Za-z0-9._:+\-]*) does not exist(?:: not found)?$`)
 
 // SnapshotKind classifies a dangling snapshot key by who owns it, which
 // decides what repairing it involves.
@@ -137,6 +142,80 @@ func DanglingSnapshotFromError(err error) (DanglingSnapshot, bool) {
 	return ParseDanglingSnapshot(err.Error())
 }
 
+// danglingLeaseRe matches containerd's lease-not-found message as it surfaces
+// through a BuildKit solve (#193):
+//
+//	lease "4igb5uptxddpdjw9lwatd9psk": not found
+//
+// This is the LEASE half of the same "two databases describe one store"
+// corruption the dangling-snapshot signature covers: BuildKit's bbolt
+// metadata still holds a cache record while the containerd lease that record
+// owns is gone (an out-of-band `ctr leases rm`, a restored containerd data
+// dir, or a crash between a lease deletion and the matching metadata clear).
+// The record loads fine — nothing on the load path checks the lease — and
+// detonates only when a build cache-hits it and tries to attach a resource
+// to the missing lease (buildkit source/containerimage/pull.go on a cached
+// FROM, cache/blobs.go setBlob on export), failing the job with the raw
+// containerd error above.
+//
+// Deliberately narrow, for the same reason as danglingSnapshotRe: repair
+// throws away build cache. Only a quoted BuildKit cache-record lease ID — 25
+// base-36 characters (identity.NewID), optionally the -view / -variants
+// derivative — followed by containerd's exact "not found" qualifies.
+// BuildKit's temporary leases have a different shape ("<nanos>-<base64>") and
+// a missing one does not indicate a persistent desync, so they do not match.
+//
+// Anchored to the END of the error string: a genuine containerd not-found is
+// always the tail of the wrap chain, while job-influenced text (a hostile
+// registry's HTTP reason phrase, a RUN command's own output echoed into a
+// failure) is followed by more wrapping ("did not complete successfully",
+// status codes, ...). Without the anchor, a fork PR could embed a fake
+// lease line in an error it controls and trigger a shared-cache prune —
+// repair must never be reachable from attacker-influenced text.
+var danglingLeaseRe = regexp.MustCompile(`lease "([a-z0-9]{25}(?:-view|-variants)?)": not found$`)
+
+// DanglingLease identifies a containerd lease BuildKit's metadata believes in
+// and containerd no longer has.
+type DanglingLease struct {
+	// ID is the missing lease's ID from the error message — a BuildKit
+	// cache record ID, since the record's lease shares its ID.
+	ID string
+}
+
+// ParseDanglingLease extracts the missing lease ID from an error message,
+// reporting false when the message is some other failure. Pure — the whole
+// detection rule, testable without standing up BuildKit.
+func ParseDanglingLease(msg string) (DanglingLease, bool) {
+	m := danglingLeaseRe.FindStringSubmatch(msg)
+	if m == nil {
+		return DanglingLease{}, false
+	}
+	return DanglingLease{ID: m[1]}, true
+}
+
+// DanglingLeaseFromError is ParseDanglingLease over an error value. Nil
+// errors report false.
+func DanglingLeaseFromError(err error) (DanglingLease, bool) {
+	if err == nil {
+		return DanglingLease{}, false
+	}
+	return ParseDanglingLease(err.Error())
+}
+
+// HealKeyFromError returns the escalation-ladder key for err's repairable
+// shared-store signature, if it has one. It is what a caller that only needs
+// the Healer bookkeeping key (e.g. to Forget after a successful retry) should
+// use, so it stays in sync with every signature healAndRetryBuild acts on.
+func HealKeyFromError(err error) (string, bool) {
+	if d, ok := DanglingSnapshotFromError(err); ok {
+		return d.ID, true
+	}
+	if l, ok := DanglingLeaseFromError(err); ok {
+		return l.ID, true
+	}
+	return "", false
+}
+
 // chainIDRe matches an OCI layer chain ID: an algorithm, a colon and a hex
 // digest. Anything else is BuildKit's own record ID.
 var chainIDRe = regexp.MustCompile(`^[a-z0-9]+(?:[.+_-][a-z0-9]+)*:[0-9a-fA-F]{32,}$`)
@@ -211,7 +290,20 @@ type Healer struct {
 // Next reports the action to take for d and records it, so a second sighting
 // of the same key escalates instead of repeating a repair that did not work.
 func (h *Healer) Next(d DanglingSnapshot) HealAction {
-	if d.ID == "" {
+	return h.nextID(d.ID)
+}
+
+// NextLease is Next for the missing-lease signature. Lease IDs and snapshot
+// keys share one ladder map: they cannot collide (a lease ID is a full
+// 25-char record ID, a repairable snapshot key is a chain ID or record ID
+// with different provenance), and even a collision would only escalate a
+// repair one rung early.
+func (h *Healer) NextLease(l DanglingLease) HealAction {
+	return h.nextID(l.ID)
+}
+
+func (h *Healer) nextID(id string) HealAction {
+	if id == "" {
 		return HealNone
 	}
 	h.mu.Lock()
@@ -220,13 +312,13 @@ func (h *Healer) Next(d DanglingSnapshot) HealAction {
 		h.seen = map[string]HealAction{}
 	}
 	next := HealPrune
-	switch h.seen[d.ID] {
+	switch h.seen[id] {
 	case HealPrune:
 		next = HealRebuild
 	case HealRebuild, HealGiveUp:
 		next = HealGiveUp
 	}
-	h.seen[d.ID] = next
+	h.seen[id] = next
 	return next
 }
 
@@ -242,8 +334,12 @@ func (h *Healer) Forget(id string) {
 // HealReport summarises one repair attempt, for logging and for the message
 // surfaced to the job when repair fails.
 type HealReport struct {
-	// Snapshot is the key that triggered the repair.
+	// Snapshot is the key that triggered the repair, when the trigger was
+	// the dangling-snapshot signature.
 	Snapshot DanglingSnapshot
+	// Lease is the key that triggered the repair, when the trigger was the
+	// missing-lease signature. Exactly one of Snapshot and Lease is set.
+	Lease DanglingLease
 	// Action is the rung of the ladder that was executed.
 	Action HealAction
 	// ImagesEvicted counts containerd image records dropped because their
@@ -258,6 +354,10 @@ type HealReport struct {
 
 // String renders the report for a log line or an error message.
 func (r HealReport) String() string {
+	if r.Lease.ID != "" {
+		return fmt.Sprintf("lease=%s action=%s bytes_released=%d rebuilt=%t",
+			r.Lease.ID, r.Action, r.BytesReleased, r.Rebuilt)
+	}
 	return fmt.Sprintf("snapshot=%s kind=%s action=%s images_evicted=%d bytes_released=%d rebuilt=%t",
 		r.Snapshot.ID, r.Snapshot.Kind, r.Action, r.ImagesEvicted, r.BytesReleased, r.Rebuilt)
 }

pkg/buildkit/heal_test.go

@@ -104,6 +104,141 @@ func TestDanglingSnapshotFromError(t *testing.T) {
 	}
 }
 
+func TestParseDanglingLease(t *testing.T) {
+	tests := []struct {
+		name   string
+		msg    string
+		wantID string
+		wantOK bool
+	}{
+		{
+			// The #193 incident string, verbatim: a cached FROM hit a
+			// cache record whose containerd lease was gone.
+			name:   "missing cache record lease",
+			msg:    `failed to solve: lease "4igb5uptxddpdjw9lwatd9psk": not found`,
+			wantID: "4igb5uptxddpdjw9lwatd9psk",
+			wantOK: true,
+		},
+		{
+			name:   "view lease derivative",
+			msg:    `failed to solve: lease "4igb5uptxddpdjw9lwatd9psk-view": not found`,
+			wantID: "4igb5uptxddpdjw9lwatd9psk-view",
+			wantOK: true,
+		},
+		{
+			name:   "compression variants lease derivative",
+			msg:    `lease "4igb5uptxddpdjw9lwatd9psk-variants": not found`,
+			wantID: "4igb5uptxddpdjw9lwatd9psk-variants",
+			wantOK: true,
+		},
+		{
+			// A wrong positive throws away the node's build cache, so the
+			// near misses matter as much as the hits. BuildKit's temporary
+			// leases ("<nanos>-<base64>") are not a persistent desync — a
+			// retry mints a fresh one — and must not trigger a repair.
+			name:   "temporary lease id is a different failure",
+			msg:    `lease "123456789-AbC_": not found`,
+			wantOK: false,
+		},
+		{
+			// containerd's other missing-lease shape carries no ID and
+			// comes from a leased-context write, not a stale record.
+			name:   "lease does not exist is a different failure",
+			msg:    "lease does not exist: not found",
+			wantOK: false,
+		},
+		{
+			// 24 chars: not an identity.NewID (always exactly 25).
+			name:   "id of the wrong length",
+			msg:    `lease "4igb5uptxddpdjw9lwatd9ps": not found`,
+			wantOK: false,
+		},
+		{
+			name:   "missing content is a different failure",
+			msg:    `failed to solve: content digest sha256:abc: not found`,
+			wantOK: false,
+		},
+		{
+			name:   "empty",
+			msg:    "",
+			wantOK: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, ok := ParseDanglingLease(tt.msg)
+			if ok != tt.wantOK {
+				t.Fatalf("ok = %v, want %v (got %+v)", ok, tt.wantOK, got)
+			}
+			if tt.wantOK && got.ID != tt.wantID {
+				t.Errorf("ID = %q, want %q", got.ID, tt.wantID)
+			}
+		})
+	}
+}
+
+func TestDanglingLeaseFromError(t *testing.T) {
+	if _, ok := DanglingLeaseFromError(nil); ok {
+		t.Fatal("nil error reported a dangling lease")
+	}
+
+	// The signature must survive wrapping: it arrives from the solver
+	// through several layers of fmt.Errorf.
+	base := errors.New(`lease "4igb5uptxddpdjw9lwatd9psk": not found`)
+	wrapped := fmt.Errorf("build failed: %w", fmt.Errorf("failed to solve: %w", base))
+	got, ok := DanglingLeaseFromError(wrapped)
+	if !ok {
+		t.Fatal("wrapped error not recognised")
+	}
+	if got.ID != "4igb5uptxddpdjw9lwatd9psk" {
+		t.Errorf("ID = %q", got.ID)
+	}
+}
+
+func TestHealKeyFromError(t *testing.T) {
+	tests := []struct {
+		name    string
+		err     error
+		wantKey string
+		wantOK  bool
+	}{
+		{
+			name:    "dangling snapshot",
+			err:     errors.New("snapshot jfvdzwv6tyfkgcimx9uaifsfh does not exist: not found"),
+			wantKey: "jfvdzwv6tyfkgcimx9uaifsfh",
+			wantOK:  true,
+		},
+		{
+			name:    "dangling lease",
+			err:     errors.New(`failed to solve: lease "4igb5uptxddpdjw9lwatd9psk": not found`),
+			wantKey: "4igb5uptxddpdjw9lwatd9psk",
+			wantOK:  true,
+		},
+		{
+			name:   "unrelated failure",
+			err:    errors.New("no space left on device"),
+			wantOK: false,
+		},
+		{
+			name:   "nil",
+			err:    nil,
+			wantOK: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			key, ok := HealKeyFromError(tt.err)
+			if ok != tt.wantOK {
+				t.Fatalf("ok = %v, want %v", ok, tt.wantOK)
+			}
+			if key != tt.wantKey {
+				t.Errorf("key = %q, want %q", key, tt.wantKey)
+			}
+		})
+	}
+}
+
 func TestClassifySnapshotKey(t *testing.T) {
 	tests := []struct {
 		id   string
@@ -160,6 +295,33 @@ func TestHealerIgnoresEmptyID(t *testing.T) {
 	if got := h.Next(DanglingSnapshot{}); got != HealNone {
 		t.Fatalf("empty ID = %v, want HealNone", got)
 	}
+	if got := h.NextLease(DanglingLease{}); got != HealNone {
+		t.Fatalf("empty lease ID = %v, want HealNone", got)
+	}
+}
+
+func TestHealerLeaseLadder(t *testing.T) {
+	// The missing-lease signature climbs the same prune → rebuild → give-up
+	// ladder as the snapshot one, keyed by the lease ID.
+	var h Healer
+	l := DanglingLease{ID: "4igb5uptxddpdjw9lwatd9psk"}
+
+	if got := h.NextLease(l); got != HealPrune {
+		t.Fatalf("first = %v, want HealPrune", got)
+	}
+	if got := h.NextLease(l); got != HealRebuild {
+		t.Fatalf("second = %v, want HealRebuild", got)
+	}
+	if got := h.NextLease(l); got != HealGiveUp {
+		t.Fatalf("third = %v, want HealGiveUp", got)
+	}
+
+	// Forget re-arms the ladder after a successful retry, via the same key
+	// HealKeyFromError hands the build handler.
+	h.Forget(l.ID)
+	if got := h.NextLease(l); got != HealPrune {
+		t.Fatalf("after Forget = %v, want HealPrune", got)
+	}
 }
 
 func TestHealerConcurrent(t *testing.T) {
@@ -237,3 +399,36 @@ func dirExists(p string) bool {
 	st, err := os.Stat(p)
 	return err == nil && st.IsDir()
 }
+
+// TestMatchersRejectJobInfluencedText pins the tail anchoring and the
+// near-miss shapes: the repair ladder throws away shared build cache, so it
+// must be unreachable from any error string a job can influence, and from
+// lease shapes that are not cache-record leases. A future "loosen the regex"
+// edit must fail here, not in production.
+func TestMatchersRejectJobInfluencedText(t *testing.T) {
+	reject := []struct{ name, msg string }{
+		// Attacker-influenced text is never the tail of the wrap chain —
+		// buildkit appends its own wrapping after job output / registry
+		// reason phrases. The anchor is what rejects these.
+		{"registry reason phrase gets wrapped",
+			`failed to solve: failed to load cache key: unexpected status from HEAD request to https://evil.example/v2/x/manifests/latest: 404 lease "aaaaaaaaaaaaaaaaaaaaaaaaa": not found (server message)`},
+		{"RUN output echoed into a failure gets wrapped",
+			`process "/bin/sh -c echo snapshot deadbeef does not exist: not found; false" did not complete successfully: exit code: 1`},
+		// Lease shapes that are not cache-record leases.
+		{"temporary lease shape", `lease "1755846719000000000-aBcD": not found`},
+		{"history lease ref_ prefix", `lease "ref_4igb5uptxddpdjw9lwatd9psk": not found`},
+		{"26-char id", `lease "4igb5uptxddpdjw9lwatd9pskx": not found`},
+		{"24-char id", `lease "4igb5uptxddpdjw9lwatd9ps": not found`},
+		{"id-less lease error", `lease does not exist: not found`},
+	}
+	for _, c := range reject {
+		t.Run(c.name, func(t *testing.T) {
+			if _, ok := ParseDanglingLease(c.msg); ok {
+				t.Fatalf("ParseDanglingLease matched %q, must reject", c.msg)
+			}
+			if _, ok := ParseDanglingSnapshot(c.msg); ok {
+				t.Fatalf("ParseDanglingSnapshot matched %q, must reject", c.msg)
+			}
+		})
+	}
+}