From 3cb2352bbb6e550435fe24fe122a4b303a88222f Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Wed, 17 Nov 2021 13:19:46 -0800 Subject: [PATCH 01/16] Add instance_id to NetworkInterface NetworkInterfaces always belong to exactly one instance throughout their lifecycle. --- common/src/api/external/mod.rs | 3 +++ common/src/sql/dbinit.sql | 2 ++ nexus/src/db/model.rs | 1 + nexus/src/db/schema.rs | 1 + openapi/sled-agent.json | 6 ++++++ sled-agent-client/src/lib.rs | 1 + 6 files changed, 14 insertions(+) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 5984551732c..82a0833c1a4 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1984,6 +1984,9 @@ pub struct NetworkInterface { /** common identifying metadata */ pub identity: IdentityMetadata, + /** The Instance to which the interface belongs. */ + pub instance_id: Uuid, + /** The VPC to which the interface belongs. */ pub vpc_id: Uuid, diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index a61b0a7a501..5dd75723cae 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -334,6 +334,8 @@ 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. */ + instance_id UUID NOT NULL, /* FK into VPC table */ vpc_id UUID NOT NULL, /* FK into VPCSubnet table. */ diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index db7798fc45a..e113e3ae6ae 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -1616,6 +1616,7 @@ pub struct NetworkInterface { #[diesel(embed)] pub identity: NetworkInterfaceIdentity, + pub instance_id: Uuid, pub vpc_id: Uuid, pub subnet_id: Uuid, pub mac: MacAddr, diff --git a/nexus/src/db/schema.rs b/nexus/src/db/schema.rs index 1e4c481242f..cbdd20376f0 100644 --- a/nexus/src/db/schema.rs +++ b/nexus/src/db/schema.rs @@ -60,6 +60,7 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, + instance_id -> Uuid, vpc_id -> Uuid, subnet_id -> Uuid, mac -> Text, diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index ef4081c4d18..c8ea74e6cd2 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -568,6 +568,11 @@ } ] }, + "instance_id": { + "description": "The Instance to which the interface belongs.", + "type": "string", + "format": "uuid" + }, "ip": { "description": "The IP address assigned to this interface.", "type": "string", @@ -594,6 +599,7 @@ }, "required": [ "identity", + "instance_id", "ip", "mac", "subnet_id", diff --git a/sled-agent-client/src/lib.rs b/sled-agent-client/src/lib.rs index 2747de9a04c..a6df6b62b63 100644 --- a/sled-agent-client/src/lib.rs +++ b/sled-agent-client/src/lib.rs @@ -245,6 +245,7 @@ impl From<&omicron_common::api::external::NetworkInterface> Self { identity: (&s.identity).into(), ip: s.ip.to_string(), + instance_id: s.instance_id, mac: s.mac.into(), subnet_id: s.subnet_id, vpc_id: s.vpc_id, From 85190d50962de6899a31b0137afa4f4b602772d7 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Mon, 29 Nov 2021 10:11:20 -0800 Subject: [PATCH 02/16] Implement a basic network interface model This model currently uses randomly generated locally administered MAC addresses. This is probably? not what we want longer term. --- common/src/api/external/mod.rs | 18 +- common/src/sql/dbinit.sql | 7 + nexus/src/db/datastore.rs | 278 ++++++++++++++++++++++++++++++- nexus/src/db/model.rs | 44 ++++- nexus/src/external_api/params.rs | 14 ++ nexus/src/nexus.rs | 48 ++++++ nexus/src/sagas.rs | 53 +++++- 7 files changed, 455 insertions(+), 7 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 82a0833c1a4..2bf66962e34 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -468,6 +468,7 @@ pub enum ResourceType { Disk, DiskAttachment, Instance, + NetworkInterface, Rack, Sled, SagaDbg, @@ -491,6 +492,7 @@ impl Display for ResourceType { ResourceType::Disk => "disk", ResourceType::DiskAttachment => "disk attachment", ResourceType::Instance => "instance", + ResourceType::NetworkInterface => "network interface", ResourceType::Rack => "rack", ResourceType::Sled => "sled", ResourceType::SagaDbg => "saga_dbg", @@ -1921,14 +1923,24 @@ impl JsonSchema for L4PortRange { /// hardware devices on a network. // NOTE: We're using the `macaddr` crate for the internal representation. But as with the `ipnet`, // this crate does not implement `JsonSchema`. -#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] +#[derive( + Clone, Copy, Debug, DeserializeFromStr, PartialEq, SerializeDisplay, +)] pub struct MacAddr(pub macaddr::MacAddr6); +impl FromStr for MacAddr { + type Err = macaddr::ParseError; + + fn from_str(s: &str) -> Result { + s.parse().map(|addr| MacAddr(addr)) + } +} + impl TryFrom for MacAddr { - type Error = macaddr::ParseError; + type Error = ::Err; fn try_from(s: String) -> Result { - s.parse().map(|addr| MacAddr(addr)) + MacAddr::from_str(s.as_ref()) } } diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 5dd75723cae..6ba1ccf8a01 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -358,6 +358,13 @@ CREATE UNIQUE INDEX ON omicron.public.network_interface ( ) 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, + ip +) 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 e4cbbcfc64e..838adf70e11 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -29,6 +29,7 @@ use crate::context::OpContext; use async_bb8_diesel::{AsyncConnection, AsyncRunQueryDsl, ConnectionManager}; use chrono::Utc; use diesel::prelude::*; +use diesel::sql_types; use diesel::{ExpressionMethods, QueryDsl, SelectableHelper}; use omicron_common::api; use omicron_common::api::external::CreateResult; @@ -38,9 +39,11 @@ use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; +use omicron_common::api::external::MacAddr; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use omicron_common::bail_unless; +use rand::{rngs::StdRng, SeedableRng}; use std::convert::TryFrom; use std::sync::Arc; use uuid::Uuid; @@ -52,8 +55,9 @@ use crate::db::{ }, model::{ ConsoleSession, Disk, DiskAttachment, DiskRuntimeState, Generation, - Instance, InstanceRuntimeState, Name, Organization, OrganizationUpdate, - OximeterInfo, ProducerEndpoint, Project, ProjectUpdate, RouterRoute, + IncompleteNetworkInterface, Instance, InstanceRuntimeState, Name, + NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, + ProducerEndpoint, Project, ProjectUpdate, RouterRoute, RouterRouteUpdate, Sled, Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, VpcSubnetUpdate, VpcUpdate, }, @@ -948,6 +952,76 @@ impl DataStore { } } + /* + * Network interfaces + */ + + /** + * Generate a unique MAC address for an interface + */ + pub fn generate_mac_address(&self) -> Result { + // Bitmasks defined in RFC7042 section 2.1 + const LOCALLY_ADMINISTERED: u8 = 0b1; + const MULTICAST: u8 = 0b10; + + // Generate a unique address from the locally administered space. + use rand::Fill; + let mut addr = [0u8; 6]; + addr.try_fill(&mut StdRng::from_entropy()) + .map_err(|_| Error::internal_error("failed to generate MAC"))?; + addr[0] = (addr[0] & !MULTICAST) | LOCALLY_ADMINISTERED; + Ok(MacAddr(macaddr::MacAddr6::from(addr)).into()) + } + + pub async fn instance_create_network_interface( + &self, + interface: IncompleteNetworkInterface, + ) -> CreateResult { + use db::schema::network_interface::dsl; + + let name = interface.identity.name.clone(); + let result = match interface.ip { + // Attempt an insert with a requested IP address + Some(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 + } + // Insert and allocate an IP address + None => { + let block = interface.subnet.ipv4_block.ok_or_else(|| { + Error::internal_error("assuming subnets all have v4 block") + })?; + let allocation_query = AllocateIpQuery { + block: ipnetwork::IpNetwork::V4(block.0 .0), + interface, + }; + diesel::insert_into(dsl::network_interface) + .values(allocation_query) + .returning(NetworkInterface::as_returning()) + .get_result_async(self.pool()) + .await + } + }; + result.map_err(|e| { + public_error_from_diesel_pool_create( + e, + ResourceType::NetworkInterface, + name.as_str(), + ) + }) + } + // Create a record for a new Oximeter instance pub async fn oximeter_create( &self, @@ -1845,6 +1919,206 @@ impl DataStore { } } +// TODO: Move this somewhere else +use diesel::pg::Pg; +struct AllocateIpQuery { + interface: IncompleteNetworkInterface, + block: ipnetwork::IpNetwork, +} + +struct AllocateIpQueryValues(AllocateIpQuery); + +impl diesel::query_builder::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) + } +} + +/// Generate the query +/// 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(1, ) as off +/// LEFT OUTER JOIN +/// network_interface +/// ON (subnet_id, ip, time_deleted IS NULL) = +/// (, + off, TRUE) +/// WHERE ip IS NULL LIMIT 1; +impl diesel::query_builder::QueryFragment for AllocateIpQuery { + fn walk_ast( + &self, + mut out: diesel::query_builder::AstPass, + ) -> diesel::QueryResult<()> { + use db::schema::network_interface::dsl; + + 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) + } + }; + let now = Utc::now(); + + 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::(&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::( + &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)?; + + // Start the offsets from 1 to exclude the network base address. + out.push_sql(" FROM generate_series(1, "); + out.push_bind_param::( + // Subtract 1 to exclude the broadcast address + &(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 diesel::query_builder::QueryId for AllocateIpQueryValues { + type QueryId = (); + const HAS_STATIC_QUERY_ID: bool = false; +} + +impl diesel::insertable::CanInsertInSingleQuery for AllocateIpQueryValues { + fn rows_to_insert(&self) -> Option { + Some(1) + } +} + +impl diesel::query_builder::QueryFragment for AllocateIpQueryValues { + fn walk_ast( + &self, + mut out: diesel::query_builder::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) + } +} + #[cfg(test)] mod test { use crate::context::OpContext; diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index e113e3ae6ae..7f50175578b 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -1610,7 +1610,36 @@ impl Into for VpcFirewallRule { } } -#[derive(Queryable, Insertable, Clone, Debug, Resource)] +/// A not fully constructed NetworkInterface. It may not yet have an IP +/// address allocated. +#[derive(Clone, Debug)] +pub struct IncompleteNetworkInterface { + pub identity: NetworkInterfaceIdentity, + + pub instance_id: Uuid, + pub vpc_id: Uuid, + pub subnet: VpcSubnet, + pub mac: MacAddr, + pub ip: Option, +} + +impl IncompleteNetworkInterface { + pub fn new( + interface_id: Uuid, + instance_id: Uuid, + vpc_id: uuid::Uuid, + subnet: VpcSubnet, + mac: MacAddr, + ip: Option, + params: params::NetworkInterfaceCreate, + ) -> Self { + let identity = + NetworkInterfaceIdentity::new(interface_id, params.identity); + Self { identity, instance_id, subnet, vpc_id, mac, ip } + } +} + +#[derive(Selectable, Queryable, Insertable, Clone, Debug, Resource)] #[table_name = "network_interface"] pub struct NetworkInterface { #[diesel(embed)] @@ -1623,6 +1652,19 @@ pub struct NetworkInterface { pub ip: ipnetwork::IpNetwork, } +impl From for external::NetworkInterface { + fn from(iface: NetworkInterface) -> Self { + Self { + identity: iface.identity(), + instance_id: iface.instance_id, + vpc_id: iface.vpc_id, + subnet_id: iface.subnet_id, + ip: iface.ip.ip(), + mac: *iface.mac, + } + } +} + // TODO: `struct SessionToken(String)` for session token #[derive(Queryable, Insertable, Clone, Debug, Selectable)] diff --git a/nexus/src/external_api/params.rs b/nexus/src/external_api/params.rs index 4d25f7aa68a..b7876fb863f 100644 --- a/nexus/src/external_api/params.rs +++ b/nexus/src/external_api/params.rs @@ -55,3 +55,17 @@ pub struct ProjectUpdate { #[serde(flatten)] pub identity: IdentityMetadataUpdateParams, } + +/* + * NETWORK INTERFACES + */ + +/** + * Create-time parameters for a [`NetworkInterface`] + */ +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct NetworkInterfaceCreate { + #[serde(flatten)] + pub identity: IdentityMetadataCreateParams, +} diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index b3db597f058..7e5ec385624 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -1394,6 +1394,54 @@ impl Nexus { .map(|_| ()) } + /** + * Creates a new network interface for this instance + */ + pub async fn instance_create_network_interface( + &self, + organization_name: &Name, + project_name: &Name, + instance_name: &Name, + vpc_name: &Name, + subnet_name: &Name, + params: ¶ms::NetworkInterfaceCreate, + ) -> CreateResult { + let instance = self + .project_lookup_instance( + organization_name, + project_name, + instance_name, + ) + .await?; + let vpc = self + .db_datastore + .vpc_fetch_by_name(&instance.project_id, vpc_name) + .await?; + let subnet = self + .db_datastore + .vpc_subnet_fetch_by_name(&vpc.id(), subnet_name) + .await?; + + let mac = self.db_datastore.generate_mac_address()?; + + 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, + mac, + ip, + params.clone(), + ); + self.db_datastore.instance_create_network_interface(interface).await + } + pub async fn project_list_vpcs( &self, organization_name: &Name, diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 998af21225f..64746ba97b4 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -10,17 +10,22 @@ */ use crate::db; +use crate::db::identity::Resource; +use crate::external_api::params; use crate::saga_interface::SagaContext; use chrono::Utc; use lazy_static::lazy_static; use omicron_common::api::external::Generation; +use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::InstanceCreateParams; use omicron_common::api::external::InstanceState; +use omicron_common::api::external::Name; use omicron_common::api::internal::nexus::InstanceRuntimeState; use omicron_common::api::internal::sled_agent::InstanceHardware; use serde::Deserialize; use serde::Serialize; use std::collections::BTreeMap; +use std::convert::TryFrom; use std::sync::Arc; use steno::new_action_noop_undo; use steno::ActionContext; @@ -154,11 +159,57 @@ async fn sic_create_instance_record( .await .map_err(ActionError::action_failed)?; + let default_name = + db::model::Name(Name::try_from("default".to_string()).unwrap()); + + let vpc = osagactx + .datastore() + .vpc_fetch_by_name(&instance.project_id, &default_name) + .await + .map_err(ActionError::action_failed)?; + let subnet = osagactx + .datastore() + .vpc_subnet_fetch_by_name(&vpc.id(), &default_name) + .await + .map_err(ActionError::action_failed)?; + + let mac = osagactx + .datastore() + .generate_mac_address() + .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, + mac, + ip, + params::NetworkInterfaceCreate { + identity: IdentityMetadataCreateParams { + // TODO: Generate a unique name here, since we're not guaranteed + // this interface name is available + name: instance.name().0.clone(), + description: "default interface".to_string(), + }, + }, + ); + let interface = osagactx + .datastore() + .instance_create_network_interface(interface) + .await + .map_err(ActionError::action_failed)?; + // TODO: Populate this with an appropriate NIC. // See also: instance_set_runtime in nexus.rs for a similar construction. Ok(InstanceHardware { runtime: instance.runtime().clone().into(), - nics: vec![], + nics: vec![interface.into()], }) } From 97725134db16786b184254a43de5eca284c0c955 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 30 Nov 2021 18:05:56 -0800 Subject: [PATCH 03/16] Move subnet IP allocation to its own file --- nexus/src/db/datastore.rs | 202 +---------------------------- nexus/src/db/mod.rs | 1 + nexus/src/db/subnet_allocation.rs | 206 ++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 201 deletions(-) create mode 100644 nexus/src/db/subnet_allocation.rs diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 838adf70e11..04cfb17516d 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -29,7 +29,6 @@ use crate::context::OpContext; use async_bb8_diesel::{AsyncConnection, AsyncRunQueryDsl, ConnectionManager}; use chrono::Utc; use diesel::prelude::*; -use diesel::sql_types; use diesel::{ExpressionMethods, QueryDsl, SelectableHelper}; use omicron_common::api; use omicron_common::api::external::CreateResult; @@ -62,6 +61,7 @@ use crate::db::{ VpcRouterUpdate, VpcSubnet, VpcSubnetUpdate, VpcUpdate, }, pagination::paginated, + subnet_allocation::AllocateIpQuery, update_and_check::{UpdateAndCheck, UpdateStatus}, }; @@ -1919,206 +1919,6 @@ impl DataStore { } } -// TODO: Move this somewhere else -use diesel::pg::Pg; -struct AllocateIpQuery { - interface: IncompleteNetworkInterface, - block: ipnetwork::IpNetwork, -} - -struct AllocateIpQueryValues(AllocateIpQuery); - -impl diesel::query_builder::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) - } -} - -/// Generate the query -/// 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(1, ) as off -/// LEFT OUTER JOIN -/// network_interface -/// ON (subnet_id, ip, time_deleted IS NULL) = -/// (, + off, TRUE) -/// WHERE ip IS NULL LIMIT 1; -impl diesel::query_builder::QueryFragment for AllocateIpQuery { - fn walk_ast( - &self, - mut out: diesel::query_builder::AstPass, - ) -> diesel::QueryResult<()> { - use db::schema::network_interface::dsl; - - 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) - } - }; - let now = Utc::now(); - - 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::(&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::( - &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)?; - - // Start the offsets from 1 to exclude the network base address. - out.push_sql(" FROM generate_series(1, "); - out.push_bind_param::( - // Subtract 1 to exclude the broadcast address - &(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 diesel::query_builder::QueryId for AllocateIpQueryValues { - type QueryId = (); - const HAS_STATIC_QUERY_ID: bool = false; -} - -impl diesel::insertable::CanInsertInSingleQuery for AllocateIpQueryValues { - fn rows_to_insert(&self) -> Option { - Some(1) - } -} - -impl diesel::query_builder::QueryFragment for AllocateIpQueryValues { - fn walk_ast( - &self, - mut out: diesel::query_builder::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) - } -} - #[cfg(test)] mod test { use crate::context::OpContext; diff --git a/nexus/src/db/mod.rs b/nexus/src/db/mod.rs index d02f40801d7..7f6d1af8f9a 100644 --- a/nexus/src/db/mod.rs +++ b/nexus/src/db/mod.rs @@ -15,6 +15,7 @@ mod pool; mod saga_recovery; mod saga_types; mod sec_store; +mod subnet_allocation; mod update_and_check; #[cfg(test)] diff --git a/nexus/src/db/subnet_allocation.rs b/nexus/src/db/subnet_allocation.rs new file mode 100644 index 00000000000..b12c3f5935e --- /dev/null +++ b/nexus/src/db/subnet_allocation.rs @@ -0,0 +1,206 @@ +use crate::db; +use crate::db::identity::Resource; +use crate::db::model::IncompleteNetworkInterface; +use chrono::Utc; +use diesel::pg::Pg; +use diesel::prelude::*; +use diesel::query_builder::*; +use diesel::sql_types; +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(1, ) AS off +/// LEFT OUTER JOIN +/// network_interface +/// ON (subnet_id, ip, time_deleted IS NULL) = +/// (, + off, TRUE) +/// WHERE ip IS NULL LIMIT 1; +pub struct AllocateIpQuery { + pub interface: IncompleteNetworkInterface, + pub block: ipnetwork::IpNetwork, +} + +/// 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; + + 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) + } + }; + let now = Utc::now(); + + 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::(&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::( + &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)?; + + // Start the offsets from 1 to exclude the network base address. + out.push_sql(" FROM generate_series(1, "); + out.push_bind_param::( + // Subtract 1 to exclude the broadcast address + &(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; +} + +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) + } +} From 6f87c143e05b47ecad3b975d3871020c3860221e Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 30 Nov 2021 18:28:53 -0800 Subject: [PATCH 04/16] Add header to subnet_allocation.rs --- nexus/src/db/subnet_allocation.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nexus/src/db/subnet_allocation.rs b/nexus/src/db/subnet_allocation.rs index b12c3f5935e..d098e49260e 100644 --- a/nexus/src/db/subnet_allocation.rs +++ b/nexus/src/db/subnet_allocation.rs @@ -1,3 +1,9 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Diesel queries used for subnet allocation + use crate::db; use crate::db::identity::Resource; use crate::db::model::IncompleteNetworkInterface; From 6c545598713458b34c7db7cc19cd563a847f0bae Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Fri, 19 Nov 2021 14:36:52 -0800 Subject: [PATCH 05/16] Convert all names to IPs --- nexus/src/db/datastore.rs | 102 +++++++++++++++++++++ nexus/src/db/model.rs | 9 +- nexus/src/nexus.rs | 188 +++++++++++++++++++++++++++++++++++++- 3 files changed, 291 insertions(+), 8 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index f457782e9db..6df7c073ebd 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -48,6 +48,7 @@ use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use omicron_common::bail_unless; use rand::{rngs::StdRng, SeedableRng}; +use std::collections::HashMap; use std::convert::TryFrom; use std::sync::Arc; use uuid::Uuid; @@ -841,6 +842,76 @@ impl DataStore { } } + /// Identify all IPs in use by each instance + // TODO: how to name/where to put this + pub async fn resolve_instances_to_ips>( + &self, + vpc: &Vpc, + instance_names: T, + ) -> Result>, Error> { + use db::schema::{instance, network_interface}; + let mut addrs = network_interface::table + .inner_join( + instance::table + .on(instance::id.eq(network_interface::instance_id)), + ) + .select((instance::name, network_interface::ip)) + .filter(instance::project_id.eq(vpc.project_id)) + .filter(instance::name.eq_any(instance_names)) + .filter(network_interface::time_deleted.is_null()) + .filter(instance::time_deleted.is_null()) + .get_results_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ResourceType::Instance, + LookupType::Other("Resolving to IPs".to_string()), + ) + })?; + + let mut result = HashMap::with_capacity(addrs.len()); + for (name, addr) in addrs.drain(..) { + result.entry(name).or_insert(vec![]).push(addr) + } + Ok(result) + } + + /// Identify all subnets in use by each VpcSubnet + // TODO: how to name/where to put this + pub async fn resolve_subnets_to_ips>( + &self, + vpc: &Vpc, + subnet_names: T, + ) -> Result>, Error> { + use db::schema::vpc_subnet; + let subnets = vpc_subnet::table + .select(VpcSubnet::as_select()) + .filter(vpc_subnet::vpc_id.eq(vpc.id())) + .filter(vpc_subnet::name.eq_any(subnet_names)) + .filter(vpc_subnet::time_deleted.is_null()) + .get_results_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ResourceType::VpcSubnet, + LookupType::Other("Resolving to IPs".to_string()), + ) + })?; + let mut result = HashMap::with_capacity(subnets.len()); + for subnet in subnets { + let entry = result.entry(subnet.name().clone()).or_insert(vec![]); + if let Some(block) = subnet.ipv4_block { + entry.push(ipnetwork::IpNetwork::V4(block.0 .0)) + } + if let Some(block) = subnet.ipv6_block { + entry.push(ipnetwork::IpNetwork::V6(block.0 .0)) + } + } + Ok(result) + } + /* * Disks */ @@ -1585,6 +1656,37 @@ impl DataStore { }) } + pub async fn vpc_resolve_to_sleds( + &self, + vpc_id: &Uuid, + ) -> Result, Error> { + use db::schema::{instance, network_interface, sled}; + + // Resolve each VNIC in the VPC to the Sled its on, so we know which + // Sleds to notify when firewall rules change. + network_interface::table + .inner_join( + instance::table + .on(instance::id.eq(network_interface::instance_id)), + ) + .inner_join( + sled::table.on(sled::id.eq(instance::active_propolis_id)), + ) + .filter(network_interface::vpc_id.eq(*vpc_id)) + .filter(network_interface::time_deleted.is_null()) + .filter(instance::time_deleted.is_null()) + .select(Sled::as_select()) + .get_results_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ResourceType::Vpc, + LookupType::Other("Resolving to sleds".to_string()), + ) + }) + } + pub async fn vpc_list_subnets( &self, vpc_id: &Uuid, diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 2a9c095f987..2c9a2e358d8 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -100,6 +100,7 @@ macro_rules! impl_enum_type { AsExpression, FromSqlRow, Eq, + Hash, PartialEq, Ord, PartialOrd, @@ -1444,7 +1445,7 @@ impl_enum_type!( #[postgres(type_name = "vpc_firewall_rule_status", type_schema = "public")] pub struct VpcFirewallRuleStatusEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow)] #[sql_type = "VpcFirewallRuleStatusEnum"] pub struct VpcFirewallRuleStatus(pub external::VpcFirewallRuleStatus); @@ -1459,7 +1460,7 @@ impl_enum_type!( #[postgres(type_name = "vpc_firewall_rule_direction", type_schema = "public")] pub struct VpcFirewallRuleDirectionEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow)] #[sql_type = "VpcFirewallRuleDirectionEnum"] pub struct VpcFirewallRuleDirection(pub external::VpcFirewallRuleDirection); @@ -1474,7 +1475,7 @@ impl_enum_type!( #[postgres(type_name = "vpc_firewall_rule_action", type_schema = "public")] pub struct VpcFirewallRuleActionEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow)] #[sql_type = "VpcFirewallRuleActionEnum"] pub struct VpcFirewallRuleAction(pub external::VpcFirewallRuleAction); @@ -1489,7 +1490,7 @@ impl_enum_type!( #[postgres(type_name = "vpc_firewall_rule_protocol", type_schema = "public")] pub struct VpcFirewallRuleProtocolEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow)] #[sql_type = "VpcFirewallRuleProtocolEnum"] pub struct VpcFirewallRuleProtocol(pub external::VpcFirewallRuleProtocol); diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index f4e08cbbe0f..69e9a387d9c 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -61,6 +61,7 @@ use oximeter_producer::register; use rand::{rngs::StdRng, RngCore, SeedableRng}; use sled_agent_client::Client as SledAgentClient; use slog::Logger; +use std::collections::{HashMap, HashSet}; use std::convert::TryInto; use std::net::SocketAddr; use std::sync::Arc; @@ -1682,16 +1683,182 @@ impl Nexus { vpc.id(), params.clone(), ); - let result = self + let rules = self .db_datastore .vpc_update_firewall_rules(&vpc.id(), rules) + .await?; + + let rules_for_sled = + self.resolve_firewall_rules_for_sled_agent(&vpc, &rules).await?; + + let sleds = self + .db_datastore + .vpc_resolve_to_sleds(&vpc.id()) .await? - .into_iter() - .map(|rule| rule.into()) - .collect(); + .iter() + .map(|sled| async move { self.sled_client(&sled.id()).await }); + + let result = rules.into_iter().map(|rule| rule.into()).collect(); Ok(result) } + // TODO: move this somewhere else + async fn resolve_firewall_rules_for_sled_agent( + &self, + vpc: &db::model::Vpc, + rules: &Vec, + ) -> Result, Error> { + // Gather list of Instances and Subnets to resolve + let mut instances = HashSet::new(); + let mut subnets = HashSet::new(); + for rule in rules { + for target in &rule.targets { + match &target.0 { + external::VpcFirewallRuleTarget::Instance(name) => { + instances.insert(name.clone().into()); + } + external::VpcFirewallRuleTarget::Subnet(name) => { + subnets.insert(name.clone().into()); + } + // TODO: How do we resolve VPC targets? + external::VpcFirewallRuleTarget::Vpc(name) => (), + } + } + + for host in rule.filter_hosts.iter().flatten() { + match &host.0 { + external::VpcFirewallRuleHostFilter::Instance(name) => { + instances.insert(name.clone().into()); + } + external::VpcFirewallRuleHostFilter::Subnet(name) => { + subnets.insert(name.clone().into()); + } + // We don't need to resolve anything for Ip + external::VpcFirewallRuleHostFilter::Ip(addr) => (), + // TODO: How do we resolve VPC targets? + external::VpcFirewallRuleHostFilter::Vpc(name) => (), + // TODO: How do we resolve InternetGateway targets? + external::VpcFirewallRuleHostFilter::InternetGateway( + name, + ) => (), + } + } + } + let instance_ips: HashMap> = + self.db_datastore + .resolve_instances_to_ips(vpc, instances) + .await? + .drain() + .map(|(name, v)| (name.0, v)) + .collect(); + let subnet_networks: HashMap< + external::Name, + Vec, + > = self + .db_datastore + .resolve_subnets_to_ips(vpc, subnets) + .await? + .drain() + .map(|(name, v)| (name.0, v)) + .collect(); + + let mut opte_rules = Vec::with_capacity(rules.len()); + for rule in rules { + let mut targets = Vec::with_capacity(rule.targets.len()); + for target in &rule.targets { + match &target.0 { + // TODO: what is the correct behavior when a name is not + // found? Options: + // 1) Fail update request (though note this can still + // arise from things like instance deletion) + // 2) Allow update request, ignore this rule (but store it + // in case it becomes valid later). This is consistent + // with the semantics of the rules. Rules with bad + // references should likely at least be flagged to users + external::VpcFirewallRuleTarget::Instance(name) => { + targets.extend_from_slice( + instance_ips + .get(&name) + .ok_or_else(|| { + Error::not_found_by_name( + ResourceType::Instance, + &name, + ) + })? + .as_slice(), + ); + } + external::VpcFirewallRuleTarget::Subnet(name) => { + targets.extend_from_slice( + subnet_networks + .get(&name) + .ok_or_else(|| { + Error::not_found_by_name( + ResourceType::VpcSubnet, + &name, + ) + })? + .as_slice(), + ); + } + // TODO: How do we resolve VPC targets? + external::VpcFirewallRuleTarget::Vpc(_name) => (), + }; + } + + let filter_hosts = match &rule.filter_hosts { + None => None, + Some(hosts) => { + let mut host_addrs = Vec::with_capacity(hosts.len()); + for host in hosts { + match &host.0 { + // TODO: See above about handling missing names + external::VpcFirewallRuleHostFilter::Instance(name) => { + host_addrs.extend_from_slice(instance_ips.get(&name).ok_or_else(|| { + Error::not_found_by_name( + ResourceType::Instance, + &name, + ) + })?.as_slice()); + } + external::VpcFirewallRuleHostFilter::Subnet(name) => { + host_addrs.extend_from_slice(subnet_networks.get(&name).ok_or_else(|| { + Error::not_found_by_name( + ResourceType::VpcSubnet, + &name, + ) + })?.as_slice()); + } + external::VpcFirewallRuleHostFilter::Ip(addr) => { + host_addrs.push(ipnetwork::IpNetwork::from(*addr)); + } + // TODO: How do we resolve VPC targets? + external::VpcFirewallRuleHostFilter::Vpc(_name) => { + } + // TODO: How do we resolve InternetGateway targets? + external::VpcFirewallRuleHostFilter::InternetGateway( + _name, + ) => (), + } + } + Some(host_addrs) + } + }; + + opte_rules.push(OpteFirewallRule { + status: rule.status, + direction: rule.direction, + targets: targets, + filter_hosts: filter_hosts, + filter_ports: rule.filter_ports.clone(), + filter_protocols: rule.filter_protocols.clone(), + action: rule.action, + priority: rule.priority, + }); + } + Ok(opte_rules) + } + pub async fn vpc_list_subnets( &self, organization_name: &Name, @@ -2381,3 +2548,16 @@ lazy_static! { } }"#).unwrap(); } + +// TODO: move this somewhere else +pub struct OpteFirewallRule { + pub status: crate::db::model::VpcFirewallRuleStatus, + pub direction: crate::db::model::VpcFirewallRuleDirection, + pub targets: Vec, + pub filter_hosts: Option>, + pub filter_ports: Option>, + pub filter_protocols: + Option>, + pub action: crate::db::model::VpcFirewallRuleAction, + pub priority: crate::db::model::VpcFirewallRulePriority, +} From 91a790d486b6f7dfd86ea817620cce5dbb918be7 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 23 Nov 2021 17:21:14 -0800 Subject: [PATCH 06/16] Make target resolution go to interfaces instead of ips --- nexus/src/db/datastore.rs | 53 +++++++++++++++++++++++++++++++++------ nexus/src/db/model.rs | 17 +++++++------ nexus/src/nexus.rs | 49 +++++++++++++++++++++++++----------- 3 files changed, 90 insertions(+), 29 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index 6df7c073ebd..a2a016d9504 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -844,18 +844,21 @@ impl DataStore { /// Identify all IPs in use by each instance // TODO: how to name/where to put this - pub async fn resolve_instances_to_ips>( + pub async fn resolve_instances_to_interfaces< + T: IntoIterator, + >( &self, vpc: &Vpc, instance_names: T, - ) -> Result>, Error> { + ) -> Result>, Error> { use db::schema::{instance, network_interface}; - let mut addrs = network_interface::table + // TODO: make sure there's an index on network_interface::instance_id + let ifaces = network_interface::table .inner_join( instance::table .on(instance::id.eq(network_interface::instance_id)), ) - .select((instance::name, network_interface::ip)) + .select((instance::name, NetworkInterface::as_select())) .filter(instance::project_id.eq(vpc.project_id)) .filter(instance::name.eq_any(instance_names)) .filter(network_interface::time_deleted.is_null()) @@ -870,9 +873,45 @@ impl DataStore { ) })?; - let mut result = HashMap::with_capacity(addrs.len()); - for (name, addr) in addrs.drain(..) { - result.entry(name).or_insert(vec![]).push(addr) + let mut result = HashMap::with_capacity(ifaces.len()); + for (name, iface) in ifaces.into_iter() { + result.entry(name).or_insert(vec![]).push(iface) + } + Ok(result) + } + + /// Identify all VNICs connected to each VpcSubnet + // TODO: how to name/where to put this + pub async fn resolve_subnets_to_interfaces>( + &self, + vpc: &Vpc, + subnet_names: T, + ) -> Result>, Error> { + use db::schema::{network_interface, vpc_subnet}; + // TODO: make sure there's an index on network_interface::subnet_id + let subnets = network_interface::table + .inner_join( + vpc_subnet::table + .on(vpc_subnet::id.eq(network_interface::subnet_id)), + ) + .select((vpc_subnet::name, NetworkInterface::as_select())) + .filter(vpc_subnet::vpc_id.eq(vpc.id())) + .filter(vpc_subnet::name.eq_any(subnet_names)) + .filter(network_interface::time_deleted.is_null()) + .filter(vpc_subnet::time_deleted.is_null()) + .get_results_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ResourceType::VpcSubnet, + LookupType::Other("Resolving to interfaces".to_string()), + ) + })?; + let mut result = HashMap::with_capacity(subnets.len()); + for (name, interface) in subnets.into_iter() { + let entry = result.entry(name).or_insert(vec![]); + entry.push(interface); } Ok(result) } diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 2c9a2e358d8..82f5f1bf2c9 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -1783,15 +1783,16 @@ pub struct NetworkInterface { pub ip: ipnetwork::IpNetwork, } +/// Conversion to the external API type. impl From for external::NetworkInterface { - fn from(iface: NetworkInterface) -> Self { - Self { - identity: iface.identity(), - instance_id: iface.instance_id, - vpc_id: iface.vpc_id, - subnet_id: iface.subnet_id, - ip: iface.ip.ip(), - mac: *iface.mac, + fn from(interface: NetworkInterface) -> external::NetworkInterface { + external::NetworkInterface { + identity: interface.identity(), + instance_id: interface.instance_id, + vpc_id: interface.vpc_id, + subnet_id: interface.subnet_id, + mac: interface.mac.into(), + ip: interface.ip.ip(), } } } diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 69e9a387d9c..e5b8ff5f1e6 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -39,6 +39,7 @@ use omicron_common::api::external::Ipv6Net; use omicron_common::api::external::ListResult; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; +use omicron_common::api::external::NetworkInterface; use omicron_common::api::external::PaginationOrder; use omicron_common::api::external::ResourceType; use omicron_common::api::external::RouteDestination; @@ -1744,23 +1745,41 @@ impl Nexus { } } } - let instance_ips: HashMap> = - self.db_datastore - .resolve_instances_to_ips(vpc, instances) - .await? - .drain() - .map(|(name, v)| (name.0, v)) - .collect(); + + // TODO-correctness: It's possible these three queries produce + // inconsistent results due to concurrent changes. These should be + // transactional + let instance_interfaces: HashMap< + external::Name, + Vec, + > = self + .db_datastore + .resolve_instances_to_interfaces(vpc, instances) + .await? + .into_iter() + .map(|(name, v)| { + (name.0, v.into_iter().map(|iface| iface.into()).collect()) + }) + .collect(); let subnet_networks: HashMap< external::Name, Vec, > = self .db_datastore - .resolve_subnets_to_ips(vpc, subnets) + .resolve_subnets_to_ips(vpc, subnets.clone()) .await? - .drain() + .into_iter() .map(|(name, v)| (name.0, v)) .collect(); + let subnet_interfaces: HashMap> = + self.db_datastore + .resolve_subnets_to_interfaces(vpc, subnets) + .await? + .into_iter() + .map(|(name, v)| { + (name.0, v.into_iter().map(|iface| iface.into()).collect()) + }) + .collect(); let mut opte_rules = Vec::with_capacity(rules.len()); for rule in rules { @@ -1777,7 +1796,7 @@ impl Nexus { // references should likely at least be flagged to users external::VpcFirewallRuleTarget::Instance(name) => { targets.extend_from_slice( - instance_ips + instance_interfaces .get(&name) .ok_or_else(|| { Error::not_found_by_name( @@ -1790,7 +1809,7 @@ impl Nexus { } external::VpcFirewallRuleTarget::Subnet(name) => { targets.extend_from_slice( - subnet_networks + subnet_interfaces .get(&name) .ok_or_else(|| { Error::not_found_by_name( @@ -1814,12 +1833,14 @@ impl Nexus { match &host.0 { // TODO: See above about handling missing names external::VpcFirewallRuleHostFilter::Instance(name) => { - host_addrs.extend_from_slice(instance_ips.get(&name).ok_or_else(|| { + for interface in instance_interfaces.get(&name).ok_or_else(|| { Error::not_found_by_name( ResourceType::Instance, &name, ) - })?.as_slice()); + })? { + host_addrs.push(interface.ip.into()); + } } external::VpcFirewallRuleHostFilter::Subnet(name) => { host_addrs.extend_from_slice(subnet_networks.get(&name).ok_or_else(|| { @@ -2553,7 +2574,7 @@ lazy_static! { pub struct OpteFirewallRule { pub status: crate::db::model::VpcFirewallRuleStatus, pub direction: crate::db::model::VpcFirewallRuleDirection, - pub targets: Vec, + pub targets: Vec, pub filter_hosts: Option>, pub filter_ports: Option>, pub filter_protocols: From 7a1b9a053f1b1a61203f6a05f32ea00c8c9cfe49 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 23 Nov 2021 19:01:52 -0800 Subject: [PATCH 07/16] Most of the way to plumbed to sled agent --- Cargo.lock | 1 + nexus/src/nexus.rs | 143 +++++++++++++++---------- openapi/sled-agent.json | 143 +++++++++++++++++++++++++ sled-agent-client/Cargo.toml | 1 + sled-agent-client/src/lib.rs | 77 +++++++++++++ sled-agent/src/http_entrypoints.rs | 30 +++++- sled-agent/src/params.rs | 30 ++++++ sled-agent/src/sim/http_entrypoints.rs | 29 ++++- 8 files changed, 393 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8463f35e178..8d026c514e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3095,6 +3095,7 @@ dependencies = [ "anyhow", "async-trait", "chrono", + "ipnetwork", "omicron-common", "percent-encoding", "progenitor", diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index e5b8ff5f1e6..7b1757b8693 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -19,6 +19,7 @@ use crate::saga_interface::SagaContext; use crate::sagas; use anyhow::Context; use async_trait::async_trait; +use futures::future::join_all; use futures::future::ready; use futures::StreamExt; use hex; @@ -1689,18 +1690,50 @@ impl Nexus { .vpc_update_firewall_rules(&vpc.id(), rules) .await?; + self.send_sled_agents_firewall_rules(&vpc, &rules).await?; + + let result = rules.into_iter().map(|rule| rule.into()).collect(); + Ok(result) + } + + async fn send_sled_agents_firewall_rules( + &self, + vpc: &db::model::Vpc, + rules: &Vec, + ) -> Result<(), Error> { let rules_for_sled = self.resolve_firewall_rules_for_sled_agent(&vpc, &rules).await?; + let sled_rules_request = + sled_agent_client::types::VpcFirewallRulesEnsureBody { + rules: rules_for_sled, + }; - let sleds = self - .db_datastore - .vpc_resolve_to_sleds(&vpc.id()) - .await? - .iter() - .map(|sled| async move { self.sled_client(&sled.id()).await }); + let vpc_to_sleds = + self.db_datastore.vpc_resolve_to_sleds(&vpc.id()).await?; + let mut sled_requests = Vec::with_capacity(vpc_to_sleds.len()); + for sled in &vpc_to_sleds { + let sled_id = sled.id(); + let vpc_id = vpc.id(); + let sled_rules_request = sled_rules_request.clone(); + sled_requests.push(async move { + self.sled_client(&sled_id) + .await? + .vpc_firewall_rules_put(&vpc_id, &sled_rules_request) + .await + }); + } + let results = join_all(sled_requests).await; + // TODO-correctness: Actually do something about the failures here + for (sled, result) in vpc_to_sleds.iter().zip(results) { + if let Err(e) = result { + warn!(self.log, "failed to update firewall rules on sled agent"; + "sled_id" => %sled.id(), + "vpc_id" => %vpc.id(), + "error" => %e); + } + } - let result = rules.into_iter().map(|rule| rule.into()).collect(); - Ok(result) + Ok(()) } // TODO: move this somewhere else @@ -1708,7 +1741,7 @@ impl Nexus { &self, vpc: &db::model::Vpc, rules: &Vec, - ) -> Result, Error> { + ) -> Result, Error> { // Gather list of Instances and Subnets to resolve let mut instances = HashSet::new(); let mut subnets = HashSet::new(); @@ -1781,7 +1814,7 @@ impl Nexus { }) .collect(); - let mut opte_rules = Vec::with_capacity(rules.len()); + let mut sled_agent_rules = Vec::with_capacity(rules.len()); for rule in rules { let mut targets = Vec::with_capacity(rule.targets.len()); for target in &rule.targets { @@ -1795,30 +1828,28 @@ impl Nexus { // with the semantics of the rules. Rules with bad // references should likely at least be flagged to users external::VpcFirewallRuleTarget::Instance(name) => { - targets.extend_from_slice( - instance_interfaces - .get(&name) - .ok_or_else(|| { - Error::not_found_by_name( - ResourceType::Instance, - &name, - ) - })? - .as_slice(), - ); + for interface in + instance_interfaces.get(&name).ok_or_else(|| { + Error::not_found_by_name( + ResourceType::Instance, + &name, + ) + })? + { + targets.push(interface.into()); + } } external::VpcFirewallRuleTarget::Subnet(name) => { - targets.extend_from_slice( - subnet_interfaces - .get(&name) - .ok_or_else(|| { - Error::not_found_by_name( - ResourceType::VpcSubnet, - &name, - ) - })? - .as_slice(), - ); + for interface in + subnet_interfaces.get(&name).ok_or_else(|| { + Error::not_found_by_name( + ResourceType::VpcSubnet, + &name, + ) + })? + { + targets.push(interface.into()); + } } // TODO: How do we resolve VPC targets? external::VpcFirewallRuleTarget::Vpc(_name) => (), @@ -1839,19 +1870,21 @@ impl Nexus { &name, ) })? { - host_addrs.push(interface.ip.into()); + host_addrs.push(ipnetwork::IpNetwork::from(interface.ip).into()); } } external::VpcFirewallRuleHostFilter::Subnet(name) => { - host_addrs.extend_from_slice(subnet_networks.get(&name).ok_or_else(|| { + for subnet in subnet_networks.get(&name).ok_or_else(|| { Error::not_found_by_name( ResourceType::VpcSubnet, &name, ) - })?.as_slice()); + })? { + host_addrs.push(subnet.clone().into()); + } } external::VpcFirewallRuleHostFilter::Ip(addr) => { - host_addrs.push(ipnetwork::IpNetwork::from(*addr)); + host_addrs.push(ipnetwork::IpNetwork::from(*addr).into()); } // TODO: How do we resolve VPC targets? external::VpcFirewallRuleHostFilter::Vpc(_name) => { @@ -1866,18 +1899,23 @@ impl Nexus { } }; - opte_rules.push(OpteFirewallRule { - status: rule.status, - direction: rule.direction, - targets: targets, - filter_hosts: filter_hosts, - filter_ports: rule.filter_ports.clone(), - filter_protocols: rule.filter_protocols.clone(), - action: rule.action, - priority: rule.priority, + sled_agent_rules.push(sled_agent_client::types::VpcFirewallRule { + status: rule.status.0.into(), + direction: rule.direction.0.into(), + targets, + filter_hosts, + filter_ports: rule + .filter_ports + .as_ref() + .map(|ports| ports.iter().map(|v| v.0.into()).collect()), + filter_protocols: rule.filter_protocols.as_ref().map( + |protocols| protocols.iter().map(|v| v.0.into()).collect(), + ), + action: rule.action.0.into(), + priority: rule.priority.0 .0, }); } - Ok(opte_rules) + Ok(sled_agent_rules) } pub async fn vpc_list_subnets( @@ -2569,16 +2607,3 @@ lazy_static! { } }"#).unwrap(); } - -// TODO: move this somewhere else -pub struct OpteFirewallRule { - pub status: crate::db::model::VpcFirewallRuleStatus, - pub direction: crate::db::model::VpcFirewallRuleDirection, - pub targets: Vec, - pub filter_hosts: Option>, - pub filter_ports: Option>, - pub filter_protocols: - Option>, - pub action: crate::db::model::VpcFirewallRuleAction, - pub priority: crate::db::model::VpcFirewallRulePriority, -} diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index ec525b2b23c..a426065088a 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -87,6 +87,38 @@ } } } + }, + "/vpc/{vpc_id}/firewall/rules": { + "put": { + "operationId": "vpc_firewall_rules_put", + "parameters": [ + { + "in": "path", + "name": "vpc_id", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/VpcFirewallRulesEnsureBody" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + } + } + } } }, "components": { @@ -547,6 +579,18 @@ "destroyed" ] }, + "IpNetwork": { + "description": "IPv4 (in dotted quad) or IPv6address, followed by a slash and prefix length", + "type": "string" + }, + "L4PortRange": { + "title": "A range of IP ports", + "description": "An inclusive-inclusive range of IP ports. The second port may be omitted to represent a single port", + "type": "string", + "pattern": "^[0-9]{1,5}(-[0-9]{1,5})?$", + "minLength": 1, + "maxLength": 11 + }, "MacAddr": { "title": "A MAC address", "description": "A Media Access Control address, in EUI-48 format", @@ -611,6 +655,105 @@ "subnet_id", "vpc_id" ] + }, + "VpcFirewallRule": { + "description": "VPC firewall rule after object name resolution has been performed by Nexus", + "type": "object", + "properties": { + "action": { + "$ref": "#/components/schemas/VpcFirewallRuleAction" + }, + "direction": { + "$ref": "#/components/schemas/VpcFirewallRuleDirection" + }, + "filter_hosts": { + "nullable": true, + "type": "array", + "items": { + "$ref": "#/components/schemas/IpNetwork" + } + }, + "filter_ports": { + "nullable": true, + "type": "array", + "items": { + "$ref": "#/components/schemas/L4PortRange" + } + }, + "filter_protocols": { + "nullable": true, + "type": "array", + "items": { + "$ref": "#/components/schemas/VpcFirewallRuleProtocol" + } + }, + "priority": { + "type": "integer", + "format": "uint16", + "minimum": 0 + }, + "status": { + "$ref": "#/components/schemas/VpcFirewallRuleStatus" + }, + "targets": { + "type": "array", + "items": { + "$ref": "#/components/schemas/NetworkInterface" + } + } + }, + "required": [ + "action", + "direction", + "priority", + "status", + "targets" + ] + }, + "VpcFirewallRuleAction": { + "type": "string", + "enum": [ + "allow", + "deny" + ] + }, + "VpcFirewallRuleDirection": { + "type": "string", + "enum": [ + "inbound", + "outbound" + ] + }, + "VpcFirewallRuleProtocol": { + "description": "The protocols that may be specified in a firewall rule's filter", + "type": "string", + "enum": [ + "TCP", + "UDP", + "ICMP" + ] + }, + "VpcFirewallRuleStatus": { + "type": "string", + "enum": [ + "disabled", + "enabled" + ] + }, + "VpcFirewallRulesEnsureBody": { + "description": "Sent to a sled agent to establish the current firewall rules for a VPC", + "type": "object", + "properties": { + "rules": { + "type": "array", + "items": { + "$ref": "#/components/schemas/VpcFirewallRule" + } + } + }, + "required": [ + "rules" + ] } } } diff --git a/sled-agent-client/Cargo.toml b/sled-agent-client/Cargo.toml index 2f233c4ebdc..4066339e3d3 100644 --- a/sled-agent-client/Cargo.toml +++ b/sled-agent-client/Cargo.toml @@ -10,6 +10,7 @@ async-trait = "0.1" progenitor = { git = "https://github.com/oxidecomputer/progenitor" } reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] } percent-encoding = "2.1.0" +ipnetwork = "0.18" [dependencies.chrono] version = "0.4" diff --git a/sled-agent-client/src/lib.rs b/sled-agent-client/src/lib.rs index c210c7d190c..86ab54aa31f 100644 --- a/sled-agent-client/src/lib.rs +++ b/sled-agent-client/src/lib.rs @@ -284,6 +284,83 @@ impl From for types::MacAddr { Self(s.0.to_string()) } } + +impl From for types::IpNetwork { + fn from(s: ipnetwork::IpNetwork) -> Self { + Self(s.to_string()) + } +} + +impl From for types::L4PortRange { + fn from(s: omicron_common::api::external::L4PortRange) -> Self { + Self(s.to_string()) + } +} + +impl From + for types::VpcFirewallRuleAction +{ + fn from(s: omicron_common::api::external::VpcFirewallRuleAction) -> Self { + match s { + omicron_common::api::external::VpcFirewallRuleAction::Allow => { + Self::Allow + } + omicron_common::api::external::VpcFirewallRuleAction::Deny => { + Self::Deny + } + } + } +} + +impl From + for types::VpcFirewallRuleDirection +{ + fn from( + s: omicron_common::api::external::VpcFirewallRuleDirection, + ) -> Self { + match s { + omicron_common::api::external::VpcFirewallRuleDirection::Inbound => { + Self::Inbound + } + omicron_common::api::external::VpcFirewallRuleDirection::Outbound => { + Self::Outbound + } + } + } +} + +impl From + for types::VpcFirewallRuleStatus +{ + fn from(s: omicron_common::api::external::VpcFirewallRuleStatus) -> Self { + match s { + omicron_common::api::external::VpcFirewallRuleStatus::Enabled => { + Self::Enabled + } + omicron_common::api::external::VpcFirewallRuleStatus::Disabled => { + Self::Disabled + } + } + } +} + +impl From + for types::VpcFirewallRuleProtocol +{ + fn from(s: omicron_common::api::external::VpcFirewallRuleProtocol) -> Self { + match s { + omicron_common::api::external::VpcFirewallRuleProtocol::Tcp => { + Self::Tcp + } + omicron_common::api::external::VpcFirewallRuleProtocol::Udp => { + Self::Udp + } + omicron_common::api::external::VpcFirewallRuleProtocol::Icmp => { + Self::Icmp + } + } + } +} /** * Exposes additional [`Client`] interfaces for use by the test suite. These * are bonus endpoints, not generated in the real client. diff --git a/sled-agent/src/http_entrypoints.rs b/sled-agent/src/http_entrypoints.rs index 718dc7b89a8..ffdcb4811ae 100644 --- a/sled-agent/src/http_entrypoints.rs +++ b/sled-agent/src/http_entrypoints.rs @@ -4,11 +4,12 @@ //! HTTP entrypoint functions for the sled agent's exposed API -use super::params::DiskEnsureBody; +use super::params::{DiskEnsureBody, VpcFirewallRulesEnsureBody}; use dropshot::endpoint; use dropshot::ApiDescription; use dropshot::HttpError; use dropshot::HttpResponseOk; +use dropshot::HttpResponseUpdatedNoContent; use dropshot::Path; use dropshot::RequestContext; use dropshot::TypedBody; @@ -29,6 +30,7 @@ pub fn api() -> SledApiDescription { fn register_endpoints(api: &mut SledApiDescription) -> Result<(), String> { api.register(instance_put)?; api.register(disk_put)?; + api.register(vpc_firewall_rules_put)?; Ok(()) } @@ -90,3 +92,29 @@ async fn disk_put( .await?, )) } + +/// Path parameters for VPC requests (sled agent API) +#[derive(Deserialize, JsonSchema)] +struct VpcPathParam { + vpc_id: Uuid, +} + +// TODO: Evaluate whether we want to keep this endpoint as is. This was +// developed ad-hoc in preparation for the Milestone 3 demo. +#[endpoint { + method = PUT, + path = "/vpc/{vpc_id}/firewall/rules", +}] +async fn vpc_firewall_rules_put( + rqctx: Arc>, + path_params: Path, + body: TypedBody, +) -> Result { + let _sa = rqctx.context(); + let _vpc_id = path_params.into_inner().vpc_id; + let _body_args = body.into_inner(); + + // TODO: Actually send the firewall rules to OPTE + + Ok(HttpResponseUpdatedNoContent()) +} diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index 993b31b6328..7946d01f51d 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -2,6 +2,11 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +use omicron_common::api::external::{ + L4PortRange, NetworkInterface, VpcFirewallRuleAction, + VpcFirewallRuleDirection, VpcFirewallRulePriority, VpcFirewallRuleProtocol, + VpcFirewallRuleStatus, +}; use omicron_common::api::internal::nexus::DiskRuntimeState; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -39,3 +44,28 @@ pub struct DiskEnsureBody { /// requested runtime state of the Disk pub target: DiskStateRequested, } + +/// Sent to a sled agent to establish the current firewall rules for a VPC +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct VpcFirewallRulesEnsureBody { + pub rules: Vec, +} + +/// VPC firewall rule after object name resolution has been performed by Nexus +#[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] +pub struct VpcFirewallRule { + pub status: VpcFirewallRuleStatus, + pub direction: VpcFirewallRuleDirection, + pub targets: Vec, + #[schemars(with = "Option>")] + pub filter_hosts: Option>, + pub filter_ports: Option>, + pub filter_protocols: Option>, + pub action: VpcFirewallRuleAction, + pub priority: VpcFirewallRulePriority, +} + +#[derive(JsonSchema)] +#[serde(remote = "ipnetwork::IpNetwork")] +/// IPv4 (in dotted quad) or IPv6address, followed by a slash and prefix length +struct IpNetworkDef(String); diff --git a/sled-agent/src/sim/http_entrypoints.rs b/sled-agent/src/sim/http_entrypoints.rs index f90cdf5af30..6e589966b14 100644 --- a/sled-agent/src/sim/http_entrypoints.rs +++ b/sled-agent/src/sim/http_entrypoints.rs @@ -6,7 +6,7 @@ * HTTP entrypoint functions for the sled agent's exposed API */ -use crate::params::DiskEnsureBody; +use crate::params::{DiskEnsureBody, VpcFirewallRulesEnsureBody}; use dropshot::endpoint; use dropshot::ApiDescription; use dropshot::HttpError; @@ -36,6 +36,7 @@ pub fn api() -> SledApiDescription { api.register(instance_poke_post)?; api.register(disk_put)?; api.register(disk_poke_post)?; + api.register(vpc_firewall_rules_put)?; Ok(()) } @@ -129,3 +130,29 @@ async fn disk_poke_post( sa.disk_poke(disk_id).await; Ok(HttpResponseUpdatedNoContent()) } + +/// Path parameters for VPC requests (sled agent API) +#[derive(Deserialize, JsonSchema)] +struct VpcPathParam { + vpc_id: Uuid, +} + +// TODO: Evaluate whether we want to keep this endpoint as is. This was +// developed ad-hoc in preparation for the Milestone 3 demo. +#[endpoint { + method = PUT, + path = "/vpc/{vpc_id}/firewall/rules", +}] +async fn vpc_firewall_rules_put( + rqctx: Arc>>, + path_params: Path, + body: TypedBody, +) -> Result { + let _sa = rqctx.context(); + let _vpc_id = path_params.into_inner().vpc_id; + let _body_args = body.into_inner(); + + // TODO: Actually send the firewall rules to OPTE + + Ok(HttpResponseUpdatedNoContent()) +} From 9e5571768214bbdc861ef2baca630259b95f857d Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Wed, 24 Nov 2021 13:26:17 -0800 Subject: [PATCH 08/16] Add indices --- common/src/sql/dbinit.sql | 5 +++++ nexus/src/db/datastore.rs | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 0405fb1cffc..767222355ec 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -438,6 +438,11 @@ CREATE UNIQUE INDEX ON omicron.public.network_interface ( ) WHERE time_deleted IS NULL; +CREATE INDEX ON omicron.public.network_interface ( + instance_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 a2a016d9504..c7fa9287daa 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -852,7 +852,7 @@ impl DataStore { instance_names: T, ) -> Result>, Error> { use db::schema::{instance, network_interface}; - // TODO: make sure there's an index on network_interface::instance_id + // TODO-performance: paginate the results of this query? let ifaces = network_interface::table .inner_join( instance::table @@ -888,7 +888,7 @@ impl DataStore { subnet_names: T, ) -> Result>, Error> { use db::schema::{network_interface, vpc_subnet}; - // TODO: make sure there's an index on network_interface::subnet_id + // TODO-performance: paginate the results of this query? let subnets = network_interface::table .inner_join( vpc_subnet::table @@ -924,6 +924,7 @@ impl DataStore { subnet_names: T, ) -> Result>, Error> { use db::schema::vpc_subnet; + // TODO-performance: paginate the results of this query? let subnets = vpc_subnet::table .select(VpcSubnet::as_select()) .filter(vpc_subnet::vpc_id.eq(vpc.id())) From 2820ec213f94674abd700210324a4ad8bc63e987 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Thu, 2 Dec 2021 12:11:46 -0800 Subject: [PATCH 09/16] Add support for VPC targets/filters and fix bad merge After the merge, active_propolis_id was used instead of active_server_id --- nexus/src/db/datastore.rs | 37 +++++++++++++++++++++++--- nexus/src/nexus.rs | 56 +++++++++++++++++++++++++++++++++------ 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index c7fa9287daa..46d94450b66 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -916,6 +916,39 @@ impl DataStore { Ok(result) } + /// Identify all VNICs connected to each Vpc + // TODO: how to name/where to put this + pub async fn resolve_vpcs_to_interfaces>( + &self, + project_id: &Uuid, + vpc_names: T, + ) -> Result>, Error> { + use db::schema::{network_interface, vpc}; + // TODO-performance: paginate the results of this query? + let interfaces = network_interface::table + .inner_join(vpc::table.on(vpc::id.eq(network_interface::vpc_id))) + .select((vpc::name, NetworkInterface::as_select())) + .filter(vpc::project_id.eq(*project_id)) + .filter(vpc::name.eq_any(vpc_names)) + .filter(network_interface::time_deleted.is_null()) + .filter(vpc::time_deleted.is_null()) + .get_results_async(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ResourceType::Vpc, + LookupType::Other("Resolving to interfaces".to_string()), + ) + })?; + let mut result = HashMap::with_capacity(interfaces.len()); + for (name, interface) in interfaces.into_iter() { + let entry = result.entry(name).or_insert(vec![]); + entry.push(interface); + } + Ok(result) + } + /// Identify all subnets in use by each VpcSubnet // TODO: how to name/where to put this pub async fn resolve_subnets_to_ips>( @@ -1709,9 +1742,7 @@ impl DataStore { instance::table .on(instance::id.eq(network_interface::instance_id)), ) - .inner_join( - sled::table.on(sled::id.eq(instance::active_propolis_id)), - ) + .inner_join(sled::table.on(sled::id.eq(instance::active_server_id))) .filter(network_interface::vpc_id.eq(*vpc_id)) .filter(network_interface::time_deleted.is_null()) .filter(instance::time_deleted.is_null()) diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 7b1757b8693..dfa5fc25b8f 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -1745,6 +1745,7 @@ impl Nexus { // Gather list of Instances and Subnets to resolve let mut instances = HashSet::new(); let mut subnets = HashSet::new(); + let mut vpcs = HashSet::new(); for rule in rules { for target in &rule.targets { match &target.0 { @@ -1754,8 +1755,14 @@ impl Nexus { external::VpcFirewallRuleTarget::Subnet(name) => { subnets.insert(name.clone().into()); } - // TODO: How do we resolve VPC targets? - external::VpcFirewallRuleTarget::Vpc(name) => (), + external::VpcFirewallRuleTarget::Vpc(name) => { + if *name != vpc.name().0 { + return Err(Error::InvalidRequest { + message: "firewall target ".to_string(), + }); + } + vpcs.insert(name.clone().into()); + } } } @@ -1768,9 +1775,16 @@ impl Nexus { subnets.insert(name.clone().into()); } // We don't need to resolve anything for Ip - external::VpcFirewallRuleHostFilter::Ip(addr) => (), + external::VpcFirewallRuleHostFilter::Ip(_) => (), // TODO: How do we resolve VPC targets? - external::VpcFirewallRuleHostFilter::Vpc(name) => (), + external::VpcFirewallRuleHostFilter::Vpc(name) => { + if *name != vpc.name().0 { + return Err(Error::InvalidRequest { + message: "firewall target ".to_string(), + }); + } + vpcs.insert(name.clone().into()); + } // TODO: How do we resolve InternetGateway targets? external::VpcFirewallRuleHostFilter::InternetGateway( name, @@ -1813,6 +1827,15 @@ impl Nexus { (name.0, v.into_iter().map(|iface| iface.into()).collect()) }) .collect(); + let vpc_interfaces: HashMap> = + self.db_datastore + .resolve_vpcs_to_interfaces(&vpc.project_id, vpcs) + .await? + .into_iter() + .map(|(name, v)| { + (name.0, v.into_iter().map(|iface| iface.into()).collect()) + }) + .collect(); let mut sled_agent_rules = Vec::with_capacity(rules.len()); for rule in rules { @@ -1851,8 +1874,18 @@ impl Nexus { targets.push(interface.into()); } } - // TODO: How do we resolve VPC targets? - external::VpcFirewallRuleTarget::Vpc(_name) => (), + external::VpcFirewallRuleTarget::Vpc(name) => { + for interface in + vpc_interfaces.get(&name).ok_or_else(|| { + Error::not_found_by_name( + ResourceType::Vpc, + &name, + ) + })? + { + targets.push(interface.into()); + } + } }; } @@ -1886,8 +1919,15 @@ impl Nexus { external::VpcFirewallRuleHostFilter::Ip(addr) => { host_addrs.push(ipnetwork::IpNetwork::from(*addr).into()); } - // TODO: How do we resolve VPC targets? - external::VpcFirewallRuleHostFilter::Vpc(_name) => { + external::VpcFirewallRuleHostFilter::Vpc(name) => { + for interface in vpc_interfaces.get(&name).ok_or_else(|| { + Error::not_found_by_name( + ResourceType::Vpc, + &name, + ) + })? { + host_addrs.push(ipnetwork::IpNetwork::from(interface.ip).into()); + } } // TODO: How do we resolve InternetGateway targets? external::VpcFirewallRuleHostFilter::InternetGateway( From 2cbb97782afc25be46a46c00600d70dbd3977895 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Fri, 3 Dec 2021 17:06:31 -0800 Subject: [PATCH 10/16] Add basic tests for the allocation query generation --- nexus/src/db/datastore.rs | 1 + nexus/src/db/model.rs | 2 +- nexus/src/db/subnet_allocation.rs | 183 +++++++++++++++++++++++++----- 3 files changed, 159 insertions(+), 27 deletions(-) diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index f457782e9db..1030a87b69c 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -1099,6 +1099,7 @@ impl DataStore { let allocation_query = AllocateIpQuery { block: ipnetwork::IpNetwork::V4(block.0 .0), interface, + now: Utc::now(), }; diesel::insert_into(dsl::network_interface) .values(allocation_query) diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 2a9c095f987..2911a4d9a87 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -1757,7 +1757,7 @@ impl IncompleteNetworkInterface { pub fn new( interface_id: Uuid, instance_id: Uuid, - vpc_id: uuid::Uuid, + vpc_id: Uuid, subnet: VpcSubnet, mac: MacAddr, ip: Option, diff --git a/nexus/src/db/subnet_allocation.rs b/nexus/src/db/subnet_allocation.rs index d098e49260e..0d957b47675 100644 --- a/nexus/src/db/subnet_allocation.rs +++ b/nexus/src/db/subnet_allocation.rs @@ -7,7 +7,7 @@ use crate::db; use crate::db::identity::Resource; use crate::db::model::IncompleteNetworkInterface; -use chrono::Utc; +use chrono::{DateTime, Utc}; use diesel::pg::Pg; use diesel::prelude::*; use diesel::query_builder::*; @@ -31,6 +31,7 @@ use uuid::Uuid; 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 @@ -65,7 +66,6 @@ impl QueryFragment for AllocateIpQuery { i64::try_from(network.size() - 1).unwrap_or(i64::MAX) } }; - let now = Utc::now(); out.push_sql("SELECT "); @@ -74,57 +74,57 @@ impl QueryFragment for AllocateIpQuery { )?; out.push_sql(" AS "); out.push_identifier(dsl::id::NAME)?; - out.push_sql(","); + 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_sql(", "); out.push_bind_param::( &self.interface.identity.description, )?; out.push_sql(" AS "); out.push_identifier(dsl::description::NAME)?; - out.push_sql(","); + out.push_sql(", "); - out.push_bind_param::(&now)?; + out.push_bind_param::(&self.now)?; out.push_sql(" AS "); out.push_identifier(dsl::time_created::NAME)?; - out.push_sql(","); + out.push_sql(", "); - out.push_bind_param::(&now)?; + out.push_bind_param::(&self.now)?; out.push_sql(" AS "); out.push_identifier(dsl::time_modified::NAME)?; - out.push_sql(","); + 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_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_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_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_sql(", "); out.push_bind_param::( &self.block.network().into(), @@ -149,15 +149,15 @@ impl QueryFragment for AllocateIpQuery { // (, + off, TRUE) out.push_sql(" ON ("); out.push_identifier(dsl::subnet_id::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::ip::NAME)?; - out.push_sql(","); + 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_sql(", "); out.push_bind_param::( &self.block.network().into(), )?; @@ -167,7 +167,7 @@ impl QueryFragment for AllocateIpQuery { // WHERE ip IS NULL LIMIT 1; out.push_sql("WHERE "); out.push_identifier(dsl::ip::NAME)?; - out.push_sql("IS NULL LIMIT 1"); + out.push_sql(" IS NULL LIMIT 1"); Ok(()) } } @@ -188,25 +188,156 @@ impl QueryFragment for AllocateIpQueryValues { use db::schema::network_interface::dsl; out.push_sql("("); out.push_identifier(dsl::id::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::name::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::description::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::time_created::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::time_modified::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::instance_id::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::vpc_id::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::subnet_id::NAME)?; - out.push_sql(","); + out.push_sql(", "); out.push_identifier(dsl::mac::NAME)?; - out.push_sql(","); + 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 crate::db::model::{ + 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 omicron_common::api::external::{ + IdentityMetadataCreateParams, Ipv4Net, MacAddr, + }; + use std::convert::TryInto; + + #[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 block: ipnetwork::Ipv4Network = "192.168.1.0/24".parse().unwrap(); + let subnet = VpcSubnet::new( + subnet_id, + vpc_id, + params::VpcSubnetCreate { + identity: IdentityMetadataCreateParams { + name: "test-subnet".to_string().try_into().unwrap(), + description: "subnet description".to_string(), + }, + ipv4_block: Some(Ipv4Net(block.clone()).into()), + ipv6_block: None, + }, + ); + 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: 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(1, $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(1, $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); + } +} From 347e66ea48cea3f8db60e94c27de2d8917b629a3 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Mon, 6 Dec 2021 11:40:20 -0800 Subject: [PATCH 11/16] Make code match RFD174 --- common/src/sql/dbinit.sql | 9 +++++++++ nexus/src/db/datastore.rs | 17 +++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 0405fb1cffc..ee29c0d2f78 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -438,6 +438,15 @@ CREATE UNIQUE INDEX ON omicron.public.network_interface ( ) WHERE time_deleted IS NULL; +/* Ensure we do not assign the same MAC twice within a VPC + * See RFD174's discussion on the scope of virtual MACs + */ +CREATE UNIQUE INDEX ON omicron.public.network_interface ( + vpc_id, + mac +) 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 1030a87b69c..d574c9b679e 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -1054,16 +1054,17 @@ impl DataStore { * Generate a unique MAC address for an interface */ pub fn generate_mac_address(&self) -> Result { - // Bitmasks defined in RFC7042 section 2.1 - const LOCALLY_ADMINISTERED: u8 = 0b1; - const MULTICAST: u8 = 0b10; - - // Generate a unique address from the locally administered space. use rand::Fill; - let mut addr = [0u8; 6]; - addr.try_fill(&mut StdRng::from_entropy()) + // Use the Oxide OUI A8 40 25 + let mut addr = [0xA8, 0x40, 0x25, 0x00, 0x00, 0x00]; + addr[3..] + .try_fill(&mut StdRng::from_entropy()) .map_err(|_| Error::internal_error("failed to generate MAC"))?; - addr[0] = (addr[0] & !MULTICAST) | LOCALLY_ADMINISTERED; + // Oxide virtual MACs are constrained to have these bits set. + addr[3] |= 0xF0; + // TODO-correctness: We should use an explicit allocator for the MACs + // given the small address space. Right now creation requests may fail + // due to MAC collision, especially given the 20-bit space. Ok(MacAddr(macaddr::MacAddr6::from(addr)).into()) } From 556c672c067719631512cc7940ea3a385bc03852 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 7 Dec 2021 14:51:57 -0800 Subject: [PATCH 12/16] Implement a basic /vpcs//subnets//ips endpoint This is an API we intend to have, but it might not look like this. Having such an API will be useful for testing the network interface logic though. --- common/src/api/external/mod.rs | 2 +- nexus/src/db/datastore.rs | 22 +++ nexus/src/external_api/http_entrypoints.rs | 41 +++++ nexus/src/nexus.rs | 21 +++ nexus/src/sagas.rs | 1 - openapi/nexus.json | 166 +++++++++++++++++++++ tools/oxapi_demo | 29 +++- 7 files changed, 279 insertions(+), 3 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 79d727d7281..a6211a87bee 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1829,7 +1829,7 @@ impl JsonSchema for MacAddr { } /// A `NetworkInterface` represents a virtual network interface device. -#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] +#[derive(ObjectIdentity, Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct NetworkInterface { /** common identifying metadata */ pub identity: IdentityMetadata, diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index d574c9b679e..f094ac26005 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -1699,6 +1699,28 @@ impl DataStore { Ok(()) } + pub async fn subnet_list_network_interfaces( + &self, + subnet_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::subnet_id.eq(*subnet_id)) + .select(NetworkInterface::as_select()) + .load_async::(self.pool()) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ResourceType::NetworkInterface, + LookupType::Other("Listing All".to_string()), + ) + }) + } + pub async fn vpc_list_routers( &self, vpc_id: &Uuid, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 97b82d31ef4..1ef8ff12343 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -44,6 +44,7 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Disk; use omicron_common::api::external::DiskAttachment; use omicron_common::api::external::Instance; +use omicron_common::api::external::NetworkInterface; use omicron_common::api::external::PaginationOrder; use omicron_common::api::external::RouterRoute; use omicron_common::api::external::RouterRouteCreateParams; @@ -111,6 +112,8 @@ pub fn external_api() -> NexusApiDescription { api.register(vpc_subnets_delete_subnet)?; api.register(vpc_subnets_put_subnet)?; + api.register(subnets_ips_get)?; + api.register(vpc_routers_get)?; api.register(vpc_routers_get_router)?; api.register(vpc_routers_post)?; @@ -1344,6 +1347,44 @@ 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. +#[endpoint { + method = GET, + path = "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/ips", + }] +async fn subnets_ips_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 handler = async { + let interfaces = nexus + .subnet_list_network_interfaces( + &path.organization_name, + &path.project_name, + &path.vpc_name, + &path.subnet_name, + &data_page_params_for(&rqctx, &query)? + .map_name(|n| Name::ref_cast(n)), + ) + .await? + .into_iter() + .map(|interfaces| interfaces.into()) + .collect(); + Ok(HttpResponseOk(ScanByName::results_page(&query, interfaces)?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /* * VPC Firewalls */ diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index f4e08cbbe0f..36995a4cdc7 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -1782,6 +1782,27 @@ impl Nexus { .await?) } + pub async fn subnet_list_network_interfaces( + &self, + organization_name: &Name, + project_name: &Name, + vpc_name: &Name, + subnet_name: &Name, + pagparams: &DataPageParams<'_, Name>, + ) -> ListResultVec { + let subnet = self + .vpc_lookup_subnet( + organization_name, + project_name, + vpc_name, + subnet_name, + ) + .await?; + self.db_datastore + .subnet_list_network_interfaces(&subnet.id(), pagparams) + .await + } + pub async fn vpc_list_routers( &self, organization_name: &Name, diff --git a/nexus/src/sagas.rs b/nexus/src/sagas.rs index 91c9872e466..62b7aacbbc3 100644 --- a/nexus/src/sagas.rs +++ b/nexus/src/sagas.rs @@ -216,7 +216,6 @@ async fn sic_create_instance_record( .await .map_err(ActionError::action_failed)?; - // TODO: Populate this with an appropriate NIC. // See also: instance_set_runtime in nexus.rs for a similar construction. Ok(InstanceHardware { runtime: instance.runtime().clone().into(), diff --git a/openapi/nexus.json b/openapi/nexus.json index f139df5cc0c..050e9796395 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -2623,6 +2623,93 @@ } } }, + "/organizations/{organization_name}/projects/{project_name}/vpcs/{vpc_name}/subnets/{subnet_name}/ips": { + "get": { + "description": "List IP addresses on a VPC subnet.", + "operationId": "subnets_ips_get", + "parameters": [ + { + "in": "query", + "name": "limit", + "schema": { + "nullable": true, + "description": "Maximum number of items returned by a single call", + "type": "integer", + "format": "uint32", + "minimum": 1 + }, + "style": "form" + }, + { + "in": "query", + "name": "page_token", + "schema": { + "nullable": true, + "description": "Token returned by previous call to retreive the subsequent page", + "type": "string" + }, + "style": "form" + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameSortMode" + }, + "style": "form" + }, + { + "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" + }, + { + "in": "path", + "name": "subnet_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + }, + { + "in": "path", + "name": "vpc_name", + "required": true, + "schema": { + "$ref": "#/components/schemas/Name" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/NetworkInterfaceResultsPage" + } + } + } + } + }, + "x-dropshot-pagination": true + } + }, "/sagas": { "get": { "description": "List all sagas (for debugging)", @@ -3199,6 +3286,14 @@ "username" ] }, + "MacAddr": { + "title": "A MAC address", + "description": "A Media Access Control address, in EUI-48 format", + "type": "string", + "pattern": "^([0-9a-fA-F]{2}:){5}[0-9a-fA-F]{2}$", + "minLength": 17, + "maxLength": 17 + }, "Name": { "title": "A name used in the API", "description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'.", @@ -3206,6 +3301,77 @@ "pattern": "[a-z](|[a-zA-Z0-9-]*[a-zA-Z0-9])", "maxLength": 63 }, + "NetworkInterface": { + "description": "A `NetworkInterface` represents a virtual network interface device.", + "type": "object", + "properties": { + "identity": { + "description": "common identifying metadata", + "allOf": [ + { + "$ref": "#/components/schemas/IdentityMetadata" + } + ] + }, + "instance_id": { + "description": "The Instance to which the interface belongs.", + "type": "string", + "format": "uuid" + }, + "ip": { + "description": "The IP address assigned to this interface.", + "type": "string", + "format": "ip" + }, + "mac": { + "description": "The MAC address assigned to this interface.", + "allOf": [ + { + "$ref": "#/components/schemas/MacAddr" + } + ] + }, + "subnet_id": { + "description": "The subnet to which the interface belongs.", + "type": "string", + "format": "uuid" + }, + "vpc_id": { + "description": "The VPC to which the interface belongs.", + "type": "string", + "format": "uuid" + } + }, + "required": [ + "identity", + "instance_id", + "ip", + "mac", + "subnet_id", + "vpc_id" + ] + }, + "NetworkInterfaceResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/NetworkInterface" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "Organization": { "description": "Client view of an [`Organization`]", "type": "object", diff --git a/tools/oxapi_demo b/tools/oxapi_demo index b54f613476a..4921de953bb 100755 --- a/tools/oxapi_demo +++ b/tools/oxapi_demo @@ -61,6 +61,15 @@ VPCS vpc_get ORGANIZATION_NAME PROJECT_NAME VPC_NAME vpc_delete ORGANIZATION_NAME PROJECT_NAME VPC_NAME +VPC SUBNETS + + vpc_subnets_list ORGANIZATION_NAME PROJECT_NAME VPC_NAME + vpc_subnet_get ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME + +VPC SUBNETS IPS + + subnet_ips_list ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME + VPC FIREWALL vpc_firewall_rules_get ORGANIZATION_NAME PROJECT_NAME VPC_NAME @@ -340,10 +349,28 @@ function cmd_vpc_get function cmd_vpc_delete { - [[ $# != 2 ]] && usage "expected PROJECT_NAME VPC_NAME" + [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME" do_curl "/organizations/$1/projects/$2/vpcs/$3" -X DELETE } +function cmd_vpc_subnets_list +{ + [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME" + do_curl "/organizations/$1/projects/$2/vpcs/$3/subnets" +} + +function cmd_vpc_subnet_get +{ + [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME" + do_curl "/organizations/$1/projects/$2/vpcs/$3/subnets/$4" +} + +function cmd_subnet_ips_list +{ + [[ $# != 4 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME SUBNET_NAME" + do_curl "/organizations/$1/projects/$2/vpcs/$3/subnets/$4/ips" +} + function cmd_vpc_firewall_rules_get { [[ $# != 3 ]] && usage "expected ORGANIZATION_NAME PROJECT_NAME VPC_NAME" From b323cd9697069899440870002f9b248d533e013b Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 7 Dec 2021 15:13:19 -0800 Subject: [PATCH 13/16] Verify that new instances get a network interface --- nexus/tests/test_instances.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/nexus/tests/test_instances.rs b/nexus/tests/test_instances.rs index 305dc013846..b95b5355ae7 100644 --- a/nexus/tests/test_instances.rs +++ b/nexus/tests/test_instances.rs @@ -13,6 +13,7 @@ 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::NetworkInterface; use omicron_nexus::TestInterfaces as _; use omicron_nexus::{external_api::params, Nexus}; use sled_agent_client::TestInterfaces as _; @@ -135,6 +136,17 @@ async fn test_instances_create_reboot_halt() { instances_eq(&instances[0], &instance); assert_eq!(instance.runtime.run_state, InstanceState::Starting); + /* Check that the instance got a network interface */ + let ips_url = format!( + "/organizations/{}/projects/{}/vpcs/default/subnets/default/ips", + 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, instance.identity.name); + /* * Now, simulate completion of instance boot and check the state reported. */ From b04d5d9741c40696c65fb879b7c7b3a53f606795 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 7 Dec 2021 17:36:47 -0800 Subject: [PATCH 14/16] Introduce a test for IP address allocation --- nexus/src/db/datastore.rs | 40 ++++++-- nexus/tests/test_subnet_allocation.rs | 137 ++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 10 deletions(-) create mode 100644 nexus/tests/test_subnet_allocation.rs diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index f094ac26005..7fd66c526c8 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -30,7 +30,10 @@ use super::identity::{Asset, Resource}; use super::Pool; use crate::authz; use crate::context::OpContext; -use async_bb8_diesel::{AsyncConnection, AsyncRunQueryDsl, ConnectionManager}; +use async_bb8_diesel::{ + AsyncConnection, AsyncRunQueryDsl, ConnectionError, ConnectionManager, + PoolError, +}; use chrono::Utc; use diesel::prelude::*; use diesel::upsert::excluded; @@ -1075,7 +1078,7 @@ impl DataStore { use db::schema::network_interface::dsl; let name = interface.identity.name.clone(); - let result = match interface.ip { + match interface.ip { // Attempt an insert with a requested IP address Some(ip) => { let row = NetworkInterface { @@ -1091,6 +1094,13 @@ impl DataStore { .returning(NetworkInterface::as_returning()) .get_result_async(self.pool()) .await + .map_err(|e| { + public_error_from_diesel_pool_create( + e, + ResourceType::NetworkInterface, + name.as_str(), + ) + }) } // Insert and allocate an IP address None => { @@ -1107,15 +1117,25 @@ impl DataStore { .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::not_found_other( + ResourceType::NetworkInterface, + "no available IP addresses".to_string(), + ) + } else { + public_error_from_diesel_pool_create( + e, + ResourceType::NetworkInterface, + name.as_str(), + ) + } + }) } - }; - result.map_err(|e| { - public_error_from_diesel_pool_create( - e, - ResourceType::NetworkInterface, - name.as_str(), - ) - }) + } } // Create a record for a new Oximeter instance diff --git a/nexus/tests/test_subnet_allocation.rs b/nexus/tests/test_subnet_allocation.rs new file mode 100644 index 00000000000..e8ae390b409 --- /dev/null +++ b/nexus/tests/test_subnet_allocation.rs @@ -0,0 +1,137 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +/*! + * Tests that subnet allocation will successfully allocate the entire space of a + * subnet and error appropriately when the space is exhausted. + */ + +use http::method::Method; +use http::StatusCode; +use omicron_common::api::external::{ + ByteCount, IdentityMetadataCreateParams, IdentityMetadataUpdateParams, + Instance, InstanceCpuCount, Ipv4Net, NetworkInterface, +}; +use omicron_nexus::external_api::params; +use std::net::IpAddr; + +use dropshot::test_util::objects_list_page; +use dropshot::test_util::objects_post; +use dropshot::test_util::ClientTestContext; +use dropshot::HttpErrorResponseBody; + +pub mod common; +use common::resource_helpers::{create_organization, create_project}; +use common::test_setup; + +async fn create_instance( + client: &ClientTestContext, + url_instances: &String, + name: &str, +) { + let new_instance = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: "".to_string(), + }, + ncpus: InstanceCpuCount(1), + memory: ByteCount::from_mebibytes_u32(256), + hostname: name.to_string(), + }; + objects_post::<_, Instance>(&client, url_instances, new_instance.clone()) + .await; +} + +async fn create_instance_expect_failure( + client: &ClientTestContext, + url_instances: &String, + name: &str, +) -> HttpErrorResponseBody { + let new_instance = params::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: "".to_string(), + }, + ncpus: InstanceCpuCount(1), + memory: ByteCount::from_mebibytes_u32(256), + hostname: name.to_string(), + }; + client + .make_request_error_body( + Method::POST, + &url_instances, + new_instance, + StatusCode::NOT_FOUND, + ) + .await +} + +#[tokio::test] +async fn test_subnet_allocation() { + let cptestctx = test_setup("test_subnet_allocation").await; + let client = &cptestctx.external_client; + + let organization_name = "test-org"; + let project_name = "springfield-squidport"; + + // Create a project that we'll use for testing. + create_organization(&client, organization_name).await; + create_project(&client, organization_name, project_name).await; + let url_instances = format!( + "/organizations/{}/projects/{}/instances", + organization_name, project_name + ); + + // Modify the default VPC to have a very small subnet so we don't need to + // issue many requests + let url_subnet = format!( + "/organizations/{}/projects/{}/vpcs/default/subnets/default", + organization_name, project_name + ); + let subnet = "192.168.42.0/30".parse().unwrap(); + let subnet_update = params::VpcSubnetUpdate { + identity: IdentityMetadataUpdateParams { + name: Some("default".parse().unwrap()), + description: None, + }, + ipv4_block: Some(Ipv4Net(subnet)), + ipv6_block: None, + }; + client + .make_request( + Method::PUT, + &url_subnet, + Some(subnet_update), + StatusCode::OK, + ) + .await + .unwrap(); + + // The valid addresses for allocation in `subnet` are 192.168.42.1 and + // 192.168.42.2. The rest are reserved as the network address and broadcast + // address. + create_instance(client, &url_instances, "i1").await; + create_instance(client, &url_instances, "i2").await; + + // 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"); + + // Verify the subnet lists the two addresses as in use + let url_ips = format!("{}/ips", url_subnet); + let network_interfaces = + objects_list_page::(client, &url_ips).await.items; + assert_eq!(network_interfaces.len(), 2); + assert_eq!( + network_interfaces[0].ip, + "192.168.42.1".parse::().unwrap() + ); + assert_eq!( + network_interfaces[1].ip, + "192.168.42.2".parse::().unwrap() + ); + + cptestctx.teardown().await; +} From dc15e54a4302e5267e3b46123977a78c9ee995c2 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Tue, 7 Dec 2021 17:45:37 -0800 Subject: [PATCH 15/16] Respect address reservations from RFD21 --- nexus/src/db/subnet_allocation.rs | 12 ++++++++---- nexus/tests/test_subnet_allocation.rs | 11 +++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/nexus/src/db/subnet_allocation.rs b/nexus/src/db/subnet_allocation.rs index 0d957b47675..5bf45c39d54 100644 --- a/nexus/src/db/subnet_allocation.rs +++ b/nexus/src/db/subnet_allocation.rs @@ -16,18 +16,22 @@ 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(1, ) AS off +/// 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. pub struct AllocateIpQuery { pub interface: IncompleteNetworkInterface, pub block: ipnetwork::IpNetwork, @@ -135,7 +139,7 @@ impl QueryFragment for AllocateIpQuery { out.push_identifier(dsl::ip::NAME)?; // Start the offsets from 1 to exclude the network base address. - out.push_sql(" FROM generate_series(1, "); + out.push_sql(" FROM generate_series(5, "); out.push_bind_param::( // Subtract 1 to exclude the broadcast address &(last_address_offset - 1), @@ -286,7 +290,7 @@ mod test { $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(1, $11) AS \"off\" LEFT OUTER JOIN \ + FROM generate_series(5, $11) AS \"off\" LEFT OUTER JOIN \ \"network_interface\" ON \ (\"subnet_id\", \"ip\", \"time_deleted\" IS NULL) = \ ($12, $13 + \"off\", TRUE) \ @@ -314,7 +318,7 @@ mod test { $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(1, $11) AS \"off\" LEFT OUTER JOIN \ + FROM generate_series(5, $11) AS \"off\" LEFT OUTER JOIN \ \"network_interface\" ON \ (\"subnet_id\", \"ip\", \"time_deleted\" IS NULL) = \ ($12, $13 + \"off\", TRUE) \ diff --git a/nexus/tests/test_subnet_allocation.rs b/nexus/tests/test_subnet_allocation.rs index e8ae390b409..5110b29f6c7 100644 --- a/nexus/tests/test_subnet_allocation.rs +++ b/nexus/tests/test_subnet_allocation.rs @@ -89,7 +89,7 @@ async fn test_subnet_allocation() { "/organizations/{}/projects/{}/vpcs/default/subnets/default", organization_name, project_name ); - let subnet = "192.168.42.0/30".parse().unwrap(); + let subnet = "192.168.42.0/29".parse().unwrap(); let subnet_update = params::VpcSubnetUpdate { identity: IdentityMetadataUpdateParams { name: Some("default".parse().unwrap()), @@ -108,9 +108,8 @@ async fn test_subnet_allocation() { .await .unwrap(); - // The valid addresses for allocation in `subnet` are 192.168.42.1 and - // 192.168.42.2. The rest are reserved as the network address and broadcast - // address. + // The valid addresses for allocation in `subnet` are 192.168.42.5 and + // 192.168.42.6. The rest are reserved as described in RFD21. create_instance(client, &url_instances, "i1").await; create_instance(client, &url_instances, "i2").await; @@ -126,11 +125,11 @@ async fn test_subnet_allocation() { assert_eq!(network_interfaces.len(), 2); assert_eq!( network_interfaces[0].ip, - "192.168.42.1".parse::().unwrap() + "192.168.42.5".parse::().unwrap() ); assert_eq!( network_interfaces[1].ip, - "192.168.42.2".parse::().unwrap() + "192.168.42.6".parse::().unwrap() ); cptestctx.teardown().await; From 09c409bebff8c5b9f23477fa44ae4b07000bd258 Mon Sep 17 00:00:00 2001 From: Tess Eisenberger Date: Wed, 8 Dec 2021 13:29:09 -0800 Subject: [PATCH 16/16] Clean up error messages, error in inetgw case --- nexus/src/nexus.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index f02ad368f89..5179efae123 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -1799,7 +1799,9 @@ impl Nexus { external::VpcFirewallRuleTarget::Vpc(name) => { if *name != vpc.name().0 { return Err(Error::InvalidRequest { - message: "firewall target ".to_string(), + message: + "cross-vpc firewall target unsupported" + .to_string(), }); } vpcs.insert(name.clone().into()); @@ -1817,19 +1819,23 @@ impl Nexus { } // We don't need to resolve anything for Ip external::VpcFirewallRuleHostFilter::Ip(_) => (), - // TODO: How do we resolve VPC targets? external::VpcFirewallRuleHostFilter::Vpc(name) => { if *name != vpc.name().0 { return Err(Error::InvalidRequest { - message: "firewall target ".to_string(), + message: + "cross-vpc firewall target unsupported" + .to_string(), }); } vpcs.insert(name.clone().into()); } // TODO: How do we resolve InternetGateway targets? - external::VpcFirewallRuleHostFilter::InternetGateway( - name, - ) => (), + external::VpcFirewallRuleHostFilter::InternetGateway(_) => { + return Err(Error::InvalidRequest { + message: "inetgw firewall host filters unsupported" + .to_string(), + }); + } } } } @@ -1971,9 +1977,7 @@ impl Nexus { } } // TODO: How do we resolve InternetGateway targets? - external::VpcFirewallRuleHostFilter::InternetGateway( - _name, - ) => (), + external::VpcFirewallRuleHostFilter::InternetGateway(_) => (), } } Some(host_addrs)