[API] Bounds-check that memory for instance is above a minimum, meets a granularity (#1030)

nexus/src/app/instance.rs

@@ -16,6 +16,7 @@ use crate::db::model::Name;
 use crate::db::queries::network_interface;
 use crate::external_api::params;
 use omicron_common::api::external;
+use omicron_common::api::external::ByteCount;
 use omicron_common::api::external::CreateResult;
 use omicron_common::api::external::DataPageParams;
 use omicron_common::api::external::DeleteResult;
@@ -56,6 +57,32 @@ impl super::Nexus {
             )));
         }
 
+        // Reject instances where the memory is not at least
+        // MIN_MEMORY_SIZE_BYTES
+        if params.memory.to_bytes() < params::MIN_MEMORY_SIZE_BYTES as u64 {
+            return Err(Error::InvalidValue {
+                label: String::from("size"),
+                message: format!(
+                    "memory must be at least {}",
+                    ByteCount::from(params::MIN_MEMORY_SIZE_BYTES)
+                ),
+            });
+        }
+
+        // Reject instances where the memory is not divisible by
+        // MIN_MEMORY_SIZE_BYTES
+        if (params.memory.to_bytes() % params::MIN_MEMORY_SIZE_BYTES as u64)
+            != 0
+        {
+            return Err(Error::InvalidValue {
+                label: String::from("size"),
+                message: format!(
+                    "memory must be divisible by {}",
+                    ByteCount::from(params::MIN_MEMORY_SIZE_BYTES)
+                ),
+            });
+        }
+
         let saga_params = Arc::new(sagas::instance_create::Params {
             serialized_authn: authn::saga::Serialized::for_opctx(opctx),
             organization_name: organization_name.clone().into(),

nexus/src/external_api/params.rs

@@ -286,6 +286,8 @@ pub struct NetworkInterfaceCreate {
 
 // INSTANCES
 
+pub const MIN_MEMORY_SIZE_BYTES: u32 = 1 << 30; // 1 GiB
+
 /// Describes an attachment of a `NetworkInterface` to an `Instance`, at the
 /// time the instance is created.
 // NOTE: VPC's are an organizing concept for networking resources, not for

nexus/test-utils/src/resource_helpers.rs

@@ -179,7 +179,7 @@ pub async fn create_instance_with_nics(
                 description: format!("instance {:?}", instance_name),
             },
             ncpus: InstanceCpuCount(4),
-            memory: ByteCount::from_mebibytes_u32(256),
+            memory: ByteCount::from_gibibytes_u32(1),
             hostname: String::from("the_host"),
             user_data:
                 b"#cloud-config\nsystem_info:\n  default_user:\n    name: oxide"

nexus/tests/integration_tests/instances.rs

@@ -42,6 +42,20 @@ use nexus_test_utils_macros::nexus_test;
 static ORGANIZATION_NAME: &str = "test-org";
 static PROJECT_NAME: &str = "springfield-squidport";
 
+fn get_project_url() -> String {
+    format!("/organizations/{}/projects/{}", ORGANIZATION_NAME, PROJECT_NAME)
+}
+
+fn get_instances_url() -> String {
+    format!("{}/instances", get_project_url())
+}
+
+async fn create_org_and_project(client: &ClientTestContext) -> Uuid {
+    create_organization(&client, ORGANIZATION_NAME).await;
+    let project = create_project(client, ORGANIZATION_NAME, PROJECT_NAME).await;
+    project.identity.id
+}
+
 #[nexus_test]
 async fn test_instances_access_before_create_returns_not_found(
     cptestctx: &ControlPlaneTestContext,
@@ -128,7 +142,7 @@ async fn test_instances_create_reboot_halt(
     let InstanceCpuCount(nfoundcpus) = instance.ncpus;
     // These particulars are hardcoded in create_instance().
     assert_eq!(nfoundcpus, 4);
-    assert_eq!(instance.memory.to_whole_mebibytes(), 256);
+    assert_eq!(instance.memory.to_whole_gibibytes(), 1);
     assert_eq!(instance.hostname, "the_host");
     assert_eq!(instance.runtime.run_state, InstanceState::Starting);
 
@@ -565,7 +579,7 @@ async fn test_instance_create_saga_removes_instance_database_record(
             description: String::from("instance to test saga unwind"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("inst"),
         user_data: vec![],
         network_interfaces: interface_params.clone(),
@@ -587,7 +601,7 @@ async fn test_instance_create_saga_removes_instance_database_record(
             description: String::from("instance to test saga unwind 2"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("inst2"),
         user_data: vec![],
         network_interfaces: interface_params,
@@ -673,7 +687,7 @@ async fn test_instance_with_single_explicit_ip_address(
             description: String::from("instance to test multiple nics"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nic-test"),
         user_data: vec![],
         network_interfaces: interface_params,
@@ -790,7 +804,7 @@ async fn test_instance_with_new_custom_network_interfaces(
             description: String::from("instance to test multiple nics"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nic-test"),
         user_data: vec![],
         network_interfaces: interface_params,
@@ -904,7 +918,7 @@ async fn test_instance_create_delete_network_interface(
             description: String::from("instance to test attaching new nic"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nic-test"),
         user_data: vec![],
         network_interfaces: params::InstanceNetworkInterfaceAttachment::None,
@@ -1162,7 +1176,7 @@ async fn test_instance_with_multiple_nics_unwinds_completely(
             description: String::from("instance to test multiple bad nics"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nic-test"),
         user_data: vec![],
         network_interfaces: interface_params,
@@ -1236,7 +1250,7 @@ async fn test_attach_one_disk_to_instance(cptestctx: &ControlPlaneTestContext) {
             description: String::from("probably serving data"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nfs"),
         user_data: vec![],
         network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
@@ -1332,7 +1346,7 @@ async fn test_attach_eight_disks_to_instance(
             description: String::from("probably serving data"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nfs"),
         user_data: vec![],
         network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
@@ -1438,7 +1452,7 @@ async fn test_cannot_attach_nine_disks_to_instance(
             description: String::from("probably serving data"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nfs"),
         user_data: vec![],
         network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
@@ -1565,7 +1579,7 @@ async fn test_cannot_attach_faulted_disks(cptestctx: &ControlPlaneTestContext) {
             description: String::from("probably serving data"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nfs"),
         user_data: vec![],
         network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
@@ -1677,7 +1691,7 @@ async fn test_disks_detached_when_instance_destroyed(
             description: String::from("probably serving data"),
         },
         ncpus: InstanceCpuCount::try_from(2).unwrap(),
-        memory: ByteCount::from_mebibytes_u32(4),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: String::from("nfs"),
         user_data: vec![],
         network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
@@ -1772,6 +1786,102 @@ async fn test_disks_detached_when_instance_destroyed(
     }
 }
 
+// Tests that an instance is rejected if the memory is less than
+// MIN_MEMORY_SIZE_BYTES
+#[nexus_test]
+async fn test_instances_memory_rejected_less_than_min_memory_size(
+    cptestctx: &ControlPlaneTestContext,
+) {
+    let client = &cptestctx.external_client;
+    create_org_and_project(client).await;
+
+    // Attempt to create the instance, observe a server error.
+    let instances_url = get_instances_url();
+    let instance_name = "just-rainsticks";
+    let instance = params::InstanceCreate {
+        identity: IdentityMetadataCreateParams {
+            name: instance_name.parse().unwrap(),
+            description: format!("instance {:?}", &instance_name),
+        },
+        ncpus: InstanceCpuCount(1),
+        memory: ByteCount::from(params::MIN_MEMORY_SIZE_BYTES / 2),
+        hostname: String::from("inst"),
+        user_data:
+            b"#cloud-config\nsystem_info:\n  default_user:\n    name: oxide"
+                .to_vec(),
+        network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
+        disks: vec![],
+    };
+
+    let error = NexusRequest::new(
+        RequestBuilder::new(client, Method::POST, &instances_url)
+            .body(Some(&instance))
+            .expect_status(Some(StatusCode::BAD_REQUEST)),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .unwrap()
+    .parsed_body::<dropshot::HttpErrorResponseBody>()
+    .unwrap();
+
+    assert_eq!(
+        error.message,
+        format!(
+            "unsupported value for \"size\": memory must be at least {}",
+            ByteCount::from(params::MIN_MEMORY_SIZE_BYTES)
+        ),
+    );
+}
+
+// Test that an instance is rejected if memory is not divisible by
+// MIN_MEMORY_SIZE
+#[nexus_test]
+async fn test_instances_memory_not_divisible_by_min_memory_size(
+    cptestctx: &ControlPlaneTestContext,
+) {
+    let client = &cptestctx.external_client;
+    create_org_and_project(client).await;
+
+    // Attempt to create the instance, observe a server error.
+    let instances_url = get_instances_url();
+    let instance_name = "just-rainsticks";
+    let instance = params::InstanceCreate {
+        identity: IdentityMetadataCreateParams {
+            name: instance_name.parse().unwrap(),
+            description: format!("instance {:?}", &instance_name),
+        },
+        ncpus: InstanceCpuCount(1),
+        memory: ByteCount::from(1024 * 1024 * 1024 + 300),
+        hostname: String::from("inst"),
+        user_data:
+            b"#cloud-config\nsystem_info:\n  default_user:\n    name: oxide"
+                .to_vec(),
+        network_interfaces: params::InstanceNetworkInterfaceAttachment::Default,
+        disks: vec![],
+    };
+
+    let error = NexusRequest::new(
+        RequestBuilder::new(client, Method::POST, &instances_url)
+            .body(Some(&instance))
+            .expect_status(Some(StatusCode::BAD_REQUEST)),
+    )
+    .authn_as(AuthnMode::PrivilegedUser)
+    .execute()
+    .await
+    .unwrap()
+    .parsed_body::<dropshot::HttpErrorResponseBody>()
+    .unwrap();
+
+    assert_eq!(
+        error.message,
+        format!(
+            "unsupported value for \"size\": memory must be divisible by {}",
+            ByteCount::from(params::MIN_MEMORY_SIZE_BYTES)
+        ),
+    );
+}
+
 async fn instance_get(
     client: &ClientTestContext,
     instance_url: &str,

nexus/tests/integration_tests/subnet_allocation.rs

@@ -52,7 +52,7 @@ async fn create_instance_expect_failure(
             description: "".to_string(),
         },
         ncpus: InstanceCpuCount(1),
-        memory: ByteCount::from_mebibytes_u32(256),
+        memory: ByteCount::from_gibibytes_u32(1),
         hostname: name.to_string(),
         user_data: vec![],
         network_interfaces,