crunchloop / crunchloop/devcontainer
Replace pre-pull workaround with BuildKit session + filesync
- Dominant language
- Go
- Stars
- 5
- Forks
- 0
- Avg merge
- 6h 17m
- Merged PRs (30d)
- 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)
Contributor guide
Assessment
This issue has not been assessed yet.