diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d3c773..a7c8dfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,60 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.5.2] - 2026-10-28 + +### Fixed + +- **compose** — `Up` on a compose-source devcontainer whose containers + already exist no longer fetches every feature from its registry and + rebuilds the final image. The primary container now carries a + `dev.containers.compose-inputs-hash` label (`LabelComposeInputsHash`): + a digest of the container-shaping part of the resolved + `devcontainer.json` (features as requested, with options; local feature + content), the loaded compose project, the primary service's base image + ID and the engine options that reshape the project. When it matches, + `Up` reattaches as the image path's `attachExisting` does: it reads the + merged metadata from the existing image's `devcontainer.metadata` label + (entrypoint wrapper, mounts including the v0.5.1 volume/tmpfs fix, env, + privileged, capAdd, lifecycle commands) and starts the existing services + from their existing images — native under the caller's `AdoptExisting`, + shell-out with `up --no-recreate`. A restart therefore needs no + registry: before, a docker-in-docker compose workspace whose host had no + route to ghcr.io failed every start after a stop. Any mismatch (changed + config or compose file, new base image, a moved image tag, `Recreate`, + `PullAlways`) takes the full path as before. Upgrade note: containers + created by an earlier version have no label, so they take the full path. + The label is part of the primary service's config, so the native backend + without `AdoptExisting` recreates such a container on its first `Up` + after upgrading (and labels it); with `AdoptExisting`, or on the + shell-out backend, the container is kept unlabelled and keeps taking the + full path until it is recreated. +- **features** — the `devcontainer.metadata` label written into feature + images kept `$` unescaped, so the Dockerfile parser expanded + `${devcontainerId}` to nothing: docker-in-docker's + `dind-var-lib-docker-${devcontainerId}` mount was recorded as + `dind-var-lib-docker-`. Any later read of the label (reattach, or the + image used as a base) lost the per-workspace volume name. The label is + now written literally, so a feature image whose metadata contains `$` gets + a new digest when it is next built. +- **features** — OCI feature lookups are local-first where the answer + cannot change: a digest reference, or an exact `X.Y.Z` version tag + resolved before, is served from the disk cache without contacting the + registry. Floating tags (`2`, `2.17`, `latest`) are still resolved + online; when the registry is unreachable (no HTTP response), the last + recorded resolution is used if cached. A registry that answers with an + error is authoritative. Extraction is now atomic (temporary directory, + then rename). Policy: `design/features.md` §8.1. + +### Added + +- `LabelComposeInputsHash` (`dev.containers.compose-inputs-hash`), stamped + on the primary container of a compose-source devcontainer. +- `runtime.ContainerDetails.ImageRef`: the image reference a container was + created from (docker's `Config.Image`). +- `feature.DiskStoreOptions.OCITransport`: the HTTP transport for OCI + registry calls. + ## [0.5.1] - 2026-10-27 ### Fixed @@ -613,7 +667,8 @@ shelling out to `@devcontainers/cli`. - `events` is doc-tagged **experimental** until v1.0.0 — type shapes may evolve without a SemVer-major bump. -[Unreleased]: https://github.com/crunchloop/devcontainer/compare/v0.5.1...HEAD +[Unreleased]: https://github.com/crunchloop/devcontainer/compare/v0.5.2...HEAD +[0.5.2]: https://github.com/crunchloop/devcontainer/compare/v0.5.1...v0.5.2 [0.5.1]: https://github.com/crunchloop/devcontainer/compare/v0.5.0...v0.5.1 [0.5.0]: https://github.com/crunchloop/devcontainer/compare/v0.4.3...v0.5.0 [0.4.3]: https://github.com/crunchloop/devcontainer/compare/v0.4.2...v0.4.3 diff --git a/README.md b/README.md index 54dbbf8..1d7846a 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not | --- | --- | --- | | `image` | ✅ | Pull, run, exec | | `build` (`dockerfile`, `context`, `args`, `target`, `cacheFrom`) | ✅ | User Dockerfile + features layered atop | -| `dockerComposeFile`, `service`, `runServices` | ✅ | compose-go parse + either shell-out to `docker compose` (default) or an in-process orchestrator (`EngineOptions.ComposeBackend = ComposeBackendNative`). The native orchestrator drives any `Runtime` implementing the compose primitives. | +| `dockerComposeFile`, `service`, `runServices` | ✅ | compose-go parse + either shell-out to `docker compose` (default) or an in-process orchestrator (`EngineOptions.ComposeBackend = ComposeBackendNative`). The native orchestrator drives any `Runtime` implementing the compose primitives. An `Up` whose containers exist from unchanged inputs restarts them from their existing images, with the feature metadata read from the image label: no feature fetch, no rebuild. | **Container config** @@ -69,7 +69,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not | Field | Status | Notes | | --- | --- | --- | -| `features` (OCI / HTTPS / local) | ✅ | DAG ordering, options validation, content-addressed cache | +| `features` (OCI / HTTPS / local) | ✅ | DAG ordering, options validation, content-addressed cache (local-first for digests and exact `X.Y.Z` versions; floating tags fall back to the last resolution when the registry is unreachable) | | `dependsOn` / `installsAfter` / `overrideFeatureInstallOrder` | ✅ | | | Pre-baked-image short-circuit | ✅ | `devcontainer.metadata` label read on Up | | `devcontainer-lock.json` | ❌ | [#26](https://github.com/crunchloop/devcontainer/issues/26) | diff --git a/design/compose-native.md b/design/compose-native.md index df103b0..c334673 100644 --- a/design/compose-native.md +++ b/design/compose-native.md @@ -399,6 +399,59 @@ Input: DownPlan { ProjectName string, RemoveImages bool, RemoveVolumes bool } service that didn't become healthy. The dependent services are not started. +### 5.4 Engine-level reattach (both backends) + +The orchestrator's config-hash reuse (§5.1) decides per container, but +it runs after the engine has already prepared the primary image: before +v0.5.2 every compose `Up` fetched every feature from its registry and +rebuilt `dc-go-final-`, even when the project's containers already +existed. A restart therefore needed the feature registry, and failed +without it. The image path never had this cost: `attachExisting` reads +the merged metadata from the existing container's image label. + +`Engine.Up` now does the same for compose sources (`reattachCompose`): + +1. The full path stamps `dev.containers.compose-inputs-hash` on the + primary container. The hash covers the inputs fixed before any + feature is fetched: the container-shaping part of the resolved + `devcontainer.json` (workspace and mounts, users, + `updateRemoteUserUID`, `containerEnv`, `runArgs`, init, privileged, + capAdd, securityOpt, overrideCommand, and the features as requested + with their options, plus a content digest of each local feature), + the loaded compose project (services, networks, volumes, secrets, + configs), the local image ID of the primary service's base image, + the engine options that reshape the project + (`ComposeBackend`, `DisableHostPortPublishing`, + `StrictFeatureVersionMatch`) and the workspace folder's owner. + Lifecycle commands, `remoteEnv`, ports attributes and + customizations are left out: `Up` re-reads them every call, and they + never reach the container or its image. +2. On a later `Up` with an existing container (and neither `Recreate`, + `PullAlways` nor an `Engine.Build` override), the engine finds the + primary container by project, service and devcontainer id, and + reattaches when its label equals the freshly computed hash and the + image tag it was created from still names the image it runs. +3. Reattach parses the image's `devcontainer.metadata` label, merges it + into the resolved config exactly as `attachExisting` does, rebuilds + the run override (entrypoint wrapper, every mount type, env, + privileged, capAdd, …) and brings the project up through the + selected backend against the existing images: native under the + caller's `AdoptExisting`, skipping sidecar builds whose image is + present; shell-out with `up -d --no-recreate`. Because the image is + the same, the orchestrator's per-service config hash matches too, so + a native `Up` without `AdoptExisting` reuses the containers instead + of recreating them. +4. Any other case — no container, no label (containers created before + v0.5.2), a different hash, a moved tag — takes the full path, + unchanged. `AdoptExisting` keeps its meaning: it governs whether the + orchestrator may reuse a container whose config drifted; reattach + only removes the fetch and rebuild in front of it. + +Like the reference CLI starting an existing container, reattach does +not re-resolve floating feature tags or look at the build context of a +`build:` primary (only its definition and the base image ID); a caller +that wants either picked up passes `Recreate`. + ## 6. Mapping to `runtime.Runtime` methods Same shape as `runtime-applecontainer.md` §8. @@ -1126,6 +1179,15 @@ Resolved during this design draft (2026-05-14): (upstream `#286`), the capability flag flips to true and the warning self-suppresses. +14. **Compose `Up` reattaches without fetching features + (2026-10-28).** §5.4. An existing project whose stored inputs hash + matches is restarted from its existing images with the metadata + read from the primary image's label, as the image path's + `attachExisting` does. Motivation: a restart of a compose workspace + with features needed the feature registry (and paid a final-image + rebuild) on every start; a workspace without registry egress could + not restart at all. + --- ## Appendix A: relationship to `compose.md` diff --git a/design/features.md b/design/features.md index 9d2d496..a249e9d 100644 --- a/design/features.md +++ b/design/features.md @@ -314,14 +314,35 @@ features/ sha256-/ blob.tgz extracted/... - index.json # ref → digest map with TTL hints + tags/ + .json # {"ref": "", "digest": "sha256:…"} ``` Cache key is **content-addressed**: OCI digest or HTTPS body sha256. A -mutable tag (`:1`) resolves through `index.json` to the digest, then -hits the disk cache. This is intentionally different from devpod's -"hash-the-ref-string" scheme — content-addressed dedupes across mirror -URLs and prevents stale-ref bugs. +tag resolves to a digest, then hits the disk cache. This is +intentionally different from devpod's "hash-the-ref-string" scheme — +content-addressed dedupes across mirror URLs and prevents stale-ref +bugs. Extraction goes to a temporary directory in the entry and is +renamed into place, so a partial extraction is never taken for a hit. + +### 8.1 OCI resolution policy (v0.5.2) + +The cache is only useful offline if the registry is not asked first. +`DiskStore` decides per reference form: + +| Reference | Policy | +| --- | --- | +| `name@sha256:…` | Local first: a cached extraction of the digest is used with no registry call. | +| `name:X.Y.Z` (exact version) | Local first: the digest recorded under `tags/` the last time this tag was resolved is used with no registry call, if its extraction is cached. Published versions are immutable by the publishing convention (the reference CLI publishes `X.Y.Z` once and moves only `X`, `X.Y` and `latest`). | +| `name:X`, `name:X.Y`, `name:latest`, any other tag | Online first, so a moved tag is picked up. If the registry is **unreachable** (no HTTP response: DNS, connect, TLS, timeout), the recorded resolution is used if its extraction is cached. | + +A registry that answers with an error (404, 401, 403, …) is +authoritative: its error is returned even when a resolution is +recorded. With nothing recorded or the content evicted, an unreachable +registry fails the fetch as before. Every successful online fetch of a +tag rewrites its record (write to a temporary file, then rename). HTTPS +features are unchanged: their content hash is only known after the +download. Concurrency: a small lockfile (`extracted/.lock`) per cache entry so parallel `Resolve` calls for the same ref don't race. diff --git a/feature/disk_store.go b/feature/disk_store.go index fe4abfc..f0396d8 100644 --- a/feature/disk_store.go +++ b/feature/disk_store.go @@ -29,6 +29,12 @@ type DiskStoreOptions struct { // fetches. Useful for Authorization on private servers. HTTPSHeaders map[string]string + // OCITransport overrides the HTTP transport for OCI registry + // calls. Nil uses go-containerregistry's default (which honors the + // standard proxy environment variables). Tests use it to make the + // registry unreachable; callers may use it for a custom proxy. + OCITransport http.RoundTripper + // HTTPSClient overrides the default *http.Client used for HTTPS // fetches. If nil, the package's default client (5-minute timeout) // is used. Tests substitute this to drive httptest servers. @@ -41,6 +47,7 @@ type DiskStoreOptions struct { type DiskStore struct { cacheDir string ociKeychain authn.Keychain + ociTransport http.RoundTripper httpsHeaders map[string]string httpsClient *http.Client } @@ -66,6 +73,7 @@ func NewDiskStore(opts DiskStoreOptions) (*DiskStore, error) { return &DiskStore{ cacheDir: dir, ociKeychain: kc, + ociTransport: opts.OCITransport, httpsHeaders: opts.HTTPSHeaders, httpsClient: opts.HTTPSClient, }, nil diff --git a/feature/dockerfile.go b/feature/dockerfile.go index 8b26940..0806c67 100644 --- a/feature/dockerfile.go +++ b/feature/dockerfile.go @@ -160,8 +160,7 @@ func generateDockerfile(plan BuildPlan) (string, error) { if err != nil { return "", fmt.Errorf("build metadata label: %w", err) } - // Wrap the JSON in strconv-quoted form so embedded "/'/$ are unambiguous. - fmt.Fprintf(&b, "LABEL %s=%s\n\n", MetadataLabel, strconv.Quote(string(label))) + fmt.Fprintf(&b, "LABEL %s=%s\n\n", MetadataLabel, dockerfileLiteral(string(label))) user := "root" if plan.RemoteUser != "" { @@ -239,3 +238,16 @@ func copyFileContents(src, dst string, mode os.FileMode) error { } return out.Close() } + +// dockerfileLiteral quotes s as a double-quoted Dockerfile word that +// evaluates to s exactly. strconv.Quote handles quotes and +// backslashes, but the Dockerfile parser still expands $VAR and +// ${VAR} inside double quotes, so every $ is escaped as well. Without +// it a metadata label carrying a feature's +// "dind-var-lib-docker-${devcontainerId}" mount was stored as +// "dind-var-lib-docker-" (the build has no such variable), and any +// later read of the label — a reattach, or this image as the base of +// another build — lost the per-workspace volume name. +func dockerfileLiteral(s string) string { + return strings.ReplaceAll(strconv.Quote(s), "$", `\$`) +} diff --git a/feature/dockerfile_test.go b/feature/dockerfile_test.go index 28ca8d6..6f12f13 100644 --- a/feature/dockerfile_test.go +++ b/feature/dockerfile_test.go @@ -301,3 +301,52 @@ func TestParseMetadataLabel_Empty(t *testing.T) { t.Errorf("got %v, want nil", got) } } + +// The metadata LABEL must carry ${...} literally: the Dockerfile parser +// expands variables inside double quotes, so an unescaped +// "dind-var-lib-docker-${devcontainerId}" mount source was stored as +// "dind-var-lib-docker-" and every later read of the label lost the +// per-workspace volume name. +func TestGenerateBuildContext_LabelKeepsDollarLiteral(t *testing.T) { + f := fixtureFeature(t, "dind", "2.0") + f.Metadata.Mounts = []config.Mount{{ + Type: config.MountVolume, + Source: "dind-var-lib-docker-${devcontainerId}", + Target: "/var/lib/docker", + }} + dst := t.TempDir() + if err := GenerateBuildContext(BuildPlan{BaseImage: "alpine:3.20", Features: []config.ResolvedFeature{f}}, dst); err != nil { + t.Fatalf("GenerateBuildContext: %v", err) + } + df, err := os.ReadFile(filepath.Join(dst, "Dockerfile")) + if err != nil { + t.Fatal(err) + } + var label string + for _, line := range strings.Split(string(df), "\n") { + if strings.HasPrefix(line, "LABEL "+MetadataLabel+"=") { + label = line + } + } + if label == "" { + t.Fatalf("no metadata LABEL line in:\n%s", df) + } + if strings.Count(label, "$") != strings.Count(label, `\$`) { + t.Errorf("LABEL has an unescaped $: %s", label) + } + if !strings.Contains(label, `dind-var-lib-docker-\${devcontainerId}`) { + t.Errorf("LABEL lost the mount source variable: %s", label) + } +} + +func TestDockerfileLiteral(t *testing.T) { + for in, want := range map[string]string{ + `plain`: `"plain"`, + `a${b}c`: `"a\${b}c"`, + `$HOME and "q" \x`: `"\$HOME and \"q\" \\x"`, + } { + if got := dockerfileLiteral(in); got != want { + t.Errorf("dockerfileLiteral(%q) = %s, want %s", in, got, want) + } + } +} diff --git a/feature/oci.go b/feature/oci.go index 05ffaee..f669a03 100644 --- a/feature/oci.go +++ b/feature/oci.go @@ -2,30 +2,76 @@ package feature import ( "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" "fmt" "os" "path/filepath" + "regexp" "strings" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) // devContainerFeatureMediaType is the OCI artifact config media type // used by the devcontainers spec for feature publication. const devContainerFeatureMediaType = "application/vnd.devcontainers" +// OCI cache policy. A feature artifact is content-addressed, so a +// cached copy is exactly as good as a download of the same digest; the +// only question is when the registry must be asked which digest a +// reference means. DiskStore answers it like this: +// +// - Digest reference (name@sha256:…): local first. A cached copy of +// that digest is used without contacting the registry. +// - Exact version tag (name:X.Y.Z): local first. Published feature +// versions are immutable by the spec's publishing convention (the +// reference CLI publishes X.Y.Z once and moves only the floating +// X, X.Y and latest tags), so the digest recorded the last time +// this tag was resolved is used without contacting the registry. +// - Floating tag (name:X, name:X.Y, name:latest, any other tag): +// online first, as before, so a moved tag is picked up. If the +// registry cannot be reached at all — no HTTP response: DNS, +// connect, TLS or timeout failures — the digest recorded the last +// time this tag was resolved is used. A registry that answers with +// an error (not found, unauthorized, …) is authoritative and its +// error is returned, as is any failure when nothing is recorded. +// +// Tag resolutions are recorded under /oci/tags/ after every +// successful online fetch. The recorded digest is only used when its +// extracted content is still in the cache. func (s *DiskStore) fetchOCI(ctx context.Context, ref string) (Fetched, error) { parsed, err := name.ParseReference(ref) if err != nil { return Fetched{}, fmt.Errorf("parse OCI ref %q: %w", ref, err) } + repo := parsed.Context().Name() - img, err := remote.Image(parsed, - remote.WithContext(ctx), - remote.WithAuthFromKeychain(s.ociKeychain), - ) + switch r := parsed.(type) { + case name.Digest: + if f, ok := s.cachedOCI(repo, r.DigestStr()); ok { + return f, nil + } + case name.Tag: + if isExactVersionTag(r.TagStr()) { + if f, ok := s.cachedTag(r); ok { + return f, nil + } + } + } + + img, err := remote.Image(parsed, s.remoteOptions(ctx)...) if err != nil { + if tag, isTag := parsed.(name.Tag); isTag && ctx.Err() == nil && registryUnreachable(err) { + if f, ok := s.cachedTag(tag); ok { + return f, nil + } + } return Fetched{}, fmt.Errorf("fetch OCI image %s: %w", ref, err) } @@ -41,20 +87,137 @@ func (s *DiskStore) fetchOCI(ctx context.Context, ref string) (Fetched, error) { if err != nil { return Fetched{}, fmt.Errorf("OCI digest: %w", err) } - pinned := parsed.Context().Name() + "@" + digest.String() + f, err := s.extractOCI(ref, repo, digest, img) + if err != nil { + return Fetched{}, err + } + if tag, isTag := parsed.(name.Tag); isTag { + // Best effort: a failed record only costs a later offline or + // local-first lookup, never this fetch. + _ = s.recordTag(tag, digest.String()) + } + return f, nil +} + +// remoteOptions are the go-containerregistry options for every OCI +// registry call. +func (s *DiskStore) remoteOptions(ctx context.Context) []remote.Option { + opts := []remote.Option{ + remote.WithContext(ctx), + remote.WithAuthFromKeychain(s.ociKeychain), + } + if s.ociTransport != nil { + opts = append(opts, remote.WithTransport(s.ociTransport)) + } + return opts +} + +// exactVersionTag matches a full semantic version with no pre-release +// or build suffix: the only tag form DiskStore treats as immutable. +var exactVersionTag = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+$`) + +func isExactVersionTag(tag string) bool { return exactVersionTag.MatchString(tag) } + +// registryUnreachable reports whether err means no HTTP response came +// back from the registry. A *transport.Error carries a response status +// (the registry answered), so it is not an offline condition. +func registryUnreachable(err error) bool { + var te *transport.Error + return !errors.As(err, &te) +} + +// ociEntryDir is the cache entry for one feature digest: +// /oci/sha256-. +func (s *DiskStore) ociEntryDir(hex string) string { + return filepath.Join(s.cacheDir, "oci", "sha256-"+hex) +} + +// cachedOCI returns the cached, extracted feature for repo@digest, if +// the cache holds a complete extraction of it. +func (s *DiskStore) cachedOCI(repo, digest string) (Fetched, bool) { + h, err := v1.NewHash(digest) + if err != nil || h.Algorithm != "sha256" { + return Fetched{}, false + } + extractedDir := filepath.Join(s.ociEntryDir(h.Hex), "extracted") + if _, err := os.Stat(filepath.Join(extractedDir, installScript)); err != nil { + return Fetched{}, false + } + meta, err := parseMetadata(extractedDir) + if err != nil { + return Fetched{}, false + } + return Fetched{Dir: extractedDir, ResolvedRef: repo + "@" + h.String(), Metadata: meta}, true +} + +// tagRecord is the on-disk record of one tag resolution. +type tagRecord struct { + Ref string `json:"ref"` + Digest string `json:"digest"` +} + +func (s *DiskStore) tagRecordPath(tag name.Tag) string { + sum := sha256.Sum256([]byte(tag.Name())) + return filepath.Join(s.cacheDir, "oci", "tags", hex.EncodeToString(sum[:])+".json") +} + +// cachedTag returns the cached feature for the digest tag last +// resolved to, if both the record and its content are present. +func (s *DiskStore) cachedTag(tag name.Tag) (Fetched, bool) { + b, err := os.ReadFile(s.tagRecordPath(tag)) + if err != nil { + return Fetched{}, false + } + var rec tagRecord + if err := json.Unmarshal(b, &rec); err != nil || rec.Ref != tag.Name() { + return Fetched{}, false + } + return s.cachedOCI(tag.Context().Name(), rec.Digest) +} + +// recordTag records that tag resolved to digest, atomically (write a +// temporary file, then rename over the record). +func (s *DiskStore) recordTag(tag name.Tag, digest string) error { + path := s.tagRecordPath(tag) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + b, err := json.Marshal(tagRecord{Ref: tag.Name(), Digest: digest}) + if err != nil { + return err + } + tmp, err := os.CreateTemp(filepath.Dir(path), ".tag-*") + if err != nil { + return err + } + if _, err := tmp.Write(b); err != nil { + tmp.Close() + os.Remove(tmp.Name()) + return err + } + if err := tmp.Close(); err != nil { + os.Remove(tmp.Name()) + return err + } + if err := os.Rename(tmp.Name(), path); err != nil { + os.Remove(tmp.Name()) + return err + } + return nil +} + +// extractOCI returns the cached extraction of digest, or downloads and +// extracts img's single layer into the cache. +func (s *DiskStore) extractOCI(ref, repo string, digest v1.Hash, img v1.Image) (Fetched, error) { + if f, ok := s.cachedOCI(repo, digest.String()); ok { + return f, nil + } + pinned := repo + "@" + digest.String() // Cache layout: /oci/sha256-/extracted/ entryDir := filepath.Join(s.cacheDir, "oci", "sha256-"+digest.Hex) extractedDir := filepath.Join(entryDir, "extracted") - // Cache hit: extracted dir already has the metadata file. - if _, err := os.Stat(filepath.Join(extractedDir, metadataFile)); err == nil { - meta, err := parseMetadata(extractedDir) - if err == nil { - return Fetched{Dir: extractedDir, ResolvedRef: pinned, Metadata: meta}, nil - } - } - if err := os.MkdirAll(entryDir, 0o755); err != nil { return Fetched{}, fmt.Errorf("mkdir %s: %w", entryDir, err) } @@ -77,15 +240,29 @@ func (s *DiskStore) fetchOCI(ctx context.Context, ref string) (Fetched, error) { } defer rc.Close() - if err := os.RemoveAll(extractedDir); err != nil { - return Fetched{}, err + // Extract beside the entry and rename into place, so the cache + // never holds a partial extraction that the local-first lookups + // above would take for a complete one. + tmpDir, err := os.MkdirTemp(entryDir, ".extract-*") + if err != nil { + return Fetched{}, fmt.Errorf("mkdir extract tmpdir: %w", err) } - if err := extractTar(rc, extractedDir); err != nil { + defer os.RemoveAll(tmpDir) + if err := extractTar(rc, tmpDir); err != nil { return Fetched{}, fmt.Errorf("extract OCI feature %s: %w", ref, err) } - if _, err := os.Stat(filepath.Join(extractedDir, installScript)); err != nil { + if _, err := os.Stat(filepath.Join(tmpDir, installScript)); err != nil { return Fetched{}, fmt.Errorf("OCI feature %s: missing %s after extract: %w", ref, installScript, err) } + if _, err := parseMetadata(tmpDir); err != nil { + return Fetched{}, err + } + if err := os.RemoveAll(extractedDir); err != nil { + return Fetched{}, err + } + if err := os.Rename(tmpDir, extractedDir); err != nil { + return Fetched{}, fmt.Errorf("install extracted feature %s: %w", ref, err) + } meta, err := parseMetadata(extractedDir) if err != nil { return Fetched{}, err diff --git a/feature/oci_cache_test.go b/feature/oci_cache_test.go new file mode 100644 index 0000000..96b7d10 --- /dev/null +++ b/feature/oci_cache_test.go @@ -0,0 +1,288 @@ +package feature + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "io" + "log" + "net" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync/atomic" + "testing" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" + + "github.com/crunchloop/devcontainer/config" +) + +// Tests for the OCI cache policy documented on fetchOCI: digest refs +// and exact version tags are local-first, floating tags are online +// first with an offline fallback to the recorded resolution, and a +// registry that answers with an error is authoritative. + +// registryMode selects what gatedTransport does with a request. +type registryMode int32 + +const ( + modeOnline registryMode = iota // pass through + modeUnreachable // fail like a refused connection + modeDenied // answer 403 for everything +) + +// gatedTransport counts registry calls and can make the registry +// unreachable or have it refuse every request. +type gatedTransport struct { + mode atomic.Int32 + calls atomic.Int64 +} + +func (g *gatedTransport) RoundTrip(req *http.Request) (*http.Response, error) { + g.calls.Add(1) + switch registryMode(g.mode.Load()) { + case modeUnreachable: + return nil, &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connect: connection refused (test)")} + case modeDenied: + return &http.Response{ + StatusCode: http.StatusForbidden, + Status: "403 Forbidden", + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(strings.NewReader(`{"errors":[{"code":"DENIED","message":"denied (test)"}]}`)), + Request: req, + }, nil + } + return http.DefaultTransport.RoundTrip(req) +} + +type cacheFixture struct { + host string + gate *gatedTransport + store *DiskStore +} + +func newCacheFixture(t *testing.T) *cacheFixture { + t.Helper() + srv := httptest.NewServer(registry.New(registry.Logger(log.New(io.Discard, "", 0)))) + t.Cleanup(srv.Close) + gate := &gatedTransport{} + store, err := NewDiskStore(DiskStoreOptions{ + CacheDir: t.TempDir(), + OCIKeychain: authn.NewMultiKeychain(), + OCITransport: gate, + }) + if err != nil { + t.Fatalf("NewDiskStore: %v", err) + } + return &cacheFixture{host: strings.TrimPrefix(srv.URL, "http://"), gate: gate, store: store} +} + +func (c *cacheFixture) ref(tag string) string { return c.host + "/features/demo:" + tag } + +// push publishes a feature artifact whose install.sh echoes marker +// under tag, and returns its manifest digest. +func (c *cacheFixture) push(t *testing.T, tag, marker string) v1.Hash { + t.Helper() + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for _, f := range []struct{ name, body string }{ + {"devcontainer-feature.json", `{"id":"demo","version":"1.2.3"}`}, + {"install.sh", "#!/bin/sh\necho " + marker + "\n"}, + } { + if err := tw.WriteHeader(&tar.Header{Name: f.name, Mode: 0o755, Size: int64(len(f.body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(f.body)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + img, err := mutate.AppendLayers( + mutate.ConfigMediaType(empty.Image, types.MediaType(devContainerFeatureMediaType)), + static.NewLayer(buf.Bytes(), "application/vnd.devcontainers.layer.v1+tar")) + if err != nil { + t.Fatal(err) + } + ref, err := name.ParseReference(c.ref(tag)) + if err != nil { + t.Fatal(err) + } + if err := remote.Write(ref, img, remote.WithTransport(http.DefaultTransport)); err != nil { + t.Fatalf("push %s: %v", ref, err) + } + d, err := img.Digest() + if err != nil { + t.Fatal(err) + } + return d +} + +func (c *cacheFixture) fetch(t *testing.T, ref string) (Fetched, error) { + t.Helper() + return c.store.Fetch(context.Background(), ref, config.FeatureSourceOCI) +} + +func (c *cacheFixture) mustFetch(t *testing.T, ref string) Fetched { + t.Helper() + f, err := c.fetch(t, ref) + if err != nil { + t.Fatalf("Fetch %s: %v", ref, err) + } + return f +} + +func (c *cacheFixture) set(m registryMode) { + c.gate.mode.Store(int32(m)) + c.gate.calls.Store(0) +} + +func wantDigest(t *testing.T, f Fetched, d v1.Hash) { + t.Helper() + if !strings.HasSuffix(f.ResolvedRef, "@"+d.String()) { + t.Fatalf("ResolvedRef = %s, want digest %s", f.ResolvedRef, d) + } +} + +func TestOCICache_DigestRefIsLocalFirst(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + c.mustFetch(t, c.ref("1.2.3")) + + c.set(modeUnreachable) + f := c.mustFetch(t, c.host+"/features/demo@"+d.String()) + wantDigest(t, f, d) + if n := c.gate.calls.Load(); n != 0 { + t.Fatalf("digest ref with cached content made %d registry calls, want 0", n) + } +} + +func TestOCICache_DigestRefNotCachedGoesOnline(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + f := c.mustFetch(t, c.host+"/features/demo@"+d.String()) + wantDigest(t, f, d) + if c.gate.calls.Load() == 0 { + t.Fatal("uncached digest ref made no registry call") + } +} + +func TestOCICache_ExactVersionTagIsLocalFirst(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) + + // Online, but the recorded resolution of an exact version is used + // without asking the registry. + c.set(modeOnline) + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) + if n := c.gate.calls.Load(); n != 0 { + t.Fatalf("cached exact version made %d registry calls, want 0", n) + } + + c.set(modeUnreachable) + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) +} + +func TestOCICache_FloatingTagIsOnlineFirst(t *testing.T) { + c := newCacheFixture(t) + d1 := c.push(t, "1", "v1") + wantDigest(t, c.mustFetch(t, c.ref("1")), d1) + + d2 := c.push(t, "1", "v2") + if d1 == d2 { + t.Fatal("test artifacts share a digest") + } + c.set(modeOnline) + wantDigest(t, c.mustFetch(t, c.ref("1")), d2) + if c.gate.calls.Load() == 0 { + t.Fatal("floating tag resolved without asking the registry") + } +} + +func TestOCICache_FloatingTagOfflineUsesRecordedResolution(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "latest", "v1") + wantDigest(t, c.mustFetch(t, c.ref("latest")), d) + + c.set(modeUnreachable) + f := c.mustFetch(t, c.ref("latest")) + wantDigest(t, f, d) + if f.Metadata.ID != "demo" { + t.Fatalf("Metadata.ID = %q, want demo", f.Metadata.ID) + } +} + +func TestOCICache_OfflineWithoutRecordFails(t *testing.T) { + c := newCacheFixture(t) + c.push(t, "1", "v1") + c.push(t, "1.2.3", "v1") + + c.set(modeUnreachable) + for _, tag := range []string{"1", "1.2.3"} { + if _, err := c.fetch(t, c.ref(tag)); err == nil { + t.Fatalf("offline fetch of never-resolved %s succeeded", tag) + } + } +} + +func TestOCICache_RegistryErrorIsAuthoritative(t *testing.T) { + c := newCacheFixture(t) + c.push(t, "1", "v1") + c.mustFetch(t, c.ref("1")) + + // The registry answers, with a refusal: that is not an offline + // condition, so the recorded resolution must not paper over it. + c.set(modeDenied) + if _, err := c.fetch(t, c.ref("1")); err == nil { + t.Fatal("fetch succeeded although the registry refused it") + } +} + +func TestOCICache_RecordNeedsContent(t *testing.T) { + c := newCacheFixture(t) + d := c.push(t, "1.2.3", "v1") + f := c.mustFetch(t, c.ref("1.2.3")) + wantDigest(t, f, d) + + // Evict the content but keep the tag record: the local-first path + // must fall through to the registry instead of returning a + // dangling directory. + if err := os.RemoveAll(c.store.ociEntryDir(d.Hex)); err != nil { + t.Fatal(err) + } + c.set(modeOnline) + wantDigest(t, c.mustFetch(t, c.ref("1.2.3")), d) + if c.gate.calls.Load() == 0 { + t.Fatal("evicted content was not re-fetched") + } +} + +func TestIsExactVersionTag(t *testing.T) { + for tag, want := range map[string]bool{ + "2.17.0": true, + "10.0.12": true, + "2": false, + "2.17": false, + "latest": false, + "2.17.0-rc.1": false, + "v2.17.0": false, + "dev": false, + } { + if got := isExactVersionTag(tag); got != want { + t.Errorf("isExactVersionTag(%q) = %v, want %v", tag, got, want) + } + } +} diff --git a/runtime/docker/inspect.go b/runtime/docker/inspect.go index d05ce7c..6afcc76 100644 --- a/runtime/docker/inspect.go +++ b/runtime/docker/inspect.go @@ -62,6 +62,7 @@ func (r *Runtime) InspectContainer(ctx context.Context, id string) (*runtime.Con out.User = c.Config.User out.Env = append([]string(nil), c.Config.Env...) out.Labels = copyLabels(c.Config.Labels) + out.ImageRef = c.Config.Image } if c.HostConfig != nil { out.Privileged = c.HostConfig.Privileged diff --git a/runtime/runtime.go b/runtime/runtime.go index 4f07e9e..cea9c02 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -230,6 +230,14 @@ type ContainerDetails struct { Mounts []MountInspect Labels map[string]string + // ImageRef is the image reference the container was created from + // (docker's Config.Image), e.g. "dc-go-final-:latest", as + // opposed to Container.Image, which inspect reports as the resolved + // image ID. Empty when the backend does not surface it. The compose + // reattach path uses it to rebuild the service's image: field + // exactly as it was when the container was created. + ImageRef string + // Security options as actually applied to the container, read back // from the backend's inspect (docker HostConfig). Lets callers // verify that RunSpec.Privileged/CapAdd/SecurityOpt — including the diff --git a/test/integration/compose_dind_test.go b/test/integration/compose_dind_test.go index 71d764f..e185d68 100644 --- a/test/integration/compose_dind_test.go +++ b/test/integration/compose_dind_test.go @@ -11,6 +11,7 @@ import ( "time" devcontainer "github.com/crunchloop/devcontainer" + "github.com/crunchloop/devcontainer/runtime" ) // dindFeatureRef is the upstream docker-in-docker feature. Its metadata @@ -139,3 +140,135 @@ services: }) } } + +// waitInnerDockerd waits for the docker-in-docker feature's dockerd. +func waitInnerDockerd(t *testing.T, ctx context.Context, eng *devcontainer.Engine, ws *devcontainer.Workspace) { + t.Helper() + deadline := time.Now().Add(2 * time.Minute) + for { + res, err := eng.Exec(ctx, ws, devcontainer.ExecOptions{ + Cmd: []string{"docker", "info", "--format", "{{.Driver}}"}, + }) + if err == nil && res.ExitCode == 0 { + return + } + if time.Now().After(deadline) { + t.Fatalf("inner dockerd never became ready: err=%v res=%+v", err, res) + } + time.Sleep(2 * time.Second) + } +} + +// TestComposeSource_DockerInDockerRestartsWithoutRegistry is the +// restart half of the docker-in-docker check: up, pull an image into +// the inner dockerd, stop every service, cut the feature registry +// off, up again. The second Up must reattach without fetching the +// feature, the /var/lib/docker volume must still be mounted, and +// `docker run hello-world` must work from the image the first boot +// pulled onto that volume. Before compose reattach, the second Up +// re-fetched docker-in-docker from ghcr.io and failed offline. +func TestComposeSource_DockerInDockerRestartsWithoutRegistry(t *testing.T) { + if testing.Short() { + t.Skip() + } + + for _, bk := range composeBackends { + t.Run(bk.name, func(t *testing.T) { + h := newReattachHarness(t, bk.backend) + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "docker-compose.yml"), ` +services: + app: + image: `+dindBaseImage+` + command: ["sh", "-c", "while sleep 1000; do :; done"] +`) + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "`+dindFeatureRef+`": {} } + }`) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute) + defer cancel() + + start := time.Now() + ws1, err := h.eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: dir, + Recreate: true, + SkipLifecycle: true, + }) + if err != nil { + t.Fatalf("first Up: %v", err) + } + t.Logf("first Up: %s, %d feature fetch(es), %d registry request(s)", + time.Since(start).Round(time.Millisecond), h.store.fetches.Load(), h.gate.calls.Load()) + cleanupComposeWorkspace(t, h.eng, ws1) + waitInnerDockerd(t, ctx, h.eng, ws1) + // Pull while the network is there; the second boot runs it + // from the volume. + if got := execOut(t, ctx, h.eng, ws1, "docker", "pull", "-q", "hello-world"); got == "" { + t.Fatal("docker pull hello-world printed nothing") + } + + project := ws1.Container.Labels["com.docker.compose.project"] + cs, err := h.rt.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{"com.docker.compose.project": project}}) + if err != nil { + t.Fatal(err) + } + for _, c := range cs { + if err := h.rt.StopContainer(ctx, c.ID, runtime.StopOptions{Timeout: 10 * time.Second}); err != nil { + t.Fatalf("stop %s: %v", c.Name, err) + } + } + h.block() + + start = time.Now() + ws2, err := h.eng.Up(ctx, devcontainer.UpOptions{ + LocalWorkspaceFolder: dir, + AdoptExisting: true, + SkipLifecycle: true, + }) + if err != nil { + t.Fatalf("second Up with the registry blocked: %v", err) + } + t.Logf("second Up (registry blocked): %s, %d feature fetch(es), %d registry request(s)", + time.Since(start).Round(time.Millisecond), h.store.fetches.Load(), h.gate.calls.Load()) + if n := h.store.fetches.Load(); n != 0 { + t.Errorf("second Up fetched %d features, want 0", n) + } + if n := h.gate.calls.Load(); n != 0 { + t.Errorf("second Up made %d registry requests, want 0", n) + } + if ws2.Container.ID != ws1.Container.ID { + t.Errorf("second Up recreated the primary: %s -> %s", ws1.Container.ID, ws2.Container.ID) + } + var dockerMount bool + for _, m := range ws2.Container.Mounts { + if m.Target == "/var/lib/docker" { + dockerMount = m.Type == "volume" + } + } + if !dockerMount { + t.Fatalf("/var/lib/docker is not a volume after reattach: %+v", ws2.Container.Mounts) + } + if !ws2.Container.Privileged { + t.Errorf("primary not privileged after reattach") + } + + waitInnerDockerd(t, ctx, h.eng, ws2) + res, err := h.eng.Exec(ctx, ws2, devcontainer.ExecOptions{ + Cmd: []string{"docker", "run", "--rm", "--pull", "never", "hello-world"}, + }) + if err != nil { + t.Fatalf("Exec docker run: %v", err) + } + if res.ExitCode != 0 || !strings.Contains(res.Stdout, "Hello from Docker!") { + t.Fatalf("docker run hello-world after offline restart failed: exit=%d stdout=%q stderr=%q", + res.ExitCode, res.Stdout, res.Stderr) + } + t.Logf("docker run --pull never hello-world after the offline restart: %q", + strings.SplitN(strings.TrimSpace(res.Stdout), "\n", 2)[0]) + }) + } +} diff --git a/test/integration/compose_reattach_test.go b/test/integration/compose_reattach_test.go new file mode 100644 index 0000000..a3cae08 --- /dev/null +++ b/test/integration/compose_reattach_test.go @@ -0,0 +1,500 @@ +//go:build integration + +package integration + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "io" + "log" + "net" + "net/http" + "net/http/httptest" + "os/exec" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" + + devcontainer "github.com/crunchloop/devcontainer" + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/feature" + "github.com/crunchloop/devcontainer/runtime" + "github.com/crunchloop/devcontainer/runtime/docker" +) + +// Compose reattach: an Up whose project containers already exist from +// identical inputs must restart them from the existing images, reading +// the feature metadata from the primary image's label — no feature +// fetch, no image build, so no registry. These tests serve a small +// feature from an in-process registry and cut the registry off before +// the second Up. + +// blockableTransport makes the feature registry unreachable on demand, +// failing the way a guest without egress does (a refused connection). +type blockableTransport struct { + blocked atomic.Bool + calls atomic.Int64 +} + +func (b *blockableTransport) RoundTrip(req *http.Request) (*http.Response, error) { + b.calls.Add(1) + if b.blocked.Load() { + return nil, &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connect: connection refused (registry blocked by test)")} + } + return http.DefaultTransport.RoundTrip(req) +} + +// countingStore counts Fetch calls on the wrapped store. +type countingStore struct { + inner feature.Store + fetches atomic.Int64 +} + +func (c *countingStore) Fetch(ctx context.Context, ref string, kind config.FeatureSourceKind) (feature.Fetched, error) { + c.fetches.Add(1) + return c.inner.Fetch(ctx, ref, kind) +} + +// reattachHarness is one engine with a blockable, counted feature +// store whose cache lives in the test's temp dir. +type reattachHarness struct { + eng *devcontainer.Engine + rt *docker.Runtime + gate *blockableTransport + store *countingStore +} + +func newReattachHarness(t *testing.T, backend devcontainer.ComposeBackend) *reattachHarness { + t.Helper() + gate := &blockableTransport{} + ds, err := feature.NewDiskStore(feature.DiskStoreOptions{ + CacheDir: t.TempDir(), + OCIKeychain: authn.NewMultiKeychain(authn.DefaultKeychain), + OCITransport: gate, + }) + if err != nil { + t.Fatalf("NewDiskStore: %v", err) + } + store := &countingStore{inner: ds} + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + rt, err := docker.New(ctx, docker.Options{}) + if err != nil { + t.Skipf("Docker daemon unavailable: %v", err) + } + t.Cleanup(func() { _ = rt.Close() }) + eng, err := devcontainer.New(devcontainer.EngineOptions{ + Runtime: rt, + FeatureStore: store, + ComposeBackend: backend, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + return &reattachHarness{eng: eng, rt: rt, gate: gate, store: store} +} + +// block cuts the registry off and resets the counters. +func (h *reattachHarness) block() { + h.gate.blocked.Store(true) + h.reset() +} + +func (h *reattachHarness) reset() { + h.gate.calls.Store(0) + h.store.fetches.Store(0) +} + +// markerFeatureMetadata declares one of everything the compose +// run override carries from feature metadata: env, a capability, a +// project volume, an entrypoint and a lifecycle command. +const markerFeatureMetadata = `{ + "id": "marker", + "version": "1.0.0", + "options": {"flavor": {"type": "string", "default": "plain"}}, + "containerEnv": {"MARKER_FEATURE": "on"}, + "capAdd": ["SYS_PTRACE"], + "mounts": [{"source": "marker-data-${devcontainerId}", "target": "/marker-data", "type": "volume"}], + "entrypoint": "/usr/local/share/marker-entry.sh", + "postStartCommand": "echo poststart >> /marker-data/poststart.log" +}` + +const markerFeatureInstall = `#!/bin/sh +set -e +mkdir -p /usr/local/share +echo "$FLAVOR" > /usr/local/share/marker-flavor +cat > /usr/local/share/marker-entry.sh <<'EOF' +#!/bin/sh +mkdir -p /marker-data +echo entry >> /marker-data/entry.log +EOF +chmod +x /usr/local/share/marker-entry.sh +` + +// serveMarkerFeature starts an in-process registry holding the marker +// feature and returns its reference. +func serveMarkerFeature(t *testing.T) string { + t.Helper() + srv := httptest.NewServer(registry.New(registry.Logger(log.New(io.Discard, "", 0)))) + t.Cleanup(srv.Close) + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for _, f := range []struct{ name, body string }{ + {"devcontainer-feature.json", markerFeatureMetadata}, + {"install.sh", markerFeatureInstall}, + } { + if err := tw.WriteHeader(&tar.Header{Name: f.name, Mode: 0o755, Size: int64(len(f.body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(f.body)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + img, err := mutate.AppendLayers( + mutate.ConfigMediaType(empty.Image, types.MediaType("application/vnd.devcontainers")), + static.NewLayer(buf.Bytes(), "application/vnd.devcontainers.layer.v1+tar")) + if err != nil { + t.Fatal(err) + } + ref := strings.TrimPrefix(srv.URL, "http://") + "/features/marker:1.0.0" + parsed, err := name.ParseReference(ref) + if err != nil { + t.Fatal(err) + } + if err := remote.Write(parsed, img); err != nil { + t.Fatalf("push marker feature: %v", err) + } + return ref +} + +// writeMarkerWorkspace writes a compose devcontainer with one app +// service and a sidecar, using the marker feature with flavor. +func writeMarkerWorkspace(t *testing.T, dir, featureRef, flavor string) { + t.Helper() + mustWrite(t, filepath.Join(dir, "docker-compose.yml"), ` +services: + app: + image: debian:bookworm-slim + command: ["sh", "-c", "while sleep 1000; do :; done"] + sidecar: + image: busybox:1.36 + command: ["sh", "-c", "while sleep 1000; do :; done"] +`) + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "`+featureRef+`": { "flavor": "`+flavor+`" } } + }`) +} + +// eventLog collects every event of one Up. +type eventLog struct { + ch chan events.Event + done chan struct{} + mu sync.Mutex + evs []events.Event +} + +func newEventLog() *eventLog { + l := &eventLog{ch: make(chan events.Event, 4096), done: make(chan struct{})} + go func() { + defer close(l.done) + for ev := range l.ch { + l.mu.Lock() + l.evs = append(l.evs, ev) + l.mu.Unlock() + } + }() + return l +} + +func (l *eventLog) close() []events.Event { + close(l.ch) + <-l.done + return l.evs +} + +func featureBuilds(evs []events.Event) int { + n := 0 + for _, ev := range evs { + if b, ok := ev.(events.BuildStartEvent); ok && b.Source == events.BuildSourceFeatures { + n++ + } + } + return n +} + +func reattachSkips(evs []events.Event) int { + n := 0 + for _, ev := range evs { + if s, ok := ev.(events.FeatureSkippedEvent); ok && s.Reason == "compose_reattach" { + n++ + } + } + return n +} + +// up runs one Up and returns the workspace and its events. +func (h *reattachHarness) up(t *testing.T, ctx context.Context, dir string, opts devcontainer.UpOptions) (*devcontainer.Workspace, []events.Event) { + t.Helper() + l := newEventLog() + opts.LocalWorkspaceFolder = dir + opts.Events = l.ch + ws, err := h.eng.Up(ctx, opts) + evs := l.close() + if err != nil { + t.Fatalf("Up: %v", err) + } + return ws, evs +} + +// stopProject stops every container of the workspace's project, as a +// host or VM stop leaves them. +func (h *reattachHarness) stopProject(t *testing.T, ctx context.Context, ws *devcontainer.Workspace) { + t.Helper() + project := ws.Container.Labels["com.docker.compose.project"] + cs, err := h.rt.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{"com.docker.compose.project": project}}) + if err != nil { + t.Fatalf("ListContainers: %v", err) + } + if len(cs) < 2 { + t.Fatalf("project %s has %d containers, want app and sidecar", project, len(cs)) + } + for _, c := range cs { + if err := h.rt.StopContainer(ctx, c.ID, runtime.StopOptions{Timeout: 5 * time.Second}); err != nil { + t.Fatalf("stop %s: %v", c.Name, err) + } + } +} + +// cleanupComposeWorkspace removes the project, its volumes and the +// images the engine tagged for it. +func cleanupComposeWorkspace(t *testing.T, eng *devcontainer.Engine, ws *devcontainer.Workspace) { + t.Helper() + t.Cleanup(func() { + ctx := context.Background() + project := ws.Container.Labels["com.docker.compose.project"] + _ = eng.Down(ctx, ws, devcontainer.DownOptions{Remove: true, RemoveVolumes: true}) + if out, err := exec.Command("docker", "ps", "-aq", "--filter", "label=com.docker.compose.project="+project).Output(); err == nil { + for _, id := range strings.Fields(string(out)) { + _ = exec.Command("docker", "rm", "-f", id).Run() + } + } + if out, err := exec.Command("docker", "volume", "ls", "-q", "--filter", "label=com.docker.compose.project="+project).Output(); err == nil { + for _, v := range strings.Fields(string(out)) { + _ = exec.Command("docker", "volume", "rm", "-f", v).Run() + } + } + _ = exec.Command("docker", "network", "rm", project+"_default").Run() + for _, prefix := range []string{"dc-go-final-", "dc-go-uid-", "dc-go-compose-base-"} { + _ = exec.Command("docker", "image", "rm", "-f", prefix+string(ws.ID)+":latest").Run() + } + }) +} + +func execOut(t *testing.T, ctx context.Context, eng *devcontainer.Engine, ws *devcontainer.Workspace, cmd ...string) string { + t.Helper() + res, err := eng.Exec(ctx, ws, devcontainer.ExecOptions{Cmd: cmd}) + if err != nil { + t.Fatalf("Exec %v: %v", cmd, err) + } + if res.ExitCode != 0 { + t.Fatalf("Exec %v: exit %d stdout=%q stderr=%q", cmd, res.ExitCode, res.Stdout, res.Stderr) + } + return strings.TrimSpace(res.Stdout) +} + +func lineCount(s string) int { + if s == "" { + return 0 + } + return len(strings.Split(s, "\n")) +} + +var composeBackends = []struct { + name string + backend devcontainer.ComposeBackend +}{ + {"native", devcontainer.ComposeBackendNative}, + {"shellout", devcontainer.ComposeBackendShellout}, +} + +// TestComposeReattach_RestartsWithoutRegistry: up, stop every +// service, cut the registry off, up again. The second Up must reattach +// — same containers, restarted, no fetch, no build — and still apply +// the feature's metadata from the image label: env, capability, +// volume, entrypoint and postStartCommand. +func TestComposeReattach_RestartsWithoutRegistry(t *testing.T) { + if testing.Short() { + t.Skip() + } + featureRef := serveMarkerFeature(t) + + for _, bk := range composeBackends { + for _, adopt := range []bool{false, true} { + sub := bk.name + if adopt { + sub += "/adopt" + } + t.Run(sub, func(t *testing.T) { + h := newReattachHarness(t, bk.backend) + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) + defer cancel() + dir := t.TempDir() + writeMarkerWorkspace(t, dir, featureRef, "plain") + + // First creation: unchanged — fetch, build, label. + ws1, evs1 := h.up(t, ctx, dir, devcontainer.UpOptions{Recreate: true}) + cleanupComposeWorkspace(t, h.eng, ws1) + if n := h.store.fetches.Load(); n != 1 { + t.Fatalf("first Up: %d feature fetches, want 1", n) + } + if featureBuilds(evs1) != 1 { + t.Fatalf("first Up: %d feature image builds, want 1", featureBuilds(evs1)) + } + if ws1.Container.Labels[devcontainer.LabelComposeInputsHash] == "" { + t.Fatalf("first Up: primary has no %s label: %v", devcontainer.LabelComposeInputsHash, ws1.Container.Labels) + } + if got := execOut(t, ctx, h.eng, ws1, "cat", "/marker-data/entry.log"); lineCount(got) != 1 { + t.Fatalf("first Up: entry.log = %q, want one entry", got) + } + + h.stopProject(t, ctx, ws1) + h.block() + + ws2, evs2 := h.up(t, ctx, dir, devcontainer.UpOptions{AdoptExisting: adopt}) + if n := h.store.fetches.Load(); n != 0 { + t.Errorf("reattach: %d feature fetches, want 0", n) + } + if n := h.gate.calls.Load(); n != 0 { + t.Errorf("reattach: %d registry requests, want 0", n) + } + if n := featureBuilds(evs2); n != 0 { + t.Errorf("reattach: %d feature image builds, want 0", n) + } + if n := reattachSkips(evs2); n != 1 { + t.Errorf("reattach: %d compose_reattach skip events, want 1", n) + } + if ws2.Container.ID != ws1.Container.ID { + t.Errorf("reattach recreated the primary: %s -> %s", ws1.Container.ID, ws2.Container.ID) + } + if ws2.Container.State != runtime.StateRunning { + t.Errorf("primary state after reattach = %s, want running", ws2.Container.State) + } + cs, err := h.rt.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{ + "com.docker.compose.project": ws2.Container.Labels["com.docker.compose.project"], + "com.docker.compose.service": "sidecar", + }}) + if err != nil || len(cs) != 1 || cs[0].State != runtime.StateRunning { + t.Errorf("sidecar after reattach = %+v (err %v), want one running", cs, err) + } + + // Metadata from the image label reached cfg and the container. + if ws2.Config.ContainerEnv["MARKER_FEATURE"] != "on" { + t.Errorf("cfg.ContainerEnv = %v, want MARKER_FEATURE=on", ws2.Config.ContainerEnv) + } + if !containsStr(ws2.Config.CapAdd, "SYS_PTRACE") || !hasCapability(ws2.Container.CapAdd, "SYS_PTRACE") { + t.Errorf("capAdd cfg=%v container=%v, want SYS_PTRACE", ws2.Config.CapAdd, ws2.Container.CapAdd) + } + var vol bool + for _, m := range ws2.Container.Mounts { + if m.Target == "/marker-data" && m.Type == "volume" { + vol = true + } + } + if !vol { + t.Errorf("no volume at /marker-data after reattach: %+v", ws2.Container.Mounts) + } + if got := execOut(t, ctx, h.eng, ws2, "printenv", "MARKER_FEATURE"); got != "on" { + t.Errorf("MARKER_FEATURE in container = %q", got) + } + // The entrypoint ran again on restart, and the lifecycle + // command from the label ran on both Ups, into the same volume. + if got := execOut(t, ctx, h.eng, ws2, "cat", "/marker-data/entry.log"); lineCount(got) != 2 { + t.Errorf("entry.log after reattach = %q, want two entries", got) + } + if got := execOut(t, ctx, h.eng, ws2, "cat", "/marker-data/poststart.log"); lineCount(got) != 2 { + t.Errorf("poststart.log after reattach = %q, want two entries", got) + } + if got := execOut(t, ctx, h.eng, ws2, "cat", "/usr/local/share/marker-flavor"); got != "plain" { + t.Errorf("marker-flavor = %q, want plain", got) + } + }) + } + } +} + +// TestComposeReattach_ConfigChangeRebuilds: a changed feature option +// changes the inputs hash, so the next Up takes the full path — it +// fetches and builds again (needing the registry) rather than +// reattaching to an image built from other inputs. +func TestComposeReattach_ConfigChangeRebuilds(t *testing.T) { + if testing.Short() { + t.Skip() + } + featureRef := serveMarkerFeature(t) + + for _, bk := range composeBackends { + t.Run(bk.name, func(t *testing.T) { + h := newReattachHarness(t, bk.backend) + ctx, cancel := context.WithTimeout(context.Background(), 8*time.Minute) + defer cancel() + dir := t.TempDir() + writeMarkerWorkspace(t, dir, featureRef, "plain") + + ws1, _ := h.up(t, ctx, dir, devcontainer.UpOptions{Recreate: true, SkipLifecycle: true}) + cleanupComposeWorkspace(t, h.eng, ws1) + h.stopProject(t, ctx, ws1) + + writeMarkerWorkspace(t, dir, featureRef, "spicy") + + // Offline, the changed inputs must not be satisfied by the + // old image: the full path runs and needs the registry. The + // exact-version tag is served from the store's cache, so the + // fetch succeeds; what matters is that it happens. + h.block() + ws2, evs := h.up(t, ctx, dir, devcontainer.UpOptions{SkipLifecycle: true}) + if n := h.store.fetches.Load(); n != 1 { + t.Errorf("Up after config change: %d feature fetches, want 1", n) + } + if n := featureBuilds(evs); n != 1 { + t.Errorf("Up after config change: %d feature image builds, want 1", n) + } + if n := reattachSkips(evs); n != 0 { + t.Errorf("Up after config change reattached (%d skip events)", n) + } + if bk.backend == devcontainer.ComposeBackendNative { + // Native without AdoptExisting recreates on drift. + if ws2.Container.ID == ws1.Container.ID { + t.Errorf("native: primary not recreated after config change") + } + if ws2.Container.Labels[devcontainer.LabelComposeInputsHash] == ws1.Container.Labels[devcontainer.LabelComposeInputsHash] { + t.Errorf("native: inputs hash label unchanged after config change") + } + if got := execOut(t, ctx, h.eng, ws2, "cat", "/usr/local/share/marker-flavor"); got != "spicy" { + t.Errorf("native: marker-flavor = %q, want spicy", got) + } + } + }) + } +} diff --git a/up.go b/up.go index bf51e9d..89649b7 100644 --- a/up.go +++ b/up.go @@ -2,6 +2,7 @@ package devcontainer import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -159,6 +160,12 @@ const ( // - existing + UpOptions.Recreate=true → stop + remove, fresh create // - no existing container → fresh create // +// For compose sources, "attach" means the project's containers are +// started from their existing images when the primary container's +// LabelComposeInputsHash matches the current inputs; features are not +// fetched and no image is built. Otherwise the full path runs (fetch, +// build, compose up). See reattachCompose. +// // Image source only in this milestone. Build / compose return // runtime.ErrNotImplemented. func (e *Engine) Up(ctx context.Context, opts UpOptions) (*Workspace, error) { @@ -227,10 +234,13 @@ func (e *Engine) Up(ctx context.Context, opts UpOptions) (*Workspace, error) { var ws *Workspace switch { case isCompose: - // `docker compose up -d` is idempotent: already-running services - // stay, stopped ones restart, missing ones get created. So we - // always go through createFreshCompose, regardless of whether - // `existing` was found — compose handles the dispatch internally. + // createFreshCompose handles both cases: with an existing + // container it first tries reattachCompose (restart the project + // from its existing images, metadata from the image label, no + // feature fetch or build) and otherwise runs the full path, where + // `docker compose up -d` / the native orchestrator keep + // already-running services, restart stopped ones and create + // missing ones. // // Reattach caveat for host-side hooks: when compose finds an // existing container, its env is already baked. secretsCommand @@ -547,11 +557,32 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon return nil, err } + // Reattach when the project's containers already exist from inputs + // identical to these: the image path's attachExisting, for compose. + // No feature fetch, no image build, so a restart needs neither the + // feature registry nor the time to rebuild the final image. + if existingContainer { + ws, err := e.reattachCompose(ctx, cfg, opts, project, src, primary, projectName, workingDir) + if !errors.Is(err, errComposeReattachMiss) { + return ws, err + } + } + baseImage, err := e.prepareComposeServiceImage(ctx, cfg, primary, workingDir, opts) if err != nil { return nil, err } + // Hash the inputs before layerFeatures and applyMetadataMerge + // mutate cfg. A hash that cannot be computed only means the + // container gets no label and a later Up cannot reattach to it. + inputsHash := "" + if base, ierr := e.runtime.InspectImage(ctx, baseImage); ierr == nil && base != nil { + if h, herr := e.composeInputsHash(cfg, opts, project, base.ID); herr == nil { + inputsHash = h + } + } + finalImage, baseLayers, err := e.layerFeatures(ctx, cfg, baseImage, opts) if err != nil { return nil, err @@ -568,6 +599,26 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon return nil, err } + runOverride := e.composeRunOverride(ctx, cfg, opts, project, src, finalImage, extraEnv, inputsHash) + return e.upComposeBackend(ctx, cfg, opts, project, src, projectName, workingDir, + finalImage, runOverride, existingContainer, false) +} + +// composeRunOverride builds the run-time override layered onto the +// primary service from the merged cfg. Shared by the full path and +// reattachCompose so both produce the same service config (and so the +// same orchestrator config hash) for the same merged metadata. +// inputsHash, when non-empty, is stamped as LabelComposeInputsHash. +func (e *Engine) composeRunOverride( + ctx context.Context, + cfg *config.ResolvedConfig, + opts UpOptions, + project *composetypes.Project, + src *config.ComposeSource, + finalImage string, + extraEnv map[string]string, + inputsHash string, +) compose.Override { bindMounts := composeBindMounts(cfg, opts) overrideLabels := map[string]string{ @@ -575,6 +626,9 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon LabelLocalWorkspaceFolder: cfg.LocalWorkspaceFolder, LabelEngine: engineIdent, } + if inputsHash != "" { + overrideLabels[LabelComposeInputsHash] = inputsHash + } overrideEnv := mergeEnv(cfg.ContainerEnv, extraEnv) // When features/image metadata declare entrypoint scripts (e.g. @@ -607,7 +661,7 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon // feature-entrypoint chain — without these, features like // docker-in-docker that declare privileged/init/entrypoint silently // fail on compose-source devcontainers. - runOverride := compose.Override{ + return compose.Override{ Service: src.Service, ExtraBindMounts: bindMounts, ExtraMounts: composeMounts(cfg, opts), @@ -620,11 +674,25 @@ func (e *Engine) createFreshCompose(ctx context.Context, cfg *config.ResolvedCon Entrypoints: cfg.Entrypoints, OriginalEntrypoint: origEntrypoint, } +} +// upComposeBackend dispatches to the selected compose backend. +// reattach marks a reattachCompose call: the native backend then skips +// sidecar builds whose image is already present. +func (e *Engine) upComposeBackend( + ctx context.Context, + cfg *config.ResolvedConfig, + opts UpOptions, + project *composetypes.Project, + src *config.ComposeSource, + projectName, workingDir, finalImage string, + runOverride compose.Override, + existingContainer, reattach bool, +) (*Workspace, error) { switch e.opts.ComposeBackend { case ComposeBackendNative: return e.upComposeNative(ctx, cfg, opts, project, src, projectName, - workingDir, finalImage, runOverride) + workingDir, finalImage, runOverride, reattach) case ComposeBackendShellout: return e.upComposeShellout(ctx, cfg, opts, project, src, projectName, workingDir, finalImage, runOverride, existingContainer) @@ -755,11 +823,12 @@ func (e *Engine) upComposeNative( src *config.ComposeSource, projectName, workingDir, finalImage string, runOverride compose.Override, + reattach bool, ) (*Workspace, error) { if err := compose.ApplyBuildOverride(project, src.Service, finalImage); err != nil { return nil, err } - if err := e.buildComposeSidecarImages(ctx, project, src, projectName, workingDir, opts); err != nil { + if err := e.buildComposeSidecarImages(ctx, project, src, projectName, workingDir, opts, reattach); err != nil { return nil, err } if err := compose.ApplyRunOverride(project, src.Service, runOverride); err != nil { @@ -911,6 +980,7 @@ func (e *Engine) buildComposeSidecarImages( src *config.ComposeSource, projectName, workingDir string, opts UpOptions, + skipPresent bool, ) error { // Selection mirrors `docker compose up `: // the named services plus their transitive dependencies (the same @@ -950,6 +1020,17 @@ func (e *Engine) buildComposeSidecarImages( if !filepath.IsAbs(ctxPath) { ctxPath = filepath.Join(workingDir, ctxPath) } + if skipPresent { + // Reattach: the existing containers were created from this + // tag; rebuilding would only re-run the build (and maybe + // pull its base) to land on the same image. + if _, err := e.runtime.InspectImage(ctx, tag); err == nil { + if err := compose.ApplyBuildOverride(project, name, tag); err != nil { + return err + } + continue + } + } opts.bus.Emit(events.BuildStartEvent{Source: events.BuildSourceDockerfile, Ref: tag}) if _, err := e.runtime.BuildImage(ctx, runtime.BuildSpec{ ContextPath: ctxPath, diff --git a/up_compose_host_ports_test.go b/up_compose_host_ports_test.go index 5c43b57..8b7c30f 100644 --- a/up_compose_host_ports_test.go +++ b/up_compose_host_ports_test.go @@ -55,7 +55,7 @@ func hostPortUp(t *testing.T, eng *Engine, out chan events.Event) { }, } if _, err := eng.upComposeNative(context.Background(), cfg, opts, project, src, - "dc-test", t.TempDir(), "app:dev", runOverride); err != nil { + "dc-test", t.TempDir(), "app:dev", runOverride, false); err != nil { t.Fatalf("upComposeNative: %v", err) } } diff --git a/up_compose_mounts_test.go b/up_compose_mounts_test.go index bc85120..3070d62 100644 --- a/up_compose_mounts_test.go +++ b/up_compose_mounts_test.go @@ -125,7 +125,7 @@ func TestUpComposeNative_CarriesFeatureVolumeMount(t *testing.T) { project.Name = "dc-test" if _, err := eng.upComposeNative(context.Background(), cfg, opts, project, &config.ComposeSource{Service: "app"}, "dc-test", t.TempDir(), "app:dev", - mountsRunOverride(cfg, opts)); err != nil { + mountsRunOverride(cfg, opts), false); err != nil { t.Fatalf("upComposeNative: %v", err) } diff --git a/up_compose_reattach.go b/up_compose_reattach.go new file mode 100644 index 0000000..56e8f82 --- /dev/null +++ b/up_compose_reattach.go @@ -0,0 +1,340 @@ +package devcontainer + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "sort" + + composetypes "github.com/compose-spec/compose-go/v2/types" + + "github.com/crunchloop/devcontainer/compose" + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/feature" + "github.com/crunchloop/devcontainer/runtime" +) + +// LabelComposeInputsHash is stamped on the primary service container +// of a compose-source devcontainer. Its value is composeInputsHash of +// everything that went into that container's image and run override +// before any feature was fetched: the container-shaping part of the +// resolved devcontainer.json (features as requested, with their +// options), the loaded compose project, the primary service's base +// image ID, and the engine options that reshape the project. +// +// A later Up whose freshly computed hash equals the stored one +// reattaches: it reads the merged metadata from the existing +// container's image label and starts the project's existing services, +// with no feature fetch and no image build — the compose counterpart +// of the image path's attachExisting. See reattachCompose. +const LabelComposeInputsHash = "dev.containers.compose-inputs-hash" + +// composeInputsHashVersion is folded into the hash so a change to what +// the hash covers invalidates every stored value (the next Up takes the +// full path once and stamps the new form) instead of comparing +// incompatible digests. +const composeInputsHashVersion = 1 + +// maxLocalFeatureHashBytes bounds how much local feature content +// composeInputsHash reads. A local feature is a handful of scripts; a +// tree past this is not one, and the hash fails rather than reading it, +// which only costs the reattach (the full path still runs). +const maxLocalFeatureHashBytes = 64 << 20 + +// composeHashedConfig is the container-shaping subset of a freshly +// resolved (pre-feature-merge) ResolvedConfig. Fields that Up re-reads +// from devcontainer.json on every call and that never reach the +// container or its image — lifecycle commands, remoteEnv, ports +// attributes, customizations, the display name — are left out, so +// editing them does not cost a feature fetch and an image rebuild. +type composeHashedConfig struct { + DevcontainerID string + Source *config.ComposeSource + LocalWorkspaceFolder string + ContainerWorkspaceFolder string + WorkspaceMount *config.Mount + ContainerUser string + RemoteUser string + UpdateRemoteUserUID *bool + ContainerEnv map[string]string + Mounts []config.Mount + RunArgs []string + Init *bool + Privileged *bool + CapAdd []string + SecurityOpt []string + OverrideCommand *bool + Features []composeHashedFeature +} + +type composeHashedFeature struct { + Ref string + SourceKind config.FeatureSourceKind + Options map[string]any + // LocalContent is the content digest of a local feature's + // directory. OCI and HTTPS features are identified by their + // reference as written, matching the reference CLI, which does not + // re-resolve feature tags when it starts an existing container. + LocalContent string `json:",omitempty"` +} + +type composeHashInput struct { + Version int + Config composeHashedConfig + // Project is the loaded compose model before any engine override. + // The project's interpolation environment and working directory are + // not hashed: they are inputs to the model, already reflected in it. + Services composetypes.Services + Networks composetypes.Networks + Volumes composetypes.Volumes + Secrets composetypes.Secrets + Configs composetypes.Configs + // BaseImageID is the local image ID of the primary service's base + // image (its image:, or the engine's build of its build:). + BaseImageID string + // Engine options that reshape the project or the feature layering. + ComposeBackend ComposeBackend + DisableHostPortPublishing bool + StrictFeatureVersionMatch bool + // HostOwner is the workspace folder's owner, which selects the + // updateRemoteUserUID reconciliation applied on Linux. + HostOwner string +} + +// composeInputsHash returns the hex digest stored as +// LabelComposeInputsHash. cfg must be the freshly resolved config, +// before layerFeatures and applyMetadataMerge mutate it; project must +// be the loaded project before any Apply*Override. +func (e *Engine) composeInputsHash(cfg *config.ResolvedConfig, opts UpOptions, project *composetypes.Project, baseImageID string) (string, error) { + src, ok := cfg.Source.(*config.ComposeSource) + if !ok { + return "", fmt.Errorf("compose inputs hash: not a compose source") + } + if baseImageID == "" { + return "", fmt.Errorf("compose inputs hash: base image ID unknown") + } + configDir := composeWorkingDir(cfg, opts) + features := make([]composeHashedFeature, 0, len(cfg.Features)) + for _, f := range cfg.Features { + hf := composeHashedFeature{Ref: f.Ref, SourceKind: f.SourceKind, Options: f.Options} + if f.SourceKind == config.FeatureSourceLocal { + digest, err := hashDirContent(feature.ResolveLocalRef(configDir, f.Ref), maxLocalFeatureHashBytes) + if err != nil { + return "", fmt.Errorf("compose inputs hash: local feature %s: %w", f.Ref, err) + } + hf.LocalContent = digest + } + features = append(features, hf) + } + in := composeHashInput{ + Version: composeInputsHashVersion, + Config: composeHashedConfig{ + DevcontainerID: cfg.DevcontainerID, + Source: src, + LocalWorkspaceFolder: cfg.LocalWorkspaceFolder, + ContainerWorkspaceFolder: cfg.ContainerWorkspaceFolder, + WorkspaceMount: cfg.WorkspaceMount, + ContainerUser: cfg.ContainerUser, + RemoteUser: cfg.RemoteUser, + UpdateRemoteUserUID: cfg.UpdateRemoteUserUID, + ContainerEnv: cfg.ContainerEnv, + Mounts: cfg.Mounts, + RunArgs: cfg.RunArgs, + Init: cfg.Init, + Privileged: cfg.Privileged, + CapAdd: cfg.CapAdd, + SecurityOpt: cfg.SecurityOpt, + OverrideCommand: cfg.OverrideCommand, + Features: features, + }, + Services: project.Services, + Networks: project.Networks, + Volumes: project.Volumes, + Secrets: project.Secrets, + Configs: project.Configs, + BaseImageID: baseImageID, + ComposeBackend: e.opts.ComposeBackend, + DisableHostPortPublishing: e.opts.DisableHostPortPublishing, + StrictFeatureVersionMatch: e.opts.StrictFeatureVersionMatch, + } + if uid, gid, ok := statOwner(cfg.LocalWorkspaceFolder); ok { + in.HostOwner = fmt.Sprintf("%d:%d", uid, gid) + } + b, err := json.Marshal(in) + if err != nil { + return "", fmt.Errorf("compose inputs hash: %w", err) + } + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:]), nil +} + +// hashDirContent digests every regular file under root (relative path, +// mode and bytes, in lexical order), reading at most limit bytes. +func hashDirContent(root string, limit int64) (string, error) { + var paths []string + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.Type().IsRegular() { + paths = append(paths, p) + } + return nil + }) + if err != nil { + return "", err + } + sort.Strings(paths) + h := sha256.New() + var total int64 + for _, p := range paths { + rel, err := filepath.Rel(root, p) + if err != nil { + return "", err + } + fi, err := os.Stat(p) + if err != nil { + return "", err + } + total += fi.Size() + if total > limit { + return "", fmt.Errorf("content exceeds %d bytes", limit) + } + _, _ = fmt.Fprintf(h, "%s\x00%o\x00%d\x00", filepath.ToSlash(rel), fi.Mode().Perm(), fi.Size()) // hash.Hash writes never fail + f, err := os.Open(p) + if err != nil { + return "", err + } + _, err = io.Copy(h, io.LimitReader(f, fi.Size())) + f.Close() + if err != nil { + return "", err + } + } + return hex.EncodeToString(h.Sum(nil)), nil +} + +// composePrimaryBaseRef is the image reference whose local ID stands +// for the primary service's base image in composeInputsHash: its +// image:, or the tag prepareComposeServiceImage builds its build: to. +func composePrimaryBaseRef(cfg *config.ResolvedConfig, svc *composetypes.ServiceConfig) string { + if svc.Build != nil { + return "dc-go-compose-base-" + cfg.DevcontainerID + ":latest" + } + return svc.Image +} + +// errComposeReattachMiss marks a reattach precondition that did not +// hold. Up then takes the full path; it is never returned to callers. +var errComposeReattachMiss = errors.New("compose reattach not possible") + +func reattachMiss(format string, args ...any) error { + return fmt.Errorf("%w: "+format, append([]any{errComposeReattachMiss}, args...)...) +} + +// reattachCompose is the compose counterpart of attachExisting. When +// the project's primary container exists and carries an +// LabelComposeInputsHash equal to the hash of the current inputs, it +// +// - reads the merged feature and image metadata from that +// container's image label, as attachExisting does, instead of +// fetching every feature and rebuilding the final image; +// - rebuilds the run override from it (entrypoint wrapper, mounts, +// env, privileged, capAdd, ...) exactly as the full path does; and +// - brings the project up through the selected backend against the +// existing images: the native orchestrator under the caller's +// AdoptExisting with sidecar builds skipped when their images are +// present, or `docker compose up --no-recreate` (which builds only +// missing images) on the shell-out backend. +// +// It returns (nil, errComposeReattachMiss-wrapped) when a precondition +// does not hold — no primary container, no or different hash label, +// the image tag the container was created from now names another +// image — and the caller falls back to the full path. cfg is not +// mutated on a miss. +func (e *Engine) reattachCompose( + ctx context.Context, + cfg *config.ResolvedConfig, + opts UpOptions, + project *composetypes.Project, + src *config.ComposeSource, + primary *composetypes.ServiceConfig, + projectName, workingDir string, +) (*Workspace, error) { + if opts.override != nil { + return nil, reattachMiss("image build requested") + } + if opts.PullPolicy == PullAlways { + return nil, reattachMiss("pull policy is always") + } + containers, err := e.runtime.ListContainers(ctx, runtime.LabelFilter{Match: map[string]string{ + compose.LabelComposeProject: projectName, + compose.LabelComposeService: src.Service, + LabelDevcontainerID: cfg.DevcontainerID, + }}) + if err != nil { + return nil, reattachMiss("list primary containers: %v", err) + } + if len(containers) != 1 { + return nil, reattachMiss("%d primary containers", len(containers)) + } + details, err := e.runtime.InspectContainer(ctx, containers[0].ID) + if err != nil || details == nil { + return nil, reattachMiss("inspect primary container: %v", err) + } + stored := details.Labels[LabelComposeInputsHash] + if stored == "" { + return nil, reattachMiss("primary container has no %s label", LabelComposeInputsHash) + } + base, err := e.runtime.InspectImage(ctx, composePrimaryBaseRef(cfg, primary)) + if err != nil || base == nil || base.ID == "" { + return nil, reattachMiss("base image not present locally: %v", err) + } + current, err := e.composeInputsHash(cfg, opts, project, base.ID) + if err != nil { + return nil, reattachMiss("%v", err) + } + if current != stored { + return nil, reattachMiss("inputs changed") + } + if details.ImageRef == "" { + return nil, reattachMiss("primary container image reference unknown") + } + img, err := e.runtime.InspectImage(ctx, details.ImageRef) + if err != nil || img == nil { + return nil, reattachMiss("image %s not present locally: %v", details.ImageRef, err) + } + if img.ID != details.Image { + // Someone re-tagged it (Engine.Build, a manual build): the tag + // no longer names the image the container runs, and a later + // recreate from it would silently switch images. + return nil, reattachMiss("image %s now names %s, container runs %s", details.ImageRef, img.ID, details.Image) + } + + // Committed: from here on cfg is merged from the label. + var layers []config.FeatureMetadata + if label := img.Labels[feature.MetadataLabel]; label != "" { + parsed, err := feature.ParseMetadataLabel(label) + if err != nil { + return nil, fmt.Errorf("compose reattach: parse %s label of %s: %w", feature.MetadataLabel, details.ImageRef, err) + } + layers = parsed + } + for _, f := range cfg.Features { + opts.bus.Emit(events.FeatureSkippedEvent{Ref: f.Ref, Reason: "compose_reattach"}) + } + applyMetadataMerge(cfg, layers, opts.LocalEnv) + + // Reattach never re-runs secretsCommand: the container's env is + // already baked (see UpOptions.RunSecretsCommand). + runOverride := e.composeRunOverride(ctx, cfg, opts, project, src, details.ImageRef, nil, stored) + return e.upComposeBackend(ctx, cfg, opts, project, src, projectName, workingDir, details.ImageRef, runOverride, true, true) +} diff --git a/up_compose_reattach_test.go b/up_compose_reattach_test.go new file mode 100644 index 0000000..67d240c --- /dev/null +++ b/up_compose_reattach_test.go @@ -0,0 +1,308 @@ +package devcontainer + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + composetypes "github.com/compose-spec/compose-go/v2/types" + + "github.com/crunchloop/devcontainer/compose" + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/runtime" +) + +// hashFixture is a resolved compose config plus its loaded project, as +// Up has them before any feature is fetched. +type hashFixture struct { + eng *Engine + cfg *config.ResolvedConfig + opts UpOptions + project *composetypes.Project +} + +// newHashFixture writes the workspace into dir (shared between the +// fixtures a test compares, so paths and the devcontainer ID match) +// and resolves it. +func newHashFixture(t *testing.T, dir, devcontainerJSON, composeYAML string) *hashFixture { + t.Helper() + if err := os.MkdirAll(filepath.Join(dir, ".devcontainer"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "docker-compose.yml"), []byte(composeYAML), 0o644); err != nil { + t.Fatal(err) + } + cfgPath := filepath.Join(dir, ".devcontainer", "devcontainer.json") + if err := os.WriteFile(cfgPath, []byte(devcontainerJSON), 0o644); err != nil { + t.Fatal(err) + } + opts := UpOptions{LocalWorkspaceFolder: dir, ConfigPath: cfgPath, LocalEnv: map[string]string{"HOME": "/home/x"}} + cfg, err := Resolve(context.Background(), ResolveOptions{LocalWorkspaceFolder: dir, ConfigPath: cfgPath, LocalEnv: opts.LocalEnv}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + src := cfg.Source.(*config.ComposeSource) + project, err := compose.Load(context.Background(), compose.LoadOptions{ + Files: src.Files, + WorkingDir: composeWorkingDir(cfg, opts), + ProjectName: composeProjectName(cfg), + Env: mapToEnvList(opts.LocalEnv), + }) + if err != nil { + t.Fatalf("compose.Load: %v", err) + } + eng, err := New(EngineOptions{Runtime: newFakeRuntime(), ComposeBackend: ComposeBackendNative}) + if err != nil { + t.Fatal(err) + } + return &hashFixture{eng: eng, cfg: cfg, opts: opts, project: project} +} + +func (f *hashFixture) hash(t *testing.T, baseID string) string { + t.Helper() + h, err := f.eng.composeInputsHash(f.cfg, f.opts, f.project, baseID) + if err != nil { + t.Fatalf("composeInputsHash: %v", err) + } + return h +} + +const hashBaseJSON = `{ + "dockerComposeFile": "../docker-compose.yml", + "service": "app", + "workspaceFolder": "/workspaces/proj", + "features": { "ghcr.io/devcontainers/features/docker-in-docker:2": { "moby": false } }, + "postStartCommand": "echo hi", + "customizations": { "vscode": { "extensions": ["a.b"] } } +}` + +const hashBaseCompose = ` +services: + app: + image: debian:bookworm-slim + command: ["sleep", "infinity"] + db: + image: postgres:17 +` + +func TestComposeInputsHash_Deterministic(t *testing.T) { + a := newHashFixture(t, t.TempDir(), hashBaseJSON, hashBaseCompose) + h1 := a.hash(t, "sha256:base") + if h2 := a.hash(t, "sha256:base"); h1 != h2 { + t.Fatalf("same inputs hashed differently: %s vs %s", h1, h2) + } + if len(h1) != 64 { + t.Fatalf("hash %q is not a sha256 hex digest", h1) + } +} + +// Everything that shapes the primary image or the project changes the +// hash, so a changed input takes the full path (fetch and build). +func TestComposeInputsHash_ChangesWithInputs(t *testing.T) { + cases := []struct { + name string + json, yaml string + baseID string + disablePort bool + }{ + {name: "base image ID", baseID: "sha256:other"}, + {name: "feature option", json: replaceOnce(t, hashBaseJSON, `"moby": false`, `"moby": true`)}, + {name: "feature version", json: replaceOnce(t, hashBaseJSON, "docker-in-docker:2", "docker-in-docker:2.17.0")}, + {name: "containerEnv", json: replaceOnce(t, hashBaseJSON, `"service": "app",`, `"service": "app", "containerEnv": {"A": "1"},`)}, + {name: "mounts", json: replaceOnce(t, hashBaseJSON, `"service": "app",`, `"service": "app", "mounts": ["type=volume,source=v,target=/v"],`)}, + {name: "primary service", yaml: replaceOnce(t, hashBaseCompose, `["sleep", "infinity"]`, `["sleep", "1000"]`)}, + {name: "sidecar service", yaml: replaceOnce(t, hashBaseCompose, "postgres:17", "postgres:18")}, + {name: "engine option", disablePort: true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + want := newHashFixture(t, dir, hashBaseJSON, hashBaseCompose).hash(t, "sha256:base") + json, yaml, baseID := hashBaseJSON, hashBaseCompose, "sha256:base" + if c.json != "" { + json = c.json + } + if c.yaml != "" { + yaml = c.yaml + } + if c.baseID != "" { + baseID = c.baseID + } + f := newHashFixture(t, dir, json, yaml) + f.eng.opts.DisableHostPortPublishing = c.disablePort + if got := f.hash(t, baseID); got == want { + t.Fatalf("hash unchanged after changing the %s", c.name) + } + }) + } +} + +// Fields Up re-reads from devcontainer.json on every call and that never +// reach the container or its image do not change the hash: editing a +// lifecycle command must not cost a feature fetch and a rebuild. +func TestComposeInputsHash_IgnoresNonContainerFields(t *testing.T) { + dir := t.TempDir() + want := newHashFixture(t, dir, hashBaseJSON, hashBaseCompose).hash(t, "sha256:base") + + edited := replaceOnce(t, hashBaseJSON, `"postStartCommand": "echo hi"`, + `"name": "renamed", "postStartCommand": "echo bye", "remoteEnv": {"X": "y"}, "forwardPorts": [3000]`) + edited = replaceOnce(t, edited, `["a.b"]`, `["c.d"]`) + if got := newHashFixture(t, dir, edited, hashBaseCompose).hash(t, "sha256:base"); got != want { + t.Fatalf("hash changed after editing name/lifecycle/remoteEnv/ports/customizations") + } +} + +func TestComposeInputsHash_LocalFeatureContent(t *testing.T) { + dir := t.TempDir() + json := replaceOnce(t, hashBaseJSON, `"ghcr.io/devcontainers/features/docker-in-docker:2": { "moby": false }`, `"./local-feature": {}`) + featDir := filepath.Join(dir, ".devcontainer", "local-feature") + if err := os.MkdirAll(featDir, 0o755); err != nil { + t.Fatal(err) + } + install := filepath.Join(featDir, "install.sh") + if err := os.WriteFile(install, []byte("#!/bin/sh\necho one\n"), 0o755); err != nil { + t.Fatal(err) + } + f := newHashFixture(t, dir, json, hashBaseCompose) + h1 := f.hash(t, "sha256:base") + if err := os.WriteFile(install, []byte("#!/bin/sh\necho two\n"), 0o755); err != nil { + t.Fatal(err) + } + if h2 := f.hash(t, "sha256:base"); h1 == h2 { + t.Fatal("hash unchanged after editing a local feature's install.sh") + } +} + +func TestComposeInputsHash_RequiresBaseImageID(t *testing.T) { + f := newHashFixture(t, t.TempDir(), hashBaseJSON, hashBaseCompose) + if _, err := f.eng.composeInputsHash(f.cfg, f.opts, f.project, ""); err == nil { + t.Fatal("hash computed without a base image ID") + } +} + +func replaceOnce(t *testing.T, s, old, new string) string { + t.Helper() + if strings.Count(s, old) != 1 { + t.Fatalf("%q does not occur exactly once", old) + } + return strings.Replace(s, old, new, 1) +} + +// listingFake answers ListContainers from the fakeRuntime's containers. +type listingFake struct{ *fakeRuntime } + +func (l listingFake) ListContainers(ctx context.Context, filter runtime.LabelFilter) ([]runtime.Container, error) { + l.mu.Lock() + defer l.mu.Unlock() + var out []runtime.Container + for _, d := range l.containersByID { + ok := true + for k, v := range filter.Match { + if d.Labels[k] != v { + ok = false + } + } + if ok { + out = append(out, d.Container) + } + } + return out, nil +} + +// reattachMissFixture is a compose workspace whose primary container +// exists with the hash of the current inputs; each test breaks one +// precondition and expects a miss (so Up falls back to the full path) +// without cfg being touched. +func reattachMissFixture(t *testing.T) (*hashFixture, *fakeRuntime, *runtime.ContainerDetails) { + t.Helper() + f := newHashFixture(t, t.TempDir(), hashBaseJSON, hashBaseCompose) + fr := newFakeRuntime() + eng, err := New(EngineOptions{Runtime: listingFake{fr}, ComposeBackend: ComposeBackendNative}) + if err != nil { + t.Fatal(err) + } + f.eng = eng + f.opts.bus = newEventBus(events.NewEmitter(nil), nil) + fr.imagesByRef["debian:bookworm-slim"] = &runtime.ImageDetails{ID: "sha256:base"} + fr.imagesByRef["dc-go-final-x:latest"] = &runtime.ImageDetails{ID: "sha256:final"} + c := &runtime.ContainerDetails{ + Container: runtime.Container{ID: "c1", Image: "sha256:final", State: runtime.StateExited}, + ImageRef: "dc-go-final-x:latest", + Labels: map[string]string{ + compose.LabelComposeProject: composeProjectName(f.cfg), + compose.LabelComposeService: "app", + LabelDevcontainerID: f.cfg.DevcontainerID, + LabelComposeInputsHash: f.hash(t, "sha256:base"), + }, + } + fr.containersByID["c1"] = c + return f, fr, c +} + +func (f *hashFixture) reattach(t *testing.T) error { + t.Helper() + src := f.cfg.Source.(*config.ComposeSource) + primary, err := compose.PrimaryService(f.project, src.Service) + if err != nil { + t.Fatal(err) + } + _, err = f.eng.reattachCompose(context.Background(), f.cfg, f.opts, f.project, src, primary, + composeProjectName(f.cfg), composeWorkingDir(f.cfg, f.opts)) + return err +} + +func TestReattachCompose_Misses(t *testing.T) { + // Control: with every precondition holding, reattach commits (here + // it then fails in the fake's orchestrator primitives, which is not + // a miss), so each case below misses for the reason it names. + t.Run("control", func(t *testing.T) { + f, _, _ := reattachMissFixture(t) + if err := f.reattach(t); errors.Is(err, errComposeReattachMiss) { + t.Fatalf("intact fixture missed: %v", err) + } + if len(f.cfg.Entrypoints) == 0 && f.cfg.OverrideCommand == nil { + t.Fatalf("committed reattach did not merge metadata into cfg") + } + }) + cases := map[string]func(f *hashFixture, fr *fakeRuntime, c *runtime.ContainerDetails){ + "no hash label (created before reattach existed)": func(_ *hashFixture, _ *fakeRuntime, c *runtime.ContainerDetails) { + delete(c.Labels, LabelComposeInputsHash) + }, + "inputs changed": func(_ *hashFixture, _ *fakeRuntime, c *runtime.ContainerDetails) { + c.Labels[LabelComposeInputsHash] = "0000" + }, + "base image moved": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + fr.imagesByRef["debian:bookworm-slim"] = &runtime.ImageDetails{ID: "sha256:newbase"} + }, + "base image missing": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + delete(fr.imagesByRef, "debian:bookworm-slim") + }, + "final image tag moved": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + fr.imagesByRef["dc-go-final-x:latest"] = &runtime.ImageDetails{ID: "sha256:rebuilt"} + }, + "no primary container": func(_ *hashFixture, fr *fakeRuntime, _ *runtime.ContainerDetails) { + delete(fr.containersByID, "c1") + }, + "pull policy always": func(f *hashFixture, _ *fakeRuntime, _ *runtime.ContainerDetails) { + f.opts.PullPolicy = PullAlways + }, + } + for name, breakIt := range cases { + t.Run(name, func(t *testing.T) { + f, fr, c := reattachMissFixture(t) + breakIt(f, fr, c) + before := *f.cfg + err := f.reattach(t) + if !errors.Is(err, errComposeReattachMiss) { + t.Fatalf("reattach = %v, want a miss", err) + } + if len(f.cfg.Mounts) != len(before.Mounts) || len(f.cfg.Entrypoints) != len(before.Entrypoints) || f.cfg.OverrideCommand != before.OverrideCommand { + t.Fatalf("cfg mutated on a miss") + } + }) + } +} diff --git a/up_compose_sidecar_test.go b/up_compose_sidecar_test.go index cd2e228..9378b01 100644 --- a/up_compose_sidecar_test.go +++ b/up_compose_sidecar_test.go @@ -73,7 +73,7 @@ func TestBuildComposeSidecarImages_BuildsNonPrimaryServices(t *testing.T) { }) src := &config.ComposeSource{Service: "app"} - if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", workingDir, sidecarUpOptions()); err != nil { + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", workingDir, sidecarUpOptions(), false); err != nil { t.Fatalf("buildComposeSidecarImages: %v", err) } @@ -120,7 +120,7 @@ func TestBuildComposeSidecarImages_HonorsRunServices(t *testing.T) { }) src := &config.ComposeSource{Service: "app", RunServices: []string{"db"}} - if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions()); err != nil { + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions(), false); err != nil { t.Fatalf("buildComposeSidecarImages: %v", err) } @@ -147,7 +147,7 @@ func TestBuildComposeSidecarImages_BuildsDependenciesOfSelection(t *testing.T) { }) src := &config.ComposeSource{Service: "app", RunServices: []string{"app"}} - if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions()); err != nil { + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions(), false); err != nil { t.Fatalf("buildComposeSidecarImages: %v", err) } if len(rt.builds) != 1 || rt.builds[0].Tag != "dc-x-db" {