crunchloop / crunchloop/devcontainer
Replace pre-pull workaround with BuildKit session + filesync
- Langage dominant
- Go
- Étoiles
- 5
- Forks
- 0
- Merge moyen
- 6 h 17 min
- PR mergées (30 j)
- 15
Description
## Context
#47 switched the docker runtime to BuildKit (`Version: build.BuilderBuildKit`) to escape the classic-builder authz-plugin pathology, but stopped short of the canonical BuildKit integration. The short-of-canonical version works (DAP is unblocked, builds are fast) but carries three real limitations that a session + filesync integration would close together.
The deferred work in #47 was scoped intentionally to avoid pulling `github.com/moby/buildkit` in as a direct dep (~100+ transitive packages: containerd, grpc, opentelemetry-extras). This issue tracks the followup.
## Limitations to address
### 1. Per-vertex progress events are dropped
Under BuildKit, dockerd emits `moby.buildkit.trace` records of the form `{"id":"moby.buildkit.trace","aux":""}` carrying buildkit's `SolveStatus`. `runtime/docker/build.go:streamBuildOutput` ignores them (the previous `stream` / `status` JSON shape is classic-builder-only), so:
- No `BuildLayerEvent` fires during the build
- No `BuildLogEvent` fires for `RUN` output
- Consumers see `BuildStart` → silence → `BuildCompleted`
The aux payloads are protobuf-encoded; decoding requires either pulling in `github.com/moby/buildkit` (which has the schema and a `progressui` renderer) or hand-rolling the proto definitions.
### 2. `extractBaseImages` can't resolve some Dockerfiles
`runtime/docker/build.go:extractBaseImages` does a naive line-based parse to find FROM image references for pre-pulling. It handles literal FROMs, ARG defaults, and BuildSpec.Args overrides — but silently drops anything whose ref still contains `$` after substitution, including:
- Runtime-supplied ARGs without defaults (the daemon resolves at build time, we don't see the value)
- Dynamic refs computed by buildx/buildkit's parser features
- Refs constructed from multiple ARGs we don't fully evaluate
When the parser drops a ref, the build falls through to BuildKit's "no active sessions" error rather than getting pre-pulled — same failure mode as before #47 for these cases. Rare in our generated Dockerfiles (`useruid.go`, `feature/dockerfile.go` produce simple, parseable FROMs) but a sharp edge for user-supplied dockerfile-source contexts.
### 3. Custom `tarDirectory` is still load-bearing
`runtime/docker/build.go:tarDirectory` is our hand-rolled context packer. #47 fixed the symlink-Linkname bug, but every other tar-encoding corner case is on us:
- `.dockerignore` is **not honored** at all
- UID/GID mapping for cross-user builds
- Hardlinks (we follow them as regular files)
- Sparse files
- Very large contexts (the io.Pipe approach is bounded, but no streaming-progress signal)
## Canonical fix
Open a buildkit session with:
- `session.Session` (`github.com/moby/buildkit/session`)
- `auth.Provider` registered on the session — even a stub returning empty creds satisfies BuildKit's "session required for remote resolution" check, eliminating the need for pre-pulling
- `filesync.FSSyncProvider` rooted at `spec.ContextPath`, with `RemoteContext: \"client-session\"` and `SessionID: sess.ID()` in `ImageBuildOptions` — replaces our `tarDirectory` entirely, handles `.dockerignore`
- aux-record decoder in `streamBuildOutput` — emits `BuildLayerEvent` per vertex transition and `BuildLogEvent` per vertex log line
Reference implementation: docker/cli's `cli/command/image/build.go` does exactly this. The auth provider there is `cli/command/registry.NewStaticCredentialsStore` wrapped in `cli/command/image/build.NewSessionAuthProvider`; for our usage we can use a stub with no creds since we already pass `AuthConfigs` separately when needed.
## Acceptance
- Per-step `BuildLayerEvent` and `BuildLogEvent` fire during a BuildKit build
- `extractBaseImages` is deleted (no longer needed — BuildKit resolves with the session)
- `prePullBaseImages` is deleted
- `tarDirectory` is deleted (replaced by buildkit's filesync provider)
- `.dockerignore` is honored on user-supplied build contexts
- `runtime/docker/build_test.go` parser tests can be deleted; new tests cover session lifecycle (open, cancel-on-ctx, cleanup)
## Cost
Adds `github.com/moby/buildkit` as a direct dep. Transitive deps include `containerd`, `grpc`, `opentelemetry-contrib` extras. Concrete size impact should be measured (`go mod why -m all | wc -l` before/after) before committing to the change.
## Repo / file pointers
- `runtime/docker/build.go:35-90` — `BuildImage` (the change site)
- `runtime/docker/build.go:107-166` — `streamBuildOutput` (rewrite for aux records)
- `runtime/docker/build.go:168-232` — `tarDirectory` (delete)
- `runtime/docker/build.go:234-410` — `prePullBaseImages`, `extractBaseImages`, helpers (delete)
- `runtime/docker/build_test.go` — parser tests (delete; add session-lifecycle tests)
## Severity
Low — DAP is unblocked, builds are fast, observability is degraded-but-not-broken. Worth doing before we have a consumer that depends on per-vertex progress events or hits a Dockerfile our parser can't resolve.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Guide de contribution
Ouvrir le guide de contribution
Piste de recherche
Commencez dans runtime/docker/build.go:35-90 et lisez l’implémentation de la session image-build de docker/cli, puis examinez streamBuildOutput aux lignes 107-166. Exécutez les tests existants de runtime/docker/build_test.go et mesurez la croissance des dépendances avec go mod why -m all | wc -l ; le travail est terminé lorsque les tests du cycle de vie de la session passent, que .dockerignore est respecté, que des événements par vertex sont émis et que les anciens chemins pre-pull, parser et tar sont supprimés.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- docker, go
- Domaine
- build-system, devops
- Type d'issue
- Refactorisation
- Difficulté
- 5/5
- Temps estimé
- Plus d'une semaine
- Activité
- Calme
- Clarté
- Clairement spécifiée
- Accessibilité débutants
- 35/100