diff --git a/Cargo.lock b/Cargo.lock index 3dc93b90add..225d5ca6dbb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4939,6 +4939,7 @@ dependencies = [ "gpui", "http 1.3.1", "http_client", + "indoc", "log", "menu", "paths", @@ -4948,6 +4949,7 @@ dependencies = [ "serde", "serde_json", "serde_json_lenient", + "serde_yaml", "settings", "shlex", "ui", diff --git a/Cargo.toml b/Cargo.toml index 6db9f292287..817018a9509 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -724,6 +724,7 @@ serde_json_lenient = { version = "0.2", features = [ "preserve_order", "raw_value", ] } +serde_yaml = "0.9.34" serde_path_to_error = "0.1.17" serde_urlencoded = "0.7" sha2 = "0.10" diff --git a/crates/dev_container/Cargo.toml b/crates/dev_container/Cargo.toml index d051b51e8bf..c235b24db4a 100644 --- a/crates/dev_container/Cargo.toml +++ b/crates/dev_container/Cargo.toml @@ -11,6 +11,7 @@ async-trait.workspace = true serde.workspace = true serde_json.workspace = true serde_json_lenient.workspace = true +serde_yaml.workspace = true yaml-rust2.workspace = true shlex.workspace = true http_client.workspace = true @@ -33,6 +34,7 @@ workspace.workspace = true [dev-dependencies] fs = { workspace = true, features = ["test-support"] } +indoc.workspace = true gpui = { workspace = true, features = ["test-support"] } project = { workspace = true, features = ["test-support"] } serde_json.workspace = true diff --git a/crates/dev_container/src/command_json.rs b/crates/dev_container/src/command_json.rs index 8226767f579..be15c9016dc 100644 --- a/crates/dev_container/src/command_json.rs +++ b/crates/dev_container/src/command_json.rs @@ -43,6 +43,43 @@ where }) } +pub(crate) async fn evaluate_yaml_command( + mut command: Command, +) -> Result, DevContainerError> +where + T: for<'de> Deserialize<'de>, +{ + let output = command.output().await.map_err(|e| { + log::error!("Error running command {:?}: {e}", command); + DevContainerError::CommandFailed(command.get_program().display().to_string()) + })?; + + deserialize_yaml_output(output).map_err(|e| { + log::error!("Error running command {:?}: {e}", command); + DevContainerError::CommandFailed(command.get_program().display().to_string()) + }) +} + +pub(crate) fn deserialize_yaml_output(output: Output) -> Result, String> +where + T: for<'de> Deserialize<'de>, +{ + if output.status.success() { + let raw = String::from_utf8_lossy(&output.stdout); + if raw.is_empty() || raw.trim() == "[]" || raw.trim() == "{}" { + return Ok(None); + } + serde_yaml::from_str(&raw) + .map(Some) + .map_err(|e| format!("Error deserializing from raw yaml: {e}")) + } else { + let std_err = String::from_utf8_lossy(&output.stderr); + Err(format!( + "Sent non-successful output; cannot deserialize. StdErr: {std_err}" + )) + } +} + pub(crate) fn deserialize_json_output(output: Output) -> Result, String> where T: for<'de> Deserialize<'de>, @@ -66,6 +103,8 @@ where mod tests { use std::process::ExitStatus; + use crate::docker::{DockerComposeConfig, DockerComposeServiceBuild}; + use super::*; fn success_output(stdout: &str) -> Output { @@ -104,4 +143,65 @@ mod tests { let result: Option = deserialize_json_output(output).unwrap(); assert_eq!(result, None); } + + #[test] + fn test_deserialize_yaml_docker_compose_config() { + let yaml = indoc::indoc! {" + name: my-project + services: + app: + image: node:18 + command: + - sleep + - infinity + build: + context: . + dockerfile: Dockerfile + db: + image: postgres:15 + volumes: {} + "}; + let output = success_output(yaml); + let result: DockerComposeConfig = deserialize_yaml_output(output) + .expect("deserialization should succeed") + .expect("result should not be None"); + + assert_eq!(result.name, Some("my-project".to_string())); + assert_eq!(result.services.len(), 2); + + let app = result + .services + .get("app") + .expect("app service should exist"); + assert_eq!(app.image, Some("node:18".to_string())); + assert_eq!( + app.command, + vec!["sleep".to_string(), "infinity".to_string()] + ); + assert_eq!( + app.build, + Some(DockerComposeServiceBuild { + context: Some(".".to_string()), + dockerfile: Some("Dockerfile".to_string()), + ..Default::default() + }) + ); + + let db = result.services.get("db").expect("db service should exist"); + assert_eq!(db.image, Some("postgres:15".to_string())); + } + + #[test] + fn test_deserialize_yaml_empty_output() { + let output = success_output(""); + let result: Option = deserialize_yaml_output(output).unwrap(); + assert_eq!(result, None); + } + + #[test] + fn test_deserialize_yaml_empty_object() { + let output = success_output("{}"); + let result: Option = deserialize_yaml_output(output).unwrap(); + assert_eq!(result, None); + } } diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index fe1dbe7b9a8..c88c4df995a 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -362,6 +362,59 @@ impl DevContainerManifest { } } + async fn copy_local_feature( + &self, + feature_ref: &str, + destination: &Path, + ) -> Result<(), DevContainerError> { + let source_path = normalize_path(&self.config_directory.join(feature_ref)); + + if !self.fs.is_dir(&source_path).await { + log::error!( + "Local feature directory '{}' not found at {:?}", + feature_ref, + source_path + ); + return Err(DevContainerError::ResourceFetchFailed); + } + + let items = fs::read_dir_items(&*self.fs, &source_path) + .await + .map_err(|e| { + log::error!( + "Failed to read local feature directory {:?}: {e}", + source_path + ); + DevContainerError::FilesystemError + })?; + + for (item_path, is_dir) in &items { + let relative = item_path.strip_prefix(&source_path).map_err(|e| { + log::error!("Failed to compute relative path for {:?}: {e}", item_path); + DevContainerError::FilesystemError + })?; + let dest_path = destination.join(relative); + + if *is_dir { + self.fs.create_dir(&dest_path).await.map_err(|e| { + log::error!("Failed to create directory {:?}: {e}", dest_path); + DevContainerError::FilesystemError + })?; + } else { + let content = self.fs.load_bytes(item_path).await.map_err(|e| { + log::error!("Failed to read file {:?}: {e}", item_path); + DevContainerError::FilesystemError + })?; + self.fs.write(&dest_path, &content).await.map_err(|e| { + log::error!("Failed to write file {:?}: {e}", dest_path); + DevContainerError::FilesystemError + })?; + } + } + + Ok(()) + } + async fn download_feature_and_dockerfile_resources(&mut self) -> Result<(), DevContainerError> { let dev_container = match &self.config { ConfigStatus::Deserialized(_) => { @@ -458,59 +511,66 @@ impl DevContainerManifest { DevContainerError::FilesystemError })?; - let oci_ref = parse_oci_feature_ref(feature_ref).ok_or_else(|| { - log::error!( - "Feature '{}' is not a supported OCI feature reference", - feature_ref - ); - DevContainerError::DevContainerParseFailed - })?; - let TokenResponse { token } = - get_oci_token(&oci_ref.registry, &oci_ref.path, &self.http_client) - .await - .map_err(|e| { - log::error!("Failed to get OCI token for feature '{}': {e}", feature_ref); - DevContainerError::ResourceFetchFailed - })?; - let manifest = get_oci_manifest( - &oci_ref.registry, - &oci_ref.path, - &token, - &self.http_client, - &oci_ref.version, - None, - ) - .await - .map_err(|e| { - log::error!( - "Failed to fetch OCI manifest for feature '{}': {e}", - feature_ref - ); - DevContainerError::ResourceFetchFailed - })?; - let digest = &manifest - .layers - .first() - .ok_or_else(|| { + if is_local_feature_ref(feature_ref) { + self.copy_local_feature(feature_ref, &feature_dir).await?; + } else { + let oci_ref = parse_oci_feature_ref(feature_ref).ok_or_else(|| { log::error!( - "OCI manifest for feature '{}' contains no layers", + "Feature '{}' is not a supported OCI feature reference", + feature_ref + ); + DevContainerError::DevContainerParseFailed + })?; + let TokenResponse { token } = + get_oci_token(&oci_ref.registry, &oci_ref.path, &self.http_client) + .await + .map_err(|e| { + log::error!( + "Failed to get OCI token for feature '{}': {e}", + feature_ref + ); + DevContainerError::ResourceFetchFailed + })?; + let manifest = get_oci_manifest( + &oci_ref.registry, + &oci_ref.path, + &token, + &self.http_client, + &oci_ref.version, + None, + ) + .await + .map_err(|e| { + log::error!( + "Failed to fetch OCI manifest for feature '{}': {e}", feature_ref ); DevContainerError::ResourceFetchFailed - })? - .digest; - download_oci_tarball( - &token, - &oci_ref.registry, - &oci_ref.path, - digest, - "application/vnd.devcontainers.layer.v1+tar", - &feature_dir, - &self.http_client, - &self.fs, - None, - ) - .await?; + })?; + let digest = &manifest + .layers + .first() + .ok_or_else(|| { + log::error!( + "OCI manifest for feature '{}' contains no layers", + feature_ref + ); + DevContainerError::ResourceFetchFailed + })? + .digest; + download_oci_tarball( + &token, + &oci_ref.registry, + &oci_ref.path, + digest, + "application/vnd.devcontainers.layer.v1+tar", + &feature_dir, + &self.http_client, + &self.fs, + None, + ) + .await?; + } let feature_json_path = &feature_dir.join("devcontainer-feature.json"); if !self.fs.is_file(feature_json_path).await { @@ -537,7 +597,7 @@ impl DevContainerManifest { let feature_manifest = FeatureManifest::new(consecutive_id, feature_dir, feature_json); - log::debug!("Downloaded OCI feature content for '{}'", feature_ref); + log::debug!("Prepared feature content for '{}'", feature_ref); let env_content = feature_manifest .write_feature_env(&self.fs, options) @@ -1176,7 +1236,7 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true Some(( source.clone(), DockerComposeVolume { - name: source.clone(), + name: Some(source.clone()), }, )) } else { @@ -2565,6 +2625,10 @@ fn extract_feature_id(feature_ref: &str) -> &str { } } +fn is_local_feature_ref(feature_ref: &str) -> bool { + feature_ref.starts_with("./") || feature_ref.starts_with("../") +} + /// Generates a shell command that looks up a user's passwd entry. /// /// Mirrors the CLI's `getEntPasswdShellCommand` in `commonUtils.ts`. @@ -2836,7 +2900,7 @@ mod test { devcontainer_manifest::{ ConfigStatus, DevContainerManifest, DockerBuildResources, DockerComposeResources, DockerInspect, extract_feature_id, find_primary_service, get_remote_user_from_config, - image_from_dockerfile, resolve_compose_dockerfile, + image_from_dockerfile, is_local_feature_ref, resolve_compose_dockerfile, }, docker::{ DockerClient, DockerComposeConfig, DockerComposeService, DockerComposeServiceBuild, @@ -3061,6 +3125,16 @@ mod test { ); } + #[test] + fn should_identify_local_feature_refs() { + assert!(is_local_feature_ref("./lsp-devtools")); + assert!(is_local_feature_ref("./some/nested/feature")); + assert!(is_local_feature_ref("../sibling-feature")); + assert!(!is_local_feature_ref("ghcr.io/devcontainers/features/go:1")); + assert!(!is_local_feature_ref("ghcr.io/user/repo/node:18.0.0")); + assert!(!is_local_feature_ref("https://example.com/feature.tgz")); + } + #[gpui::test] async fn should_create_correct_docker_run_command(cx: &mut TestAppContext) { let mut metadata = HashMap::new(); @@ -4145,7 +4219,7 @@ ENV DOCKER_BUILDKIT=1 volumes: HashMap::from([( "dind-var-lib-docker-42dad4b4ca7b8ced".to_string(), DockerComposeVolume { - name: "dind-var-lib-docker-42dad4b4ca7b8ced".to_string(), + name: Some("dind-var-lib-docker-42dad4b4ca7b8ced".to_string()), }, )]), }; @@ -5156,6 +5230,116 @@ chmod +x ./install.sh })) } + #[cfg(not(target_os = "windows"))] + #[gpui::test] + async fn test_spawns_devcontainer_with_local_feature(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + env_logger::try_init().ok(); + let given_devcontainer_contents = r#" + { + "name": "cli-local-feature-test", + "image": "test_image:latest", + "features": { + "./lsp-devtools": { + "version": "0.1.0" + } + } + } + "#; + + let (test_dependencies, mut devcontainer_manifest) = + init_default_devcontainer_manifest(cx, given_devcontainer_contents) + .await + .unwrap(); + + test_dependencies + .fs + .insert_tree( + format!("{TEST_PROJECT_PATH}/.devcontainer/lsp-devtools"), + serde_json::json!({ + "devcontainer-feature.json": r#"{ + "id": "lsp-devtools", + "version": "0.1.0", + "name": "LSP Devtools", + "options": { + "version": { + "type": "string", + "default": "latest" + } + } + }"#, + "install.sh": "#!/bin/sh\nset -e\necho 'Installing lsp-devtools'", + }), + ) + .await; + + devcontainer_manifest.parse_nonremote_vars().unwrap(); + + let _devcontainer_up = devcontainer_manifest.build_and_run().await.unwrap(); + + let files = test_dependencies.fs.files(); + + let feature_dockerfile = files + .iter() + .find(|f| { + f.file_name() + .is_some_and(|s| s.display().to_string() == "Dockerfile.extended") + }) + .expect("Dockerfile.extended should be generated"); + let feature_dockerfile = test_dependencies.fs.load(feature_dockerfile).await.unwrap(); + + assert!( + feature_dockerfile.contains("lsp-devtools_0"), + "Dockerfile.extended should reference the local feature. Got:\n{}", + feature_dockerfile + ); + + let install_wrapper = files + .iter() + .find(|f| { + f.file_name() + .is_some_and(|s| s.display().to_string() == "devcontainer-features-install.sh") + && f.to_str().is_some_and(|s| s.contains("/lsp-devtools_")) + }) + .expect("Install wrapper should be generated for local feature"); + let install_wrapper = test_dependencies.fs.load(install_wrapper).await.unwrap(); + assert!( + install_wrapper.contains("./lsp-devtools"), + "Install wrapper should reference the local feature path. Got:\n{}", + install_wrapper + ); + + let feature_env = files + .iter() + .find(|f| { + f.file_name() + .is_some_and(|s| s.display().to_string() == "devcontainer-features.env") + && f.to_str().is_some_and(|s| s.contains("/lsp-devtools_")) + }) + .expect("Feature env file should be generated for local feature"); + let feature_env = test_dependencies.fs.load(feature_env).await.unwrap(); + assert!( + feature_env.contains("VERSION=0.1.0"), + "Feature env should contain user-provided version override. Got:\n{}", + feature_env + ); + + let install_sh = files + .iter() + .find(|f| { + f.file_name() + .is_some_and(|s| s.display().to_string() == "install.sh") + && f.to_str().is_some_and(|s| s.contains("/lsp-devtools_")) + }) + .expect("install.sh should be copied from the local feature directory"); + let install_sh = test_dependencies.fs.load(install_sh).await.unwrap(); + assert!( + install_sh.contains("Installing lsp-devtools"), + "install.sh should have the original content. Got:\n{}", + install_sh + ); + } + #[cfg(not(target_os = "windows"))] #[gpui::test] async fn test_spawns_devcontainer_with_plain_image(cx: &mut TestAppContext) { diff --git a/crates/dev_container/src/docker.rs b/crates/dev_container/src/docker.rs index be0fe0ed81b..cccbb4373ed 100644 --- a/crates/dev_container/src/docker.rs +++ b/crates/dev_container/src/docker.rs @@ -5,7 +5,8 @@ use serde::{Deserialize, Deserializer, Serialize, de}; use util::command::Command; use crate::{ - command_json::evaluate_json_command, devcontainer_api::DevContainerError, + command_json::{evaluate_json_command, evaluate_yaml_command}, + devcontainer_api::DevContainerError, devcontainer_json::MountDefinition, }; @@ -126,6 +127,7 @@ where #[derive(Debug, Clone, Deserialize, Serialize, Eq, PartialEq, Default)] pub(crate) struct DockerComposeService { + #[serde(skip_serializing_if = "Option::is_none")] pub(crate) image: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) entrypoint: Option>, @@ -143,7 +145,11 @@ pub(crate) struct DockerComposeService { pub(crate) build: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) privileged: Option, - #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[serde( + default, + skip_serializing_if = "Vec::is_empty", + deserialize_with = "deserialize_compose_volumes" + )] pub(crate) volumes: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) env_file: Option>, @@ -161,7 +167,8 @@ pub(crate) struct DockerComposeService { #[derive(Debug, Clone, Deserialize, Serialize, Eq, PartialEq, Default)] pub(crate) struct DockerComposeVolume { - pub(crate) name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) name: Option, } #[derive(Debug, Clone, Deserialize, Serialize, Eq, PartialEq, Default)] @@ -169,7 +176,7 @@ pub(crate) struct DockerComposeConfig { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) name: Option, pub(crate) services: HashMap, - #[serde(default)] + #[serde(default, deserialize_with = "deserialize_compose_top_level_volumes")] pub(crate) volumes: HashMap, } @@ -251,7 +258,7 @@ impl Docker { for file_path in config_files { command.args(&["-f", &file_path.display().to_string()]); } - command.args(&["config", "--format", "json"]); + command.arg("config"); command } } @@ -277,7 +284,7 @@ impl DockerClient for Docker { config_files: &Vec, ) -> Result, DevContainerError> { let command = self.create_docker_compose_config_command(config_files); - evaluate_json_command(command).await + evaluate_yaml_command(command).await } async fn docker_compose_build( @@ -526,6 +533,106 @@ where deserializer.deserialize_any(LabelsVisitor) } +fn deserialize_compose_volumes<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + #[derive(Deserialize)] + #[serde(untagged)] + enum VolumeItem { + Object(MountDefinition), + String(String), + } + + let items = Vec::::deserialize(deserializer)?; + items + .into_iter() + .map(|item| match item { + VolumeItem::Object(mount) => Ok(mount), + VolumeItem::String(s) => parse_compose_volume_string(&s) + .ok_or_else(|| de::Error::custom(format!("invalid volume string: {s}"))), + }) + .collect() +} + +/// Parses Docker Compose short volume syntax: `[SOURCE:]TARGET[:MODE]`. +/// A leading drive letter (e.g. `C:`) on the source is treated as part of the +/// path rather than as a source/target separator. +fn parse_compose_volume_string(s: &str) -> Option { + let bytes = s.as_bytes(); + + // Find the colon that separates source from target, skipping a possible + // Windows drive-letter prefix (single ASCII letter followed by `:`). + let separator_start = if bytes.len() >= 2 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && bytes.get(2).map_or(false, |&b| b == b'/' || b == b'\\') + { + // Skip past the drive letter prefix (e.g. "C:\") + 3 + } else { + 0 + }; + + if let Some(colon_pos) = s[separator_start..].find(':') { + let colon_pos = colon_pos + separator_start; + let source = &s[..colon_pos]; + + let rest = &s[colon_pos + 1..]; + + // `rest` may itself start with a Windows drive letter, so skip past + // that before looking for a second colon that would delimit the mode. + let mode_search_start = if rest.len() >= 2 + && rest.as_bytes()[0].is_ascii_alphabetic() + && rest.as_bytes()[1] == b':' + { + 2 + } else { + 0 + }; + + let (target, _mode) = if let Some(pos) = rest[mode_search_start..].find(':') { + let pos = pos + mode_search_start; + (&rest[..pos], Some(&rest[pos + 1..])) + } else { + (rest, None) + }; + + if target.is_empty() { + return None; + } + + Some(MountDefinition { + source: Some(source.to_string()), + target: target.to_string(), + mount_type: None, + }) + } else { + // No colon at all — anonymous volume with only a target path + if s.is_empty() { + return None; + } + Some(MountDefinition { + source: None, + target: s.to_string(), + mount_type: None, + }) + } +} + +fn deserialize_compose_top_level_volumes<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let map: HashMap> = HashMap::deserialize(deserializer)?; + Ok(map + .into_iter() + .map(|(key, value)| (key, value.unwrap_or_default())) + .collect()) +} + fn deserialize_nullable_vec<'de, D, T>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, @@ -966,7 +1073,7 @@ mod test { volumes: HashMap::from([( "postgres-data".to_string(), DockerComposeVolume { - name: "devcontainer_postgres-data".to_string(), + name: Some("devcontainer_postgres-data".to_string()), }, )]), }; @@ -1093,6 +1200,73 @@ mod test { assert_eq!(service.volumes[0].mount_type, Some("tmpfs".to_string())); } + #[test] + fn should_deserialize_compose_inline_volume_strings() { + let given_yaml = indoc::indoc! {r#" + name: devcontainer + services: + app: + image: node:18 + volumes: + - postgres-data:/var/lib/postgresql/data + - /host/path:/container/path + - /anonymous/volume + - type: bind + source: /explicit + target: /mnt/explicit + volumes: + postgres-data: + name: devcontainer_postgres-data + "#}; + + let config: DockerComposeConfig = serde_yaml::from_str(given_yaml).unwrap(); + let service = config.services.get("app").unwrap(); + assert_eq!(service.volumes.len(), 4); + + assert_eq!(service.volumes[0].source, Some("postgres-data".to_string())); + assert_eq!(service.volumes[0].target, "/var/lib/postgresql/data"); + assert_eq!(service.volumes[0].mount_type, None); + + assert_eq!(service.volumes[1].source, Some("/host/path".to_string())); + assert_eq!(service.volumes[1].target, "/container/path"); + + assert_eq!(service.volumes[2].source, None); + assert_eq!(service.volumes[2].target, "/anonymous/volume"); + + assert_eq!(service.volumes[3].source, Some("/explicit".to_string())); + assert_eq!(service.volumes[3].target, "/mnt/explicit"); + assert_eq!(service.volumes[3].mount_type, Some("bind".to_string())); + } + + #[test] + fn should_deserialize_compose_top_level_volumes_with_null_value() { + let given_yaml = indoc::indoc! {r#" + name: devcontainer + services: + app: + image: node:18 + volumes: + postgres-data: + named-vol: + name: custom-name + "#}; + + let config: DockerComposeConfig = serde_yaml::from_str(given_yaml).unwrap(); + assert_eq!(config.volumes.len(), 2); + + let bare = config + .volumes + .get("postgres-data") + .expect("bare volume should exist"); + assert_eq!(bare.name, None); + + let named = config + .volumes + .get("named-vol") + .expect("named volume should exist"); + assert_eq!(named.name, Some("custom-name".to_string())); + } + #[test] fn should_deserialize_inspect_without_labels() { let given_config = r#"