dev_container: Make volumes key and source key in volume mounts optional (#53137)

Peter Siegel and KyleBarton created 

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>

Change summary

crates/dev_container/src/devcontainer_json.rs     | 67 +++++++++-------
crates/dev_container/src/devcontainer_manifest.rs | 13 +-
crates/dev_container/src/docker.rs                | 50 ++++++++++++
3 files changed, 93 insertions(+), 37 deletions(-)

Detailed changes

crates/dev_container/src/devcontainer_json.rs 🔗 

@@ -60,7 +60,8 @@ pub(crate) enum ShutdownAction {
 #[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
 #[serde(rename_all = "camelCase")]
 pub(crate) struct MountDefinition {
-    pub(crate) source: String,
+    #[serde(default)]
+    pub(crate) source: Option<String>,
     pub(crate) target: String,
     #[serde(rename = "type")]
     pub(crate) mount_type: Option<String>,
@@ -68,23 +69,23 @@ pub(crate) struct MountDefinition {
 
 impl Display for MountDefinition {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(
-            f,
-            "type={},source={},target={},consistency=cached",
-            self.mount_type.clone().unwrap_or_else(|| {
-                if self.source.starts_with('/')
-                    || self.source.starts_with("\\\\")
-                    || self.source.get(1..3) == Some(":\\")
-                    || self.source.get(1..3) == Some(":/")
+        let mount_type = self.mount_type.clone().unwrap_or_else(|| {
+            if let Some(source) = &self.source {
+                if source.starts_with('/')
+                    || source.starts_with("\\\\")
+                    || source.get(1..3) == Some(":\\")
+                    || source.get(1..3) == Some(":/")
                 {
-                    "bind".to_string()
-                } else {
-                    "volume".to_string()
+                    return "bind".to_string();
                 }
-            }),
-            self.source,
-            self.target
-        )
+            }
+            "volume".to_string()
+        });
+        write!(f, "type={}", mount_type)?;
+        if let Some(source) = &self.source {
+            write!(f, ",source={}", source)?;
+        }
+        write!(f, ",target={},consistency=cached", self.target)
     }
 }
 
@@ -447,8 +448,6 @@ where
                 }
             }
 
-            let source = source
-                .ok_or_else(|| D::Error::custom(format!("mount string missing 'source': {}", s)))?;
             let target = target
                 .ok_or_else(|| D::Error::custom(format!("mount string missing 'target': {}", s)))?;
 
@@ -502,9 +501,6 @@ where
                     }
                 }
 
-                let source = source.ok_or_else(|| {
-                    D::Error::custom(format!("mount string missing 'source': {}", s))
-                })?;
                 let target = target.ok_or_else(|| {
                     D::Error::custom(format!("mount string missing 'target': {}", s))
                 })?;
@@ -873,7 +869,7 @@ mod test {
                 ])),
                 container_user: Some("myUser".to_string()),
                 mounts: Some(vec![MountDefinition {
-                    source: "/localfolder/app".to_string(),
+                    source: Some("/localfolder/app".to_string()),
                     target: "/workspaces/app".to_string(),
                     mount_type: Some("volume".to_string()),
                 }]),
@@ -882,7 +878,7 @@ mod test {
                 override_command: Some(true),
                 workspace_folder: Some("/workspaces".to_string()),
                 workspace_mount: Some(MountDefinition {
-                    source: "/app".to_string(),
+                    source: Some("/app".to_string()),
                     target: "/workspaces/app".to_string(),
                     mount_type: Some("bind".to_string())
                 }),
@@ -1316,12 +1312,12 @@ mod test {
                 container_user: Some("myUser".to_string()),
                 mounts: Some(vec![
                     MountDefinition {
-                        source: "/localfolder/app".to_string(),
+                        source: Some("/localfolder/app".to_string()),
                         target: "/workspaces/app".to_string(),
                         mount_type: Some("volume".to_string()),
                     },
                     MountDefinition {
-                        source: "dev-containers-cli-bashhistory".to_string(),
+                        source: Some("dev-containers-cli-bashhistory".to_string()),
                         target: "/home/node/commandhistory".to_string(),
                         mount_type: None,
                     }
@@ -1331,7 +1327,7 @@ mod test {
                 override_command: Some(true),
                 workspace_folder: Some("/workspaces".to_string()),
                 workspace_mount: Some(MountDefinition {
-                    source: "/folder".to_string(),
+                    source: Some("/folder".to_string()),
                     target: "/workspace".to_string(),
                     mount_type: Some("bind".to_string())
                 }),
@@ -1356,7 +1352,7 @@ mod test {
     #[test]
     fn mount_definition_should_use_bind_type_for_unix_absolute_paths() {
         let mount = MountDefinition {
-            source: "/home/user/project".to_string(),
+            source: Some("/home/user/project".to_string()),
             target: "/workspaces/project".to_string(),
             mount_type: None,
         };
@@ -1372,7 +1368,7 @@ mod test {
     #[test]
     fn mount_definition_should_use_bind_type_for_windows_unc_paths() {
         let mount = MountDefinition {
-            source: "\\\\server\\share\\project".to_string(),
+            source: Some("\\\\server\\share\\project".to_string()),
             target: "/workspaces/project".to_string(),
             mount_type: None,
         };
@@ -1388,7 +1384,7 @@ mod test {
     #[test]
     fn mount_definition_should_use_bind_type_for_windows_absolute_paths() {
         let mount = MountDefinition {
-            source: "C:\\Users\\mrg\\cli".to_string(),
+            source: Some("C:\\Users\\mrg\\cli".to_string()),
             target: "/workspaces/cli".to_string(),
             mount_type: None,
         };
@@ -1400,4 +1396,17 @@ mod test {
             "Expected mount type 'bind' for Windows absolute path, but got: {rendered}"
         );
     }
+
+    #[test]
+    fn mount_definition_should_omit_source_when_none() {
+        let mount = MountDefinition {
+            source: None,
+            target: "/tmp".to_string(),
+            mount_type: Some("tmpfs".to_string()),
+        };
+
+        let rendered = mount.to_string();
+
+        assert_eq!(rendered, "type=tmpfs,target=/tmp,consistency=cached");
+    }
 }

crates/dev_container/src/devcontainer_manifest.rs 🔗 

@@ -1092,11 +1092,12 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true
             .filter_map(|mount| {
                 if let Some(mount_type) = &mount.mount_type
                     && mount_type.to_lowercase() == "volume"
+                    && let Some(source) = &mount.source
                 {
                     Some((
-                        mount.source.clone(),
+                        source.clone(),
                         DockerComposeVolume {
-                            name: mount.source.clone(),
+                            name: source.clone(),
                         },
                     ))
                 } else {
@@ -1744,7 +1745,7 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true
         };
 
         Ok(MountDefinition {
-            source: self.local_workspace_folder(),
+            source: Some(self.local_workspace_folder()),
             target: format!("/workspaces/{}", project_directory_name.display()),
             mount_type: None,
         })
@@ -3576,7 +3577,7 @@ ENV DOCKER_BUILDKIT=1
                         ])),
                         volumes: vec![
                             MountDefinition {
-                                source: "dind-var-lib-docker-42dad4b4ca7b8ced".to_string(),
+                                source: Some("dind-var-lib-docker-42dad4b4ca7b8ced".to_string()),
                                 target: "/var/lib/docker".to_string(),
                                 mount_type: Some("volume".to_string())
                             }
@@ -4644,7 +4645,7 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true
                                     additional_contexts: None,
                                 }),
                                 volumes: vec![MountDefinition {
-                                    source: "../..".to_string(),
+                                    source: Some("../..".to_string()),
                                     target: "/workspaces".to_string(),
                                     mount_type: Some("bind".to_string()),
                                 }],
@@ -4657,7 +4658,7 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true
                             DockerComposeService {
                                 image: Some("postgres:14.1".to_string()),
                                 volumes: vec![MountDefinition {
-                                    source: "postgres-data".to_string(),
+                                    source: Some("postgres-data".to_string()),
                                     target: "/var/lib/postgresql/data".to_string(),
                                     mount_type: Some("volume".to_string()),
                                 }],

crates/dev_container/src/docker.rs 🔗 

@@ -141,6 +141,7 @@ pub(crate) struct DockerComposeService {
     pub(crate) build: Option<DockerComposeServiceBuild>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) privileged: Option<bool>,
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
     pub(crate) volumes: Vec<MountDefinition>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) env_file: Option<Vec<String>>,
@@ -1042,7 +1043,7 @@ mod test {
                         ),
                         volumes: vec![MountDefinition {
                             mount_type: Some("bind".to_string()),
-                            source: "/path/to".to_string(),
+                            source: Some("/path/to".to_string()),
                             target: "/workspaces".to_string(),
                         }],
                         network_mode: Some("service:db".to_string()),
@@ -1072,7 +1073,7 @@ mod test {
                         image: Some("postgres:14.1".to_string()),
                         volumes: vec![MountDefinition {
                             mount_type: Some("volume".to_string()),
-                            source: "postgres-data".to_string(),
+                            source: Some("postgres-data".to_string()),
                             target: "/var/lib/postgresql/data".to_string(),
                         }],
                         ..Default::default()
@@ -1164,6 +1165,51 @@ mod test {
         assert!(config.volumes.is_empty());
     }
 
+    #[test]
+    fn should_deserialize_compose_with_missing_volumes_field() {
+        let given_config = r#"
+        {
+            "name": "devcontainer",
+            "services": {
+                "sidecar": {
+                    "image": "ubuntu:24.04"
+                }
+            }
+        }
+        "#;
+
+        let config: DockerComposeConfig = serde_json_lenient::from_str(given_config).unwrap();
+        let service = config.services.get("sidecar").unwrap();
+        assert!(service.volumes.is_empty());
+    }
+
+    #[test]
+    fn should_deserialize_compose_volume_without_source() {
+        let given_config = r#"
+        {
+            "name": "devcontainer",
+            "services": {
+                "app": {
+                    "image": "ubuntu:24.04",
+                    "volumes": [
+                        {
+                            "type": "tmpfs",
+                            "target": "/tmp"
+                        }
+                    ]
+                }
+            }
+        }
+        "#;
+
+        let config: DockerComposeConfig = serde_json_lenient::from_str(given_config).unwrap();
+        let service = config.services.get("app").unwrap();
+        assert_eq!(service.volumes.len(), 1);
+        assert_eq!(service.volumes[0].source, None);
+        assert_eq!(service.volumes[0].target, "/tmp");
+        assert_eq!(service.volumes[0].mount_type, Some("tmpfs".to_string()));
+    }
+
     #[test]
     fn should_deserialize_inspect_without_labels() {
         let given_config = r#"