Commit Graph
18 Commits
Author SHA1 Message Date
Sandro Meier 5430029668 dev_container: Resolve compose service build args in Dockerfile expansion (#54270)
For docker-compose-based dev containers, build args live on the primary
compose service's `build.args`, not on `dev_container.build` (which is
`None` in the compose case). `expanded_dockerfile_content` only
consulted the latter, so `${…}` references in the service's Dockerfile —
e.g. `FROM ${BASE_IMAGE}` — were never substituted. Downstream callers
then invoked `docker inspect "${BASE_IMAGE}"` and `docker pull
"${BASE_IMAGE}"`, both of which fail and the dev container fails to
start.

A new unit test `test_expands_compose_service_args_in_dockerfile` mounts
a Dockerfile with `FROM ${BASE_IMAGE}` backed by a compose service whose
`build.args` define `BASE_IMAGE=test_image:latest`, and asserts both
`expanded_dockerfile_content` and `image_from_dockerfile` produce the
resolved reference.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed dev container startup failing for docker-compose configs whose
service Dockerfile uses build-arg substitution in the FROM line (for
example, `FROM ${BASE_IMAGE}`).
2026-04-21 16:44:44 -07:00
Ivan Mironov 30403fa913 Fix devcontainer localEnv/containerEnv substitution when variable is undefined (#53728)
Existing code replaces only the variables that are present in the
environment. It ignores references to undefined variables and leaves
them as is.

Consider following devcontainer.json:

```json
{
  ...
  "remoteEnv": {
    "RUSTFLAGS": "${localEnv:RUSTFLAGS}"
  },
  ...
}
```

Existing code will set RUSTFLAGS inside the dev container to a correct
value only if RUSTFLAGS is defined outside the dev container. If
RUSTFLAGS is not defined outside, Zed will erroneously set RUSTFLAGS to
`${localEnv:RUSTFLAGS}`.

This patch fixes this by replacing such references with either empty
strings or provided default values (`${localEnv:VARNAME:default}`).

This patch also removes replacement of `\` with `/` for environment
variables. Existing code makes no sense as simple replacement will not
magically make Windows paths valid on *nix. It is also guaranteed to
break things: think about passing passwords via env vars, for example.

I also noticed that, for some reason, existing code serializes k-v maps
to JSON, then tries to replace the references, and after that
deserializes modified JSON back. I believe that this is horribly wrong
and may have some security implications. This is out of the scope of
this patch. And I hope that somebody from the Zed team will provide the
reasoning behind this code.

Self-Review Checklist:

- [X] I've reviewed my own diff for quality, security, and reliability
- [X] Unsafe blocks (if any) have justifying comments
- [X] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [X] Tests cover the new/changed behavior
- [X] Performance impact has been considered and is acceptable

~~Closes #ISSUE~~ This pull request is also a bug report.

Release Notes:

- Fixed substitution of `${localEnv:VARNAME}` and
`${containerEnv:VARNAME}` in devcontainer.json when variable is not
defined
2026-04-20 16:12:17 -04:00
KyleBarton dc487861ed Use dockerfile build args correctly, respect context path, fix image aliasing (#54210)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #54193

Additionally, respects `cacheFrom` and `options` args defined in
devcontainer.json's `build` object. Also, addresses a bug which was
hiding these problems by overriding the primary build target with
aliasing.

Release Notes:

- Fixed an image aliasing but, respect build context and build args in
dev containers
2026-04-20 08:38:53 -07:00
KyleBarton 9172f4bb23 Use proper pathing for windows & nix in test (#54129)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes N/A

Release Notes:

- N/A
2026-04-16 16:33:55 -07:00
Bennet Bo Fenner 460c7d5f66 Remove test_dockerfile_location_with_compose_context_parent test (#54119)
Removing it as it seems to constantly fail on CI on Windows. cc
@KyleBarton

```
FAIL [   0.693s] (1511/5475) dev_container devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent
  stdout ───

    running 1 test
    test devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent ... FAILED

    failures:

    failures:
        devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 62 filtered out; finished in 0.64s
    
  stderr ───

    thread 'devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent' (1992) panicked at crates\dev_container\src\devcontainer_manifest.rs:3817:9:
    assertion `left == right` failed
      left: None
     right: Some("C:\\\\path\\to\\local\\project\\.devcontainer/Dockerfile")
    stack backtrace:
       0: std::panicking::panic_handler
                 at /rustc/e408947bfd200af42db322daf0fadfe7e26d3bd1/library\std\src\panicking.rs:689
       1: core::panicking::panic_fmt
                 at /rustc/e408947bfd200af42db322daf0fadfe7e26d3bd1/library\core\src\panicking.rs:80
       2: core::panicking::assert_failed_inner
                 at /rustc/e408947bfd200af42db322daf0fadfe7e26d3bd1/library\core\src\panicking.rs:439
       3: core::panicking::assert_failed<enum2$<core::option::Option<std::path::PathBuf> >,enum2$<core::option::Option<std::path::PathBuf> > >
                 at C:\Rust\.rustup\toolchains\1.94.1-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\panicking.rs:394
       4: dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent::__test_dockerfile_location_with_compose_context_parent::async_fn$0
                 at .\src\devcontainer_manifest.rs:3817
       5: gpui::executor::impl$4::block_test::async_block$0<tuple$<>,enum2$<dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent::__test_dockerfile_location_with_compose_context_parent::async_fn_env$0> >
                 at C:\actions-runner\_work\zed\zed\crates\gpui\src\executor.rs:440
       6: scheduler::test_scheduler::impl$1::block
                 at C:\actions-runner\_work\zed\zed\crates\scheduler\src\test_scheduler.rs:538
       7: gpui::executor::ForegroundExecutor::block_test<tuple$<>,enum2$<dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent::__test_dockerfile_location_with_compose_context_parent::async_fn_env$0> >
                 at C:\actions-runner\_work\zed\zed\crates\gpui\src\executor.rs:447
       8: dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent::closure$0
                 at .\src\devcontainer_manifest.rs:3796
       9: gpui::test::run_test::closure$0
                 at C:\actions-runner\_work\zed\zed\crates\gpui\src\test.rs:109
      10: std::panicking::catch_unwind::do_call<gpui::test::run_test::closure_env$0,tuple$<> >
                 at C:\Rust\.rustup\toolchains\1.94.1-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:581
      11: alloc::vec::spec_from_iter::impl$0::from_iter<tuple$<gpui::app::entity_map::EntityId,alloc::boxed::Box<dyn$<core::any::Any>,alloc::alloc::Global> >,core::iter::adapters::filter_map::FilterMap<alloc::vec::drain::Drain<gpui::app::entity_map::EntityId,alloc::
      12: std::panicking::catch_unwind
                 at C:\Rust\.rustup\toolchains\1.94.1-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:544
      13: std::panic::catch_unwind<gpui::test::run_test::closure_env$0,tuple$<> >
                 at C:\Rust\.rustup\toolchains\1.94.1-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:359
      14: gpui::test::run_test
                 at C:\actions-runner\_work\zed\zed\crates\gpui\src\test.rs:106
      15: dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent
                 at .\src\devcontainer_manifest.rs:3796
      16: dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent::closure$0
                 at .\src\devcontainer_manifest.rs:3796
      17: core::ops::function::FnOnce::call_once<dev_container::devcontainer_manifest::test::test_dockerfile_location_with_compose_context_parent::closure_env$0,tuple$<> >
                 at C:\Rust\.rustup\toolchains\1.94.1-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:250
      18: core::ops::function::FnOnce::call_once
                 at /rustc/e408947bfd200af42db322daf0fadfe7e26d3bd1/library\core\src\ops\function.rs:250
    note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
```

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- x ] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [ ] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #ISSUE

Release Notes:

- N/A
2026-04-16 19:49:32 +00:00
KyleBarton e8e6907b18 Simplify remote workspace folder calculation for Dev Containers (#53829)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53635 

Release Notes:

- Fixed issues with Windows pathing when calculating target workspace
directory for dev containers
2026-04-16 11:50:08 -07:00
Toni AlataloandClaude Opus 4.6 7fde238bce dev_container: Resolve compose dockerfile path relative to build context (#53860)
## Context

When a Docker Compose service specifies `context: ..` and `dockerfile:
.devcontainer/Dockerfile`, Zed resolves the dockerfile path relative to
the compose file's directory instead of the build context. This produces
a doubled path like `.devcontainer/.devcontainer/Dockerfile` which
doesn't exist.

Per the [Docker Compose
spec](https://docs.docker.com/reference/compose-file/build/#dockerfile),
the `dockerfile` field is relative to the build context directory.

The fix resolves the context directory first (relative to the compose
file), then joins the dockerfile path to that.

Closes #53473

## Prior art

This fix is extracted from #53170 by @zdeneklapes, which addresses this
bug among several other dev container startup issues. This PR isolates
the dockerfile path resolution fix into a focused change to make it
easier to review and merge independently.

Differences from #53170:
- **Scope**: Only the dockerfile-relative-to-context fix, not the other
fixes (compose build args preservation, remote user fallback, Docker
inspect labels, etc.)
- **Implementation**: Inline resolution in `dockerfile_location()`
rather than separate helper methods
- **Absolute path handling**: Handles absolute dockerfile and context
paths
- **Tests**: Two test cases — compose file inside `.devcontainer/` with
`context: ..`, and compose file at project root with `context: .`

## How to Review

Single file change in
`crates/dev_container/src/devcontainer_manifest.rs`:
- **Fix** (line 234-252): Resolve build context relative to compose file
directory, then join dockerfile to that, instead of joining dockerfile
to `config_directory` directly. Uses `normalize_path` to resolve `..`
components.
- **Tests**: Two new `FakeDocker` compose config entries and
corresponding tests asserting correct resolved paths.

## Self-Review Checklist

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed docker-compose `dockerfile` path being resolved relative to the
compose file instead of the build `context` directory.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-16 09:56:32 -07:00
KyleBarton 1357695634 Use remote_user to execute on_create_command and update_content_command (#54020)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53995

Release Notes:

- Improved dev container remote script execution to more closely align
with the reference implementation
2026-04-16 09:00:41 -07:00
8efe882ef2 dev_container: Detect actual buildx availability instead of assuming Docker has it (#53910)
`supports_compose_buildkit()` returned `!self.is_podman()`, assuming
every Docker install has buildx. On setups like Colima where the buildx
CLI plugin isn't installed, this causes builds to fail with "classic
builder doesn't support additional contexts" since `DOCKER_BUILDKIT=1`
alone isn't enough without the plugin.

Now probes for `docker buildx version` at construction time and caches
the result. When buildx isn't found, the existing scratch-image fallback
path (same one Podman uses) kicks in instead.

Closes #53890

Release Notes:

- Fixed dev container builds failing on Docker installations without the
buildx plugin.

---------

Co-authored-by: krisswee <krisswee@users.noreply.github.com>
Co-authored-by: KyleBarton <kjb@initialcapacity.io>
2026-04-16 08:44:48 -07:00
KyleBarton 5cee9c5d63 Respect runArgs for dev containers (#53931)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53503

Release Notes:

- Fixed gap where runArgs were not respected when specified in
devcontainer.json
2026-04-15 08:24:34 -07:00
KyleBarton e215d79c54 Fix dockerfile image and alias parsing (#53538)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #52928

Release Notes:

- Fixed handling of multi-stage and stage-specified dockerfiles in dev
container manifests
- Fixed the way we find the base image in a dev container when build
args need expansion
2026-04-09 13:27:29 -07:00
KyleBarton 297a04b8ac Appropriately deserialize app_port in devcontainer.json (#53322)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53263

Properly deserializes the app_port field, even if it is an array of
strings. Also, removes app_port from consideration when building a
docker-compose-based dev container, since the spec specifies that it
only applies to non-docker-compose dev containers.

Release Notes:

- Fixed app_port deserialization in dev container
2026-04-07 10:58:35 -07:00
Peter SiegelandKyleBarton d1af853505 dev_container: Preserve build context for docker-compose Dockerfiles (#53140)
When a Docker Compose service specifies a build context, the generated
override file was replacing it with an empty context directory. This
meant Dockerfiles that reference files relative to their build context
(e.g. `COPY . /app`) would fail. The fix preserves the original build
context from the compose service, falling back to the empty context
directory only when no context was specified.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed docker compose Dockerfile build context not being preserved in
dev_container integration.

---------

Co-authored-by: KyleBarton <kjb@initialcapacity.io>
2026-04-06 12:13:17 -07:00
Peter SiegelandKyleBarton 0410960442 dev_container: Make volumes key and source key in volume mounts optional (#53137)
Fixes some issues concerning volume mounts in the `dev_container`
integration:
1. Docker Compose services that don't define a volumes key cause
deserialization to fail because the field was required. This field is
not strictly necessary, i.e. for other services in a docker compose
devcontainer configuration which the editor is not attached to.
1. Volume mounts where source or target is absent (e.g. `tmpfs` mounts
that only need a target) also fail to parse because both fields were
required. This makes the source key optional, matching the Docker
Compose spec.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed devcontainer initialization erroneously requiring each service
to have a volumes key.
- Fixed devcontainer initialization erroneously requiring source keys
for all volume mounts.

---------

Co-authored-by: KyleBarton <kjb@initialcapacity.io>
2026-04-06 11:46:54 -07:00
KyleBarton f21062860f Ensure updateUID gets run for docker-compose and plain images (#53106)
Dev Containers should run a script which updates the remote UID of the
image user, so that files are still accessible. This was being run
incorrectly (on the Docker-compose side) or not at all (in the case of a
plain dev container image). This change fixes this

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53081

Release Notes:

- Fixed dev container behavior for configs which use images without a
dockerfile
2026-04-06 09:01:51 -07:00
KyleBarton 2ff42499e3 Use an object for docker compose ports rather than raw string (#53090)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53048

Release Notes:

- Fixed serialization error with Docker Compose for dev containers
2026-04-03 11:51:31 -07:00
Om ChillureandKyleBarton 1cc3b6f386 Fix/devcontainer compose labels (#53057)
Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53042

Release Notes:

- Fixed dev container failing to open when Docker Compose file contains
`labels`

---------

Co-authored-by: KyleBarton <kjb@initialcapacity.io>
2026-04-03 16:31:18 +00:00
KyleBarton 4b1bc30b49 Dev containers native implementation (#52338)
## Context

Closes #11473

In-house Zed implementation of devcontainers. Replaces the dependency on
the [reference implementation](https://github.com/devcontainers/cli) via
Node.

This enables additional features with this implementation:
1. Zed extensions can be specified in the `customizations` block, via
this syntax in `devcontainer.json:
```
...
  "customizations": {
    "zed": {
      "extensions": ["vue", "ruby"],
    },
  },

```
2.
[forwardPorts](https://containers.dev/implementors/json_reference/#general-properties)
are supported for multiple ports proxied to the host

## How to Review

<!-- Help reviewers focus their attention:
- For small PRs: note what to focus on (e.g., "error handling in
foo.rs")
- For large PRs (>400 LOC): provide a guided tour — numbered list of
files/commits to read in order. (The `large-pr` label is applied
automatically.)
     - See the review process guidelines for comment conventions -->

## Self-Review Checklist

<!-- Check before requesting review: -->
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Improved devcontainer implementation by moving initialization and
creation in-house
2026-04-01 08:16:27 -07:00