From cd12d5f98dc522ed87b985b25f2534a866ea83e8 Mon Sep 17 00:00:00 2001 From: Kunall Banerjee Date: Mon, 20 Apr 2026 13:05:12 -0400 Subject: [PATCH] dev_container: Make all `PortAttributes` fields optional (#53799) The Dev Container [metadata reference](https://containers.dev/implementors/json_reference/#port-attributes) defines all five `PortAttributes` fields (`elevateIfNeeded`, `label`, `onAutoForward`, `protocol`, `requireLocalPort`) as optional with defined defaults. The struct required all of them, causing deserialization failures for any `portsAttributes` entry that omitted a field. Add `#[serde(default)]` to each field, wrap `label` and `protocol` in `Option` (spec default: not set), and derive `Default` on `OnAutoForward` with `Notify` as the default variant (spec default: `notify`). The bool fields already default to false via `serde(default)`, matching the spec. 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 - [ ] Performance impact has been considered and is acceptable Closes #53686. Release Notes: - Made all `PortAttributes` fields optional according to the Dev Container spec --- crates/dev_container/src/devcontainer_json.rs | 103 ++++++++++++++---- 1 file changed, 82 insertions(+), 21 deletions(-) diff --git a/crates/dev_container/src/devcontainer_json.rs b/crates/dev_container/src/devcontainer_json.rs index 3b580aff484ffe976455dddb4efc565c662139d4..482be951c6bbf78ddb517dbdbba72d3759178cc6 100644 --- a/crates/dev_container/src/devcontainer_json.rs +++ b/crates/dev_container/src/devcontainer_json.rs @@ -19,9 +19,10 @@ pub(crate) enum PortAttributeProtocol { Http, } -#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Deserialize, Serialize, Eq, PartialEq)] #[serde(rename_all = "camelCase")] pub(crate) enum OnAutoForward { + #[default] Notify, OpenBrowser, OpenBrowserOnce, @@ -33,11 +34,16 @@ pub(crate) enum OnAutoForward { #[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] #[serde(rename_all = "camelCase")] pub(crate) struct PortAttributes { - label: String, + #[serde(default)] + label: Option, + #[serde(default)] on_auto_forward: OnAutoForward, + #[serde(default)] elevate_if_needed: bool, + #[serde(default)] require_local_port: bool, - protocol: PortAttributeProtocol, + #[serde(default)] + protocol: Option, } #[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] @@ -824,30 +830,30 @@ mod test { ( "3000".to_string(), PortAttributes { - label: "This Port".to_string(), + label: Some("This Port".to_string()), on_auto_forward: OnAutoForward::Notify, elevate_if_needed: false, require_local_port: true, - protocol: PortAttributeProtocol::Https + protocol: Some(PortAttributeProtocol::Https) } ), ( "db:5432".to_string(), PortAttributes { - label: "This Port too".to_string(), + label: Some("This Port too".to_string()), on_auto_forward: OnAutoForward::Silent, elevate_if_needed: true, require_local_port: false, - protocol: PortAttributeProtocol::Http + protocol: Some(PortAttributeProtocol::Http) } ) ])), other_ports_attributes: Some(PortAttributes { - label: "Other Ports".to_string(), + label: Some("Other Ports".to_string()), on_auto_forward: OnAutoForward::OpenBrowser, elevate_if_needed: true, require_local_port: true, - protocol: PortAttributeProtocol::Https + protocol: Some(PortAttributeProtocol::Https) }), update_remote_user_uid: Some(true), remote_env: Some(HashMap::from([ @@ -1043,30 +1049,30 @@ mod test { ( "3000".to_string(), PortAttributes { - label: "This Port".to_string(), + label: Some("This Port".to_string()), on_auto_forward: OnAutoForward::Notify, elevate_if_needed: false, require_local_port: true, - protocol: PortAttributeProtocol::Https + protocol: Some(PortAttributeProtocol::Https) } ), ( "db:5432".to_string(), PortAttributes { - label: "This Port too".to_string(), + label: Some("This Port too".to_string()), on_auto_forward: OnAutoForward::Silent, elevate_if_needed: true, require_local_port: false, - protocol: PortAttributeProtocol::Http + protocol: Some(PortAttributeProtocol::Http) } ) ])), other_ports_attributes: Some(PortAttributes { - label: "Other Ports".to_string(), + label: Some("Other Ports".to_string()), on_auto_forward: OnAutoForward::OpenBrowser, elevate_if_needed: true, require_local_port: true, - protocol: PortAttributeProtocol::Https + protocol: Some(PortAttributeProtocol::Https) }), update_remote_user_uid: Some(true), remote_env: Some(HashMap::from([ @@ -1271,30 +1277,30 @@ mod test { ( "3000".to_string(), PortAttributes { - label: "This Port".to_string(), + label: Some("This Port".to_string()), on_auto_forward: OnAutoForward::Notify, elevate_if_needed: false, require_local_port: true, - protocol: PortAttributeProtocol::Https + protocol: Some(PortAttributeProtocol::Https) } ), ( "db:5432".to_string(), PortAttributes { - label: "This Port too".to_string(), + label: Some("This Port too".to_string()), on_auto_forward: OnAutoForward::Silent, elevate_if_needed: true, require_local_port: false, - protocol: PortAttributeProtocol::Http + protocol: Some(PortAttributeProtocol::Http) } ) ])), other_ports_attributes: Some(PortAttributes { - label: "Other Ports".to_string(), + label: Some("Other Ports".to_string()), on_auto_forward: OnAutoForward::OpenBrowser, elevate_if_needed: true, require_local_port: true, - protocol: PortAttributeProtocol::Https + protocol: Some(PortAttributeProtocol::Https) }), update_remote_user_uid: Some(true), remote_env: Some(HashMap::from([ @@ -1504,6 +1510,60 @@ mod test { assert_eq!(rendered, "type=tmpfs,target=/tmp,consistency=cached"); } + #[test] + fn should_deserialize_port_attributes_with_missing_optional_fields() { + let json = r#" + { + "image": "nginx", + "portsAttributes": { + "8080": { + "label": "app", + "onAutoForward": "silent" + } + } + } + "#; + + let result = deserialize_devcontainer_json(json); + assert!( + result.is_ok(), + "Expected deserialization to succeed with partial portsAttributes, got: {:?}", + result.err() + ); + + let devcontainer = result.unwrap(); + let port_attrs = devcontainer.ports_attributes.unwrap(); + let attrs = port_attrs.get("8080").unwrap(); + assert_eq!(attrs.elevate_if_needed, false); + assert_eq!(attrs.require_local_port, false); + } + + #[test] + fn should_deserialize_port_attributes_with_all_fields_omitted() { + let json = r#" + { + "image": "nginx", + "portsAttributes": { + "3000": {} + } + } + "#; + + let result = deserialize_devcontainer_json(json); + assert!( + result.is_ok(), + "Expected deserialization to succeed with empty portsAttributes, got: {:?}", + result.err() + ); + + let devcontainer = result.unwrap(); + let port_attrs = devcontainer.ports_attributes.unwrap(); + let attrs = port_attrs.get("3000").unwrap(); + assert_eq!(attrs.on_auto_forward, OnAutoForward::Notify); + assert_eq!(attrs.elevate_if_needed, false); + assert_eq!(attrs.require_local_port, false); + } + #[test] fn should_fail_validation_with_workspace_mount_only() { let given_image_container_json = r#" @@ -1579,6 +1639,7 @@ mod test { )) ); } + #[test] fn should_pass_validation_with_workspace_folder_for_docker_compose() { let given_image_container_json = r#"