Repository: firecracker-microvm/firecracker
File: src/firecracker/src/api_server/request/hotplug.rs (Rust)

Code before:
fn parse_put_vcpu_hotplug(body: &Body) -> Result<ParsedRequest, RequestError> {
    METRICS.hotplug.vcpu_hotplug_request_count.inc();
    let config = serde_json::from_slice::<HotplugVcpuConfig>(body.raw()).map_err(|err| {
        METRICS.hotplug.vcpu_hotplug_request_fails.inc();
        err
    })?;

    Ok(ParsedRequest::new_sync(VmmAction::HotplugVcpu(config)))
}

#[cfg(test)]
mod tests {

    use hotplug::{parse_put_hotplug, parse_put_vcpu_hotplug};
    use vmm::{logger::{IncMetric, METRICS}, rpc_interface::VmmAction, vmm_config::hotplug::{HotplugVcpuConfig, HotplugVcpuError}};
    use crate::api_server::parsed_request::tests::vmm_action_from_request;

    use super::super::*;

    #[test]
    fn test_parse_put_hotplug() {
        // Case 1. Invalid resource type
        parse_put_hotplug(&Body::new("invalid body"), Some("invalid")).unwrap_err();

        // Case 2. No resource type
        parse_put_hotplug(&Body::new("invalid body"), None).unwrap_err();

        // Case 3. vCPU Resource
        let body = r#"{
            "vcpu_count": 4
        }"#;

        let expected_config = HotplugVcpuConfig {
            vcpu_count: 4
        };

        assert_eq!(
            vmm_action_from_request(parse_put_hotplug(&Body::new(body), Some("vcpu")).unwrap()),
            VmmAction::HotplugVcpu(expected_config)
        );
    }


}


Review comment (bug):
nit: Since these are explicitly testing incorrect values for the "resource type" parameter, we should use valid bodies, to ensure that the error we are receiving is actually related to the resource type (and not some other invalid configuration we happen to also pass). Alternatively, you could do some assertion on the returned error (e.g. by checking the error variant is, say, `InvalidPathMethod` in the first case, and looking at the method/error string)

Code after:
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
use vmm::logger::{IncMetric, METRICS};
use vmm::rpc_interface::VmmAction;
use vmm::vmm_config::hotplug::HotplugRequestConfig;

use super::super::parsed_request::{ParsedRequest, RequestError};
use super::Body;

pub(crate) fn parse_put_hotplug(body: &Body) -> Result<ParsedRequest, RequestError> {
    METRICS.put_api_requests.hotplug.inc();
    METRICS.hotplug.hotplug_request_count.inc();
    let config = serde_json::from_slice::<HotplugRequestConfig>(body.raw()).map_err(|err| {
        METRICS.hotplug.hotplug_request_fails.inc();
        err
    })?;
    Ok(ParsedRequest::new_sync(VmmAction::HotplugRequest(config)))
}

#[cfg(test)]
mod tests {

    use hotplug::parse_put_hotplug;
    use vmm::rpc_interface::VmmAction;
    use vmm::vmm_config::hotplug::{HotplugRequestConfig, HotplugVcpuConfig};

    use super::super::*;
    use crate::api_server::parsed_request::tests::vmm_action_from_request;

    #[test]
    fn test_parse_put_hotplug() {
        // Case 1. Invalid body
        parse_put_hotplug(&Body::new("invalid body")).unwrap_err();

        // Case 2. vCPU Resource
        let body = r#"{
            "Vcpu" : { "add": 4 }
        }"#;

        let expected_config = HotplugVcpuConfig { add: 4 };

        assert_eq!(
            vmm_action_from_request(parse_put_hotplug(&Body::new(body)).unwrap()),
            VmmAction::HotplugRequest(HotplugRequestConfig::Vcpu(expected_config))
        );
    }
}