From bcaa1b5231fc804c8b02517ada62fd48cccfa371 Mon Sep 17 00:00:00 2001 From: Sandro Meier Date: Wed, 22 Apr 2026 01:44:44 +0200 Subject: [PATCH] dev_container: Resolve compose service build args in Dockerfile expansion (#54270) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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}`). --- .../src/devcontainer_manifest.rs | 93 +++++++++++++++++-- 1 file changed, 87 insertions(+), 6 deletions(-) diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index fe5a5e1837f47128ab70bf20ffc53571d892bb01..952b48bd9b259500a3d67185c92dad4ad9e28778 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -2113,12 +2113,24 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true return Err(DevContainerError::DevContainerParseFailed); }; - let devcontainer_args = self - .dev_container() - .build - .as_ref() - .and_then(|b| b.args.clone()) - .unwrap_or_default(); + // For docker-compose configs the build args live on the primary + // compose service rather than on dev_container.build. + let devcontainer_args = match self.dev_container().build_type() { + DevContainerBuildType::DockerCompose => { + let compose = self.docker_compose_manifest().await?; + find_primary_service(&compose, self)? + .1 + .build + .and_then(|b| b.args) + .unwrap_or_default() + } + _ => self + .dev_container() + .build + .as_ref() + .and_then(|b| b.args.clone()) + .unwrap_or_default(), + }; let contents = self.fs.load(&dockerfile_path).await.map_err(|e| { log::error!("Failed to load Dockerfile: {e}"); DevContainerError::FilesystemError @@ -5066,6 +5078,46 @@ FROM docker.io/hexpm/elixir:1.21-erlang-28.4.1-debian-trixie-20260316-slim AS de ) } + #[gpui::test] + async fn test_expands_compose_service_args_in_dockerfile(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + env_logger::try_init().ok(); + + let given_devcontainer_contents = r#" + { + "dockerComposeFile": "docker-compose-with-args.yml", + "service": "app", + } + "#; + + let (test_dependencies, mut devcontainer_manifest) = + init_default_devcontainer_manifest(cx, given_devcontainer_contents) + .await + .unwrap(); + + test_dependencies + .fs + .atomic_write( + PathBuf::from(TEST_PROJECT_PATH).join(".devcontainer/Dockerfile"), + "FROM ${BASE_IMAGE}\nUSER root\n".to_string(), + ) + .await + .unwrap(); + + devcontainer_manifest.parse_nonremote_vars().unwrap(); + + let expanded = devcontainer_manifest + .expanded_dockerfile_content() + .await + .unwrap(); + + assert_eq!(expanded, "FROM test_image:latest\nUSER root"); + + let base_image = + image_from_dockerfile(expanded, &None).expect("base image resolves from compose args"); + assert_eq!(base_image, "test_image:latest"); + } + #[test] fn test_aliases_dockerfile_with_pre_existing_aliases_for_build() {} @@ -5306,6 +5358,35 @@ FROM docker.io/hexpm/elixir:1.21-erlang-28.4.1-debian-trixie-20260316-slim AS de volumes: HashMap::new(), })); } + if config_files.len() == 1 + && config_files.get(0) + == Some( + &project_path + .join(".devcontainer") + .join("docker-compose-with-args.yml"), + ) + { + return Ok(Some(DockerComposeConfig { + name: None, + services: HashMap::from([( + "app".to_string(), + DockerComposeService { + build: Some(DockerComposeServiceBuild { + context: Some(".".to_string()), + dockerfile: Some("Dockerfile".to_string()), + args: Some(HashMap::from([( + "BASE_IMAGE".to_string(), + "test_image:latest".to_string(), + )])), + additional_contexts: None, + target: None, + }), + ..Default::default() + }, + )]), + ..Default::default() + })); + } if config_files.len() == 1 && config_files.get(0) == Some(