fix(dind): scope built image tags per job (latent concurrent-build race)

pkg/dind/buildkit_build.go

@@ -88,8 +88,10 @@ func (s *Server) handleImageBuildBuildkit(bk *buildkit.Server) http.HandlerFunc
 			}
 		}()
 
-		// 2. Translate Docker build options → SolveOpt.
-		opt, err := dockerBuildOptsToSolveOpt(r, ctxDir)
+		// 2. Translate Docker build options → SolveOpt. Tags are scoped
+		// per job (see scopedBuildRef) because every job's build lands in
+		// the one shared buildkit containerd namespace.
+		opt, err := dockerBuildOptsToSolveOpt(r, ctxDir, s.jobID)
 		if err != nil {
 			writeJSON(w, http.StatusBadRequest, map[string]string{
 				"message": fmt.Sprintf("build options: %v", err),
@@ -224,7 +226,10 @@ func extractBuildContextToTempDir(r io.Reader) (resultDir string, retErr error)
 // a BuildKit SolveOpt. This is the heart of the arch doc's "option
 // translation" table. Not complete — MVP subset: -t, -f, --target,
 // --build-arg, --no-cache, --pull, --label.
-func dockerBuildOptsToSolveOpt(r *http.Request, ctxDir string) (bkclient.SolveOpt, error) {
+//
+// jobID scopes the export tags (see scopedBuildRef) so concurrent jobs
+// don't race on identical tag names in the shared buildkit namespace.
+func dockerBuildOptsToSolveOpt(r *http.Request, ctxDir, jobID string) (bkclient.SolveOpt, error) {
 	q := r.URL.Query()
 
 	dockerfile := q.Get("dockerfile")
@@ -283,14 +288,20 @@ func dockerBuildOptsToSolveOpt(r *http.Request, ctxDir string) (bkclient.SolveOp
 	}
 
 	// Image export: write the result into ephemerd's containerd image
-	// store under the requested tag(s). buildkit's image exporter accepts
-	// a comma-separated list under "name" — register all of them so a
-	// subsequent docker push <tag> finds the image regardless of which
-	// -t the user picked. Registry push happens later via the existing
-	// pkg/dind push path, not during build.
+	// store under the requested tag(s), scoped per job so two concurrent
+	// jobs building the same tag don't overwrite each other's image
+	// record. buildkit's image exporter accepts a comma-separated list
+	// under "name" — register all of them so a subsequent docker push
+	// <tag> finds the image regardless of which -t the user picked.
+	// Registry push happens later via the existing pkg/dind push path,
+	// which applies the same scoping on lookup.
 	exportAttrs := map[string]string{}
 	if len(tags) > 0 {
-		exportAttrs["name"] = strings.Join(tags, ",")
+		scoped := make([]string, len(tags))
+		for i, t := range tags {
+			scoped[i] = scopedBuildRef(jobID, t)
+		}
+		exportAttrs["name"] = strings.Join(scoped, ",")
 	}
 	exports := []bkclient.ExportEntry{{
 		Type:  "image",

pkg/dind/buildkit_build_test.go

@@ -14,7 +14,7 @@ import (
 
 func TestDockerBuildOptsToSolveOpt_MinimalInput(t *testing.T) {
 	req := httptest.NewRequest("POST", "/build?t=alpine:local&dockerfile=Dockerfile", nil)
-	opt, err := dockerBuildOptsToSolveOpt(req, "/tmp/ctx")
+	opt, err := dockerBuildOptsToSolveOpt(req, "/tmp/ctx", "test-job")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -30,8 +30,8 @@ func TestDockerBuildOptsToSolveOpt_MinimalInput(t *testing.T) {
 	if opt.Exports[0].Type != "image" {
 		t.Errorf("export type = %q, want image", opt.Exports[0].Type)
 	}
-	if opt.Exports[0].Attrs["name"] != "alpine:local" {
-		t.Errorf("export name = %q, want alpine:local", opt.Exports[0].Attrs["name"])
+	if got, want := opt.Exports[0].Attrs["name"], "build.ephemerd.local/test-job/alpine:local"; got != want {
+		t.Errorf("export name = %q, want %q", got, want)
 	}
 	if opt.LocalDirs["context"] != "/tmp/ctx" {
 		t.Errorf("LocalDirs[context] = %q, want /tmp/ctx", opt.LocalDirs["context"])
@@ -48,7 +48,7 @@ func TestDockerBuildOptsToSolveOpt_AllOptions(t *testing.T) {
 		`&buildargs={"VERSION":"1.0","DEBUG":"true"}` +
 		`&labels={"org.ephpm.test":"yes"}`
 	req := httptest.NewRequest("POST", "/build?"+q, nil)
-	opt, err := dockerBuildOptsToSolveOpt(req, "/ctx")
+	opt, err := dockerBuildOptsToSolveOpt(req, "/ctx", "test-job")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -72,7 +72,7 @@ func TestDockerBuildOptsToSolveOpt_AllOptions(t *testing.T) {
 
 func TestDockerBuildOptsToSolveOpt_InvalidBuildargsJSON(t *testing.T) {
 	req := httptest.NewRequest("POST", "/build?buildargs=not-json", nil)
-	_, err := dockerBuildOptsToSolveOpt(req, "/ctx")
+	_, err := dockerBuildOptsToSolveOpt(req, "/ctx", "test-job")
 	if err == nil {
 		t.Fatal("want error for malformed buildargs JSON, got nil")
 	}

pkg/dind/buildscope.go

@@ -0,0 +1,62 @@
+package dind
+
+import "strings"
+
+// buildScopeRegistry is the synthetic registry hostname used to scope
+// job-built image tags inside the shared "buildkit" containerd namespace.
+// It never resolves on any network — it exists only to make the scoped
+// name parse as a valid Docker reference.
+const buildScopeRegistry = "build.ephemerd.local"
+
+// scopedBuildRef translates a user-supplied image tag into the job-scoped
+// name that build results are stored under in the shared buildkit
+// containerd namespace.
+//
+// All jobs share one BuildKit worker writing into one containerd
+// namespace ("buildkit"). Without scoping, two concurrent jobs that both
+// `docker build -t ephpm:dev` race on the same image record — last build
+// wins, and the loser pushes/loads the other job's binary. Observed in
+// the wild as an E2E matrix job asserting on PHP 8.4 and getting the
+// 8.5 build (ephpm/ephpm#67, #68).
+//
+// The transform is invisible to the workflow: the job's docker CLI keeps
+// using its own tag; only the storage name inside the buildkit namespace
+// carries the scope. Example:
+//
+//	ephpm:dev → build.ephemerd.local/ephemerd-github-ephpm-quick-mendel/ephpm:dev
+//
+// jobID comes from the runner container ID, which containerd has already
+// validated against its identifier grammar (alphanumerics, '.', '-', '_')
+// — apart from uppercase, which Docker reference paths reject, so it is
+// lowercased here. The user ref is left untouched: Docker already
+// enforces lowercase repository names at build time, and tags are
+// case-sensitive so they must not be folded. A leading registry
+// qualifier on the user tag (e.g. ghcr.io/owner/img:v1) folds into the
+// scoped path unchanged; both "ghcr.io" and underscores are valid
+// reference path components.
+func scopedBuildRef(jobID, ref string) string {
+	if jobID == "" || ref == "" {
+		return ref
+	}
+	return buildScopeRegistry + "/" + strings.ToLower(jobID) + "/" + ref
+}
+
+// pushLookupCandidates returns the ordered list of image names that a
+// docker push of fullRef should try in the buildkit namespace.
+//
+// The Linux Docker CLI canonicalizes refs with the docker.io/ registry
+// prefix before POSTing the push, but BuildKit's containerd exporter
+// stores the image under whatever short name the build's `-t` tag
+// carried (e.g. "ephpm/ephemerd:..." with no prefix). Job-scoped forms
+// are tried first — those are where this job's own builds live. The
+// unscoped fallbacks cover images staged into the buildkit namespace
+// by paths other than this job's docker build (tests, future tooling).
+func pushLookupCandidates(jobID, fullRef string) []string {
+	stripped := strings.TrimPrefix(fullRef, "docker.io/")
+	return dedup(
+		scopedBuildRef(jobID, fullRef),
+		scopedBuildRef(jobID, stripped),
+		fullRef,
+		stripped,
+	)
+}

pkg/dind/buildscope_test.go

@@ -0,0 +1,184 @@
+package dind
+
+import (
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/distribution/reference"
+)
+
+func TestScopedBuildRef(t *testing.T) {
+	tests := []struct {
+		name  string
+		jobID string
+		ref   string
+		want  string
+	}{
+		{
+			name:  "simple tag",
+			jobID: "ephemerd-github-ephpm-quick-mendel",
+			ref:   "ephpm:dev",
+			want:  "build.ephemerd.local/ephemerd-github-ephpm-quick-mendel/ephpm:dev",
+		},
+		{
+			name:  "repo path",
+			jobID: "job1",
+			ref:   "ephpm/e2e:dev",
+			want:  "build.ephemerd.local/job1/ephpm/e2e:dev",
+		},
+		{
+			name:  "registry-qualified ref folds into path",
+			jobID: "job1",
+			ref:   "ghcr.io/ephpm/ephemerd:v1",
+			want:  "build.ephemerd.local/job1/ghcr.io/ephpm/ephemerd:v1",
+		},
+		{
+			name:  "uppercase jobID is lowercased",
+			jobID: "Job-ABC",
+			ref:   "img:tag",
+			want:  "build.ephemerd.local/job-abc/img:tag",
+		},
+		{
+			name:  "case-sensitive tag preserved",
+			jobID: "job1",
+			ref:   "img:V1.2-RC",
+			want:  "build.ephemerd.local/job1/img:V1.2-RC",
+		},
+		{
+			name:  "empty jobID is passthrough",
+			jobID: "",
+			ref:   "img:tag",
+			want:  "img:tag",
+		},
+		{
+			name:  "empty ref is passthrough",
+			jobID: "job1",
+			ref:   "",
+			want:  "",
+		},
+		{
+			name:  "underscored job id",
+			jobID: "ephemerd-github-ephpm-quick_mendel",
+			ref:   "ephpm:dev",
+			want:  "build.ephemerd.local/ephemerd-github-ephpm-quick_mendel/ephpm:dev",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := scopedBuildRef(tt.jobID, tt.ref)
+			if got != tt.want {
+				t.Errorf("scopedBuildRef(%q, %q) = %q, want %q", tt.jobID, tt.ref, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestScopedBuildRef_ProducesValidReferences asserts the scoped names parse
+// under the Docker reference grammar — BuildKit's containerd image exporter
+// rejects invalid refs, so a scoping scheme that produces unparseable names
+// would break every docker build.
+func TestScopedBuildRef_ProducesValidReferences(t *testing.T) {
+	jobIDs := []string{
+		"ephemerd-github-ephpm-quick-mendel",
+		"ephemerd-github-ephpm-quick_mendel",
+		"ephemerd-github-php-sdk-noble_galileo",
+	}
+	refs := []string{
+		"ephpm:dev",
+		"ephpm-e2e:dev",
+		"ephpm/sub:v1.2.3",
+		"ghcr.io/ephpm/ephemerd:runner-ci-linux-amd64",
+		"img:UPPER-Tag_ok.1",
+	}
+	for _, j := range jobIDs {
+		for _, r := range refs {
+			scoped := scopedBuildRef(j, r)
+			if _, err := reference.ParseNormalizedNamed(scoped); err != nil {
+				t.Errorf("scoped ref %q does not parse: %v", scoped, err)
+			}
+		}
+	}
+}
+
+// TestDockerBuildOptsToSolveOpt_ScopesTags asserts the build translation
+// rewrites every -t tag into its job-scoped form, so concurrent jobs
+// building identical tags land on distinct image records in the shared
+// buildkit namespace. Regression test for the E2E matrix race where one
+// job's `docker build -t ephpm:dev` overwrote the other's and the loser
+// served the wrong PHP version (ephpm/ephpm#67, #68).
+func TestDockerBuildOptsToSolveOpt_ScopesTags(t *testing.T) {
+	r := httptest.NewRequest("POST", "/build?t=ephpm:dev&t=ephpm-e2e:dev", nil)
+	opt, err := dockerBuildOptsToSolveOpt(r, t.TempDir(), "ephemerd-github-ephpm-jobA")
+	if err != nil {
+		t.Fatalf("dockerBuildOptsToSolveOpt: %v", err)
+	}
+	if len(opt.Exports) != 1 {
+		t.Fatalf("exports = %d, want 1", len(opt.Exports))
+	}
+	got := opt.Exports[0].Attrs["name"]
+	want := "build.ephemerd.local/ephemerd-github-ephpm-joba/ephpm:dev," +
+		"build.ephemerd.local/ephemerd-github-ephpm-joba/ephpm-e2e:dev"
+	if got != want {
+		t.Errorf("export name = %q, want %q", got, want)
+	}
+}
+
+func TestPushLookupCandidates(t *testing.T) {
+	got := pushLookupCandidates("jobA", "docker.io/ephpm:dev")
+	want := []string{
+		"build.ephemerd.local/joba/docker.io/ephpm:dev",
+		"build.ephemerd.local/joba/ephpm:dev",
+		"docker.io/ephpm:dev",
+		"ephpm:dev",
+	}
+	if len(got) != len(want) {
+		t.Fatalf("candidates = %v, want %v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("candidate[%d] = %q, want %q", i, got[i], want[i])
+		}
+	}
+
+	// Scoped forms must come before unscoped: a job pushing its own build
+	// must never accidentally resolve another path's unscoped record first.
+	if !strings.HasPrefix(got[0], buildScopeRegistry+"/") || !strings.HasPrefix(got[1], buildScopeRegistry+"/") {
+		t.Error("scoped candidates must be tried before unscoped fallbacks")
+	}
+}
+
+func TestPushLookupCandidates_NoDockerPrefix(t *testing.T) {
+	got := pushLookupCandidates("jobA", "ephpm:dev")
+	want := []string{
+		"build.ephemerd.local/joba/ephpm:dev",
+		"ephpm:dev",
+	}
+	if len(got) != len(want) {
+		t.Fatalf("candidates = %v, want %v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("candidate[%d] = %q, want %q", i, got[i], want[i])
+		}
+	}
+}
+
+// TestDockerBuildOptsToSolveOpt_DistinctJobsDistinctNames is the actual
+// race condition expressed as a property: same tag, different jobs, must
+// produce different storage names.
+func TestDockerBuildOptsToSolveOpt_DistinctJobsDistinctNames(t *testing.T) {
+	mk := func(jobID string) string {
+		r := httptest.NewRequest("POST", "/build?t=ephpm:dev", nil)
+		opt, err := dockerBuildOptsToSolveOpt(r, t.TempDir(), jobID)
+		if err != nil {
+			t.Fatalf("dockerBuildOptsToSolveOpt(%s): %v", jobID, err)
+		}
+		return opt.Exports[0].Attrs["name"]
+	}
+	a := mk("ephemerd-github-ephpm-php84")
+	b := mk("ephemerd-github-ephpm-php85")
+	if a == b {
+		t.Errorf("two jobs building the same tag produced the same storage name %q — the matrix race is back", a)
+	}
+}

pkg/dind/registry.go

@@ -168,24 +168,20 @@ func (s *Server) handleImagePush(w http.ResponseWriter, r *http.Request, refPath
 	cfg, _ := s.resolveAuthForRef(r, fullRef)
 
 	// Look up the image. Buildkit puts built images in its own namespace
-	// (configured via buildkit.Config.ContainerdNamespace = "buildkit").
+	// (configured via buildkit.Config.ContainerdNamespace = "buildkit"),
+	// stored under per-job scoped names (see scopedBuildRef) so concurrent
+	// jobs building the same tag don't collide.
 	const buildkitNS = "buildkit"
 	ctx := namespaces.WithNamespace(r.Context(), buildkitNS)
 
-	// Look up the image. The Linux Docker CLI canonicalizes refs with the
-	// docker.io/ registry prefix before POSTing the push, but BuildKit's
-	// containerd exporter stores the image under whatever short name the
-	// build's `-t` tag carried (e.g. "ephpm/ephemerd:..." with no prefix).
-	// Try the original first, then strip a leading docker.io/.
-	img, err := s.client.GetImage(ctx, fullRef)
-	if err != nil {
-		if alt, ok := strings.CutPrefix(fullRef, "docker.io/"); ok {
-			if alt2, err2 := s.client.GetImage(ctx, alt); err2 == nil {
-				img, err = alt2, nil
-			}
+	var img client.Image
+	var imgErr error
+	for _, name := range pushLookupCandidates(s.jobID, fullRef) {
+		if img, imgErr = s.client.GetImage(ctx, name); imgErr == nil {
+			break
 		}
 	}
-	if err != nil {
+	if err := imgErr; err != nil {
 		writeJSON(w, http.StatusNotFound, map[string]string{
 			"message": fmt.Sprintf("image not found in buildkit namespace: %s: %v", fullRef, err),
 		})