api_server: implemented IntoParsedRequest...

andreeaflorescu · andreeaflorescu · commit b23a56e5b4e6 · 2018-10-17T16:10:39.000+03:00
... for BootSourceConfig, NetworkInterfaceBody and
APILoggerDescription.

Signed-off-by: Andreea Florescu &lt;fandree@amazon.com&gt;
diff --git a/api_server/src/http_service.rs b/api_server/src/http_service.rs
@@ -161,7 +161,7 @@ fn parse_boot_source_req<'a>(
                 .map_err(|e| {
                     METRICS.put_api_requests.boot_source_fails.inc();
                     Error::SerdeJson(e)
-                })?.into_parsed_request()
+                })?.into_parsed_request(None, method)
                 .map_err(|s| {
                     METRICS.put_api_requests.boot_source_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -210,20 +210,19 @@ fn parse_drives_req<'a>(
         1 if method == Method::Get => Ok(ParsedRequest::Dummy),
 
         1 if method == Method::Put => {
+            let unwrapped_id = id_from_path.ok_or(Error::InvalidID)?;
             METRICS.put_api_requests.drive_count.inc();
 
             let device_cfg = serde_json::from_slice::<BlockDeviceConfig>(body).map_err(|e| {
                 METRICS.put_api_requests.drive_fails.inc();
                 Error::SerdeJson(e)
             })?;
-            device_cfg.check_id(id_from_path.unwrap()).map_err(|s| {
-                METRICS.put_api_requests.drive_fails.inc();
-                Error::Generic(StatusCode::BadRequest, format!("{:?}", s))
-            })?;
-            Ok(device_cfg.into_parsed_request(None, method).map_err(|s| {
-                METRICS.put_api_requests.drive_fails.inc();
-                Error::Generic(StatusCode::BadRequest, s)
-            })?)
+            Ok(device_cfg
+                .into_parsed_request(Some(unwrapped_id.to_string()), method)
+                .map_err(|s| {
+                    METRICS.put_api_requests.drive_fails.inc();
+                    Error::Generic(StatusCode::BadRequest, s)
+                })?)
         }
 
         1 if method == Method::Patch => {
@@ -261,7 +260,7 @@ fn parse_logger_req<'a>(
                 .map_err(|e| {
                     METRICS.put_api_requests.logger_fails.inc();
                     Error::SerdeJson(e)
-                })?.into_parsed_request()
+                })?.into_parsed_request(None, method)
                 .map_err(|s| {
                     METRICS.put_api_requests.logger_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -332,7 +331,7 @@ fn parse_netif_req<'a>(
                 .map_err(|e| {
                     METRICS.put_api_requests.network_fails.inc();
                     Error::SerdeJson(e)
-                })?.into_parsed_request(unwrapped_id)
+                })?.into_parsed_request(Some(unwrapped_id.to_string()), method)
                 .map_err(|s| {
                     METRICS.put_api_requests.network_fails.inc();
                     Error::Generic(StatusCode::BadRequest, s)
@@ -873,7 +872,7 @@ mod tests {
             rate_limiter: None,
         };
 
-        match drive_desc.into_parsed_request(None, Method::Put) {
+        match drive_desc.into_parsed_request(Some(String::from("bar")), Method::Put) {
             Ok(pr) => match parse_drives_req(
                 &"/foo/bar"[1..].split_terminator('/').collect(),
                 &"/foo/bar",
@@ -1142,7 +1141,7 @@ mod tests {
             allow_mmds_requests: false,
         };
 
-        match netif.into_parsed_request("bar") {
+        match netif.into_parsed_request(Some(String::from("bar")), Method::Put) {
             Ok(pr) => match parse_netif_req(
                 &"/foo/bar"[1..].split_terminator('/').collect(),
                 &"/foo/bar",
diff --git a/api_server/src/request/boot_source.rs b/api_server/src/request/boot_source.rs
@@ -1,10 +1,10 @@
 use std::result;
 
 use futures::sync::oneshot;
-use hyper::{Response, StatusCode};
+use hyper::{Method, Response, StatusCode};
 
 use http_service::{json_fault_message, json_response};
-use request::{GenerateResponse, ParsedRequest, VmmAction};
+use request::{GenerateResponse, IntoParsedRequest, ParsedRequest, VmmAction};
 
 #[derive(Debug, Deserialize, PartialEq, Serialize)]
 #[serde(deny_unknown_fields)]
@@ -44,8 +44,12 @@ impl GenerateResponse for BootSourceConfigError {
     }
 }
 
-impl BootSourceConfig {
-    pub fn into_parsed_request(self) -> result::Result<ParsedRequest, String> {
+impl IntoParsedRequest for BootSourceConfig {
+    fn into_parsed_request(
+        self,
+        _: Option<String>,
+        _: Method,
+    ) -> result::Result<ParsedRequest, String> {
         let (sender, receiver) = oneshot::channel();
         Ok(ParsedRequest::Sync(
             VmmAction::ConfigureBootSource(self, sender),
@@ -85,9 +89,12 @@ mod tests {
             boot_args: Some(String::from("foobar")),
         };
         let (sender, receiver) = oneshot::channel();
-        assert!(body.into_parsed_request().eq(&Ok(ParsedRequest::Sync(
-            VmmAction::ConfigureBootSource(same_body, sender),
-            receiver
-        ))))
+        assert!(
+            body.into_parsed_request(None, Method::Put)
+                .eq(&Ok(ParsedRequest::Sync(
+                    VmmAction::ConfigureBootSource(same_body, sender),
+                    receiver
+                )))
+        )
     }
 }
diff --git a/api_server/src/request/drive.rs b/api_server/src/request/drive.rs
@@ -98,9 +98,15 @@ impl IntoParsedRequest for PatchDrivePayload {
 impl IntoParsedRequest for BlockDeviceConfig {
     fn into_parsed_request(
         self,
-        _: Option<String>,
+        id_from_path: Option<String>,
         method: Method,
     ) -> result::Result<ParsedRequest, String> {
+        let id_from_path = id_from_path.unwrap_or(String::new());
+        if id_from_path != self.drive_id {
+            return Err(String::from(
+                "The id from the path does not match the id from the body!",
+            ));
+        }
         let (sender, receiver) = oneshot::channel();
         match method {
             Method::Put => Ok(ParsedRequest::Sync(
@@ -336,14 +342,14 @@ mod tests {
         assert!(
             &desc
                 .clone()
-                .into_parsed_request(None, Method::Options)
+                .into_parsed_request(Some(String::from("foo")), Method::Options)
                 .eq(&Ok(ParsedRequest::Dummy))
         );
         let (sender, receiver) = oneshot::channel();
         assert!(
             &desc
                 .clone()
-                .into_parsed_request(None, Method::Put)
+                .into_parsed_request(Some(String::from("foo")), Method::Put)
                 .eq(&Ok(ParsedRequest::Sync(
                     VmmAction::InsertBlockDevice(desc, sender),
                     receiver
diff --git a/api_server/src/request/logger.rs b/api_server/src/request/logger.rs
@@ -1,10 +1,10 @@
 use std::result;
 
 use futures::sync::oneshot;
-use hyper::{Response, StatusCode};
+use hyper::{Method, Response, StatusCode};
 
 use http_service::{json_fault_message, json_response};
-use request::{GenerateResponse, ParsedRequest, VmmAction};
+use request::{GenerateResponse, IntoParsedRequest, ParsedRequest, VmmAction};
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
 pub enum APILoggerLevel {
@@ -44,8 +44,12 @@ impl GenerateResponse for APILoggerError {
     }
 }
 
-impl APILoggerDescription {
-    pub fn into_parsed_request(self) -> result::Result<ParsedRequest, String> {
+impl IntoParsedRequest for APILoggerDescription {
+    fn into_parsed_request(
+        self,
+        _: Option<String>,
+        _: Method,
+    ) -> result::Result<ParsedRequest, String> {
         let (sender, receiver) = oneshot::channel();
         Ok(ParsedRequest::Sync(
             VmmAction::ConfigureLogger(self, sender),
@@ -86,12 +90,12 @@ mod tests {
             show_log_origin: None,
         };
         format!("{:?}", desc);
-        assert!(&desc.clone().into_parsed_request().is_ok());
+        assert!(&desc.clone().into_parsed_request(None, Method::Put).is_ok());
         let (sender, receiver) = oneshot::channel();
         assert!(
             &desc
                 .clone()
-                .into_parsed_request()
+                .into_parsed_request(None, Method::Put)
                 .eq(&Ok(ParsedRequest::Sync(
                     VmmAction::ConfigureLogger(desc, sender),
                     receiver
diff --git a/api_server/src/request/net.rs b/api_server/src/request/net.rs
@@ -1,14 +1,14 @@
 use std::result;
 
 use futures::sync::oneshot;
-use hyper::{Response, StatusCode};
+use hyper::{Method, Response, StatusCode};
 
 use super::VmmAction;
 
 use data_model::vm::{DeviceState, RateLimiterDescription};
 use http_service::{json_fault_message, json_response};
 use net_util::{MacAddr, TapError};
-use request::{GenerateResponse, ParsedRequest};
+use request::{GenerateResponse, IntoParsedRequest, ParsedRequest};
 
 // This struct represents the strongly typed equivalent of the json body from net iface
 // related requests.
@@ -75,8 +75,13 @@ impl GenerateResponse for NetworkInterfaceError {
     }
 }
 
-impl NetworkInterfaceBody {
-    pub fn into_parsed_request(self, id_from_path: &str) -> result::Result<ParsedRequest, String> {
+impl IntoParsedRequest for NetworkInterfaceBody {
+    fn into_parsed_request(
+        self,
+        id_from_path: Option<String>,
+        _: Method,
+    ) -> result::Result<ParsedRequest, String> {
+        let id_from_path = id_from_path.unwrap_or(String::new());
         if id_from_path != self.iface_id {
             return Err(String::from(
                 "The id from the path does not match the id from the body!",
@@ -112,12 +117,17 @@ mod tests {
             allow_mmds_requests: false,
         };
 
-        assert!(netif.clone().into_parsed_request("bar").is_err());
+        assert!(
+            netif
+                .clone()
+                .into_parsed_request(Some(String::from("bar")), Method::Put)
+                .is_err()
+        );
         let (sender, receiver) = oneshot::channel();
         assert!(
             netif
                 .clone()
-                .into_parsed_request("foo")
+                .into_parsed_request(Some(String::from("foo")), Method::Put)
                 .eq(&Ok(ParsedRequest::Sync(
                     VmmAction::InsertNetworkDevice(netif, sender),
                     receiver
diff --git a/data_model/src/vm/devices/drive.rs b/data_model/src/vm/devices/drive.rs
@@ -37,16 +37,6 @@ pub struct BlockDeviceConfig {
 }
 
 impl BlockDeviceConfig {
-    pub fn check_id(&self, id: &str) -> result::Result<(), String> {
-        if self.drive_id != String::from(id) {
-            Err(String::from(
-                "The id from the path does not match the id from the body!",
-            ))
-        } else {
-            Ok(())
-        }
-    }
-
     pub fn get_partuuid(&self) -> Option<&String> {
         self.partuuid.as_ref()
     }
diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs
@@ -1363,7 +1363,7 @@ impl Vmm {
             VmmAction::ConfigureBootSource(boot_source_body, sender) => {
                 let boxed_response = Box::new(self.configure_boot_source(
                     boot_source_body.kernel_image_path,
-                    boot_source_body.boot_args
+                    boot_source_body.boot_args,
                 ));
                 Vmm::send_response(boxed_response, sender);
             }

Original file line number	Diff line number	Diff line change
`@@ -1363,7 +1363,7 @@ impl Vmm {`
`1363`	`1363`	`VmmAction::ConfigureBootSource(boot_source_body, sender) => {`
`1364`	`1364`	`let boxed_response = Box::new(self.configure_boot_source(`
`1365`	`1365`	`boot_source_body.kernel_image_path,`
`1366`		`- boot_source_body.boot_args`
	`1366`	`+ boot_source_body.boot_args,`
`1367`	`1367`	`));`
`1368`	`1368`	`Vmm::send_response(boxed_response, sender);`
`1369`	`1369`	`}`