crunchloop / crunchloop/devcontainer

Replace pre-pull workaround with BuildKit session + filesync

Ouverte
#50 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
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

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.