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<T>` (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
This commit is contained in:
Kunall Banerjee authored and GitHub committed
1 parent
f9593f71aa
commit
3663fdb5cc
1 file changed
+82
-21
@@ -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<String>,
|
||||
#[serde(default)]
|
||||
on_auto_forward: OnAutoForward,
|
||||
#[serde(default)]
|
||||
elevate_if_needed: bool,
|
||||
#[serde(default)]
|
||||
require_local_port: bool,
|
||||
protocol: PortAttributeProtocol,
|
||||
#[serde(default)]
|
||||
protocol: Option<PortAttributeProtocol>,
|
||||
}
|
||||
|
||||
#[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#"
|
||||
|
||||
Reference in new issue
Block a user