diff --git a/README.adoc b/README.adoc index 9c555d19571..812a53e7168 100644 --- a/README.adoc +++ b/README.adoc @@ -645,6 +645,11 @@ documents being checked in. In general, changes any service API **require the following set of build steps**: * Make changes to the service API +* Build the package for the modified service alone. This can be done by changing + directories there, or `cargo build -p `. This is step is important, + to avoid the circular dependency at this point. One needs to update this one + OpenAPI document, without rebuilding the other components that depend on a + now-outdated spec. * Update the OpenAPI document by running the relevant test with overwrite set: `EXPECTORATE=overwrite cargo test test_nexus_openapi_internal` (changing the test name as necessary) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 5df89b2da28..a0aec6172f0 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1774,20 +1774,22 @@ pub struct NetworkInterface { #[serde(flatten)] pub identity: IdentityMetadata, - /** The Instance to which the interface belongs. */ + /// The Instance to which the interface belongs. pub instance_id: Uuid, - /** The VPC to which the interface belongs. */ + /// The VPC to which the interface belongs. pub vpc_id: Uuid, - /** The subnet to which the interface belongs. */ + /// The subnet to which the interface belongs. pub subnet_id: Uuid, - /** The MAC address assigned to this interface. */ + /// The MAC address assigned to this interface. pub mac: MacAddr, - /** The IP address assigned to this interface. */ + /// The IP address assigned to this interface. pub ip: IpAddr, + // TODO-correctness: We need to split this into an optional V4 and optional + // V6 address, at least one of which must be specified. } #[cfg(test)] diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 531f37f26f0..d332f60b030 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -465,8 +465,13 @@ CREATE TABLE omicron.public.network_interface ( time_modified TIMESTAMPTZ NOT NULL, /* Indicates that the object has been deleted */ time_deleted TIMESTAMPTZ, - /* FK into Instance table. */ + + /* FK into Instance table. + * Note that interfaces are always attached to a particular instance. + * IP addresses may be reserved, but this is a different resource. + */ instance_id UUID NOT NULL, + /* FK into VPC table */ vpc_id UUID NOT NULL, /* FK into VPCSubnet table. */ @@ -483,12 +488,6 @@ CREATE TABLE omicron.public.network_interface ( * as moving IPs between NICs on different instances, etc. */ -CREATE UNIQUE INDEX ON omicron.public.network_interface ( - vpc_id, - name -) WHERE - time_deleted IS NULL; - /* Ensure we do not assign the same address twice within a subnet */ CREATE UNIQUE INDEX ON omicron.public.network_interface ( subnet_id, @@ -505,6 +504,18 @@ CREATE UNIQUE INDEX ON omicron.public.network_interface ( ) WHERE time_deleted IS NULL; +/* + * Index used to verify that an Instance's networking is contained + * within a single VPC. + */ +CREATE UNIQUE INDEX ON omicron.public.network_interface ( + instance_id, + name +) +STORING (vpc_id) +WHERE + time_deleted IS NULL; + CREATE TYPE omicron.public.vpc_router_kind AS ENUM ( 'system', 'custom' diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 591c677ef7d..43a392eadc5 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -79,8 +79,9 @@ use crate::db::{ }, pagination::paginated, pagination::paginated_multicolumn, - subnet_allocation::AllocateIpQuery, subnet_allocation::FilterConflictingVpcSubnetRangesQuery, + subnet_allocation::InsertNetworkInterfaceQuery, + subnet_allocation::NetworkInterfaceError, subnet_allocation::SubnetError, update_and_check::{UpdateAndCheck, UpdateStatus}, }; @@ -1698,97 +1699,142 @@ impl DataStore { /* * Network interfaces */ - pub async fn instance_create_network_interface( &self, interface: IncompleteNetworkInterface, - ) -> CreateResult { + ) -> Result { use db::schema::network_interface::dsl; + let query = InsertNetworkInterfaceQuery { + interface: interface.clone(), + now: Utc::now(), + }; + diesel::insert_into(dsl::network_interface) + .values(query) + .returning(NetworkInterface::as_returning()) + .get_result_async(self.pool()) + .await + .map_err(|e| NetworkInterfaceError::from_pool(e, &interface)) + } - // TODO: Longer term, it would be nice to decouple the IP allocation - // (and MAC allocation) from the NetworkInterface table, so that - // retrying from parallel inserts doesn't need to happen here. - - let name = interface.identity.name.clone(); - match interface.ip { - // Attempt an insert with a requested IP address - Some(ip) => { - interface.subnet.contains(ip)?; - let row = NetworkInterface { - identity: interface.identity, - instance_id: interface.instance_id, - vpc_id: interface.vpc_id, - subnet_id: interface.subnet.id(), - mac: interface.mac, - ip: ip.into(), - }; - diesel::insert_into(dsl::network_interface) - .values(row) - .returning(NetworkInterface::as_returning()) - .get_result_async(self.pool()) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::Conflict( - ResourceType::NetworkInterface, - name.as_str(), - ), - ) - }) - } - // Insert and allocate an IP address - None => { - let allocation_query = AllocateIpQuery { - block: ipnetwork::IpNetwork::V4( - interface.subnet.ipv4_block.0 .0, + /// Delete all network interfaces attached to the given instance. + // NOTE: This is mostly useful in the context of sagas, but might be helpful + // in other situations, such as moving an instance between VPC Subnets. + pub async fn instance_delete_all_network_interfaces( + &self, + instance_id: &Uuid, + ) -> DeleteResult { + use db::schema::network_interface::dsl; + let now = Utc::now(); + diesel::update(dsl::network_interface) + .filter(dsl::instance_id.eq(*instance_id)) + .filter(dsl::time_deleted.is_null()) + .set(dsl::time_deleted.eq(now)) + .execute_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Instance, + LookupType::ById(*instance_id), ), - interface, - now: Utc::now(), - }; - diesel::insert_into(dsl::network_interface) - .values(allocation_query) - .returning(NetworkInterface::as_returning()) - .get_result_async(self.pool()) - .await - .map_err(|e| { - if let PoolError::Connection(ConnectionError::Query( - diesel::result::Error::NotFound, - )) = e - { - Error::InvalidRequest { - message: "no available IP addresses" - .to_string(), - } - } else { - public_error_from_diesel_pool( - e, - ErrorHandler::Conflict( - ResourceType::NetworkInterface, - name.as_str(), - ), - ) - } - }) - } - } + ) + })?; + Ok(()) } pub async fn instance_delete_network_interface( &self, - network_interface_id: &Uuid, + interface_id: &Uuid, ) -> DeleteResult { use db::schema::network_interface::dsl; + let now = Utc::now(); + let result = diesel::update(dsl::network_interface) + .filter(dsl::id.eq(*interface_id)) + .filter(dsl::time_deleted.is_null()) + .set((dsl::time_deleted.eq(now),)) + .check_if_exists::(*interface_id) + .execute_and_check(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::NetworkInterface, + LookupType::ById(*interface_id), + ), + ) + })?; + match result.status { + UpdateStatus::Updated => Ok(()), + UpdateStatus::NotUpdatedButExists => { + let interface = &result.found; + if interface.time_deleted().is_some() { + // Already deleted + Ok(()) + } else { + Err(Error::internal_error(&format!( + "failed to delete network interface: {}", + interface_id + ))) + } + } + } + } + + pub async fn subnet_lookup_network_interface( + &self, + subnet_id: &Uuid, + interface_name: &Name, + ) -> LookupResult { + use db::schema::network_interface::dsl; + + dsl::network_interface + .filter(dsl::subnet_id.eq(*subnet_id)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::name.eq(interface_name.clone())) + .select(db::model::NetworkInterface::as_select()) + .get_result_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::NetworkInterface, + LookupType::ByName(interface_name.to_string()), + ), + ) + }) + } - // TODO-correctness: Do not allow deleting interfaces on running - // instances until we support hotplug + /// List network interfaces associated with a given instance. + pub async fn instance_list_network_interfaces( + &self, + instance_id: &Uuid, + pagparams: &DataPageParams<'_, Name>, + ) -> ListResultVec { + use db::schema::network_interface::dsl; + paginated(dsl::network_interface, dsl::name, &pagparams) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::instance_id.eq(*instance_id)) + .select(NetworkInterface::as_select()) + .load_async::(self.pool()) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } - let now = Utc::now(); - diesel::update(dsl::network_interface) + /// Get a network interface by name attached to an instance + pub async fn instance_lookup_network_interface( + &self, + instance_id: &Uuid, + interface_name: &Name, + ) -> LookupResult { + use db::schema::network_interface::dsl; + dsl::network_interface + .filter(dsl::instance_id.eq(*instance_id)) + .filter(dsl::name.eq(interface_name.clone())) .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(*network_interface_id)) - .set(dsl::time_deleted.eq(now)) - .returning(NetworkInterface::as_returning()) + .select(NetworkInterface::as_select()) .get_result_async(self.pool()) .await .map_err(|e| { @@ -1796,11 +1842,10 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::NetworkInterface, - LookupType::ById(*network_interface_id), + LookupType::ByName(interface_name.to_string()), ), ) - })?; - Ok(()) + }) } // Create a record for a new Oximeter instance diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 8870ff0b6f1..e9615092fd2 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -33,6 +33,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::net::IpAddr; +use std::net::Ipv4Addr; +use std::net::Ipv6Addr; use std::net::SocketAddr; use uuid::Uuid; @@ -320,6 +322,23 @@ pub struct Ipv4Net(pub external::Ipv4Net); NewtypeFrom! { () pub struct Ipv4Net(external::Ipv4Net); } NewtypeDeref! { () pub struct Ipv4Net(external::Ipv4Net); } +impl Ipv4Net { + /// Check if an address is a valid user-requestable address for this subnet + pub fn check_requestable_addr(&self, addr: Ipv4Addr) -> bool { + self.contains(addr) + && ( + // First N addresses are reserved + self.iter() + .take(defaults::NUM_INITIAL_RESERVED_IP_ADDRESSES) + .all(|this| this != addr) + ) + && ( + // Last address in the subnet is reserved + addr != self.broadcast() + ) + } +} + impl ToSql for Ipv4Net where DB: Backend, @@ -404,6 +423,16 @@ impl Ipv6Net { .expect("Failed to create random subnet"); Some(Self(external::Ipv6Net(net))) } + + /// Check if an address is a valid user-requestable address for this subnet + pub fn check_requestable_addr(&self, addr: Ipv6Addr) -> bool { + // Only the first N addresses are reserved + self.contains(addr) + && self + .iter() + .take(defaults::NUM_INITIAL_RESERVED_IP_ADDRESSES) + .all(|this| this != addr) + } } impl ToSql for Ipv6Net @@ -1004,7 +1033,7 @@ impl_enum_type!( #[postgres(type_name = "instance_state", type_schema = "public")] pub struct InstanceStateEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Debug, PartialEq, AsExpression, FromSqlRow)] #[sql_type = "InstanceStateEnum"] pub struct InstanceState(pub external::InstanceState); @@ -1413,29 +1442,29 @@ impl VpcSubnet { /// /// - The subnet has an allocated block of the same version as the address /// - The allocated block contains the address. - pub fn contains(&self, addr: IpAddr) -> Result<(), external::Error> { - match addr { + /// - The address is not reserved. + pub fn check_requestable_addr( + &self, + addr: IpAddr, + ) -> Result<(), external::Error> { + let subnet = match addr { IpAddr::V4(addr) => { - if self.ipv4_block.contains(addr) { - Ok(()) - } else { - Err(external::Error::invalid_request(&format!( - "Address '{}' not in IPv4 subnet '{}'", - addr, self.ipv4_block.0, - ))) + if self.ipv4_block.check_requestable_addr(addr) { + return Ok(()); } + ipnetwork::IpNetwork::V4(self.ipv4_block.0 .0) } IpAddr::V6(addr) => { - if self.ipv6_block.contains(addr) { - Ok(()) - } else { - Err(external::Error::invalid_request(&format!( - "Address '{}' not in IPv6 subnet '{}'", - addr, self.ipv6_block.0, - ))) + if self.ipv6_block.check_requestable_addr(addr) { + return Ok(()); } + ipnetwork::IpNetwork::V6(self.ipv6_block.0 .0) } - } + }; + Err(external::Error::invalid_request(&format!( + "Address '{}' not in subnet '{}' or is reserved for rack services", + addr, subnet, + ))) } } @@ -2005,12 +2034,14 @@ impl IncompleteNetworkInterface { vpc_id: Uuid, subnet: VpcSubnet, mac: MacAddr, + identity: external::IdentityMetadataCreateParams, ip: Option, - params: params::NetworkInterfaceCreate, - ) -> Self { - let identity = - NetworkInterfaceIdentity::new(interface_id, params.identity); - Self { identity, instance_id, subnet, vpc_id, mac, ip } + ) -> Result { + if let Some(ip) = ip { + subnet.check_requestable_addr(ip)?; + }; + let identity = NetworkInterfaceIdentity::new(interface_id, identity); + Ok(Self { identity, instance_id, subnet, vpc_id, mac, ip }) } } @@ -2025,6 +2056,11 @@ pub struct NetworkInterface { pub subnet_id: Uuid, pub mac: MacAddr, pub ip: ipnetwork::IpNetwork, + // TODO-correctness: We need to split this into an optional V4 and optional V6 address, at + // least one of which will always be specified. + // + // If user requests an address of either kind, give exactly that and not the other. + // If neither is specified, auto-assign one of each? } impl From for external::NetworkInterface { @@ -2173,7 +2209,7 @@ mod tests { use std::net::Ipv6Addr; #[test] - fn test_vpc_subnet_contains() { + fn test_vpc_subnet_check_requestable_addr() { let ipv4_block = Ipv4Net("192.168.0.0/16".parse::().unwrap()); let ipv6_block = Ipv6Net("fd00::/48".parse::().unwrap()); @@ -2181,27 +2217,54 @@ mod tests { name: "net-test-vpc".parse().unwrap(), description: "A test VPC".parse().unwrap(), }; - let vpc = VpcSubnet::new( + let subnet = VpcSubnet::new( Uuid::new_v4(), Uuid::new_v4(), identity, ipv4_block, ipv6_block, ); - assert!(vpc - .contains(IpAddr::from(Ipv4Addr::new(192, 168, 1, 1))) + // Within subnet + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv4Addr::new( + 192, 168, 1, 10 + ))) .is_ok()); - assert!(vpc - .contains(IpAddr::from(Ipv4Addr::new(192, 160, 1, 1))) + // Network address is reserved + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv4Addr::new(192, 168, 0, 0))) .is_err()); - assert!(vpc - .contains(IpAddr::from(Ipv6Addr::new( - 0xfd00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 + // Broadcast address is reserved + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv4Addr::new( + 192, 168, 255, 255 + ))) + .is_err()); + // Within subnet, but reserved + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv4Addr::new(192, 168, 0, 1))) + .is_err()); + // Not within subnet + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv4Addr::new(192, 160, 1, 1))) + .is_err()); + + // Within subnet + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv6Addr::new( + 0xfd00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10 ))) .is_ok()); - assert!(vpc - .contains(IpAddr::from(Ipv6Addr::new( - 0xfc00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 + // Within subnet, but reserved + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv6Addr::new( + 0xfd00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01 + ))) + .is_err()); + // Not within subnet + assert!(subnet + .check_requestable_addr(IpAddr::from(Ipv6Addr::new( + 0xfc00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10 ))) .is_err()); } @@ -2238,4 +2301,21 @@ mod tests { ); assert_eq!(base.random_subnet(base.prefix()), Some(base)); } + + #[test] + fn test_ip_subnet_check_requestable_address() { + let subnet = super::Ipv4Net(Ipv4Net("192.168.0.0/16".parse().unwrap())); + assert!(subnet.check_requestable_addr("192.168.0.10".parse().unwrap())); + assert!(subnet.check_requestable_addr("192.168.1.0".parse().unwrap())); + assert!(!subnet.check_requestable_addr("192.168.0.0".parse().unwrap())); + assert!(subnet.check_requestable_addr("192.168.0.255".parse().unwrap())); + assert!( + !subnet.check_requestable_addr("192.168.255.255".parse().unwrap()) + ); + + let subnet = super::Ipv6Net(Ipv6Net("fd00::/64".parse().unwrap())); + assert!(subnet.check_requestable_addr("fd00::a".parse().unwrap())); + assert!(!subnet.check_requestable_addr("fd00::1".parse().unwrap())); + assert!(subnet.check_requestable_addr("fd00::1:1".parse().unwrap())); + } } diff --git a/nexus/src/db/subnet_allocation.rs b/nexus/src/db/subnet_allocation.rs index ece69aa0e4d..b1b1bba0bb7 100644 --- a/nexus/src/db/subnet_allocation.rs +++ b/nexus/src/db/subnet_allocation.rs @@ -12,217 +12,32 @@ use diesel::pg::Pg; use diesel::prelude::*; use diesel::query_builder::*; use diesel::sql_types; +use ipnetwork::IpNetwork; use omicron_common::api::external; use std::convert::TryFrom; use uuid::Uuid; -/// Used for allocating an IP as part of [`NetworkInterface`] construction. -/// -/// This is a query equivalent to: -/// SELECT AS id, AS name, AS description, -/// AS time_created, AS time_modified, -/// AS instance_id, AS vpc_id, -/// AS subnet_id, AS mac, + off AS ip -/// FROM -/// generate_series(5, ) AS off -/// LEFT OUTER JOIN -/// network_interface -/// ON (subnet_id, ip, time_deleted IS NULL) = -/// (, + off, TRUE) -/// WHERE ip IS NULL LIMIT 1; -/// -/// Note that generate_series receives a start value of 5 in accordance with -/// RFD 21's reservation of addresses 0 through 4 in a subnet. See -/// , for details. -// TODO-performance: This query scales linearly with the number of IPs -// allocated, which is highly undesirable. It will also return the same -// candidate address to two parallel executors, which will cause additional -// retries. -pub struct AllocateIpQuery { - pub interface: IncompleteNetworkInterface, - pub block: ipnetwork::IpNetwork, - pub now: DateTime, -} - -/// Used for using AllocateIpQuery with an INSERT statement. Do not use this -/// directly, instead pass an instance of [`AllocateIpQuery`] to -/// [`InsertStatement::values`]. -pub struct AllocateIpQueryValues(AllocateIpQuery); - -impl QueryId for AllocateIpQuery { - type QueryId = (); - const HAS_STATIC_QUERY_ID: bool = false; -} - -impl Insertable for AllocateIpQuery { - type Values = AllocateIpQueryValues; - - fn values(self) -> Self::Values { - AllocateIpQueryValues(self) - } -} - -impl QueryFragment for AllocateIpQuery { - fn walk_ast(&self, mut out: AstPass) -> diesel::QueryResult<()> { - use db::schema::network_interface::dsl; - - // Generate last address in the range. - // - // NOTE: First subtraction is to convert from the subnet size to an - // offset, since `generate_series` is inclusive of the last value. - // Example: 256 -> 255. - let last_address_offset = match self.block { - ipnetwork::IpNetwork::V4(network) => network.size() as i64 - 1, - ipnetwork::IpNetwork::V6(network) => { - // If we're allocating from a v6 subnet with more than 2^63 - 1 - // addresses, just cap the size we'll explore. This will never - // fail in practice since we're never going to be storing 2^64 - // rows in the network_interface table. - i64::try_from(network.size() - 1).unwrap_or(i64::MAX) - } - }; - - out.push_sql("SELECT "); - - out.push_bind_param::( - &self.interface.identity.id, - )?; - out.push_sql(" AS "); - out.push_identifier(dsl::id::NAME)?; - out.push_sql(", "); - - out.push_bind_param::( - &self.interface.identity.name.to_string(), - )?; - out.push_sql(" AS "); - out.push_identifier(dsl::name::NAME)?; - out.push_sql(", "); - - out.push_bind_param::( - &self.interface.identity.description, - )?; - out.push_sql(" AS "); - out.push_identifier(dsl::description::NAME)?; - out.push_sql(", "); - - out.push_bind_param::(&self.now)?; - out.push_sql(" AS "); - out.push_identifier(dsl::time_created::NAME)?; - out.push_sql(", "); - - out.push_bind_param::(&self.now)?; - out.push_sql(" AS "); - out.push_identifier(dsl::time_modified::NAME)?; - out.push_sql(", "); - - out.push_bind_param::( - &self.interface.instance_id, - )?; - out.push_sql(" AS "); - out.push_identifier(dsl::instance_id::NAME)?; - out.push_sql(", "); - - out.push_bind_param::(&self.interface.vpc_id)?; - out.push_sql(" AS "); - out.push_identifier(dsl::vpc_id::NAME)?; - out.push_sql(", "); - - out.push_bind_param::( - &self.interface.subnet.id(), - )?; - out.push_sql(" AS "); - out.push_identifier(dsl::subnet_id::NAME)?; - out.push_sql(", "); - - out.push_bind_param::( - &self.interface.mac.to_string(), - )?; - out.push_sql(" AS "); - out.push_identifier(dsl::mac::NAME)?; - out.push_sql(", "); - - out.push_bind_param::( - &self.block.network().into(), - )?; - out.push_sql(" + "); - out.push_identifier("off")?; - out.push_sql(" AS "); - out.push_identifier(dsl::ip::NAME)?; - - // Skip the initial reserved addresses and the broadcast address. - out.push_sql(" FROM generate_series(5, "); - out.push_bind_param::( - &(last_address_offset - 1), - )?; - out.push_sql(") AS "); - out.push_identifier("off")?; - out.push_sql(" LEFT OUTER JOIN "); - dsl::network_interface.from_clause().walk_ast(out.reborrow())?; - - // ON (subnet_id, ip, time_deleted IS NULL) = - // (, + off, TRUE) - out.push_sql(" ON ("); - out.push_identifier(dsl::subnet_id::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::ip::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::time_deleted::NAME)?; - out.push_sql(" IS NULL) = ("); - out.push_bind_param::( - &self.interface.subnet.id(), - )?; - out.push_sql(", "); - out.push_bind_param::( - &self.block.network().into(), - )?; - out.push_sql(" + "); - out.push_identifier("off")?; - out.push_sql(", TRUE) "); - // WHERE ip IS NULL LIMIT 1; - out.push_sql("WHERE "); - out.push_identifier(dsl::ip::NAME)?; - out.push_sql(" IS NULL LIMIT 1"); - Ok(()) - } -} - -impl QueryId for AllocateIpQueryValues { - type QueryId = (); - const HAS_STATIC_QUERY_ID: bool = false; -} +// Helper to return the offset of the last valid/allocatable IP in a subnet. +fn generate_last_address_offset(subnet: &ipnetwork::IpNetwork) -> i64 { + // Generate last address in the range. + // + // NOTE: First subtraction is to convert from the subnet size to an + // offset, since `generate_series` is inclusive of the last value. + // Example: 256 -> 255. + let last_address_offset = match subnet { + ipnetwork::IpNetwork::V4(network) => network.size() as i64 - 1, + ipnetwork::IpNetwork::V6(network) => { + // If we're allocating from a v6 subnet with more than 2^63 - 1 + // addresses, just cap the size we'll explore. This will never + // fail in practice since we're never going to be storing 2^64 + // rows in the network_interface table. + i64::try_from(network.size() - 1).unwrap_or(i64::MAX) + } + }; -impl diesel::insertable::CanInsertInSingleQuery for AllocateIpQueryValues { - fn rows_to_insert(&self) -> Option { - Some(1) - } -} - -impl QueryFragment for AllocateIpQueryValues { - fn walk_ast(&self, mut out: AstPass) -> diesel::QueryResult<()> { - use db::schema::network_interface::dsl; - out.push_sql("("); - out.push_identifier(dsl::id::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::name::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::description::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::time_created::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::time_modified::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::instance_id::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::vpc_id::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::subnet_id::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::mac::NAME)?; - out.push_sql(", "); - out.push_identifier(dsl::ip::NAME)?; - out.push_sql(") "); - self.0.walk_ast(out) - } + // This subtraction is because the last address in a subnet is + // explicitly reserved for Oxide use. + last_address_offset - 1 } /// Errors related to allocating VPC Subnets. @@ -490,142 +305,771 @@ impl QueryFragment for FilterConflictingVpcSubnetRangesQueryValues { } } +/// Errors related to inserting or attaching a NetworkInterface +#[derive(Debug)] +pub enum NetworkInterfaceError { + /// The instance specified for this interface is already associated with a + /// different VPC from this interface. + InstanceSpansMultipleVpcs(Uuid), + /// There are no available IP addresses in the requested subnet + NoAvailableIpAddresses, + /// An explicitly-requested IP address is already in use + IpAddressNotAvailable(std::net::IpAddr), + /// A primary key violation, which is intentionally caused in some cases + /// during instance creation sagas. + DuplicatePrimaryKey(Uuid), + /// Any other error + External(external::Error), +} + +impl NetworkInterfaceError { + /// Construct a `NetworkInterfaceError` from a database error + /// + /// This catches the various errors that the `InsertNetworkInterfaceQuery` + /// can generate, especially the intentional errors that indicate either IP + /// address exhaustion or an attempt to attach an interface to an instance + /// that is already associated with another VPC. + pub fn from_pool( + e: async_bb8_diesel::PoolError, + interface: &IncompleteNetworkInterface, + ) -> Self { + use crate::db::error; + use async_bb8_diesel::ConnectionError; + use async_bb8_diesel::PoolError; + use diesel::result::Error; + match e { + // Catch the specific errors designed to communicate the failures we + // want to distinguish + PoolError::Connection(ConnectionError::Query( + Error::DatabaseError(_, _), + )) => decode_database_error(e, interface), + // Any other error at all is a bug + _ => NetworkInterfaceError::External( + error::public_error_from_diesel_pool( + e, + error::ErrorHandler::Server, + ), + ), + } + } + + /// Convert this error into an external one. + pub fn into_external(self) -> external::Error { + match self { + NetworkInterfaceError::NoAvailableIpAddresses => { + external::Error::invalid_request( + "No available IP addresses for interface", + ) + } + NetworkInterfaceError::InstanceSpansMultipleVpcs(_) => { + external::Error::invalid_request(concat!( + "Networking may not span multiple VPCs, but the ", + "requested instance is associated with another VPC" + )) + } + NetworkInterfaceError::IpAddressNotAvailable(ip) => { + external::Error::invalid_request(&format!( + "The IP address '{}' is not available", + ip + )) + } + NetworkInterfaceError::DuplicatePrimaryKey(id) => { + external::Error::InternalError { + internal_message: format!( + "Found duplicate primary key '{}' when inserting network interface", + id + ), + } + } + NetworkInterfaceError::External(e) => e, + } + } +} + +/// Decode an error from the database to determine why our NIC query failed. +/// +/// When inserting network interfaces, we use the `InsertNetworkInterfaceQuery`, +/// which is designed to fail in particular ways depending on the requested +/// data. For example, if the client requests a new NIC on an instance, where +/// that instance already has a NIC from a VPC that's different from the new +/// one, we handle that here. +/// +/// This function works by inspecting the detailed error messages, including +/// indexes used or constraints violated, to determine the cause of the failure. +/// As such, it naturally is extremely tightly coupled to the database itself, +/// including the software version and our schema. +fn decode_database_error( + err: async_bb8_diesel::PoolError, + interface: &IncompleteNetworkInterface, +) -> NetworkInterfaceError { + use crate::db::error; + use async_bb8_diesel::ConnectionError; + use async_bb8_diesel::PoolError; + use diesel::result::DatabaseErrorKind; + use diesel::result::Error; + + // Error message generated when we attempt to insert an interface in a + // different VPC from the interface(s) already associated with the instance + const MULTIPLE_VPC_ERROR_MESSAGE: &str = + r#"could not parse "" as type uuid: uuid: incorrect UUID length: "#; + + // Error message generated when we attempt to insert NULL in the `ip` + // column, which only happens when we run out of IPs in the subnet. + const IP_EXHAUSTION_ERROR_MESSAGE: &str = + r#"null value in column "ip" violates not-null constraint"#; + + // The name of the index whose uniqueness is violated if we try to assign an + // IP that is already allocated to another interface in the same subnet. + const IP_NOT_AVAILABLE_CONSTRAINT: &str = + "network_interface_subnet_id_ip_key"; + + // The name of the index whose uniqueness is violated if we try to assign a + // name to an interface that is already used for another interface on the + // same instance. + const NAME_CONFLICT_CONSTRAINT: &str = + "network_interface_instance_id_name_key"; + + // The primary key constraint. This is intended only to be caught, and + // usually ignored, in sagas. UUIDs are allocated in one node of the saga, + // and then the NICs in a following node. In that case, primary key + // conflicts would be expected, if we're recovering from a crash during that + // node and replaying it, without first having unwound the whole saga. This + // should _only_ be ignored in that case. In any other circumstance, this + // should likely be converted to a 500-level server error + const PRIMARY_KEY_CONSTRAINT: &str = "primary"; + + match err { + // If the address allocation subquery fails, we'll attempt to insert + // NULL for the `ip` column. This checks that the non-NULL constraint on + // that colum has been violated. + PoolError::Connection(ConnectionError::Query( + Error::DatabaseError(DatabaseErrorKind::NotNullViolation, ref info), + )) if info.message() == IP_EXHAUSTION_ERROR_MESSAGE => { + NetworkInterfaceError::NoAvailableIpAddresses + } + + // This catches the error intentionally introduced by the + // `push_ensure_unique_vpc_expression` subquery, which generates a + // UUID parsing error if an instance is already associated with + // another VPC. + PoolError::Connection(ConnectionError::Query( + Error::DatabaseError(DatabaseErrorKind::Unknown, info), + )) if info.message() == MULTIPLE_VPC_ERROR_MESSAGE => { + NetworkInterfaceError::InstanceSpansMultipleVpcs( + interface.instance_id, + ) + } + + // This path looks specifically at constraint names. + PoolError::Connection(ConnectionError::Query( + Error::DatabaseError(DatabaseErrorKind::UniqueViolation, ref info), + )) => match info.constraint_name() { + // Constraint violated if a user-requested IP address has + // already been assigned within the same VPC Subnet. + Some(constraint) if constraint == IP_NOT_AVAILABLE_CONSTRAINT => { + let ip = interface + .ip + .unwrap_or_else(|| std::net::Ipv4Addr::UNSPECIFIED.into()); + NetworkInterfaceError::IpAddressNotAvailable(ip) + } + + // Constraint violated if the user-requested name is already + // assigned to an interface on this instance. + Some(constraint) if constraint == NAME_CONFLICT_CONSTRAINT => { + NetworkInterfaceError::External( + error::public_error_from_diesel_pool( + err, + error::ErrorHandler::Conflict( + external::ResourceType::NetworkInterface, + interface.identity.name.as_str(), + ), + ), + ) + } + + // Primary key constraint violation. See notes above. + Some(constraint) if constraint == PRIMARY_KEY_CONSTRAINT => { + NetworkInterfaceError::DuplicatePrimaryKey( + interface.identity.id, + ) + } + + // Any other constraint violation is a bug + _ => NetworkInterfaceError::External( + error::public_error_from_diesel_pool( + err, + error::ErrorHandler::Server, + ), + ), + }, + + // Any other error at all is a bug + _ => NetworkInterfaceError::External( + error::public_error_from_diesel_pool( + err, + error::ErrorHandler::Server, + ), + ), + } +} + +/// Add a subquery intended to verify that an Instance's networking does not +/// span multiple VPCs. +/// +/// As described in RFD 21, an Instance's networking is confined to a single +/// VPC. That is, any NetworkInterfaces attached to an Instance must all have +/// the same VPC ID. This function adds a subquery, shown below, that fails in a +/// specific way (parsing error) if that invariant is violated. The basic +/// structure of the query is: +/// +/// ```text +/// CAST(IF(, '', '') AS UUID) +/// ``` +/// +/// This selects either the actual VPC UUID (as a string) or the empty string, +/// if any existing VPC IDs for this instance are the same. If true, we cast the +/// VPC ID string back to a UUID. If false, we try to cast the empty string, +/// which fails in a detectable way. +/// +/// Details +/// ------- +/// +/// The exact query generated looks like this: +/// +/// ```sql +/// CAST(IF( +/// COALESCE( +/// ( +/// SELECT vpc_id +/// FROM network_interface +/// WHERE +/// time_deleted IS NULL AND +/// instance_id = +/// LIMIT 1 +/// ), +/// +/// ) = , +/// '', -- UUID as a string +/// '' +/// ) AS UUID) +/// ``` +/// +/// This uses a partial index on the `network_interface` table to look up the +/// first record with the provided `instance_id`, if any. It then compares that +/// stored `vpc_id` to the one provided to this query. If those IDs match, then +/// the ID is returned. If they do _not_ match, the `IF` statement returns an +/// empty string, which it tries to cast as a UUID. That fails, in a detectable +/// way, so that we can check this case as distinct from other errors. +/// +/// Note that the `COALESCE` expression is there to handle the case where there +/// _is_ no record with the given `instance_id`. In that case, the `vpc_id` +/// provided is returned directly, so everything works as if the IDs matched. +fn push_ensure_unique_vpc_expression( + mut out: AstPass, + vpc_id: &Uuid, + instance_id: &Uuid, +) -> diesel::QueryResult<()> { + use db::schema::network_interface::dsl; + + out.push_sql("CAST(IF(COALESCE((SELECT "); + out.push_identifier(dsl::vpc_id::NAME)?; + out.push_sql(" FROM "); + dsl::network_interface.from_clause().walk_ast(out.reborrow())?; + out.push_sql(" WHERE "); + out.push_identifier(dsl::time_deleted::NAME)?; + out.push_sql(" IS NULL AND "); + out.push_identifier(dsl::instance_id::NAME)?; + out.push_sql(" = "); + out.push_bind_param::(instance_id)?; + out.push_sql(" LIMIT 1), "); + out.push_bind_param::(vpc_id)?; + out.push_sql(") = "); + out.push_bind_param::(vpc_id)?; + out.push_sql(", "); + + // NOTE: This bind-parameter is intentionally a string, rather than a UUID. + // + // This query relies on the fact that it generates a parsing error in the + // case where there is an interface attached to a VPC that's _different_ + // from the VPC of the candidate interface. This is so that we can + // distinguish this error case from the one where there is no IP address + // available. + // + // To do that, we generate a query like: + // + // ``` + // CAST(IF(, '', '') AS UUID) + // ``` + // + // That empty-string cannot be cast to a UUID, so we get a parsing error, + // but only if the condition _succeeds_. It's not evaluated otherwise. + // + // However, if we push this parameter as a UUID explicitly, the database + // looks at the parts of the `IF` statement, and tries to make them a common + // type, a UUID. That's the exact error we're trying to produce, but it's + // evaluated too early. So we ensure both are strings here, and then ask the + // DB to cast them after that condition is evaluated. + out.push_bind_param::(&vpc_id.to_string())?; + out.push_sql(", '') AS UUID)"); + Ok(()) +} + +/// Push a subquery that selects the next available IP address from a subnet. +/// +/// This adds a subquery like: +/// +/// ```sql +/// SELECT +/// + address_offset AS ip +/// FROM +/// generate_series(5, ) AS address_offset +/// LEFT OUTER JOIN +/// network_interface +/// ON +/// (subnet_id, ip, time_deleted IS NULL) = +/// ( + address_offset, TRUE) +/// WHERE ip IS NULL LIMIT 1 +/// ``` +/// +/// This is a linear, sequential scan for an IP from the subnet that's not yet +/// been allocated. We'd ultimately like a better-performing allocation +/// strategy; for example, we might be able to keep the lowest unallocated +/// address for each subnet, and atomically return and increment that. +/// +/// This would work fine, but explicit reservations of IP addresses complicate +/// the picture. We'd need a more complex data structure to manage the ranges of +/// available address for each subnet, especially to manage coalescing those +/// ranges as addresses are released back to the pool. +fn push_select_next_available_ip_subquery( + mut out: AstPass, + subnet: &IpNetwork, + subnet_id: &Uuid, +) -> diesel::QueryResult<()> { + use db::schema::network_interface::dsl; + let last_address_offset = generate_last_address_offset(&subnet); + let network_address = IpNetwork::from(subnet.network()); + out.push_sql("SELECT "); + out.push_bind_param::(&network_address)?; + out.push_sql(" + "); + out.push_identifier("address_offset")?; + out.push_sql(" AS "); + out.push_identifier(dsl::ip::NAME)?; + + out.push_sql( + format!( + " FROM generate_series({}, ", + crate::defaults::NUM_INITIAL_RESERVED_IP_ADDRESSES + ) + .as_str(), + ); + out.push_bind_param::(&last_address_offset)?; + out.push_sql(") AS "); + out.push_identifier("address_offset")?; + out.push_sql(" LEFT OUTER JOIN "); + dsl::network_interface.from_clause().walk_ast(out.reborrow())?; + out.push_sql(" ON ("); + out.push_identifier(dsl::subnet_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::ip::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_deleted::NAME)?; + out.push_sql(" IS NULL) = ("); + out.push_bind_param::(&subnet_id)?; + out.push_sql(", "); + out.push_bind_param::(&network_address)?; + out.push_sql(" + "); + out.push_identifier("address_offset")?; + out.push_sql(", TRUE) "); + out.push_sql("WHERE "); + out.push_identifier(dsl::ip::NAME)?; + out.push_sql(" IS NULL LIMIT 1"); + Ok(()) +} + +/// Subquery used to insert a _new_ `NetworkInterface` from parameters. +/// +/// This function is used to construct a query that allows inserting a +/// `NetworkInterface`, supporting both optionally allocating a new IP address +/// and verifying that the attached instance's networking is contained within a +/// single VPC. The general query looks like: +/// +/// ```sql +/// +/// SELECT AS id, AS name, AS description, +/// AS time_created, AS time_modified, +/// NULL AS time_deleted, AS instance_id, AS vpc_id, +/// AS subnet_id, AS mac, +/// ``` +/// +/// Instance validation +/// ------------------- +/// +/// This query generates a CTE that checks that the requested instance is not +/// already associated with another VPC (since an instance's networking cannot +/// span multiple VPCs). This query is designed to fail in a particular way if +/// that invariant is violated, so that we can detect and report that case to +/// the user. See [`push_ensure_unique_vpc_expression`] for details of that +/// subquery, including how it fails. +/// +/// IP allocation subquery +/// ---------------------- +/// +/// If the user explicitly requests an IP address, this part of the query is +/// just that exact IP. The query may still fail if the IP is already in use, +/// which is detected and forwarded as a client error. +/// +/// If the user wants an address allocated, then this generates a subquery that +/// tries to find the next available IP address (if any). See +/// [`push_select_next_available_ip_subquery`] for details on that allocation +/// subquery. If that fails, due to address exhaustion, this is detected and +/// forwarded to the caller. +/// +/// Errors +/// ------ +/// +/// See [`NetworkInterfaceError`] for the errors caught and propagated by this +/// query. +/// +/// Notes +/// ----- +/// +/// This query is designed so that, if the instance-validation subquery fails, +/// we do not run the address allocation query. This is just for performance; +/// since the allocation query runs in a time proportional to the smallest +/// unallocated address in the subnet, we'd like to avoid that if the query will +/// just fail the other, VPC-validation check. +/// +/// It's not easy to verify that this is indeed the case, since running `EXPLAIN +/// ANALYZE` to get details about the number of rows read can't work, as the +/// query will fail. By putting this in a CTE, prior to the rest of the main +/// query, it seems likely that the database will run that portion first. In +/// particular, [this +/// note](https://www.cockroachlabs.com/docs/v21.2/subqueries#performance-best-practices) +/// claims that scalar subqueries, which generate a single value as this one +/// does, are completely executed and stored in memory before the surrounding +/// query starts. Thus the instance-validation subquery should run entirely +/// before the remainder of the query. +/// +/// It's still possible that the engine runs the IP allocation subquery too, +/// either before or concurrently with the instance-validation subquery. It's +/// not clear how to test for this. But if this does become obvious, that +/// portion of the query might need to be placed behind a conditional evaluation +/// expression, such as `IF` or `COALESCE`, which only runs the subquery when +/// the instance-validation check passes. +fn push_interface_allocation_subquery( + mut out: AstPass, + interface: &IncompleteNetworkInterface, + now: &DateTime, +) -> diesel::QueryResult<()> { + use db::schema::network_interface::dsl; + // Push the CTE that ensures that any other interface with the same + // instance_id also has the same vpc_id. See + // `push_ensure_unique_vpc_expression` for more details. This ultimately + // fails the query if the requested instance is already associated with + // a different VPC. + out.push_sql("WITH vpc("); + out.push_identifier(dsl::vpc_id::NAME)?; + out.push_sql(") AS "); + out.push_sql("(SELECT "); + push_ensure_unique_vpc_expression( + out.reborrow(), + &interface.vpc_id, + &interface.instance_id, + )?; + out.push_sql(") "); + + // Push the columns, values and names, that are named directly. These + // are known regardless of whether we're allocating an IP address. These + // are all written as `SELECT AS , AS , ... + out.push_sql("SELECT "); + out.push_bind_param::(&interface.identity.id)?; + out.push_sql(" AS "); + out.push_identifier(dsl::id::NAME)?; + out.push_sql(", "); + + out.push_bind_param::( + &interface.identity.name.as_str(), + )?; + out.push_sql(" AS "); + out.push_identifier(dsl::name::NAME)?; + out.push_sql(", "); + + out.push_bind_param::( + &interface.identity.description.as_str(), + )?; + out.push_sql(" AS "); + out.push_identifier(dsl::description::NAME)?; + out.push_sql(", "); + + out.push_bind_param::>(&now)?; + out.push_sql(" AS "); + out.push_identifier(dsl::time_created::NAME)?; + out.push_sql(", "); + + out.push_bind_param::>(&now)?; + out.push_sql(" AS "); + out.push_identifier(dsl::time_modified::NAME)?; + out.push_sql(", "); + + out.push_bind_param::, Option>>(&None)?; + out.push_sql(" AS "); + out.push_identifier(dsl::time_deleted::NAME)?; + out.push_sql(", "); + + out.push_bind_param::(&interface.instance_id)?; + out.push_sql(" AS "); + out.push_identifier(dsl::instance_id::NAME)?; + out.push_sql(", "); + + // Tiny subquery to select the `vpc_id` from the preceding CTE. + out.push_sql("(SELECT vpc_id FROM vpc) AS "); + out.push_identifier(dsl::vpc_id::NAME)?; + out.push_sql(", "); + + out.push_bind_param::(&interface.subnet.id())?; + out.push_sql(" AS "); + out.push_identifier(dsl::subnet_id::NAME)?; + out.push_sql(", "); + + out.push_bind_param::(&interface.mac.to_string())?; + out.push_sql(" AS "); + out.push_identifier(dsl::mac::NAME)?; + out.push_sql(", "); + + // If the user specified an IP address, then insert it by value. If they + // did not, meaning we're allocating the next available one on their + // behalf, then insert that subquery here. + if let Some(ip) = interface.ip { + out.push_bind_param::(&ip.into())?; + } else { + let subnet = + ipnetwork::IpNetwork::from(interface.subnet.ipv4_block.0 .0); + out.push_sql("("); + push_select_next_available_ip_subquery( + out.reborrow(), + &subnet, + &interface.subnet.id(), + )?; + out.push_sql(")"); + } + out.push_sql(" AS "); + out.push_identifier(dsl::ip::NAME)?; + Ok(()) +} + +/// Type used to insert conditionally insert a network interface. +/// +/// This type implements a query that does one of two things +/// +/// - Insert a new network interface, performing validation and possibly IP +/// allocation +/// - Return an existing interface record, if it has the same primary key. +/// +/// The first case is implemented in the [`push_interface_allocation_subquery`] +/// function. See that function's documentation for the details. +/// +/// The second case is implemented in this type's `walk_ast` method. +/// +/// Details +/// ------- +/// +/// The `push_interface_allocation_subquery` performs a number of validations on +/// the data provided, such as verifying that a requested IP address isn't +/// already assigned, or ensuring that the instance that will receive this +/// interface isn't already associated with another VPC. +/// +/// However, the query is also meant to run during an instance creation saga. In +/// that case, the guardrails and unique indexes on this table make it +/// impossible for the query to both be idempotent and also catch these +/// constraints. +/// +/// For example, imaging the instance creation saga crashes partway through +/// allocation a list of NICs. That node of the saga will be replayed during +/// saga recovery. This will create a record with exactly the same UUID for each +/// interface, which will ulimately result in a conflicting primary key error +/// from the database. This is both intentional and integral to the sagas +/// correct functioning. We catch this error deliberately, assuming that the +/// uniqueness of 128-bit UUIDs guarantees that the only practical situation +/// under which this can occur is a saga node replay after a crash. +/// +/// Query structure +/// --------------- +/// +/// This query looks like the following: +/// +/// ```text +/// SELECT (candidate).* FROM (SELECT COALESCE( +/// , +/// +/// ) +/// ``` +/// +/// That is, we return the exact record that's already in the database, if there +/// is one, or run the entire validating query otherwise. In the context of +/// sagas, this is helpful because we generate the UUIDs for the interfaces in a +/// separate saga node, prior to inserting any interfaces. So if we have a +/// record with that exact UUID, we assert that it must be the record from the +/// saga itself. Note that, at this point, we return only the primary key, since +/// it's sufficiently unlikely that there's an existing key whose other data +/// does _not_ match the data we wanted to insert in a saga. +/// +/// The odd syntax in the initial section, `SELECT (candidate).*` is because the +/// result of the `COALESCE` expression is a tuple. That is CockroachDB's syntax +/// for expanding a tuple into its constituent columns. +/// +/// Note that the result of this expression is ultimately inserted into the +/// `network_interface` table. The way that fails (VPC-validation, IP +/// exhaustion, primary key violation), is used for either forwarding an error +/// on to the client (in the case of IP exhaustion, for example), or continuing +/// with a saga (for PK uniqueness violations). See [`NetworkInterfaceError`] +/// for a summary of the error conditions and their meaning, and the functions +/// constructing the subqueries in this type for more details. +#[derive(Debug, Clone)] +pub struct InsertNetworkInterfaceQuery { + pub interface: IncompleteNetworkInterface, + pub now: DateTime, +} + +impl QueryId for InsertNetworkInterfaceQuery { + type QueryId = (); + const HAS_STATIC_QUERY_ID: bool = false; +} + +impl Insertable + for InsertNetworkInterfaceQuery +{ + type Values = InsertNetworkInterfaceQueryValues; + + fn values(self) -> Self::Values { + InsertNetworkInterfaceQueryValues(self) + } +} + +impl QueryFragment for InsertNetworkInterfaceQuery { + fn walk_ast(&self, mut out: AstPass) -> diesel::QueryResult<()> { + use db::schema::network_interface::dsl; + let push_columns = |mut out: AstPass| -> diesel::QueryResult<()> { + out.push_identifier(dsl::id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::name::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::description::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_created::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_modified::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_deleted::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::instance_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::vpc_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::subnet_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::mac::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::ip::NAME)?; + Ok(()) + }; + + out.push_sql("SELECT (candidate).* FROM (SELECT COALESCE(("); + + // Add subquery to find exactly the record we might have already + // inserted during a saga. + out.push_sql("SELECT "); + push_columns(out.reborrow())?; + out.push_sql(" FROM "); + dsl::network_interface.from_clause().walk_ast(out.reborrow())?; + out.push_sql(" WHERE "); + out.push_identifier(dsl::id::NAME)?; + out.push_sql(" = "); + out.push_bind_param::( + &self.interface.identity.id, + )?; + out.push_sql(" AND "); + out.push_identifier(dsl::time_deleted::NAME)?; + out.push_sql(" IS NULL)"); + + // Push the main, data-validating subquery. + out.push_sql(", ("); + push_interface_allocation_subquery( + out.reborrow(), + &self.interface, + &self.now, + )?; + out.push_sql(")) AS candidate)"); + Ok(()) + } +} + +/// Type used to add the results of the `InsertNetworkInterfaceQuery` as values +/// in a Diesel statement, e.g., `insert_into(network_interface).values(query).` +/// Not for direct use. +pub struct InsertNetworkInterfaceQueryValues(InsertNetworkInterfaceQuery); + +impl QueryId for InsertNetworkInterfaceQueryValues { + type QueryId = (); + const HAS_STATIC_QUERY_ID: bool = false; +} + +impl diesel::insertable::CanInsertInSingleQuery + for InsertNetworkInterfaceQueryValues +{ + fn rows_to_insert(&self) -> Option { + Some(1) + } +} + +impl QueryFragment for InsertNetworkInterfaceQueryValues { + fn walk_ast(&self, mut out: AstPass) -> diesel::QueryResult<()> { + use db::schema::network_interface::dsl; + out.push_sql("("); + out.push_identifier(dsl::id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::name::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::description::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_created::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_modified::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::time_deleted::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::instance_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::vpc_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::subnet_id::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::mac::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::ip::NAME)?; + out.push_sql(") "); + self.0.walk_ast(out) + } +} + #[cfg(test)] mod test { - use super::AllocateIpQuery; use super::FilterConflictingVpcSubnetRangesQuery; + use super::NetworkInterfaceError; use super::SubnetError; use crate::db::model::{ - IncompleteNetworkInterface, NetworkInterface, VpcSubnet, + self, IncompleteNetworkInterface, NetworkInterface, VpcSubnet, }; - use crate::db::schema::network_interface; - use crate::external_api::params; - use chrono::{DateTime, NaiveDateTime, Utc}; use diesel::pg::Pg; - use diesel::prelude::*; use nexus_test_utils::db::test_setup_database; use omicron_common::api::external::{ - IdentityMetadataCreateParams, Ipv4Net, Ipv6Net, MacAddr, Name, + Error, IdentityMetadataCreateParams, Ipv4Net, Ipv6Net, Name, }; use omicron_test_utils::dev; use std::convert::TryInto; use std::sync::Arc; use uuid::Uuid; - #[test] - fn test_verify_query() { - let interface_id = - uuid::Uuid::parse_str("223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d0") - .unwrap(); - let instance_id = - uuid::Uuid::parse_str("223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d1") - .unwrap(); - let vpc_id = - uuid::Uuid::parse_str("223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d2") - .unwrap(); - let subnet_id = - uuid::Uuid::parse_str("223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d3") - .unwrap(); - let ipv4_block: ipnetwork::Ipv4Network = - "192.168.1.0/24".parse().unwrap(); - let ipv6_block: ipnetwork::Ipv6Network = "fd00::/48".parse().unwrap(); - let subnet = VpcSubnet::new( - subnet_id, - vpc_id, - IdentityMetadataCreateParams { - name: "test-subnet".to_string().try_into().unwrap(), - description: "subnet description".to_string(), - }, - Ipv4Net(ipv4_block.clone()).into(), - Ipv6Net(ipv6_block), - ); - let mac = - MacAddr(macaddr::MacAddr6::from([0xA8, 0x40, 0x25, 0x0, 0x0, 0x1])) - .into(); - let interface = IncompleteNetworkInterface::new( - interface_id, - instance_id, - vpc_id, - subnet, - mac, - None, - params::NetworkInterfaceCreate { - identity: IdentityMetadataCreateParams { - name: "test-iface".to_string().try_into().unwrap(), - description: "interface description".to_string(), - }, - }, - ); - let select = AllocateIpQuery { - interface, - block: ipv4_block.into(), - now: DateTime::::from_utc( - NaiveDateTime::from_timestamp(0, 0), - Utc, - ), - }; - let query = diesel::debug_query::(&select).to_string(); - - let expected_query = "SELECT \ - $1 AS \"id\", $2 AS \"name\", $3 AS \"description\", \ - $4 AS \"time_created\", $5 AS \"time_modified\", \ - $6 AS \"instance_id\", $7 AS \"vpc_id\", $8 AS \"subnet_id\", \ - $9 AS \"mac\", $10 + \"off\" AS \"ip\" \ - FROM generate_series(5, $11) AS \"off\" LEFT OUTER JOIN \ - \"network_interface\" ON \ - (\"subnet_id\", \"ip\", \"time_deleted\" IS NULL) = \ - ($12, $13 + \"off\", TRUE) \ - WHERE \"ip\" IS NULL LIMIT 1 -- \ - binds: [223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d0, \"test-iface\", \ - \"interface description\", 1970-01-01T00:00:00Z, \ - 1970-01-01T00:00:00Z, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d1, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d2, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d3, \"A8:40:25:00:00:01\", \ - V4(Ipv4Network { addr: 192.168.1.0, prefix: 32 }), 254, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d3, \ - V4(Ipv4Network { addr: 192.168.1.0, prefix: 32 })]"; - assert_eq!(query, expected_query); - - let insert = diesel::insert_into(network_interface::table) - .values(select) - .returning(NetworkInterface::as_returning()); - let query = diesel::debug_query::(&insert).to_string(); - let expected_query = "INSERT INTO \"network_interface\" \ - (\"id\", \"name\", \"description\", \"time_created\", \ - \"time_modified\", \"instance_id\", \"vpc_id\", \"subnet_id\", \ - \"mac\", \"ip\") \ - SELECT $1 AS \"id\", $2 AS \"name\", $3 AS \"description\", \ - $4 AS \"time_created\", $5 AS \"time_modified\", \ - $6 AS \"instance_id\", $7 AS \"vpc_id\", $8 AS \"subnet_id\", \ - $9 AS \"mac\", $10 + \"off\" AS \"ip\" \ - FROM generate_series(5, $11) AS \"off\" LEFT OUTER JOIN \ - \"network_interface\" ON \ - (\"subnet_id\", \"ip\", \"time_deleted\" IS NULL) = \ - ($12, $13 + \"off\", TRUE) \ - WHERE \"ip\" IS NULL LIMIT 1 \ - RETURNING \"network_interface\".\"id\", \ - \"network_interface\".\"name\", \ - \"network_interface\".\"description\", \ - \"network_interface\".\"time_created\", \ - \"network_interface\".\"time_modified\", \ - \"network_interface\".\"time_deleted\", \ - \"network_interface\".\"instance_id\", \ - \"network_interface\".\"vpc_id\", \ - \"network_interface\".\"subnet_id\", \ - \"network_interface\".\"mac\", \"network_interface\".\"ip\" -- \ - binds: [223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d0, \"test-iface\", \ - \"interface description\", 1970-01-01T00:00:00Z, \ - 1970-01-01T00:00:00Z, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d1, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d2, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d3, \"A8:40:25:00:00:01\", \ - V4(Ipv4Network { addr: 192.168.1.0, prefix: 32 }), 254, \ - 223cb7f7-0d3a-4a4e-a5e1-ad38ecb785d3, \ - V4(Ipv4Network { addr: 192.168.1.0, prefix: 32 })]"; - assert_eq!(query, expected_query); - } - #[test] fn test_filter_conflicting_vpc_subnet_ranges_query_string() { use crate::db::identity::Resource; @@ -802,4 +1246,347 @@ mod test { db.cleanup().await.unwrap(); logctx.cleanup_successful(); } + + #[tokio::test] + async fn test_insert_network_interface_query() { + // Setup the test database + let logctx = dev::test_setup_log("test_insert_network_interface_query"); + let log = logctx.log.new(o!()); + let mut db = test_setup_database(&log).await; + let cfg = crate::db::Config { url: db.pg_config().clone() }; + let pool = Arc::new(crate::db::Pool::new(&cfg)); + let db_datastore = + Arc::new(crate::db::DataStore::new(Arc::clone(&pool))); + + // Two test VpcSubnets, in different VPCs. The IPv4 range has space for + // 16 addresses, less the 6 that are reserved. + let ipv4_block = Ipv4Net("172.30.0.0/28".parse().unwrap()); + let ipv6_block = Ipv6Net("fd12:3456:7890::/64".parse().unwrap()); + let subnet_name = "subnet-a".to_string().try_into().unwrap(); + let other_subnet_name = "subnet-b".to_string().try_into().unwrap(); + let description = "some description".to_string(); + let vpc_id = "d402369d-c9ec-c5ad-9138-9fbee732d53e".parse().unwrap(); + let other_vpc_id = + "093ad2db-769b-e3c2-bc1c-b46e84ce5532".parse().unwrap(); + let subnet_id = "093ad2db-769b-e3c2-bc1c-b46e84ce5532".parse().unwrap(); + let other_subnet_id = + "695debcc-e197-447d-ffb2-976150a7b7cf".parse().unwrap(); + let subnet = VpcSubnet::new( + subnet_id, + vpc_id, + IdentityMetadataCreateParams { + name: subnet_name, + description: description.to_string(), + }, + ipv4_block, + ipv6_block, + ); + let other_subnet = VpcSubnet::new( + other_subnet_id, + other_vpc_id, + IdentityMetadataCreateParams { + name: other_subnet_name, + description: description.to_string(), + }, + ipv4_block, + ipv6_block, + ); + + // Insert a network interface with a known valid IP address, attached to + // a specific instance. + let instance_id = + "90d8542f-52dc-cacb-fa2b-ea0940d6bcb7".parse().unwrap(); + let requested_ip = "172.30.0.5".parse().unwrap(); + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-a".parse().unwrap(), + description: String::from("description"), + }, + Some(requested_ip), + ) + .unwrap(); + let inserted_interface = db_datastore + .instance_create_network_interface(interface.clone()) + .await + .expect("Failed to insert interface with known-good IP address"); + assert_interfaces_eq(&interface, &inserted_interface); + assert_eq!( + inserted_interface.ip.ip(), + requested_ip, + "The requested IP address should be available when no interfaces exist in the table" + ); + + // Insert an interface on the same instance, but with an + // automatically-assigned IP address. It should have the next address. + let expected_address = + "172.30.0.6".parse::().unwrap(); + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-b".parse().unwrap(), + description: String::from("description"), + }, + None, + ) + .unwrap(); + let inserted_interface = db_datastore + .instance_create_network_interface(interface.clone()) + .await + .expect("Failed to insert interface with known-good IP address"); + assert_interfaces_eq(&interface, &inserted_interface); + assert_eq!( + inserted_interface.ip.ip(), + expected_address, + "Failed to automatically assign the next available IP address" + ); + + // Inserting an interface with the same IP should fail. + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-c".parse().unwrap(), + description: String::from("description"), + }, + Some(requested_ip), + ) + .unwrap(); + let result = + db_datastore.instance_create_network_interface(interface).await; + assert!( + matches!( + result, + Err(NetworkInterfaceError::IpAddressNotAvailable(_)) + ), + "Requesting an interface with an existing IP should fail" + ); + + // Inserting an interface with a new IP but the same name should + // generate an invalid request error. + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-b".parse().unwrap(), + description: String::from("description"), + }, + None, + ) + .unwrap(); + let result = + db_datastore.instance_create_network_interface(interface).await; + assert!( + matches!( + result, + Err(NetworkInterfaceError::External(Error::ObjectAlreadyExists { .. })), + ), + "Requesting an interface with the same name on the same instance should fail" + ); + + // Inserting an interface that is attached to the same instance, but in a different VPC, + // should fail regardless of whether the IP is explicitly requested or allocated. + for addr in [Some(expected_address), None] { + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + other_vpc_id, + other_subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-a".parse().unwrap(), + description: String::from("description"), + }, + addr, + ) + .unwrap(); + let result = + db_datastore.instance_create_network_interface(interface).await; + assert!( + matches!(result, Err(NetworkInterfaceError::InstanceSpansMultipleVpcs(_))), + "Attaching an interface to an instance which already has one in a different VPC should fail" + ); + } + + // At this point, we should have allocated 2 addresses in this VPC + // Subnet. That has a subnet of 172.30.0.0/28, so 16 total addresses are + // available, but there are 6 reserved. Assert that we fail after + // allocating 16 - 6 - 2 = 8 more interfaces, with an address exhaustion error. + for i in 0..8 { + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: format!("interface-{}", i).try_into().unwrap(), + description: String::from("description"), + }, + None, + ) + .unwrap(); + let result = + db_datastore.instance_create_network_interface(interface).await; + assert!( + result.is_ok(), + "We should be able to allocate 8 more interfaces successfully", + ); + } + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-d".parse().unwrap(), + description: String::from("description"), + }, + None, + ) + .unwrap(); + let result = + db_datastore.instance_create_network_interface(interface).await; + assert!( + matches!( + result, + Err(NetworkInterfaceError::NoAvailableIpAddresses) + ), + "Address exhaustion should be detected and handled" + ); + + // We should _not_ fail to allocate two interfaces with the same name if + // they're in the same VPC and VPC Subnet, but on different instances. + // another instance. + for _ in 0..2 { + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + Uuid::new_v4(), // New instance ID + other_vpc_id, + other_subnet.clone(), + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-e".parse().unwrap(), // Same name + description: String::from("description"), + }, + None, + ) + .unwrap(); + let result = + db_datastore.instance_create_network_interface(interface).await; + assert!( + result.is_ok(), + concat!( + "Should be able to allocate multiple interfaces with the same name ", + "as long as they're attached to different instances", + ), + ); + } + + db.cleanup().await.unwrap(); + logctx.cleanup_successful(); + } + + // Test equality of a complete/inserted interface, for parts that are always known. + fn assert_interfaces_eq( + incomplete: &IncompleteNetworkInterface, + inserted: &NetworkInterface, + ) { + use crate::db::identity::Resource; + assert_eq!(inserted.id(), incomplete.identity.id); + assert_eq!(inserted.name(), &incomplete.identity.name); + assert_eq!(inserted.description(), incomplete.identity.description); + assert_eq!(inserted.instance_id, incomplete.instance_id); + assert_eq!(inserted.vpc_id, incomplete.vpc_id); + assert_eq!(inserted.subnet_id, incomplete.subnet.id()); + assert_eq!(inserted.mac, incomplete.mac); + } + + // Test that inserting a record into the database with the same primary key + // returns the exact same record. + // + // This is an explicit test for the first expression within the `COALESCE` + // part of the query. That is specifically designed to be executed during + // sagas. In that case, we construct the UUIDs of each interface in one + // action, and then in the next, create and insert each interface. + #[tokio::test] + async fn test_insert_network_interface_with_identical_primary_key() { + // Setup the test database + let logctx = dev::test_setup_log( + "test_insert_network_interface_with_identical_primary_key", + ); + let log = logctx.log.new(o!()); + let mut db = test_setup_database(&log).await; + let cfg = crate::db::Config { url: db.pg_config().clone() }; + let pool = Arc::new(crate::db::Pool::new(&cfg)); + let db_datastore = + Arc::new(crate::db::DataStore::new(Arc::clone(&pool))); + let ipv4_block = Ipv4Net("172.30.0.0/28".parse().unwrap()); + let ipv6_block = Ipv6Net("fd12:3456:7890::/64".parse().unwrap()); + let subnet_name = "subnet-a".to_string().try_into().unwrap(); + let description = "some description".to_string(); + let vpc_id = "d402369d-c9ec-c5ad-9138-9fbee732d53e".parse().unwrap(); + let subnet_id = "093ad2db-769b-e3c2-bc1c-b46e84ce5532".parse().unwrap(); + let subnet = VpcSubnet::new( + subnet_id, + vpc_id, + IdentityMetadataCreateParams { + name: subnet_name, + description: description.to_string(), + }, + ipv4_block, + ipv6_block, + ); + let instance_id = + "90d8542f-52dc-cacb-fa2b-ea0940d6bcb7".parse().unwrap(); + let requested_ip = "172.30.0.5".parse().unwrap(); + let interface = IncompleteNetworkInterface::new( + Uuid::new_v4(), + instance_id, + vpc_id, + subnet, + model::MacAddr::new().unwrap(), + IdentityMetadataCreateParams { + name: "interface-a".parse().unwrap(), + description: String::from("description"), + }, + Some(requested_ip), + ) + .unwrap(); + let inserted_interface = db_datastore + .instance_create_network_interface(interface.clone()) + .await + .expect("Failed to insert interface with known-good IP address"); + + // Attempt to insert the exact same record again. + let result = db_datastore + .instance_create_network_interface(interface.clone()) + .await; + if let Err(NetworkInterfaceError::DuplicatePrimaryKey(key)) = result { + assert_eq!(key, inserted_interface.identity.id); + } else { + panic!( + "Expected a NetworkInterfaceError::DuplicatePrimaryKey \ + error when inserting the exact same interface" + ); + } + + db.cleanup().await.unwrap(); + logctx.cleanup_successful(); + } } diff --git a/nexus/src/defaults.rs b/nexus/src/defaults.rs index 88a8a3166d6..d7c22abfc04 100644 --- a/nexus/src/defaults.rs +++ b/nexus/src/defaults.rs @@ -23,6 +23,9 @@ pub const MIN_VPC_IPV4_SUBNET_PREFIX: u8 = 8; /// NOTE: This is the maximum _prefix_, which sets the minimum subnet size. pub const MAX_VPC_IPV4_SUBNET_PREFIX: u8 = 26; +/// The number of reserved addresses at the beginning of a subnet range. +pub const NUM_INITIAL_RESERVED_IP_ADDRESSES: usize = 5; + lazy_static! { /// The default IPv4 subnet range assigned to the default VPC Subnet, when /// the VPC is created, if one is not provided in the request. See diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 369ace2a406..3a9ca9fed18 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -115,7 +115,12 @@ pub fn external_api() -> NexusApiDescription { api.register(vpc_subnets_delete_subnet)?; api.register(vpc_subnets_put_subnet)?; - api.register(subnets_ips_get)?; + api.register(subnet_network_interfaces_get)?; + + api.register(instance_network_interfaces_post)?; + api.register(instance_network_interfaces_get)?; + api.register(instance_network_interfaces_get_interface)?; + api.register(instance_network_interfaces_delete_interface)?; api.register(vpc_routers_get)?; api.register(vpc_routers_get_router)?; @@ -1111,6 +1116,151 @@ async fn instance_disks_detach( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +/// List network interfaces attached to this instance. +#[endpoint { + method = GET, + path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces", + tags = ["instances"], +}] +async fn instance_network_interfaces_get( + rqctx: Arc>>, + query_params: Query, + path_params: Path, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let query = query_params.into_inner(); + let path = path_params.into_inner(); + let organization_name = &path.organization_name; + let project_name = &path.project_name; + let instance_name = &path.instance_name; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let interfaces = nexus + .instance_list_network_interfaces( + &opctx, + &organization_name, + &project_name, + &instance_name, + &data_page_params_for(&rqctx, &query)? + .map_name(|n| Name::ref_cast(n)), + ) + .await? + .into_iter() + .map(|d| d.into()) + .collect(); + Ok(HttpResponseOk(ScanByName::results_page(&query, interfaces)?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Create a network interface for an instance. +#[endpoint { + method = POST, + path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces", + tags = ["instances"], +}] +async fn instance_network_interfaces_post( + rqctx: Arc>>, + path_params: Path, + interface_params: TypedBody, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let organization_name = &path.organization_name; + let project_name = &path.project_name; + let instance_name = &path.instance_name; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let iface = nexus + .instance_create_network_interface( + &opctx, + &organization_name, + &project_name, + &instance_name, + &interface_params.into_inner(), + ) + .await?; + Ok(HttpResponseCreated(iface.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +pub struct NetworkInterfacePathParam { + pub organization_name: Name, + pub project_name: Name, + pub instance_name: Name, + pub interface_name: Name, +} + +/// Detach a network interface from an instance. +#[endpoint { + method = DELETE, + path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name}", + tags = ["instances"], +}] +async fn instance_network_interfaces_delete_interface( + rqctx: Arc>>, + path_params: Path, +) -> Result { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let organization_name = &path.organization_name; + let project_name = &path.project_name; + let instance_name = &path.instance_name; + let interface_name = &path.interface_name; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + nexus + .instance_delete_network_interface( + &opctx, + organization_name, + project_name, + instance_name, + interface_name, + ) + .await?; + Ok(HttpResponseDeleted()) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Get an interface attached to an instance. +#[endpoint { + method = GET, + path = "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name}", + tags = ["instances"], +}] +async fn instance_network_interfaces_get_interface( + rqctx: Arc>>, + path_params: Path, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let organization_name = &path.organization_name; + let project_name = &path.project_name; + let instance_name = &path.instance_name; + let interface_name = &path.interface_name; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let interface = nexus + .instance_lookup_network_interface( + &opctx, + organization_name, + project_name, + instance_name, + interface_name, + ) + .await?; + Ok(HttpResponseOk(NetworkInterface::from(interface))) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /* * VPCs */ @@ -1456,18 +1606,13 @@ async fn vpc_subnets_put_subnet( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } -/** - * List IP addresses on a VPC subnet. - */ -// TODO-correctness: This API has not actually been specified in an RFD yet, and -// may not actually be what we want. It is being implemented here to give our -// testing introspection into network interfaces. +/// List network interfaces in a VPC subnet. #[endpoint { method = GET, - path = "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/ips", + path = "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/network-interfaces", tags = ["subnets"], }] -async fn subnets_ips_get( +async fn subnet_network_interfaces_get( rqctx: Arc>>, query_params: Query, path_params: Path, diff --git a/nexus/src/external_api/params.rs b/nexus/src/external_api/params.rs index 40f37ec63e4..6e9b48a7272 100644 --- a/nexus/src/external_api/params.rs +++ b/nexus/src/external_api/params.rs @@ -12,6 +12,7 @@ use omicron_common::api::external::{ }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; +use std::net::IpAddr; use uuid::Uuid; /* @@ -70,12 +71,60 @@ pub struct ProjectUpdate { pub struct NetworkInterfaceCreate { #[serde(flatten)] pub identity: IdentityMetadataCreateParams, + /// The VPC in which to create the interface. + pub vpc_name: Name, + /// The VPC Subnet in which to create the interface. + pub subnet_name: Name, + /// The IP address for the interface. One will be auto-assigned if not provided. + pub ip: Option, } /* * INSTANCES */ +/// 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 +// instances. It's true that all networking resources for an instance must +// belong to a single VPC, but we don't consider instances to be "scoped" to a +// VPC in the same way that they are scoped to projects, for example. +// +// This is slightly different than some other cloud providers, such as AWS, +// which use VPCs as both a networking concept, and a container more similar to +// our concept of a project. One example for why this is useful is that "moving" +// an instance to a new VPC can be done by detaching any interfaces in the +// original VPC and attaching interfaces in the new VPC. +// +// This type then requires the VPC identifiers, exactly because instances are +// _not_ scoped to a VPC, and so the VPC and/or VPC Subnet names are not present +// in the path of endpoints handling instance operations. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(tag = "type", content = "params")] +pub enum InstanceNetworkInterfaceAttachment { + /// Create one or more `NetworkInterface`s for the `Instance` + Create(InstanceNetworkInterfaceCreate), + + /// Default networking setup, which creates a single interface with an + /// auto-assigned IP address from project's "default" VPC and "default" VPC + /// Subnet. + Default, + + /// No network interfaces at all will be created for the instance. + None, +} + +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct InstanceNetworkInterfaceCreate { + pub params: Vec, +} + +impl Default for InstanceNetworkInterfaceAttachment { + fn default() -> Self { + Self::Default + } +} + /** * Create-time parameters for an [`Instance`](omicron_common::api::external::Instance) */ @@ -86,6 +135,10 @@ pub struct InstanceCreate { pub ncpus: InstanceCpuCount, pub memory: ByteCount, pub hostname: String, /* TODO-cleanup different type? */ + + /// The network interfaces to be created for this instance. + #[serde(default)] + pub network_interfaces: InstanceNetworkInterfaceAttachment, } /** @@ -158,8 +211,9 @@ pub struct VpcSubnetCreate { pub struct VpcSubnetUpdate { #[serde(flatten)] pub identity: IdentityMetadataUpdateParams, - // TODO-correctness: It seems fraught to allow changing these, since it can - // invalidate arbitrary sub-resources (e.g., network interfaces). + // TODO-correctness: These need to be removed. Changing these is effectively + // creating a new resource, so we should require explicit + // deletion/recreation by the client. pub ipv4_block: Option, pub ipv6_block: Option, } @@ -229,6 +283,14 @@ pub struct DiskIdentifier { pub disk: Name, } +/// Parameters for the +/// [`NetworkInterface`](omicron_common::api::external::NetworkInterface) to be +/// attached or detached to an instance. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct NetworkInterfaceIdentifier { + pub interface_name: Name, +} + /* * BUILT-IN USERS * diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 54bbb302e26..8f33b9635bc 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -14,6 +14,7 @@ use crate::db; use crate::db::identity::{Asset, Resource}; use crate::db::model::DatasetKind; use crate::db::model::Name; +use crate::db::subnet_allocation::NetworkInterfaceError; use crate::db::subnet_allocation::SubnetError; use crate::defaults; use crate::external_api::params; @@ -1541,6 +1542,160 @@ impl Nexus { .map(|_| ()) } + /// Lists network interfaces attached to the instance. + pub async fn instance_list_network_interfaces( + &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, + instance_name: &Name, + pagparams: &DataPageParams<'_, Name>, + ) -> ListResultVec { + let authz_instance = self + .db_datastore + .instance_lookup_by_path( + organization_name, + project_name, + instance_name, + ) + .await?; + opctx.authorize(authz::Action::ListChildren, &authz_instance).await?; + self.db_datastore + .instance_list_network_interfaces(&authz_instance.id(), pagparams) + .await + } + + /// Create a network interface attached to the provided instance. + // TODO-performance: Add a version of this that accepts the instance ID + // directly. This will avoid all the internal database lookups in the event + // that we create many NICs for the same instance, such as in a saga. + pub async fn instance_create_network_interface( + &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, + instance_name: &Name, + params: ¶ms::NetworkInterfaceCreate, + ) -> CreateResult { + let authz_project = self + .db_datastore + .project_lookup_by_path(organization_name, project_name) + .await?; + let (authz_instance, db_instance) = self + .db_datastore + .instance_fetch(opctx, &authz_project, instance_name) + .await?; + opctx.authorize(authz::Action::Modify, &authz_instance).await?; + + // TODO-completeness: We'd like to relax this once hot-plug is supported + let stopped = + db::model::InstanceState::new(external::InstanceState::Stopped); + if db_instance.runtime_state.state != stopped { + return Err(external::Error::invalid_request( + "Instance must be stopped to attach a new network interface", + )); + } + + // NOTE: We need to lookup the VPC and VPC Subnet, since we need both + // IDs for creating the network interface. + let vpc_name = db::model::Name(params.vpc_name.clone()); + let subnet_name = db::model::Name(params.subnet_name.clone()); + let vpc = self + .project_lookup_vpc(organization_name, project_name, &vpc_name) + .await?; + let subnet = self + .db_datastore + .vpc_subnet_fetch_by_name(&vpc.id(), &subnet_name) + .await?; + let mac = db::model::MacAddr::new()?; + let interface_id = Uuid::new_v4(); + let interface = db::model::IncompleteNetworkInterface::new( + interface_id, + authz_instance.id(), + vpc.id(), + subnet, + mac, + params.identity.clone(), + params.ip, + )?; + let interface = self + .db_datastore + .instance_create_network_interface(interface) + .await + .map_err(NetworkInterfaceError::into_external)?; + Ok(interface) + } + + /// Delete a network interface from the provided instance. + pub async fn instance_delete_network_interface( + &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, + instance_name: &Name, + interface_name: &Name, + ) -> DeleteResult { + let authz_project = self + .db_datastore + .project_lookup_by_path(organization_name, project_name) + .await?; + let (authz_instance, db_instance) = self + .db_datastore + .instance_fetch(opctx, &authz_project, instance_name) + .await?; + opctx.authorize(authz::Action::Modify, &authz_instance).await?; + + // TODO-completeness: We'd like to relax this once hot-plug is supported + let stopped = + db::model::InstanceState::new(external::InstanceState::Stopped); + if db_instance.runtime_state.state != stopped { + return Err(external::Error::invalid_request( + "Instance must be stopped to detach a network interface", + )); + } + // TODO-cleanup: It's annoying that we need to look up the interface by + // name, which gets a single record, and then soft-delete that one + // record. We should be able to do both at once, but the + // `update_and_check` tools only operate on the primary key. That means + // we need to fetch the whole record first. + let interface = self + .db_datastore + .instance_lookup_network_interface( + &authz_instance.id(), + interface_name, + ) + .await?; + self.db_datastore + .instance_delete_network_interface(&interface.id()) + .await + } + + /// Fetch a network interface attached to the given instance. + pub async fn instance_lookup_network_interface( + &self, + opctx: &OpContext, + organization_name: &Name, + project_name: &Name, + instance_name: &Name, + interface_name: &Name, + ) -> LookupResult { + let authz_instance = self + .db_datastore + .instance_lookup_by_path( + organization_name, + project_name, + instance_name, + ) + .await?; + opctx.authorize(authz::Action::ListChildren, &authz_instance).await?; + self.db_datastore + .instance_lookup_network_interface( + &authz_instance.id(), + interface_name, + ) + .await + } + pub async fn project_list_vpcs( &self, opctx: &OpContext, @@ -1826,7 +1981,6 @@ impl Nexus { vpc_name: &Name, subnet_name: &Name, ) -> LookupResult { - // TODO: join projects, vpcs, and subnets and do this in one query let vpc = self .project_lookup_vpc(organization_name, project_name, vpc_name) .await?; diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 4302857cbd4..ba97a8cc07b 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -37,6 +37,7 @@ use omicron_common::api::internal::sled_agent::InstanceHardware; use omicron_common::backoff::{self, BackoffError}; use serde::Deserialize; use serde::Serialize; +use slog::warn; use slog::Logger; use std::collections::BTreeMap; use std::convert::{TryFrom, TryInto}; @@ -149,14 +150,41 @@ pub fn saga_instance_create() -> SagaTemplate { new_action_noop_undo(sic_alloc_server), ); + // NOTE: The separation of the ID-allocation and NIC creation nodes is + // intentional. + // + // The Nexus API supports creating multiple network interfaces at the time + // an instance is provisioned. However, each NIC creation is independent, + // and each can fail. For example, someone might specify multiple NICs with + // the same IP address. The first will be created successfully, but the + // later ones will fail. We need to handle this case gracefully, and always + // delete any NICs we create, even if the NIC creation node itself fails. + // + // To do that, we create an action that only allocates the UUIDs for each + // interface. This has an undo action that actually deletes any NICs for the + // instance to be provisioned. The forward action is infallible, so this + // undo action will always run, even (and especially) if the NIC creation + // action fails. + // + // It's also important that we allocate the UUIDs first. It's possible that + // we crash partway through the NIC creation action. In this case, the saga + // recovery machinery will pick it up where it left off, without first + // destroying the NICs we created before crashing. By allocating the UUIDs + // first, we can make the insertion idempotent, by ignoring conflicts on the + // UUID. template_builder.append( - "network_interface", - "CreateNetworkInterface", + "network_interface_ids", + "NetworkInterfaceIds", ActionFunc::new_action( - sic_create_network_interface, - sic_create_network_interface_undo, + sic_allocate_network_interface_ids, + sic_create_network_interfaces_undo, ), ); + template_builder.append( + "network_interfaces", + "CreateNetworkInterfaces", + new_action_noop_undo(sic_create_network_interfaces), + ); template_builder.append( "initial_runtime", @@ -184,72 +212,235 @@ async fn sic_alloc_server( .map_err(ActionError::action_failed) } -async fn sic_create_network_interface( +async fn sic_allocate_network_interface_ids( + sagactx: ActionContext, +) -> Result, ActionError> { + match sagactx.saga_params().create_params.network_interfaces { + params::InstanceNetworkInterfaceAttachment::None => Ok(vec![]), + params::InstanceNetworkInterfaceAttachment::Default => { + Ok(vec![Uuid::new_v4()]) + } + params::InstanceNetworkInterfaceAttachment::Create( + ref create_params, + ) => { + let mut ids = Vec::with_capacity(create_params.params.len()); + for _ in 0..create_params.params.len() { + ids.push(Uuid::new_v4()); + } + Ok(ids) + } + } +} + +async fn sic_create_network_interfaces( sagactx: ActionContext, -) -> Result { +) -> Result>, ActionError> { + match sagactx.saga_params().create_params.network_interfaces { + params::InstanceNetworkInterfaceAttachment::None => Ok(None), + params::InstanceNetworkInterfaceAttachment::Default => { + sic_create_default_network_interface(&sagactx).await + } + params::InstanceNetworkInterfaceAttachment::Create( + ref create_params, + ) => { + sic_create_custom_network_interfaces( + &sagactx, + &create_params.params, + ) + .await + } + } +} + +/// Create one or more custom (non-default) network interfaces for the provided +/// instance. +async fn sic_create_custom_network_interfaces( + sagactx: &ActionContext, + interface_params: &[params::NetworkInterfaceCreate], +) -> Result>, ActionError> { + if interface_params.is_empty() { + return Ok(Some(vec![])); + } + let osagactx = sagactx.user_data(); - let params = sagactx.saga_params(); + let saga_params = sagactx.saga_params(); let instance_id = sagactx.lookup::("instance_id")?; + let ids = sagactx.lookup::>("network_interface_ids")?; + let vpc = osagactx + .datastore() + .vpc_fetch_by_name( + &saga_params.project_id, + &db::model::Name::from(interface_params[0].vpc_name.clone()), + ) + .await + .map_err(ActionError::action_failed)?; - let default_name = - db::model::Name(Name::try_from("default".to_string()).unwrap()); + // Check that all VPC names are the same. + // + // This isn't strictly necessary, as the queries would fail below, but it's + // easier to handle here. + if interface_params.iter().any(|p| p.vpc_name != vpc.name().0) { + return Err(ActionError::action_failed(Error::invalid_request( + "All interfaces must be in the same VPC", + ))); + } + + let mut interfaces = Vec::with_capacity(interface_params.len()); + if ids.len() != interface_params.len() { + return Err(ActionError::action_failed(Error::internal_error( + "found differing number of network interface IDs and interface parameters" + ))); + } + for (interface_id, params) in ids.into_iter().zip(interface_params.iter()) { + // TODO-correctness: It seems racy to fetch the subnet and create the + // interface in separate requests, but outside of a transaction. This + // should probably either be in a transaction, or the + // `subnet_create_network_interface` function/query needs some JOIN + // on the `vpc_subnet` table. + let subnet = osagactx + .datastore() + .vpc_subnet_fetch_by_name( + &vpc.id(), + &db::model::Name::from(params.subnet_name.clone()), + ) + .await + .map_err(ActionError::action_failed)?; + let mac = + db::model::MacAddr::new().map_err(ActionError::action_failed)?; + let interface = db::model::IncompleteNetworkInterface::new( + interface_id, + instance_id, + vpc.id(), + subnet.clone(), + mac, + params.identity.clone(), + params.ip, + ) + .map_err(ActionError::action_failed)?; + let result = osagactx + .datastore() + .instance_create_network_interface(interface) + .await; + + use crate::db::subnet_allocation::NetworkInterfaceError; + let interface = match result { + Ok(interface) => Ok(interface), + + // Detect the specific error arising from this node being partially + // completed. + // + // The query used to insert network interfaces first checks for an + // existing record with the same primary key. It will attempt to + // insert that record if it exists, which obviously fails with a + // primary key violation. (If the record does _not_ exist, one will + // be inserted as usual, see + // `db::subnet_name::InsertNetworkInterfaceQuery` for details). + // + // In this one specific case, we're asserting that any primary key + // duplicate arises because this saga node ran partway and then + // crashed. The saga recovery machinery will replay just this node, + // without first unwinding it, so any previously-inserted interfaces + // will still exist. This is expected. + Err(NetworkInterfaceError::DuplicatePrimaryKey(_)) => { + // TODO-observability: We should bump a counter here. + let log = osagactx.log(); + warn!( + log, + "Detected duplicate primary key during saga to \ + create network interfaces for instance '{}'. \ + This likely occurred because \ + the saga action 'sic_create_custom_network_interfaces' crashed \ + and has been recovered.", + instance_id; + "primary_key" => interface_id.to_string(), + ); + + // Refetch the interface itself, to serialize it for the next + // saga node. + osagactx + .datastore() + .instance_lookup_network_interface(&instance_id, &db::model::Name(params.identity.name.clone())) + .await + } + Err(e) => Err(e.into_external()), + } + .map_err(ActionError::action_failed)?; + interfaces.push(NetworkInterface::from(interface)) + } + Ok(Some(interfaces)) +} + +/// Create the default network interface for an instance during the create saga +async fn sic_create_default_network_interface( + sagactx: &ActionContext, +) -> Result>, ActionError> { + let osagactx = sagactx.user_data(); + let saga_params = sagactx.saga_params(); + let instance_id = sagactx.lookup::("instance_id")?; + let default_name = Name::try_from("default".to_string()).unwrap(); + let internal_default_name = db::model::Name::from(default_name.clone()); + let interface_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: default_name.clone(), + description: format!( + "default interface for {}", + saga_params.create_params.identity.name, + ), + }, + vpc_name: default_name.clone(), + subnet_name: default_name.clone(), + ip: None, // Request an IP address allocation + }; let vpc = osagactx .datastore() - .vpc_fetch_by_name(¶ms.project_id, &default_name) + .vpc_fetch_by_name( + &saga_params.project_id, + &internal_default_name.clone(), + ) .await .map_err(ActionError::action_failed)?; let subnet = osagactx .datastore() - .vpc_subnet_fetch_by_name(&vpc.id(), &default_name) + .vpc_subnet_fetch_by_name(&vpc.id(), &internal_default_name) .await .map_err(ActionError::action_failed)?; let mac = db::model::MacAddr::new().map_err(ActionError::action_failed)?; let interface_id = Uuid::new_v4(); - // Request an allocation - let ip = None; let interface = db::model::IncompleteNetworkInterface::new( interface_id, instance_id, - // TODO-correctness: vpc_id here is used for name uniqueness. Should - // interface names be unique to the subnet's VPC or to the - // VPC associated with the instance's default interface? vpc.id(), - subnet, + subnet.clone(), mac, - ip, - params::NetworkInterfaceCreate { - identity: IdentityMetadataCreateParams { - // By naming the interface after the instance id, we should - // avoid name conflicts on creation. - name: format!("default-{}", instance_id).parse().unwrap(), - description: format!( - "default interface for {}", - params.create_params.identity.name - ), - }, - }, - ); - + interface_params.identity.clone(), + interface_params.ip, + ) + .map_err(ActionError::action_failed)?; let interface = osagactx .datastore() .instance_create_network_interface(interface) .await + .map_err(db::subnet_allocation::NetworkInterfaceError::into_external) .map_err(ActionError::action_failed)?; - Ok(interface.into()) + Ok(Some(vec![interface.into()])) } -async fn sic_create_network_interface_undo( +async fn sic_create_network_interfaces_undo( sagactx: ActionContext, ) -> Result<(), anyhow::Error> { + // We issue a request to delete any interfaces associated with this instance. + // In the case we failed partway through allocating interfaces, we won't + // have cached the interface records in the saga log, but they're definitely + // still in the database. Just delete every interface that exists, even if + // there are zero such records. let osagactx = sagactx.user_data(); - let network_interface = - sagactx.lookup::("network_interface")?; - + let instance_id = sagactx.lookup::("instance_id")?; osagactx .datastore() - .instance_delete_network_interface(&network_interface.identity.id) - .await?; + .instance_delete_all_network_interfaces(&instance_id) + .await + .map_err(ActionError::action_failed)?; Ok(()) } @@ -261,8 +452,9 @@ async fn sic_create_instance_record( let sled_uuid = sagactx.lookup::("server_id"); let instance_id = sagactx.lookup::("instance_id"); let propolis_uuid = sagactx.lookup::("propolis_id"); - let network_interface = - sagactx.lookup::("network_interface")?; + let nics = sagactx + .lookup::>>("network_interfaces")? + .unwrap_or_default(); let runtime = InstanceRuntimeState { run_state: InstanceState::Creating, @@ -292,10 +484,7 @@ async fn sic_create_instance_record( .map_err(ActionError::action_failed)?; // See also: instance_set_runtime in nexus.rs for a similar construction. - Ok(InstanceHardware { - runtime: instance.runtime().clone().into(), - nics: vec![network_interface], - }) + Ok(InstanceHardware { runtime: instance.runtime().clone().into(), nics }) } async fn sic_instance_ensure( diff --git a/nexus/test-utils/src/http_testing.rs b/nexus/test-utils/src/http_testing.rs index 91bcb544556..210ea2db71e 100644 --- a/nexus/test-utils/src/http_testing.rs +++ b/nexus/test-utils/src/http_testing.rs @@ -18,7 +18,7 @@ use std::fmt::Debug; /// properties of the response for testing // // When testing an HTTP server, we make varying requests to the server and -// verify a bunch of properties about it's behavior. A lot of things can go +// verify a bunch of properties about its behavior. A lot of things can go // wrong along the way: // // - failed to serialize request body @@ -380,6 +380,7 @@ where } /// Represents a response from an HTTP server +#[derive(Debug)] pub struct TestResponse { pub status: http::StatusCode, pub headers: http::HeaderMap, diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index 66edea81d78..60301bdd5db 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -179,6 +179,7 @@ pub async fn start_sled_agent( nexus_address, dropshot: ConfigDropshot { bind_address: SocketAddr::new("127.0.0.1".parse().unwrap(), 0), + request_body_max_bytes: 2048, ..Default::default() }, /* TODO-cleanup this is unused */ diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 637b49e809e..a7c6f7e3b26 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -141,6 +141,8 @@ pub async fn create_instance( ncpus: InstanceCpuCount(4), memory: ByteCount::from_mebibytes_u32(256), hostname: String::from("the_host"), + network_interfaces: + params::InstanceNetworkInterfaceAttachment::Default, }, ) .await diff --git a/nexus/tests/config.test.toml b/nexus/tests/config.test.toml index d6dd93e1a6f..4ded6ef38a4 100644 --- a/nexus/tests/config.test.toml +++ b/nexus/tests/config.test.toml @@ -31,10 +31,12 @@ url = "postgresql://root@127.0.0.1:0/omicron?sslmode=disable" # [dropshot_external] bind_address = "127.0.0.1:0" +request_body_max_bytes = 2048 # port must be 0. see above [dropshot_internal] bind_address = "127.0.0.1:0" +request_body_max_bytes = 2048 # # NOTE: for the test suite, if mode = "file", the file path MUST be the sentinel diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index e5baf5c81e6..e694abc63a8 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -11,15 +11,18 @@ use http::StatusCode; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; +use omicron_common::api::external::ByteCount; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; use omicron_common::api::external::InstanceState; +use omicron_common::api::external::Ipv4Net; use omicron_common::api::external::Name; use omicron_common::api::external::NetworkInterface; use omicron_nexus::TestInterfaces as _; use omicron_nexus::{external_api::params, Nexus}; use sled_agent_client::TestInterfaces as _; +use std::convert::TryFrom; use std::sync::Arc; use uuid::Uuid; @@ -141,6 +144,8 @@ async fn test_instances_create_reboot_halt( ncpus: instance.ncpus, memory: instance.memory, hostname: instance.hostname.clone(), + network_interfaces: + params::InstanceNetworkInterfaceAttachment::Default, })) .expect_status(Some(StatusCode::BAD_REQUEST)), ) @@ -164,17 +169,14 @@ async fn test_instances_create_reboot_halt( /* Check that the instance got a network interface */ let ips_url = format!( - "/organizations/{}/projects/{}/vpcs/default/subnets/default/ips", + "/organizations/{}/projects/{}/vpcs/default/subnets/default/network-interfaces", ORGANIZATION_NAME, PROJECT_NAME ); let network_interfaces = objects_list_page::(client, &ips_url).await.items; assert_eq!(network_interfaces.len(), 1); assert_eq!(network_interfaces[0].instance_id, instance.identity.id); - assert_eq!( - network_interfaces[0].identity.name, - format!("default-{}", instance.identity.id).parse::().unwrap() - ); + assert_eq!(network_interfaces[0].identity.name, "default"); /* * Now, simulate completion of instance boot and check the state reported. @@ -530,6 +532,467 @@ async fn test_instances_invalid_creation_returns_bad_request( .starts_with("unable to parse body: invalid value: integer `-3`")); } +// Basic test requesting an interface with a specific IP address. +#[nexus_test] +async fn test_instance_with_single_explicit_ip_address( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + // Create test organization and project + create_organization(&client, ORGANIZATION_NAME).await; + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + ORGANIZATION_NAME, PROJECT_NAME + ); + let _ = create_project(&client, ORGANIZATION_NAME, PROJECT_NAME).await; + + // Create the parameters for the interface. + let default_name = "default".parse::().unwrap(); + let requested_address = "172.30.0.10".parse::().unwrap(); + let if0_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("if0")).unwrap(), + description: String::from("first custom interface"), + }, + vpc_name: default_name.clone(), + subnet_name: default_name.clone(), + ip: Some(requested_address), + }; + let interface_params = params::InstanceNetworkInterfaceAttachment::Create( + params::InstanceNetworkInterfaceCreate { + params: vec![if0_params.clone()], + }, + ); + + // Create the parameters for the instance itself, and create it. + let instance_params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("nic-test-inst")).unwrap(), + description: String::from("instance to test multiple nics"), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_mebibytes_u32(4), + hostname: String::from("nic-test"), + network_interfaces: interface_params, + }; + let response = + NexusRequest::objects_post(client, &url_instances, &instance_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to create instance with two network interfaces"); + let instance = response.parsed_body::().unwrap(); + + // Get the interface, and verify it has the requested address + let url_interface = format!( + "{}/{}/network-interfaces/{}", + url_instances, instance_params.identity.name, if0_params.identity.name, + ); + let interface = NexusRequest::object_get(client, &url_interface) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to get network interface for new instance") + .parsed_body::() + .expect("Failed to parse a network interface"); + assert_eq!(interface.instance_id, instance.identity.id); + assert_eq!(interface.identity.name, if0_params.identity.name); + assert_eq!( + interface.ip, requested_address, + "Interface was not assigned the requested IP address" + ); +} + +// Test creating two new interfaces for an instance, at creation time. +#[nexus_test] +async fn test_instance_with_new_custom_network_interfaces( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + // Create test organization and project + create_organization(&client, ORGANIZATION_NAME).await; + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + ORGANIZATION_NAME, PROJECT_NAME + ); + let _ = create_project(&client, ORGANIZATION_NAME, PROJECT_NAME).await; + + // Create a VPC Subnet other than the default. + // + // We'll create one interface in the default VPC Subnet and one in this new + // VPC Subnet. + let url_vpc_subnets = format!( + "/organizations/{}/projects/{}/vpcs/{}/subnets", + ORGANIZATION_NAME, PROJECT_NAME, "default", + ); + let default_name = Name::try_from(String::from("default")).unwrap(); + let non_default_subnet_name = + Name::try_from(String::from("non-default-subnet")).unwrap(); + let vpc_subnet_params = params::VpcSubnetCreate { + identity: IdentityMetadataCreateParams { + name: non_default_subnet_name.clone(), + description: String::from("A non-default subnet"), + }, + ipv4_block: Ipv4Net("172.31.0.0/24".parse().unwrap()), + ipv6_block: None, + }; + let _response = NexusRequest::objects_post( + client, + &url_vpc_subnets, + &vpc_subnet_params, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to create custom VPC Subnet"); + + // TODO-testing: We'd like to assert things about this VPC Subnet we just + // created, but the `vpc_subnets_post` endpoint in Nexus currently returns + // the "private" `omicron_nexus::db::model::VpcSubnet` type. That should be + // converted to return the public `omicron_common::external` type, which is + // work tracked in https://github.com/oxidecomputer/omicron/issues/388. + + // Create the parameters for the interfaces. These will be created during + // the saga for instance creation. + let if0_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("if0")).unwrap(), + description: String::from("first custom interface"), + }, + vpc_name: default_name.clone(), + subnet_name: default_name.clone(), + ip: None, + }; + let if1_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("if1")).unwrap(), + description: String::from("second custom interface"), + }, + vpc_name: default_name.clone(), + subnet_name: non_default_subnet_name.clone(), + ip: None, + }; + let interface_params = params::InstanceNetworkInterfaceAttachment::Create( + params::InstanceNetworkInterfaceCreate { + params: vec![if0_params.clone(), if1_params.clone()], + }, + ); + + // Create the parameters for the instance itself, and create it. + let instance_params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("nic-test-inst")).unwrap(), + description: String::from("instance to test multiple nics"), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_mebibytes_u32(4), + hostname: String::from("nic-test"), + network_interfaces: interface_params, + }; + let response = + NexusRequest::objects_post(client, &url_instances, &instance_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to create instance with two network interfaces"); + let instance = response.parsed_body::().unwrap(); + + // Check that both interfaces actually appear correct. + let ip_url = |subnet_name: &Name| { + format!( + "/organizations/{}/projects/{}/vpcs/{}/subnets/{}/network-interfaces", + ORGANIZATION_NAME, PROJECT_NAME, "default", subnet_name + ) + }; + + // The first interface is in the default VPC Subnet + let interfaces = NexusRequest::iter_collection_authn::( + client, + ip_url(&default_name).as_str(), + "", + Some(100), + ) + .await + .expect("Failed to get interfaces in default VPC Subnet"); + assert_eq!( + interfaces.all_items.len(), + 1, + "Should be a single interface in the default subnet" + ); + let if0 = &interfaces.all_items[0]; + assert_eq!(if0.identity.name, if0_params.identity.name); + assert_eq!(if0.identity.description, if0_params.identity.description); + assert_eq!(if0.instance_id, instance.identity.id); + assert_eq!(if0.ip, std::net::IpAddr::V4("172.30.0.5".parse().unwrap())); + + let interfaces1 = NexusRequest::iter_collection_authn::( + client, + ip_url(&non_default_subnet_name).as_str(), + "", + Some(100), + ) + .await + .expect("Failed to get interfaces in non-default VPC Subnet"); + assert_eq!( + interfaces1.all_items.len(), + 1, + "Should be a single interface in the non-default subnet" + ); + let if1 = &interfaces1.all_items[0]; + + // TODO-testing: Add this test once the `VpcSubnet` type can be + // deserialized. + // assert_eq!(if1.subnet_id, non_default_vpc_subnet.id); + + assert_eq!(if1.identity.name, if1_params.identity.name); + assert_eq!(if1.identity.description, if1_params.identity.description); + assert_eq!(if1.ip, std::net::IpAddr::V4("172.31.0.5".parse().unwrap())); + assert_eq!(if1.instance_id, instance.identity.id); + assert_eq!(if0.vpc_id, if1.vpc_id); + assert_ne!( + if0.subnet_id, if1.subnet_id, + "Two interfaces should be created in different subnets" + ); +} + +#[nexus_test] +async fn test_instance_create_delete_network_interface( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + + // Create test organization and project + create_organization(&client, ORGANIZATION_NAME).await; + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + ORGANIZATION_NAME, PROJECT_NAME + ); + let _ = create_project(&client, ORGANIZATION_NAME, PROJECT_NAME).await; + + // Create an instance with no network interfaces + let instance_params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("nic-attach-test-inst")).unwrap(), + description: String::from("instance to test attaching new nic"), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_mebibytes_u32(4), + hostname: String::from("nic-test"), + network_interfaces: params::InstanceNetworkInterfaceAttachment::None, + }; + let response = + NexusRequest::objects_post(client, &url_instances, &instance_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to create instance with two network interfaces"); + let instance = response.parsed_body::().unwrap(); + let url_instance = + format!("{}/{}", url_instances, instance.identity.name.as_str()); + + // Verify there are no interfaces + let url_interfaces = format!( + "/organizations/{}/projects/{}/instances/{}/network-interfaces", + ORGANIZATION_NAME, PROJECT_NAME, instance.identity.name, + ); + let interfaces = NexusRequest::iter_collection_authn::( + client, + url_interfaces.as_str(), + "", + Some(100), + ) + .await + .expect("Failed to get interfaces for instance"); + assert!( + interfaces.all_items.is_empty(), + "Expected no network interfaces for instance" + ); + + // Parameters for the interface to create/attach + let if_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: "if0".parse().unwrap(), + description: String::from("a new nic"), + }, + vpc_name: "default".parse().unwrap(), + subnet_name: "default".parse().unwrap(), + ip: Some("172.30.0.10".parse().unwrap()), + }; + let url_interface = + format!("{}/{}", url_interfaces, if_params.identity.name.as_str()); + + // We should not be able to create an interface while the instance is running. + // + // NOTE: Need to use RequestBuilder manually because `expect_failure` does not allow setting + // the body. + let builder = RequestBuilder::new( + client, + http::Method::POST, + url_interfaces.as_str(), + ) + .body(Some(&if_params)) + .expect_status(Some(http::StatusCode::BAD_REQUEST)); + let err = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Should not be able to create network interface on running instance") + .parsed_body::() + .expect("Failed to parse error response body"); + assert_eq!( + err.message, + "Instance must be stopped to attach a new network interface", + "Expected an InvalidRequest response when creating an interface on a running instance" + ); + + // Stop the instance + let instance = + instance_post(client, url_instance.as_str(), InstanceOp::Stop).await; + instance_simulate(nexus, &instance.identity.id).await; + + // Verify we can now make the request again + let response = + NexusRequest::objects_post(client, url_interfaces.as_str(), &if_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to create network interface on stopped instance"); + let iface = response.parsed_body::().unwrap(); + assert_eq!(iface.identity.name, if_params.identity.name); + assert_eq!(iface.ip, if_params.ip.unwrap()); + + // Restart the instance, verify the interface is still correct. + let instance = + instance_post(client, url_instance.as_str(), InstanceOp::Start).await; + instance_simulate(nexus, &instance.identity.id).await; + + let iface0 = NexusRequest::object_get(client, url_interface.as_str()) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to get interface") + .parsed_body::() + .expect("Failed to parse network interface from body"); + assert_eq!(iface.identity.id, iface0.identity.id); + assert_eq!(iface.ip, iface0.ip); + + // Verify we cannot delete the interface while the instance is running + let err = NexusRequest::expect_failure( + client, + http::StatusCode::BAD_REQUEST, + http::Method::DELETE, + url_interface.as_str(), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect( + "Should not be able to delete network interface on running instance", + ) + .parsed_body::() + .expect("Failed to parse error response body"); + assert_eq!( + err.message, + "Instance must be stopped to detach a network interface", + "Expected an InvalidRequest response when detaching an interface from a running instance" + ); + + // Stop the instance and verify we can delete the interface + let instance = + instance_post(client, url_instance.as_str(), InstanceOp::Stop).await; + instance_simulate(nexus, &instance.identity.id).await; + NexusRequest::object_delete(client, url_interface.as_str()) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to delete interface from stopped instance"); +} + +/// This test specifically creates two NICs, the second of which will fail the +/// saga on purpose, since its IP address is the same. This is to verify that +/// the initial NIC is also deleted. +#[nexus_test] +async fn test_instance_with_multiple_nics_unwinds_completely( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + /* Create a project that we'll use for testing. */ + create_organization(&client, ORGANIZATION_NAME).await; + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + ORGANIZATION_NAME, PROJECT_NAME + ); + let _ = create_project(&client, ORGANIZATION_NAME, PROJECT_NAME).await; + + // Create two interfaces, with the same IP addresses. + let default_name = "default".parse::().unwrap(); + let if0_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("if0")).unwrap(), + description: String::from("first custom interface"), + }, + vpc_name: default_name.clone(), + subnet_name: default_name.clone(), + ip: Some("172.30.0.6".parse().unwrap()), + }; + let if1_params = params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("if1")).unwrap(), + description: String::from("second custom interface"), + }, + vpc_name: default_name.clone(), + subnet_name: default_name.clone(), + ip: Some("172.30.0.6".parse().unwrap()), + }; + let interface_params = params::InstanceNetworkInterfaceAttachment::Create( + params::InstanceNetworkInterfaceCreate { + params: vec![if0_params.clone(), if1_params.clone()], + }, + ); + + // Create the parameters for the instance itself, and create it. + let instance_params = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: Name::try_from(String::from("nic-fail-test-inst")).unwrap(), + description: String::from("instance to test multiple bad nics"), + }, + ncpus: InstanceCpuCount::try_from(2).unwrap(), + memory: ByteCount::from_mebibytes_u32(4), + hostname: String::from("nic-test"), + network_interfaces: interface_params, + }; + let builder = + RequestBuilder::new(client, http::Method::POST, &url_instances) + .body(Some(&instance_params)) + .expect_status(Some(http::StatusCode::BAD_REQUEST)); + let response = NexusRequest::new(builder) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Expected instance saga to fail"); + assert_eq!(response.status, http::StatusCode::BAD_REQUEST); + + // Verify that there are no NICs at all in the subnet. + let url_nics = format!( + "/organizations/{}/projects/{}/vpcs/{}/subnets/{}/network-interfaces", + ORGANIZATION_NAME, PROJECT_NAME, "default", "default" + ); + let interfaces = NexusRequest::iter_collection_authn::( + client, &url_nics, "", None, + ) + .await + .expect("failed to list network interfaces") + .all_items; + assert!( + interfaces.is_empty(), + "There should be no network interfaces in the subnet" + ); +} + async fn instance_get( client: &ClientTestContext, instance_url: &str, diff --git a/nexus/tests/integration_tests/subnet_allocation.rs b/nexus/tests/integration_tests/subnet_allocation.rs index a23cba39df4..7aaa55b9387 100644 --- a/nexus/tests/integration_tests/subnet_allocation.rs +++ b/nexus/tests/integration_tests/subnet_allocation.rs @@ -41,6 +41,7 @@ async fn create_instance_expect_failure( ncpus: InstanceCpuCount(1), memory: ByteCount::from_mebibytes_u32(256), hostname: name.to_string(), + network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, }; NexusRequest::new( @@ -104,10 +105,10 @@ async fn test_subnet_allocation(cptestctx: &ControlPlaneTestContext) { // This should fail from address exhaustion let error = create_instance_expect_failure(client, &url_instances, "i3").await; - assert_eq!(error.message, "no available IP addresses"); + assert_eq!(error.message, "No available IP addresses for interface"); // Verify the subnet lists the two addresses as in use - let url_ips = format!("{}/ips", url_subnet); + let url_ips = format!("{}/network-interfaces", url_subnet); let mut network_interfaces = objects_list_page::(client, &url_ips).await.items; assert_eq!(network_interfaces.len(), 2); diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 528eb2f9aa1..894706acfbc 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -211,6 +211,7 @@ lazy_static! { ncpus: InstanceCpuCount(1), memory: ByteCount::from_gibibytes_u32(16), hostname: String::from("demo-instance"), + network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, }; } diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index e7f4fa0fe11..2eae202df1c 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -21,6 +21,10 @@ OPERATION ID URL PATH instance_disks_attach /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/attach instance_disks_detach /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks/detach instance_disks_get /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/disks +instance_network_interfaces_delete_interface /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name} +instance_network_interfaces_get /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces +instance_network_interfaces_get_interface /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name} +instance_network_interfaces_post /organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces project_instances_delete_instance /organizations/{organization_name}/projects/{project_name}/instances/{instance_name} project_instances_get /organizations/{organization_name}/projects/{project_name}/instances project_instances_get_instance /organizations/{organization_name}/projects/{project_name}/instances/{instance_name} @@ -88,7 +92,7 @@ hardware_sleds_get_sled /hardware/sleds/{sled_id} API operations found with tag "subnets" OPERATION ID URL PATH -subnets_ips_get /organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/ips +subnet_network_interfaces_get /organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/network-interfaces vpc_subnets_delete_subnet /organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name} vpc_subnets_get /organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets vpc_subnets_get_subnet /organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name} diff --git a/openapi/nexus.json b/openapi/nexus.json index 7f792a37277..b5aba3bab21 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1483,6 +1483,278 @@ } } }, + "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces": { + "get": { + "tags": [ + "instances" + ], + "summary": "List network interfaces attached to this instance.", + "operationId": "instance_network_interfaces_get", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + }, + "style": "form" + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retreive the subsequent page", + "schema": { + "nullable": true, + "type": "string" + }, + "style": "form" + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameSortMode" + }, + "style": "form" + }, + { + "in": "path", + "name": "instance_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "organization_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "project_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NetworkInterfaceResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + }, + "post": { + "tags": [ + "instances" + ], + "summary": "Create a network interface for an instance.", + "operationId": "instance_network_interfaces_post", + "parameters": [ + { + "in": "path", + "name": "instance_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "organization_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "project_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NetworkInterfaceCreate" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NetworkInterface" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/network-interfaces/{interface_name}": { + "get": { + "tags": [ + "instances" + ], + "summary": "Get an interface attached to an instance.", + "operationId": "instance_network_interfaces_get_interface", + "parameters": [ + { + "in": "path", + "name": "instance_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "interface_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "organization_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "project_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NetworkInterface" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "delete": { + "tags": [ + "instances" + ], + "summary": "Detach a network interface from an instance.", + "operationId": "instance_network_interfaces_delete_interface", + "parameters": [ + { + "in": "path", + "name": "instance_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "interface_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "organization_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "project_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/organizations/{organization_name}/projects/{project_name}/instances/{instance_name}/reboot": { "post": { "tags": [ @@ -3134,13 +3406,13 @@ } } }, - "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/ips": { + "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/network-interfaces": { "get": { "tags": [ "subnets" ], - "summary": "List IP addresses on a VPC subnet.", - "operationId": "subnets_ips_get", + "summary": "List network interfaces in a VPC subnet.", + "operationId": "subnet_network_interfaces_get", "parameters": [ { "in": "query", @@ -4065,6 +4337,14 @@ }, "ncpus": { "$ref": "#/components/schemas/InstanceCpuCount" + }, + "network_interfaces": { + "description": "The network interfaces to be created for this instance.", + "allOf": [ + { + "$ref": "#/components/schemas/InstanceNetworkInterfaceAttachment" + } + ] } }, "required": [ @@ -4088,6 +4368,74 @@ "dst_sled_uuid" ] }, + "InstanceNetworkInterfaceAttachment": { + "description": "Describes an attachment of a `NetworkInterface` to an `Instance`, at the time the instance is created.", + "oneOf": [ + { + "description": "Create one or more `NetworkInterface`s for the `Instance`", + "type": "object", + "properties": { + "params": { + "$ref": "#/components/schemas/InstanceNetworkInterfaceCreate" + }, + "type": { + "type": "string", + "enum": [ + "Create" + ] + } + }, + "required": [ + "params", + "type" + ] + }, + { + "description": "Default networking setup, which creates a single interface with an auto-assigned IP address from project's \"default\" VPC and \"default\" VPC Subnet.", + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "Default" + ] + } + }, + "required": [ + "type" + ] + }, + { + "description": "No network interfaces at all will be created for the instance.", + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "None" + ] + } + }, + "required": [ + "type" + ] + } + ] + }, + "InstanceNetworkInterfaceCreate": { + "type": "object", + "properties": { + "params": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NetworkInterfaceCreate" + } + } + }, + "required": [ + "params" + ] + }, "InstanceResultsPage": { "description": "A single page of results", "type": "object", @@ -4275,6 +4623,46 @@ "vpc_id" ] }, + "NetworkInterfaceCreate": { + "description": "Create-time parameters for a [`NetworkInterface`](omicron_common::api::external::NetworkInterface)", + "type": "object", + "properties": { + "description": { + "type": "string" + }, + "ip": { + "nullable": true, + "description": "The IP address for the interface. One will be auto-assigned if not provided.", + "type": "string", + "format": "ip" + }, + "name": { + "$ref": "#/components/schemas/Name" + }, + "subnet_name": { + "description": "The VPC Subnet in which to create the interface.", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + }, + "vpc_name": { + "description": "The VPC in which to create the interface.", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + } + }, + "required": [ + "description", + "name", + "subnet_name", + "vpc_name" + ] + }, "NetworkInterfaceResultsPage": { "description": "A single page of results", "type": "object", diff --git a/sled-agent/src/bin/sled-agent-sim.rs b/sled-agent/src/bin/sled-agent-sim.rs index 3cd7a84384f..c4c1a4a9d98 100644 --- a/sled-agent/src/bin/sled-agent-sim.rs +++ b/sled-agent/src/bin/sled-agent-sim.rs @@ -72,6 +72,7 @@ async fn do_run() -> Result<(), CmdError> { nexus_address: args.nexus_addr, dropshot: ConfigDropshot { bind_address: args.sled_agent_addr, + request_body_max_bytes: 2048, ..Default::default() }, log: ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info }, diff --git a/sled-agent/src/bin/sled-agent.rs b/sled-agent/src/bin/sled-agent.rs index 4da820751ff..4edfe39f853 100644 --- a/sled-agent/src/bin/sled-agent.rs +++ b/sled-agent/src/bin/sled-agent.rs @@ -116,6 +116,7 @@ async fn do_run() -> Result<(), CmdError> { id: config.id, dropshot: ConfigDropshot { bind_address: config.bootstrap_address, + request_body_max_bytes: 2048, ..Default::default() }, log: ConfigLogging::StderrTerminal { diff --git a/tools/oxapi_demo b/tools/oxapi_demo index 4571c5c0e1e..fba15826caf 100755 --- a/tools/oxapi_demo +++ b/tools/oxapi_demo @@ -51,6 +51,13 @@ INSTANCES instance_detach_disk ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME DISK_NAME instance_list_disks ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME + instance_create_nic ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME VPC_NAME + SUBNET_NAME INTERFACE_NAME + instance_delete_nic ORGANIZATION_NAME PROJECT_NAME INTERFACE_NAME VPC_NAME + SUBNET_NAME INTERFACE_NAME + instance_list_nics ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME + instance_get_nic ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME + DISKS disk_create_demo ORGANIZATION_NAME PROJECT_NAME DISK_NAME @@ -67,10 +74,12 @@ VPC SUBNETS vpc_subnets_list ORGANIZATION_NAME PROJECT_NAME VPC_NAME vpc_subnet_get ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME + vpc_subnet_create_demo ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME + IPV4_NET -VPC SUBNETS IPS +VPC SUBNET NETWORK INTERFACES - subnet_ips_list ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME + subnet_list_nics ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME VPC FIREWALL @@ -380,10 +389,47 @@ function cmd_vpc_subnet_get do_curl "/organizations/$1/projects/$2/vpcs/$3/subnets/$4" } -function cmd_subnet_ips_list +function cmd_instance_create_nic +{ + [[ $# != 6 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME + VPC_NAME SUBNET_NAME INTERFACE_NAME" + mkjson \ + name="$6" \ + description="an interface called $6" \ + vpc_name="$4" \ + subnet_name="$5" | + do_curl_authn \ + "/organizations/$1/projects/$2/instances/$3/network-interfaces" \ + -X POST -T - +} + +function cmd_instance_delete_nic +{ + [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME + INTERFACE_NAME" + do_curl_authn \ + "/organizations/$1/projects/$2/instances/$3/network-interfaces/$4" -X \ + DELETE +} + +function cmd_subnet_list_nics { [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME" - do_curl "/organizations/$1/projects/$2/vpcs/$3/subnets/$4/ips" + do_curl "/organizations/$1/projects/$2/vpcs/$3/subnets/$4/network-interfaces" +} + +function cmd_instance_get_nic +{ + [[ $# != 5 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME INTERFACE_NAME" + do_curl \ + "/organizations/$1/projects/$2/instances/$3/network-interfaces/$4" +} + +function cmd_instance_list_nics +{ + [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME INSTANCE_NAME" + do_curl_authn \ + "/organizations/$1/projects/$2/instances/$3/network-interfaces" } function cmd_vpc_firewall_rules_get