Skip to content

Commit

Permalink
api_server: remove dumbo crate dev dependency
Browse files Browse the repository at this point in the history 
The unit tests would use dumbo to obtain a MacAddr
and then create a NetworkInterfaceConfig. This is unnecessary
and also wrong since the api_server should rather exercise
serialization capabilities.

Signed-off-by: Diana Popa <[email protected]>
  • Loading branch information
@dianpopa @alxiord
dianpopa authored and alxiord committed Jan 23, 2020
1 parent 2143732 commit f72e4ab
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 78 deletions.
1 change: 0 additions & 1 deletion Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions src/api_server/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,3 @@ vmm = { path = "../vmm" }

[dev-dependencies]
libc = ">=0.2.39"

dumbo = { path = "../dumbo" }
124 changes: 49 additions & 75 deletions src/api_server/src/request/net.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ use vmm::vmm_config::net::{NetworkInterfaceConfig, NetworkInterfaceUpdateConfig}

pub fn parse_put_net(body: &Body, id_from_path: Option<&&str>) -> Result<ParsedRequest, Error> {
METRICS.patch_api_requests.network_count.inc();
let id = match id_from_path {
Some(&id) => checked_id(id)?,
None => {
return Err(Error::EmptyID);
}
let id = if let Some(id) = id_from_path {
checked_id(id)?
} else {
return Err(Error::EmptyID);
};

let netif = serde_json::from_slice::<NetworkInterfaceConfig>(body.raw()).map_err(|e| {
Expand All @@ -30,11 +29,10 @@ pub fn parse_put_net(body: &Body, id_from_path: Option<&&str>) -> Result<ParsedR

pub fn parse_patch_net(body: &Body, id_from_path: Option<&&str>) -> Result<ParsedRequest, Error> {
METRICS.put_api_requests.network_count.inc();
let id = match id_from_path {
Some(&id) => checked_id(id)?,
None => {
return Err(Error::EmptyID);
}
let id = if let Some(id) = id_from_path {
checked_id(id)?
} else {
return Err(Error::EmptyID);
};

let netif =
Expand All @@ -55,100 +53,77 @@ pub fn parse_patch_net(body: &Body, id_from_path: Option<&&str>) -> Result<Parse

#[cfg(test)]
mod tests {
extern crate dumbo;
extern crate vmm;

use self::dumbo::MacAddr;
use super::*;

use serde_json;

use self::vmm::vmm_config::RateLimiterConfig;

fn get_dummy_netif(
iface_id: String,
host_dev_name: String,
mac: &str,
) -> NetworkInterfaceConfig {
NetworkInterfaceConfig {
iface_id,
host_dev_name,
guest_mac: Some(MacAddr::parse_str(mac).unwrap()),
rx_rate_limiter: None,
tx_rate_limiter: None,
allow_mmds_requests: false,
}
}
use super::*;

#[test]
fn test_parse_netif_request() {
fn parse_put_net_request() {
let body = r#"{
"iface_id": "foo",
"host_dev_name": "bar",
"guest_mac": "12:34:56:78:9A:BC",
"allow_mmds_requests": false
}"#;
// 1. Exercise infamous "The id from the path does not match id from the body!".
assert!(parse_put_net(&Body::new(body), Some(&"bar")).is_err());
assert!(parse_put_net(&Body::new(body), Some(&"foo")).is_ok());
// 2. The `id_from_path` cannot be None.
assert!(parse_put_net(&Body::new(body), None).is_err());

let netif_clone = get_dummy_netif(
String::from("foo"),
String::from("bar"),
"12:34:56:78:9A:BC",
);
// 3. Success case.
let netif_clone = serde_json::from_str::<NetworkInterfaceConfig>(body).unwrap();
match parse_put_net(&Body::new(body), Some(&"foo")) {
Ok(ParsedRequest::Sync(VmmAction::InsertNetworkDevice(netif))) => {
assert_eq!(netif, netif_clone)
}
_ => panic!("Test failed."),
}
}

#[test]
fn test_network_interface_body_serialization_and_deserialization() {
let netif_clone = NetworkInterfaceConfig {
iface_id: String::from("foo"),
host_dev_name: String::from("bar"),
guest_mac: Some(MacAddr::parse_str("12:34:56:78:9A:BC").unwrap()),
rx_rate_limiter: Some(RateLimiterConfig::default()),
tx_rate_limiter: Some(RateLimiterConfig::default()),
allow_mmds_requests: true,
};

// This is the json encoding of the netif variable.
let body = r#"{
// 4. Serde error for invalid field (bytes instead of bandwidth).
let body = r#"
{
"iface_id": "foo",
"host_dev_name": "bar",
"guest_mac": "12:34:56:78:9A:bc",
"rx_rate_limiter": {
"bytes": {
"size": 62500,
"refill_time": 1000
}
},
"tx_rate_limiter": {
},
"allow_mmds_requests": true
"bytes": {
"size": 62500,
"refill_time": 1000
}
}
}"#;

match parse_put_net(&Body::new(body), Some(&"foo")) {
Ok(ParsedRequest::Sync(VmmAction::InsertNetworkDevice(netif))) => {
assert!(parse_put_net(&Body::new(body), Some(&"foo")).is_err());
}

#[test]
fn parse_patch_net_request() {
let body = r#"{
"iface_id": "foo",
"rx_rate_limiter": {
},
"tx_rate_limiter": {
}
}"#;
// 1. Exercise infamous "The id from the path does not match id from the body!".
assert!(parse_patch_net(&Body::new(body), Some(&"bar")).is_err());
// 2. The `id_from_path` cannot be None.
assert!(parse_patch_net(&Body::new(body), None).is_err());

// 3. Success case.
let netif_clone = serde_json::from_str::<NetworkInterfaceUpdateConfig>(body).unwrap();
match parse_patch_net(&Body::new(body), Some(&"foo")) {
Ok(ParsedRequest::Sync(VmmAction::UpdateNetworkInterface(netif))) => {
assert_eq!(netif, netif_clone)
}
_ => panic!("Test failed."),
}

// Check that guest_mac and rate limiters are truly optional.
let body_no_mac = r#"{
"iface_id": "foo",
"host_dev_name": "bar"
}"#;

assert!(serde_json::from_str::<NetworkInterfaceConfig>(body_no_mac).is_ok());

assert!(parse_put_net(&Body::new(body), Some(&"bar")).is_err());
assert!(parse_patch_net(&Body::new(body), Some(&"bar")).is_err());
}

#[test]
fn test_network_interface_body_error_handling() {
// Serde error for invalid field (bytes instead of bandwidth).
// 4. Serde error for invalid field (bytes instead of bandwidth).
let body = r#"
{
"iface_id": "foo",
Expand All @@ -165,7 +140,6 @@ mod tests {
}
}
}"#;

assert!(parse_patch_net(&Body::new(body), Some(&"foo")).is_err());
}
}

0 comments on commit f72e4ab

Please sign in to comment.