From ba182ae3f3f84df4118f596ea45c16d7e9df3af1 Mon Sep 17 00:00:00 2001
From: Takahiro Itazuri <itazur@amazon.com>
Date: Thu, 21 Sep 2023 17:28:41 +0000
Subject: [PATCH] vmm: Deprecate static CPU templates

Deprecate the `cpu_template` field in PUT and PATCH reqeusts on the
`/machine-config` API by following the runbook for API changes.

To differentiate between explicitly specifying `None` and not specifying
anything, wrap the `cpu_template` of the `MachineConfig` with `Option`.
With this, it can notify the deprecation of only users using the field.

Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
---
 CHANGELOG.md                                  |   5 +
 docs/cpu_templates/cpu-templates.md           |   7 ++
 .../src/request/machine_configuration.rs      | 108 ++++++++++++++++--
 src/api_server/swagger/firecracker.yaml       |   2 +
 src/vmm/src/cpu_config/templates.rs           |   5 +-
 src/vmm/src/vmm_config/machine_config.rs      |   8 +-
 .../integration_tests/functional/test_api.py  |  31 +++++
 7 files changed, 150 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6d78a20aa53..778e5f13902 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -48,6 +48,11 @@
 - Added support for the /dev/userfaultfd device available on linux kernels >=
   6.1. This is the default for creating UFFD handlers on these kernel versions.
   If it is unavailable, Firecracker falls back to the userfaultfd syscall.
+- Deprecated `cpu_template` field in `PUT` and `PATCH` requests on `/machine-config`
+  API, which is used to set a static CPU template. Custom CPU templates added in
+  v1.4.0 are available as an improved iteration of the static CPU templates. For
+  more information about the transition from static CPU templates to custom CPU
+  templates, please refer to [this GitHub discussion](https://github.com/firecracker-microvm/firecracker/discussions/4135).
 
 ### Fixed
 
diff --git a/docs/cpu_templates/cpu-templates.md b/docs/cpu_templates/cpu-templates.md
index fdcea70be77..f5cad8cbe66 100644
--- a/docs/cpu_templates/cpu-templates.md
+++ b/docs/cpu_templates/cpu-templates.md
@@ -34,6 +34,13 @@ Firecracker supports two types of CPU templates:
 - Custom CPU templates - users can create their own CPU templates in json
   format and pass them to Firecracker
 
+> **Note**
+Static CPU templates are deprecated starting from v1.5.0 and will be removed in
+accordance with our deprecation policy. Even after the removal, custom CPU
+templates are available as an improved iteration of static CPU templates. For
+more information about the transition from static CPU templates to custom CPU
+templates, please refer to [this GitHub discussion](https://github.com/firecracker-microvm/firecracker/discussions/4135).
+
 > **Note**
 CPU templates for ARM (both static and custom) require the following patch
 to be available in the host kernel: [Support writable CPU ID registers from userspace](https://lore.kernel.org/kvm/20230212215830.2975485-1-jingzhangos@google.com/#t).
diff --git a/src/api_server/src/request/machine_configuration.rs b/src/api_server/src/request/machine_configuration.rs
index 4dacc48aea2..f2b8d464ac4 100644
--- a/src/api_server/src/request/machine_configuration.rs
+++ b/src/api_server/src/request/machine_configuration.rs
@@ -20,11 +20,25 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Err
         err
     })?;
 
+    // Check for the presence of deprecated `cpu_template` field.
+    let mut deprecation_message = None;
+    if config.cpu_template.is_some() {
+        // `cpu_template` field in request is deprecated.
+        METRICS.deprecated_api.deprecated_http_api_calls.inc();
+        deprecation_message = Some("PUT /machine-config: cpu_template field is deprecated.");
+    }
+
+    // Convert `MachineConfig` to `MachineConfigUpdate`.
     let config_update = MachineConfigUpdate::from(config);
 
-    Ok(ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(
-        config_update,
-    )))
+    // Construct the `ParsedRequest` object.
+    let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
+    // If `cpu_template` 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)
 }
 
 pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, Error> {
@@ -39,9 +53,22 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, E
         return method_to_error(Method::Patch);
     }
 
-    Ok(ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(
-        config_update,
-    )))
+    // Check for the presence of deprecated `cpu_template` field.
+    let mut deprecation_message = None;
+    if config_update.cpu_template.is_some() {
+        // `cpu_template` field in request is deprecated.
+        METRICS.deprecated_api.deprecated_http_api_calls.inc();
+        deprecation_message = Some("PATCH /machine-config: cpu_template field is deprecated.");
+    }
+
+    // Construct the `ParsedRequest` object.
+    let mut parsed_req = ParsedRequest::new_sync(VmmAction::UpdateVmConfiguration(config_update));
+    // If `cpu_template` 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)]
@@ -49,7 +76,7 @@ mod tests {
     use vmm::cpu_config::templates::StaticCpuTemplate;
 
     use super::*;
-    use crate::parsed_request::tests::vmm_action_from_request;
+    use crate::parsed_request::tests::{depr_action_from_req, vmm_action_from_request};
 
     #[test]
     fn test_parse_get_machine_config_request() {
@@ -79,6 +106,23 @@ mod tests {
             "vcpu_count": 8,
             "mem_size_mib": 1024
         }"#;
+        let expected_config = MachineConfigUpdate {
+            vcpu_count: Some(8),
+            mem_size_mib: Some(1024),
+            smt: Some(false),
+            cpu_template: None,
+            track_dirty_pages: Some(false),
+        };
+        assert_eq!(
+            vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
+            VmmAction::UpdateVmConfiguration(expected_config)
+        );
+
+        let body = r#"{
+            "vcpu_count": 8,
+            "mem_size_mib": 1024,
+            "cpu_template": "None"
+        }"#;
         let expected_config = MachineConfigUpdate {
             vcpu_count: Some(8),
             mem_size_mib: Some(1024),
@@ -101,7 +145,7 @@ mod tests {
             vcpu_count: Some(8),
             mem_size_mib: Some(1024),
             smt: Some(false),
-            cpu_template: Some(StaticCpuTemplate::None),
+            cpu_template: None,
             track_dirty_pages: Some(true),
         };
         assert_eq!(
@@ -149,7 +193,7 @@ mod tests {
                 vcpu_count: Some(8),
                 mem_size_mib: Some(1024),
                 smt: Some(true),
-                cpu_template: Some(StaticCpuTemplate::None),
+                cpu_template: None,
                 track_dirty_pages: Some(true),
             };
             assert_eq!(
@@ -201,4 +245,50 @@ mod tests {
         let body = r#"{}"#;
         assert!(parse_patch_machine_config(&Body::new(body)).is_err());
     }
+
+    #[test]
+    fn test_depr_cpu_template_in_put_req() {
+        // Test that the deprecation message is shown when `cpu_template` is specified.
+        let body = r#"{
+            "vcpu_count": 8,
+            "mem_size_mib": 1024,
+            "cpu_template": "None"
+        }"#;
+        depr_action_from_req(
+            parse_put_machine_config(&Body::new(body)).unwrap(),
+            Some("PUT /machine-config: cpu_template field is deprecated.".to_string()),
+        );
+
+        // Test that the deprecation message is not shown when `cpu_template` is not specified.
+        let body = r#"{
+            "vcpu_count": 8,
+            "mem_size_mib": 1024
+        }"#;
+        let (_, mut parsing_info) = parse_put_machine_config(&Body::new(body))
+            .unwrap()
+            .into_parts();
+        assert!(parsing_info.take_deprecation_message().is_none());
+    }
+
+    #[test]
+    fn test_depr_cpu_template_in_patch_req() {
+        // Test that the deprecation message is shown when `cpu_template` is specified.
+        let body = r#"{
+            "vcpu_count": 8,
+            "cpu_template": "None"
+        }"#;
+        depr_action_from_req(
+            parse_patch_machine_config(&Body::new(body)).unwrap(),
+            Some("PATCH /machine-config: cpu_template field is deprecated.".to_string()),
+        );
+
+        // Test that the deprecation message is not shown when `cpu_template` is not specified.
+        let body = r#"{
+            "vcpu_count": 8
+        }"#;
+        let (_, mut parsing_info) = parse_patch_machine_config(&Body::new(body))
+            .unwrap()
+            .into_parts();
+        assert!(parsing_info.take_deprecation_message().is_none());
+    }
 }
diff --git a/src/api_server/swagger/firecracker.yaml b/src/api_server/swagger/firecracker.yaml
index a7341ac9965..8b515a12c69 100644
--- a/src/api_server/swagger/firecracker.yaml
+++ b/src/api_server/swagger/firecracker.yaml
@@ -826,6 +826,8 @@ definitions:
     description:
       The CPU Template defines a set of flags to be disabled from the microvm so that
       the features exposed to the guest are the same as in the selected instance type.
+      This parameter has been deprecated and it will be removed in future Firecracker
+      release.
     enum:
       - C3
       - T2
diff --git a/src/vmm/src/cpu_config/templates.rs b/src/vmm/src/cpu_config/templates.rs
index 24b7a1b1a44..838f7dd2f69 100644
--- a/src/vmm/src/cpu_config/templates.rs
+++ b/src/vmm/src/cpu_config/templates.rs
@@ -74,9 +74,8 @@ impl From<&Option<CpuTemplateType>> for StaticCpuTemplate {
     }
 }
 
-// This conversion will be used when converting `&VmConfig` to `MachineConfig`
-// to respond `GET /machine-config` and `GET /vm`.
-#[allow(dead_code)]
+// This conversion is used when converting `&VmConfig` to `MachineConfig` to
+// respond `GET /machine-config` and `GET /vm`.
 impl From<&CpuTemplateType> for StaticCpuTemplate {
     fn from(value: &CpuTemplateType) -> Self {
         match value {
diff --git a/src/vmm/src/vmm_config/machine_config.rs b/src/vmm/src/vmm_config/machine_config.rs
index fcaded94636..1a4266053d6 100644
--- a/src/vmm/src/vmm_config/machine_config.rs
+++ b/src/vmm/src/vmm_config/machine_config.rs
@@ -40,8 +40,8 @@ pub struct MachineConfig {
     #[serde(default, deserialize_with = "deserialize_smt")]
     pub smt: bool,
     /// A CPU template that it is used to filter the CPU features exposed to the guest.
-    #[serde(default, skip_serializing_if = "StaticCpuTemplate::is_none")]
-    pub cpu_template: StaticCpuTemplate,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub cpu_template: Option<StaticCpuTemplate>,
     /// Enables or disables dirty page tracking. Enabling allows incremental snapshots.
     #[serde(default)]
     pub track_dirty_pages: bool,
@@ -122,7 +122,7 @@ impl From<MachineConfig> for MachineConfigUpdate {
             vcpu_count: Some(cfg.vcpu_count),
             mem_size_mib: Some(cfg.mem_size_mib),
             smt: Some(cfg.smt),
-            cpu_template: Some(cfg.cpu_template),
+            cpu_template: cfg.cpu_template,
             track_dirty_pages: Some(cfg.track_dirty_pages),
         }
     }
@@ -212,7 +212,7 @@ impl From<&VmConfig> for MachineConfig {
             vcpu_count: value.vcpu_count,
             mem_size_mib: value.mem_size_mib,
             smt: value.smt,
-            cpu_template: (&value.cpu_template).into(),
+            cpu_template: value.cpu_template.as_ref().map(|template| template.into()),
             track_dirty_pages: value.track_dirty_pages,
         }
     }
diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py
index b2cf173ee24..9dae036db3e 100644
--- a/tests/integration_tests/functional/test_api.py
+++ b/tests/integration_tests/functional/test_api.py
@@ -428,6 +428,37 @@ def test_api_machine_config(test_microvm_with_api):
     assert json["machine-config"]["smt"] is False
 
 
+def test_negative_machine_config_api(test_microvm_with_api):
+    """
+    Test the deprecated `cpu_template` field in PUT and PATCH requests on
+    `/machine-config` API is handled correctly.
+
+    When using the `cpu_template` field (even if the value is "None"), the HTTP
+    response header should have "Deprecation: true".
+    """
+    test_microvm = test_microvm_with_api
+    test_microvm.spawn()
+
+    # Use `cpu_template` field in PUT /machine-config
+    response = test_microvm.api.machine_config.put(
+        vcpu_count=2,
+        mem_size_mib=256,
+        cpu_template="None",
+    )
+    assert response.headers["deprecation"]
+    assert (
+        "PUT /machine-config: cpu_template field is deprecated."
+        in test_microvm.log_data
+    )
+
+    # Use `cpu_template` field in PATCH /machine-config
+    response = test_microvm.api.machine_config.patch(cpu_template="None")
+    assert (
+        "PATCH /machine-config: cpu_template field is deprecated."
+        in test_microvm.log_data
+    )
+
+
 @nonci_on_arm
 def test_api_cpu_config(test_microvm_with_api, custom_cpu_template):
     """