Deprecate vsock_id

CHANGELOG.md

@@ -12,6 +12,10 @@
 - Added permanent HTTP endpoint for `GET` on `/version` for getting the
   Firecracker version.
 
+### Changed
+
+- Deprecated `vsock_id` body field in `PUT`s on `/vsock`.
+
 ### Fixed
 
 - Fixed incorrect propagation of init parameters in kernel commandline.

docs/vsock.md

@@ -88,7 +88,7 @@ images/vsock-connections.png?raw=true
 
 ## Setting up the virtio-vsock device
 
-The virtio-vsock device will require an ID, a CID, and the path to a backing
+The virtio-vsock device will require a CID, and the path to a backing
 AF_UNIX socket:
 
 ```bash
@@ -97,7 +97,6 @@ curl --unix-socket /tmp/firecracker.socket -i \
   -H 'Accept: application/json' \
   -H 'Content-Type: application/json' \
   -d '{
-      "vsock_id": "1",
       "guest_cid": 3,
       "uds_path": "./v.sock"
   }'

src/api_server/src/lib.rs

@@ -14,8 +14,10 @@ use std::path::PathBuf;
 use std::sync::{mpsc, Arc, Mutex};
 use std::{fmt, io};
 
-use crate::parsed_request::ParsedRequest;
-use logger::{debug, error, info, update_metric_with_elapsed_time, ProcessTimeReporter, METRICS};
+use crate::parsed_request::{ParsedRequest, RequestAction};
+use logger::{
+    debug, error, info, update_metric_with_elapsed_time, warn, ProcessTimeReporter, METRICS,
+};
 pub use micro_http::{
     Body, HttpServer, Method, Request, RequestError, Response, ServerError, ServerRequest,
     ServerResponse, StatusCode, Version,
@@ -244,16 +246,25 @@ impl ApiServer {
         request: &Request,
         request_processing_start_us: u64,
     ) -> Response {
-        match ParsedRequest::try_from_request(request) {
-            Ok(ParsedRequest::Sync(vmm_action)) => {
-                self.serve_vmm_action_request(vmm_action, request_processing_start_us)
-            }
-            Ok(ParsedRequest::GetMMDS) => self.get_mmds(),
-            Ok(ParsedRequest::PatchMMDS(value)) => self.patch_mmds(value),
-            Ok(ParsedRequest::PutMMDS(value)) => self.put_mmds(value),
-            Ok(ParsedRequest::ShutdownInternal) => {
-                self.shutdown_flag = true;
-                Response::new(Version::Http11, StatusCode::NoContent)
+        match ParsedRequest::try_from_request(request).map(|r| r.into_parts()) {
+            Ok((req_action, mut parsing_info)) => {
+                let mut response = match req_action {
+                    RequestAction::Sync(vmm_action) => {
+                        self.serve_vmm_action_request(vmm_action, request_processing_start_us)
+                    }
+                    RequestAction::GetMMDS => self.get_mmds(),
+                    RequestAction::PatchMMDS(value) => self.patch_mmds(value),
+                    RequestAction::PutMMDS(value) => self.put_mmds(value),
+                    RequestAction::ShutdownInternal => {
+                        self.shutdown_flag = true;
+                        Response::new(Version::Http11, StatusCode::NoContent)
+                    }
+                };
+                if let Some(message) = parsing_info.take_deprecation_message() {
+                    warn!("{}", message);
+                    response.set_deprecation();
+                }
+                response
             }
             Err(e) => {
                 error!("{}", e);

src/api_server/src/parsed_request.rs

@@ -27,15 +27,54 @@ use micro_http::{Body, Method, Request, Response, StatusCode, Version};
 use logger::{error, info};
 use vmm::rpc_interface::{VmmAction, VmmActionError};
 
-pub(crate) enum ParsedRequest {
+pub(crate) enum RequestAction {
     GetMMDS,
     PatchMMDS(Value),
     PutMMDS(Value),
     Sync(Box<VmmAction>),
     ShutdownInternal, // !!! not an API, used by shutdown to thread::join the API thread
 }
 
+#[derive(Default)]
+#[cfg_attr(test, derive(PartialEq))]
+pub(crate) struct ParsingInfo {
+    deprecation_message: Option<String>,
+}
+
+impl ParsingInfo {
+    pub fn append_deprecation_message(&mut self, message: &str) {
+        match self.deprecation_message.as_mut() {
+            None => self.deprecation_message = Some(message.to_owned()),
+            Some(s) => (*s).push_str(message),
+        }
+    }
+
+    pub fn take_deprecation_message(&mut self) -> Option<String> {
+        self.deprecation_message.take()
+    }
+}
+
+pub(crate) struct ParsedRequest {
+    action: RequestAction,
+    parsing_info: ParsingInfo,
+}
+
 impl ParsedRequest {
+    pub(crate) fn new(action: RequestAction) -> Self {
+        Self {
+            action,
+            parsing_info: Default::default(),
+        }
+    }
+
+    pub(crate) fn into_parts(self) -> (RequestAction, ParsingInfo) {
+        (self.action, self.parsing_info)
+    }
+
+    pub(crate) fn parsing_info(&mut self) -> &mut ParsingInfo {
+        &mut self.parsing_info
+    }
+
     pub(crate) fn try_from_request(request: &Request) -> Result<ParsedRequest, Error> {
         let request_uri = request.uri().get_abs_path().to_string();
         log_received_api_request(describe(
@@ -78,7 +117,9 @@ impl ParsedRequest {
             (Method::Put, "network-interfaces", Some(body)) => {
                 parse_put_net(body, path_tokens.get(1))
             }
-            (Method::Put, "shutdown-internal", None) => Ok(ParsedRequest::ShutdownInternal),
+            (Method::Put, "shutdown-internal", None) => {
+                Ok(ParsedRequest::new(RequestAction::ShutdownInternal))
+            }
             (Method::Put, "snapshot", Some(body)) => parse_put_snapshot(body, path_tokens.get(1)),
             (Method::Put, "vsock", Some(body)) => parse_put_vsock(body),
             (Method::Put, _, None) => method_to_error(Method::Put),
@@ -145,7 +186,7 @@ impl ParsedRequest {
 
     /// Helper function to avoid boiler-plate code.
     pub(crate) fn new_sync(vmm_action: VmmAction) -> ParsedRequest {
-        ParsedRequest::Sync(Box::new(vmm_action))
+        ParsedRequest::new(RequestAction::Sync(Box::new(vmm_action)))
     }
 }
 
@@ -281,15 +322,19 @@ pub(crate) mod tests {
 
     impl PartialEq for ParsedRequest {
         fn eq(&self, other: &ParsedRequest) -> bool {
-            match (self, other) {
-                (&ParsedRequest::Sync(ref sync_req), &ParsedRequest::Sync(ref other_sync_req)) => {
+            if self.parsing_info.deprecation_message != other.parsing_info.deprecation_message {
+                return false;
+            }
+
+            match (&self.action, &other.action) {
+                (RequestAction::Sync(ref sync_req), RequestAction::Sync(ref other_sync_req)) => {
                     sync_req == other_sync_req
                 }
-                (&ParsedRequest::GetMMDS, &ParsedRequest::GetMMDS) => true,
-                (&ParsedRequest::PutMMDS(ref val), &ParsedRequest::PutMMDS(ref other_val)) => {
+                (RequestAction::GetMMDS, RequestAction::GetMMDS) => true,
+                (RequestAction::PutMMDS(ref val), RequestAction::PutMMDS(ref other_val)) => {
                     val == other_val
                 }
-                (&ParsedRequest::PatchMMDS(ref val), &ParsedRequest::PatchMMDS(ref other_val)) => {
+                (RequestAction::PatchMMDS(ref val), RequestAction::PatchMMDS(ref other_val)) => {
                     val == other_val
                 }
                 _ => false,
@@ -298,8 +343,21 @@ pub(crate) mod tests {
     }
 
     pub(crate) fn vmm_action_from_request(req: ParsedRequest) -> VmmAction {
-        match req {
-            ParsedRequest::Sync(vmm_action) => *vmm_action,
+        match req.action {
+            RequestAction::Sync(vmm_action) => *vmm_action,
+            _ => panic!("Invalid request"),
+        }
+    }
+
+    pub(crate) fn depr_action_from_req(req: ParsedRequest, msg: Option<String>) -> VmmAction {
+        let (action_req, mut parsing_info) = req.into_parts();
+        match action_req {
+            RequestAction::Sync(vmm_action) => {
+                let req_msg = parsing_info.take_deprecation_message();
+                assert!(req_msg.is_some());
+                assert_eq!(req_msg, msg);
+                *vmm_action
+            }
             _ => panic!("Invalid request"),
         }
     }
@@ -853,8 +911,8 @@ pub(crate) mod tests {
             .unwrap();
         assert!(connection.try_read().is_ok());
         let req = connection.pop_parsed_request().unwrap();
-        match ParsedRequest::try_from_request(&req).unwrap() {
-            ParsedRequest::ShutdownInternal => (),
+        match ParsedRequest::try_from_request(&req).unwrap().into_parts() {
+            (RequestAction::ShutdownInternal, _) => (),
             _ => panic!("wrong parsed request"),
         };
     }

src/api_server/src/request/instance_info.rs

@@ -13,11 +13,12 @@ pub(crate) fn parse_get_instance_info() -> Result<ParsedRequest, Error> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::RequestAction;
 
     #[test]
     fn test_parse_get_instance_info_request() {
-        match parse_get_instance_info() {
-            Ok(ParsedRequest::Sync(action)) if *action == VmmAction::GetVmInstanceInfo => {}
+        match parse_get_instance_info().unwrap().into_parts() {
+            (RequestAction::Sync(action), _) if *action == VmmAction::GetVmInstanceInfo => {}
             _ => panic!("Test failed."),
         }
     }

src/api_server/src/request/mmds.rs

@@ -1,15 +1,15 @@
 // Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::parsed_request::{Error, ParsedRequest};
+use crate::parsed_request::{Error, ParsedRequest, RequestAction};
 use crate::request::Body;
 use logger::{IncMetric, METRICS};
 use micro_http::StatusCode;
 use vmm::rpc_interface::VmmAction::SetMmdsConfiguration;
 
 pub(crate) fn parse_get_mmds() -> Result<ParsedRequest, Error> {
     METRICS.get_api_requests.mmds_count.inc();
-    Ok(ParsedRequest::GetMMDS)
+    Ok(ParsedRequest::new(RequestAction::GetMMDS))
 }
 
 pub(crate) fn parse_put_mmds(
@@ -18,12 +18,12 @@ pub(crate) fn parse_put_mmds(
 ) -> Result<ParsedRequest, Error> {
     METRICS.put_api_requests.mmds_count.inc();
     match path_second_token {
-        None => Ok(ParsedRequest::PutMMDS(
+        None => Ok(ParsedRequest::new(RequestAction::PutMMDS(
             serde_json::from_slice(body.raw()).map_err(|e| {
                 METRICS.put_api_requests.mmds_fails.inc();
                 Error::SerdeJson(e)
             })?,
-        )),
+        ))),
         Some(&"config") => Ok(ParsedRequest::new_sync(SetMmdsConfiguration(
             serde_json::from_slice(body.raw()).map_err(|e| {
                 METRICS.put_api_requests.mmds_fails.inc();
@@ -42,12 +42,12 @@ pub(crate) fn parse_put_mmds(
 
 pub(crate) fn parse_patch_mmds(body: &Body) -> Result<ParsedRequest, Error> {
     METRICS.patch_api_requests.mmds_count.inc();
-    Ok(ParsedRequest::PatchMMDS(
+    Ok(ParsedRequest::new(RequestAction::PatchMMDS(
         serde_json::from_slice(body.raw()).map_err(|e| {
             METRICS.patch_api_requests.mmds_fails.inc();
             Error::SerdeJson(e)
         })?,
-    ))
+    )))
 }
 
 #[cfg(test)]

src/api_server/src/request/version.rs

@@ -13,11 +13,12 @@ pub(crate) fn parse_get_version() -> Result<ParsedRequest, Error> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::RequestAction;
 
     #[test]
     fn test_parse_get_version_request() {
-        match parse_get_version() {
-            Ok(ParsedRequest::Sync(action)) if *action == VmmAction::GetVmmVersion => {}
+        match parse_get_version().unwrap().into_parts() {
+            (RequestAction::Sync(action), _) if *action == VmmAction::GetVmmVersion => {}
             _ => panic!("Test failed."),
         }
     }

src/api_server/src/request/vsock.rs

@@ -4,32 +4,71 @@
 use super::super::VmmAction;
 use crate::parsed_request::{Error, ParsedRequest};
 use crate::request::Body;
+use logger::{IncMetric, METRICS};
 use vmm::vmm_config::vsock::VsockDeviceConfig;
 
 pub(crate) fn parse_put_vsock(body: &Body) -> Result<ParsedRequest, Error> {
-    Ok(ParsedRequest::new_sync(VmmAction::SetVsockDevice(
-        serde_json::from_slice::<VsockDeviceConfig>(body.raw()).map_err(Error::SerdeJson)?,
-    )))
+    METRICS.put_api_requests.vsock_count.inc();
+    let vsock_cfg = serde_json::from_slice::<VsockDeviceConfig>(body.raw()).map_err(|e| {
+        METRICS.put_api_requests.vsock_fails.inc();
+        Error::SerdeJson(e)
+    })?;
+
+    // Check for the presence of deprecated `vsock_id` field.
+    let mut deprecation_message = None;
+    if vsock_cfg.vsock_id.is_some() {
+        // vsock_id field in request is deprecated.
+        METRICS.deprecated_api.deprecated_http_api_calls.inc();
+        deprecation_message = Some("PUT /vsock: vsock_id field is deprecated.");
+    }
+
+    // Construct the `ParsedRequest` object.
+    let mut parsed_req = ParsedRequest::new_sync(VmmAction::SetVsockDevice(vsock_cfg));
+    // If `vsock_id` was present, set the deprecation message in `parsing_info`.
+    if let Some(msg) = deprecation_message {
+        parsed_req.parsing_info().append_deprecation_message(msg);
+    }
+
+    Ok(parsed_req)
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::parsed_request::tests::depr_action_from_req;
 
     #[test]
     fn test_parse_put_vsock_request() {
         let body = r#"{
-                "vsock_id": "foo",
                 "guest_cid": 42,
                 "uds_path": "vsock.sock"
               }"#;
         assert!(parse_put_vsock(&Body::new(body)).is_ok());
 
         let body = r#"{
-                "vsock_id": "foo",
                 "guest_cid": 42,
                 "invalid_field": false
               }"#;
         assert!(parse_put_vsock(&Body::new(body)).is_err());
     }
+
+    #[test]
+    fn test_depr_vsock_id() {
+        let body = r#"{
+                "vsock_id": "foo",
+                "guest_cid": 42,
+                "uds_path": "vsock.sock"
+              }"#;
+        depr_action_from_req(
+            parse_put_vsock(&Body::new(body)).unwrap(),
+            Some("PUT /vsock: vsock_id field is deprecated.".to_string()),
+        );
+
+        let body = r#"{
+                "guest_cid": 42,
+                "uds_path": "vsock.sock"
+              }"#;
+        let (_, mut parsing_info) = parse_put_vsock(&Body::new(body)).unwrap().into_parts();
+        assert!(!parsing_info.take_deprecation_message().is_some());
+    }
 }

src/api_server/swagger/firecracker.yaml

@@ -1147,7 +1147,6 @@ definitions:
     required:
       - guest_cid
       - uds_path
-      - vsock_id
     properties:
       guest_cid:
         type: integer
@@ -1158,3 +1157,4 @@ definitions:
         description: Path to UNIX domain socket, used to proxy vsock connections.
       vsock_id:
         type: string
+        description: This parameter has been deprecated since v1.0.0.

src/devices/src/virtio/vsock/mod.rs

@@ -18,6 +18,7 @@ use std::os::unix::io::AsRawFd;
 use crate::virtio::persist::Error as VirtioStateError;
 
 pub use self::defs::uapi::VIRTIO_ID_VSOCK as TYPE_VSOCK;
+pub use self::defs::VSOCK_DEV_ID;
 pub use self::device::Vsock;
 pub use self::unix::{Error as VsockUnixBackendError, VsockUnixBackend};
 

src/logger/src/metrics.rs

@@ -397,6 +397,10 @@ pub struct PutRequestsMetrics {
     pub mmds_count: SharedIncMetric,
     /// Number of failures in creating a new mmds.
     pub mmds_fails: SharedIncMetric,
+    /// Number of PUTs for creating a vsock device.
+    pub vsock_count: SharedIncMetric,
+    /// Number of failures in creating a vsock device.
+    pub vsock_fails: SharedIncMetric,
 }
 
 /// Metrics specific to PATCH API Requests for counting user triggered actions and/or failures.