diff --git a/common/Cargo.toml b/common/Cargo.toml index 3941f5303e..ebb8c8c9b4 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -38,6 +38,7 @@ parse-display.workspace = true progenitor.workspace = true omicron-workspace-hack.workspace = true once_cell.workspace = true +regress.workspace = true [dev-dependencies] camino-tempfile.workspace = true diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index dc3537fbb2..cdc929d89b 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -696,6 +696,109 @@ impl TryFrom for Generation { } } +/// An RFC-1035-compliant hostname. +#[derive( + Clone, Debug, Deserialize, Display, Eq, PartialEq, SerializeDisplay, +)] +#[display("{0}")] +#[serde(try_from = "String", into = "String")] +pub struct Hostname(String); + +impl Hostname { + /// Return the hostname as a string slice. + pub fn as_str(&self) -> &str { + self.0.as_str() + } +} + +// Regular expression for hostnames. +// +// Each name is a dot-separated sequence of labels. Each label is supposed to +// be an "LDH": letter, dash, or hyphen. Hostnames can consist of one label, or +// many, separated by a `.`. While _domain_ names are allowed to end in a `.`, +// making them fully-qualified, hostnames are not. +// +// Note that labels are allowed to contain a hyphen, but may not start or end +// with one. See RFC 952, "Lexical grammar" section. +// +// Note that we need to use a regex engine capable of lookbehind to support +// this, since we need to check that labels don't end with a `-`. +const HOSTNAME_REGEX: &str = r#"^([a-zA-Z0-9]+[a-zA-Z0-9\-]*(?\x00`. +const HOSTNAME_MAX_LEN: u32 = 253; + +impl FromStr for Hostname { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + anyhow::ensure!( + s.len() <= HOSTNAME_MAX_LEN as usize, + "Max hostname length is {HOSTNAME_MAX_LEN}" + ); + let re = regress::Regex::new(HOSTNAME_REGEX).unwrap(); + if re.find(s).is_some() { + Ok(Hostname(s.to_string())) + } else { + anyhow::bail!("Hostnames must comply with RFC 1035") + } + } +} + +impl TryFrom<&str> for Hostname { + type Error = ::Err; + + fn try_from(s: &str) -> Result { + s.parse() + } +} + +impl TryFrom for Hostname { + type Error = ::Err; + + fn try_from(s: String) -> Result { + s.as_str().parse() + } +} + +// Custom implementation of JsonSchema for Hostname to ensure RFC-1035-style +// validation +impl JsonSchema for Hostname { + fn schema_name() -> String { + "Hostname".to_string() + } + + fn json_schema( + _: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + metadata: Some(Box::new(schemars::schema::Metadata { + title: Some("An RFC-1035-compliant hostname".to_string()), + description: Some( + "A hostname identifies a host on a network, and \ + is usually a dot-delimited sequence of labels, \ + where each label contains only letters, digits, \ + or the hyphen. See RFCs 1035 and 952 for more details." + .to_string(), + ), + ..Default::default() + })), + instance_type: Some(schemars::schema::SingleOrVec::Single( + Box::new(schemars::schema::InstanceType::String), + )), + string: Some(Box::new(schemars::schema::StringValidation { + max_length: Some(HOSTNAME_MAX_LEN), + min_length: Some(1), + pattern: Some(HOSTNAME_REGEX.to_string()), + })), + ..Default::default() + }) + } +} + // General types used to implement API resources /// Identifies a type of API resource @@ -939,7 +1042,7 @@ pub struct Instance { /// memory allocated for this Instance pub memory: ByteCount, /// RFC1035-compliant hostname for the Instance. - pub hostname: String, // TODO-cleanup different type? + pub hostname: String, #[serde(flatten)] pub runtime: InstanceRuntimeState, @@ -2737,6 +2840,7 @@ mod test { VpcFirewallRuleUpdateParams, }; use crate::api::external::Error; + use crate::api::external::Hostname; use crate::api::external::ResourceType; use std::convert::TryFrom; use std::str::FromStr; @@ -3460,4 +3564,24 @@ mod test { let conv = mac.to_i64(); assert_eq!(original, conv); } + + #[test] + fn test_hostname_from_str() { + assert!(Hostname::from_str("name").is_ok()); + assert!(Hostname::from_str("a.good.name").is_ok()); + assert!(Hostname::from_str("another.very-good.name").is_ok()); + assert!(Hostname::from_str("0name").is_ok()); + assert!(Hostname::from_str("name0").is_ok()); + assert!(Hostname::from_str("0name0").is_ok()); + + assert!(Hostname::from_str("").is_err()); + assert!(Hostname::from_str("no_no").is_err()); + assert!(Hostname::from_str("no.fqdns.").is_err()); + assert!(Hostname::from_str("empty..label").is_err()); + assert!(Hostname::from_str("-hypen.cannot.start").is_err()); + assert!(Hostname::from_str("hypen.-cannot.start").is_err()); + assert!(Hostname::from_str("hypen.cannot.end-").is_err()); + assert!(Hostname::from_str("hyphen-cannot-end-").is_err()); + assert!(Hostname::from_str(&"too-long".repeat(100)).is_err()); + } } diff --git a/common/src/api/internal/nexus.rs b/common/src/api/internal/nexus.rs index 780e60b1a2..3972e011cf 100644 --- a/common/src/api/internal/nexus.rs +++ b/common/src/api/internal/nexus.rs @@ -5,8 +5,8 @@ //! APIs exposed by Nexus. use crate::api::external::{ - ByteCount, DiskState, Generation, InstanceCpuCount, InstanceState, IpNet, - SemverVersion, Vni, + ByteCount, DiskState, Generation, Hostname, InstanceCpuCount, + InstanceState, IpNet, SemverVersion, Vni, }; use chrono::{DateTime, Utc}; use parse_display::{Display, FromStr}; @@ -36,8 +36,7 @@ pub struct InstanceProperties { pub ncpus: InstanceCpuCount, pub memory: ByteCount, /// RFC1035-compliant hostname for the instance. - // TODO-cleanup different type? - pub hostname: String, + pub hostname: Hostname, } /// The dynamic runtime properties of an instance: its current VMM ID (if any), diff --git a/end-to-end-tests/src/instance_launch.rs b/end-to-end-tests/src/instance_launch.rs index c1da731c35..f27261e82d 100644 --- a/end-to-end-tests/src/instance_launch.rs +++ b/end-to-end-tests/src/instance_launch.rs @@ -64,7 +64,7 @@ async fn instance_launch() -> Result<()> { .body(InstanceCreate { name: generate_name("instance")?, description: String::new(), - hostname: "localshark".into(), // 🦈 + hostname: "localshark".parse().unwrap(), // 🦈 memory: ByteCount(1024 * 1024 * 1024), ncpus: InstanceCpuCount(2), disks: vec![InstanceDiskAttachment::Attach { diff --git a/nexus/db-model/src/instance.rs b/nexus/db-model/src/instance.rs index e10f8c2603..f7731ff903 100644 --- a/nexus/db-model/src/instance.rs +++ b/nexus/db-model/src/instance.rs @@ -46,7 +46,10 @@ pub struct Instance { pub memory: ByteCount, /// The instance's hostname. - // TODO-cleanup: Different type? + // TODO-cleanup: We use a validated wrapper type in the API, but not in + // between the database. This is to handle existing names that do not pass + // the new validation. We should swap this for a SQL-serializable validated + // type. #[diesel(column_name = hostname)] pub hostname: String, @@ -81,7 +84,7 @@ impl Instance { user_data: params.user_data.clone(), ncpus: params.ncpus.into(), memory: params.memory.into(), - hostname: params.hostname.clone(), + hostname: params.hostname.to_string(), boot_on_fault: false, runtime_state, } diff --git a/nexus/db-queries/src/db/datastore/instance.rs b/nexus/db-queries/src/db/datastore/instance.rs index ca7efe32f7..acea7bb4e3 100644 --- a/nexus/db-queries/src/db/datastore/instance.rs +++ b/nexus/db-queries/src/db/datastore/instance.rs @@ -97,7 +97,11 @@ impl From for omicron_common::api::external::Instance { project_id: value.instance.project_id, ncpus: value.instance.ncpus.into(), memory: value.instance.memory.into(), - hostname: value.instance.hostname, + hostname: value + .instance + .hostname + .parse() + .expect("found invalid hostname in the database"), runtime: omicron_common::api::external::InstanceRuntimeState { run_state: *run_state.state(), time_run_state_updated, diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index 392e669243..54595a8444 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -998,7 +998,7 @@ mod tests { identity: IdentityMetadataCreateParams { name: String::from(name).parse().unwrap(), description: format!("instance {}", name) }, ncpus: InstanceCpuCount(omicron_common::api::external::InstanceCpuCount(1)).into(), memory: ByteCount(omicron_common::api::external::ByteCount::from_gibibytes_u32(1)).into(), - hostname: "test".into(), + hostname: "test".parse().unwrap(), ssh_public_keys: None, user_data: vec![], network_interfaces: Default::default(), diff --git a/nexus/db-queries/src/db/queries/network_interface.rs b/nexus/db-queries/src/db/queries/network_interface.rs index 3cfbead2f7..ae3e2c8ead 100644 --- a/nexus/db-queries/src/db/queries/network_interface.rs +++ b/nexus/db-queries/src/db/queries/network_interface.rs @@ -1738,7 +1738,7 @@ mod tests { }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(4), - hostname: "inst".to_string(), + hostname: "inst".parse().unwrap(), user_data: vec![], ssh_public_keys: Some(Vec::new()), network_interfaces: InstanceNetworkInterfaceAttachment::None, diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index eb78d4179c..4b52b597ba 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -1010,6 +1010,23 @@ impl super::Nexus { ) -> Result<(), Error> { opctx.authorize(authz::Action::Modify, authz_instance).await?; + // Check that the hostname is valid. + // + // TODO-cleanup: This can be removed when we are confident that no + // instances exist prior to the addition of strict hostname validation + // in the API. + let Ok(hostname) = db_instance.hostname.parse() else { + let msg = format!( + "The instance hostname '{}' is no longer valid. \ + To access the data on its disks, this instance \ + must be deleted, and a new one created with the \ + relevant disks. The new hostname will be validated \ + at that time.", + db_instance.hostname, + ); + return Err(Error::invalid_request(&msg)); + }; + // Gather disk information and turn that into DiskRequests let disks = self .db_datastore @@ -1175,7 +1192,7 @@ impl super::Nexus { properties: InstanceProperties { ncpus: db_instance.ncpus.into(), memory: db_instance.memory.into(), - hostname: db_instance.hostname.clone(), + hostname, }, nics, source_nat, diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index ed1b23fe82..88dd0ae36e 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -1102,7 +1102,7 @@ pub mod test { }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index 067e2d79ed..0e253913b0 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -235,7 +235,7 @@ mod test { }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: vec![], ssh_public_keys: Some(Vec::new()), network_interfaces: diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index 5e91b8fed1..ff3ff66e78 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -568,7 +568,7 @@ mod tests { }, ncpus: InstanceCpuCount(2), memory: ByteCount::from_gibibytes_u32(2), - hostname: String::from(INSTANCE_NAME), + hostname: INSTANCE_NAME.parse().unwrap(), user_data: b"#cloud-config".to_vec(), ssh_public_keys: Some(Vec::new()), network_interfaces: diff --git a/nexus/src/app/sagas/instance_start.rs b/nexus/src/app/sagas/instance_start.rs index b4cc6f4cc6..b1d9506c31 100644 --- a/nexus/src/app/sagas/instance_start.rs +++ b/nexus/src/app/sagas/instance_start.rs @@ -747,7 +747,7 @@ mod test { }, ncpus: InstanceCpuCount(2), memory: ByteCount::from_gibibytes_u32(2), - hostname: String::from(INSTANCE_NAME), + hostname: INSTANCE_NAME.parse().unwrap(), user_data: b"#cloud-config".to_vec(), ssh_public_keys: Some(Vec::new()), network_interfaces: diff --git a/nexus/src/app/sagas/snapshot_create.rs b/nexus/src/app/sagas/snapshot_create.rs index d80b1b9029..e017ab377b 100644 --- a/nexus/src/app/sagas/snapshot_create.rs +++ b/nexus/src/app/sagas/snapshot_create.rs @@ -1940,7 +1940,7 @@ mod test { }, ncpus: InstanceCpuCount(2), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("base_instance"), + hostname: "base-instance".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 254723d32b..764332c5bc 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -518,7 +518,7 @@ pub async fn create_instance_with( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), @@ -532,6 +532,23 @@ pub async fn create_instance_with( .await } +/// Creates an instance, asserting a status code and returning the error. +/// +/// Note that this accepts any serializable body, which allows users to create +/// invalid inputs to test our parameter validation. +pub async fn create_instance_with_error( + client: &ClientTestContext, + project_name: &str, + body: &T, + status: StatusCode, +) -> HttpErrorResponseBody +where + T: serde::Serialize, +{ + let url = format!("/v1/instances?project={project_name}"); + object_create_error(client, &url, body, status).await +} + pub async fn create_vpc( client: &ClientTestContext, project_name: &str, diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 38e248471b..cd04bb6018 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -421,7 +421,7 @@ pub static DEMO_INSTANCE_CREATE: Lazy = }, ncpus: InstanceCpuCount(1), memory: ByteCount::from_gibibytes_u32(16), - hostname: String::from("demo-instance"), + hostname: "demo-instance".parse().unwrap(), user_data: vec![], ssh_public_keys: Some(Vec::new()), network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index e5d1c2f143..09f91a2288 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -74,7 +74,8 @@ use dropshot::{HttpErrorResponseBody, ResultsPage}; use nexus_test_utils::identity_eq; use nexus_test_utils::resource_helpers::{ - create_instance, create_instance_with, create_project, + create_instance, create_instance_with, create_instance_with_error, + create_project, }; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::shared::SiloRole; @@ -165,6 +166,64 @@ async fn test_instances_access_before_create_returns_not_found( ); } +// Regression tests for https://github.com/oxidecomputer/omicron/issues/4923. +#[nexus_test] +async fn test_cannot_create_instance_with_bad_hostname( + cptestctx: &ControlPlaneTestContext, +) { + test_create_instance_with_bad_hostname_impl(cptestctx, "bad_hostname") + .await; +} + +#[nexus_test] +async fn test_cannot_create_instance_with_empty_hostname( + cptestctx: &ControlPlaneTestContext, +) { + test_create_instance_with_bad_hostname_impl(cptestctx, "").await; +} + +async fn test_create_instance_with_bad_hostname_impl( + cptestctx: &ControlPlaneTestContext, + hostname: &str, +) { + let client = &cptestctx.external_client; + let _project = create_project_and_pool(client).await; + + // Create an instance, with what should be an invalid hostname. + // + // We'll do this by creating a _valid_ set of parameters, convert it to + // JSON, and then muck with the hostname. + let instance_name = "happy-accident"; + let params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: instance_name.parse().unwrap(), + description: format!("instance {:?}", instance_name), + }, + ncpus: InstanceCpuCount(4), + memory: ByteCount::from_gibibytes_u32(1), + hostname: "the-host".parse().unwrap(), + user_data: + b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" + .to_vec(), + network_interfaces: Default::default(), + external_ips: vec![], + disks: vec![], + start: false, + ssh_public_keys: None, + }; + let mut body: serde_json::Value = + serde_json::from_str(&serde_json::to_string(¶ms).unwrap()).unwrap(); + body["hostname"] = hostname.into(); + let err = create_instance_with_error( + client, + PROJECT_NAME, + &body, + StatusCode::BAD_REQUEST, + ) + .await; + assert!(err.message.contains("Hostnames must comply with RFC 1035")); +} + #[nexus_test] async fn test_instance_access(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; @@ -231,7 +290,7 @@ async fn test_instances_create_reboot_halt( // These particulars are hardcoded in create_instance(). assert_eq!(nfoundcpus, 4); assert_eq!(instance.memory.to_whole_gibibytes(), 1); - assert_eq!(instance.hostname, "the_host"); + assert_eq!(instance.hostname.as_str(), "the-host"); assert_eq!(instance.runtime.run_state, InstanceState::Starting); // Attempt to create a second instance with a conflicting name. @@ -247,7 +306,7 @@ async fn test_instances_create_reboot_halt( }, ncpus: instance.ncpus, memory: instance.memory, - hostname: instance.hostname.clone(), + hostname: instance.hostname.parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: @@ -1220,7 +1279,7 @@ async fn test_instances_create_stopped_start( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: @@ -1388,7 +1447,7 @@ async fn test_instance_using_image_from_other_project_fails( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: "stolen".into(), + hostname: "stolen".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: @@ -1463,7 +1522,7 @@ async fn test_instance_create_saga_removes_instance_database_record( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: interface_params.clone(), @@ -1491,7 +1550,7 @@ async fn test_instance_create_saga_removes_instance_database_record( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("inst2"), + hostname: "inst2".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: interface_params, @@ -1580,7 +1639,7 @@ async fn test_instance_with_single_explicit_ip_address( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nic-test"), + hostname: "nic-test".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: interface_params, @@ -1694,7 +1753,7 @@ async fn test_instance_with_new_custom_network_interfaces( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nic-test"), + hostname: "nic-test".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: interface_params, @@ -1808,7 +1867,7 @@ async fn test_instance_create_delete_network_interface( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nic-test"), + hostname: "nic-test".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::None, @@ -2049,7 +2108,7 @@ async fn test_instance_update_network_interfaces( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nic-test"), + hostname: "nic-test".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::None, @@ -2442,7 +2501,7 @@ async fn test_instance_with_multiple_nics_unwinds_completely( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nic-test"), + hostname: "nic-test".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: interface_params, @@ -2508,7 +2567,7 @@ async fn test_attach_one_disk_to_instance(cptestctx: &ControlPlaneTestContext) { }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -2568,7 +2627,7 @@ async fn test_instance_create_attach_disks( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(3), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -2665,7 +2724,7 @@ async fn test_instance_create_attach_disks_undo( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -2750,7 +2809,7 @@ async fn test_attach_eight_disks_to_instance( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -2831,7 +2890,7 @@ async fn test_cannot_attach_nine_disks_to_instance( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -2926,7 +2985,7 @@ async fn test_cannot_attach_faulted_disks(cptestctx: &ControlPlaneTestContext) { }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -3010,7 +3069,7 @@ async fn test_disks_detached_when_instance_destroyed( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfs"), + hostname: "nfs".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -3101,7 +3160,7 @@ async fn test_disks_detached_when_instance_destroyed( }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("nfsv2"), + hostname: "nfsv2".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -3161,7 +3220,7 @@ async fn test_instances_memory_rejected_less_than_min_memory_size( }, ncpus: InstanceCpuCount(1), memory: ByteCount::from(MIN_MEMORY_BYTES_PER_INSTANCE / 2), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), @@ -3211,7 +3270,7 @@ async fn test_instances_memory_not_divisible_by_min_memory_size( }, ncpus: InstanceCpuCount(1), memory: ByteCount::from(1024 * 1024 * 1024 + 300), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), @@ -3261,7 +3320,7 @@ async fn test_instances_memory_greater_than_max_size( ncpus: InstanceCpuCount(1), memory: ByteCount::try_from(MAX_MEMORY_BYTES_PER_INSTANCE + (1 << 30)) .unwrap(), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), @@ -3347,7 +3406,7 @@ async fn test_instance_create_with_ssh_keys( // By default should transfer all profile keys ssh_public_keys: None, start: false, - hostname: instance_name.to_string(), + hostname: instance_name.parse().unwrap(), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, external_ips: vec![], @@ -3393,7 +3452,7 @@ async fn test_instance_create_with_ssh_keys( // Should only transfer the first key ssh_public_keys: Some(vec![user_keys[0].identity.name.clone().into()]), start: false, - hostname: instance_name.to_string(), + hostname: instance_name.parse().unwrap(), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, external_ips: vec![], @@ -3438,7 +3497,7 @@ async fn test_instance_create_with_ssh_keys( // Should transfer no keys ssh_public_keys: Some(vec![]), start: false, - hostname: instance_name.to_string(), + hostname: instance_name.parse().unwrap(), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, external_ips: vec![], @@ -3556,7 +3615,7 @@ async fn test_cannot_provision_instance_beyond_cpu_capacity( }, ncpus, memory: ByteCount::from_gibibytes_u32(1), - hostname: config.0.to_string(), + hostname: config.0.parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: @@ -3610,7 +3669,7 @@ async fn test_cannot_provision_instance_beyond_cpu_limit( }, ncpus: too_many_cpus, memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("test"), + hostname: "test".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, @@ -3662,7 +3721,7 @@ async fn test_cannot_provision_instance_beyond_ram_capacity( }, ncpus: InstanceCpuCount::try_from(i64::from(1)).unwrap(), memory: ByteCount::try_from(config.1).unwrap(), - hostname: config.0.to_string(), + hostname: config.0.parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: @@ -3913,7 +3972,7 @@ async fn test_instance_ephemeral_ip_from_correct_pool( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, external_ips: vec![params::ExternalIpCreate::Ephemeral { @@ -3978,7 +4037,7 @@ async fn test_instance_ephemeral_ip_from_orphan_pool( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, external_ips: vec![params::ExternalIpCreate::Ephemeral { @@ -4039,7 +4098,7 @@ async fn test_instance_ephemeral_ip_no_default_pool_error( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: vec![], network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, external_ips: vec![params::ExternalIpCreate::Ephemeral { @@ -4172,7 +4231,7 @@ async fn test_instance_allow_only_one_ephemeral_ip( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), @@ -4300,7 +4359,7 @@ async fn test_instance_create_in_silo(cptestctx: &ControlPlaneTestContext) { }, ncpus: InstanceCpuCount::try_from(2).unwrap(), memory: ByteCount::from_gibibytes_u32(4), - hostname: String::from("inst"), + hostname: "inst".parse().unwrap(), user_data: vec![], ssh_public_keys: None, network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, diff --git a/nexus/tests/integration_tests/projects.rs b/nexus/tests/integration_tests/projects.rs index a89f2508ac..e13aca0cd0 100644 --- a/nexus/tests/integration_tests/projects.rs +++ b/nexus/tests/integration_tests/projects.rs @@ -155,7 +155,7 @@ async fn test_project_deletion_with_instance( }, ncpus: InstanceCpuCount(4), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("the_host"), + hostname: "the-host".parse().unwrap(), user_data: b"none".to_vec(), ssh_public_keys: Some(Vec::new()), network_interfaces: diff --git a/nexus/tests/integration_tests/quotas.rs b/nexus/tests/integration_tests/quotas.rs index c0422d0030..4e3335d04c 100644 --- a/nexus/tests/integration_tests/quotas.rs +++ b/nexus/tests/integration_tests/quotas.rs @@ -78,7 +78,7 @@ impl ResourceAllocator { }, ncpus: InstanceCpuCount(cpus), memory: ByteCount::from_gibibytes_u32(memory), - hostname: "host".to_string(), + hostname: "host".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), ssh_public_keys: Some(Vec::new()), diff --git a/nexus/tests/integration_tests/snapshots.rs b/nexus/tests/integration_tests/snapshots.rs index 9a2ee3d310..3731a80668 100644 --- a/nexus/tests/integration_tests/snapshots.rs +++ b/nexus/tests/integration_tests/snapshots.rs @@ -119,7 +119,7 @@ async fn test_snapshot_basic(cptestctx: &ControlPlaneTestContext) { }, ncpus: InstanceCpuCount(2), memory: ByteCount::from_gibibytes_u32(1), - hostname: String::from("base_instance"), + hostname: "base-instance".parse().unwrap(), user_data: b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" .to_vec(), diff --git a/nexus/tests/integration_tests/subnet_allocation.rs b/nexus/tests/integration_tests/subnet_allocation.rs index 3c9e18817f..3362d5a4ac 100644 --- a/nexus/tests/integration_tests/subnet_allocation.rs +++ b/nexus/tests/integration_tests/subnet_allocation.rs @@ -56,7 +56,7 @@ async fn create_instance_expect_failure( }, ncpus: InstanceCpuCount(1), memory: ByteCount::from_gibibytes_u32(1), - hostname: name.to_string(), + hostname: name.parse().unwrap(), user_data: vec![], ssh_public_keys: Some(Vec::new()), network_interfaces, diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index bda6a876ee..6cb878084d 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -9,7 +9,7 @@ use crate::external_api::shared; use base64::Engine; use chrono::{DateTime, Utc}; use omicron_common::api::external::{ - AddressLotKind, ByteCount, IdentityMetadataCreateParams, + AddressLotKind, ByteCount, Hostname, IdentityMetadataCreateParams, IdentityMetadataUpdateParams, InstanceCpuCount, IpNet, Ipv4Net, Ipv6Net, Name, NameOrId, PaginationOrder, RouteDestination, RouteTarget, SemverVersion, @@ -1004,7 +1004,7 @@ pub struct InstanceCreate { pub identity: IdentityMetadataCreateParams, pub ncpus: InstanceCpuCount, pub memory: ByteCount, - pub hostname: String, // TODO-cleanup different type? + pub hostname: Hostname, /// User data for instance initialization systems (such as cloud-init). /// Must be a Base64-encoded string, as specified in RFC 4648 § 4 (+ and / diff --git a/openapi/nexus.json b/openapi/nexus.json index 98073c8625..7aedd1b523 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -12221,6 +12221,14 @@ "start_time" ] }, + "Hostname": { + "title": "An RFC-1035-compliant hostname", + "description": "A hostname identifies a host on a network, and is usually a dot-delimited sequence of labels, where each label contains only letters, digits, or the hyphen. See RFCs 1035 and 952 for more details.", + "type": "string", + "pattern": "^([a-zA-Z0-9]+[a-zA-Z0-9\\-]*(?