From 22bfd6eef93bcacf8af66979f26a4abaec931a7e Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 3 Apr 2023 14:02:59 -0700 Subject: [PATCH 1/6] MacAddr::random_system --- nexus/db-model/src/macaddr.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nexus/db-model/src/macaddr.rs b/nexus/db-model/src/macaddr.rs index ff5d91427b..4d816f0b79 100644 --- a/nexus/db-model/src/macaddr.rs +++ b/nexus/db-model/src/macaddr.rs @@ -26,6 +26,8 @@ impl MacAddr { // for an initial discussion of the customer/system address range split. pub const MIN_GUEST_ADDR: i64 = 0xA8_40_25_F0_00_00; pub const MAX_GUEST_ADDR: i64 = 0xA8_40_25_FE_FF_FF; + pub const MIN_SYSTEM_ADDR: i64 = 0xA8_40_25_FF_00_00; + pub const MAX_SYSTEM_ADDR: i64 = 0xA8_40_25_FF_FF_FF; /// Generate a random MAC address for a guest network interface pub fn random_guest() -> Self { @@ -34,6 +36,13 @@ impl MacAddr { Self::from_i64(value) } + /// Generate a random MAC address in the system address range + pub fn random_system() -> Self { + let value = thread_rng() + .gen_range(Self::MIN_SYSTEM_ADDR..=Self::MAX_SYSTEM_ADDR); + Self::from_i64(value) + } + /// Construct a MAC address from its i64 big-endian byte representation. // NOTE: This is the representation used in the database. pub(crate) fn from_i64(value: i64) -> Self { From 1b5fe367314a08760725545695fddd7a7dad6e90 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Tue, 4 Apr 2023 15:49:08 -0700 Subject: [PATCH 2/6] Remove completed TODO --- nexus/src/app/network_interface.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/nexus/src/app/network_interface.rs b/nexus/src/app/network_interface.rs index f714decbe9..d1940009a8 100644 --- a/nexus/src/app/network_interface.rs +++ b/nexus/src/app/network_interface.rs @@ -75,14 +75,6 @@ impl super::Nexus { // NOTE: We need to lookup the VPC and VPC Subnet, since we need both // IDs for creating the network interface. - // - // TODO-correctness: There are additional races here. The VPC and VPC - // Subnet could both be deleted between the time we fetch them and - // actually insert the record for the interface. The solution is likely - // to make both objects implement `DatastoreCollection` for their - // children, and then use `VpcSubnet::insert_resource` inside the - // `instance_create_network_interface` method. See - // https://github.com/oxidecomputer/omicron/issues/738. let vpc_name = db::model::Name(params.vpc_name.clone()); let subnet_name = db::model::Name(params.subnet_name.clone()); let (.., authz_vpc, authz_subnet, db_subnet) = From 76a8ae0b76872652c334d303fc8238646152d643 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Tue, 4 Apr 2023 19:16:35 -0700 Subject: [PATCH 3/6] Generalize network_interface table to allow for new kinds of NICs. Previously, all NIC records in the DB were tied to a guest instance. In enabling OPTE usage for services, it'd be nice to be able to reuse a lot of the same NetworkInterface machinery we already have without duplicating it completely. This commit adds a new `kind` column to the `network_interface` table which at the moment will be either 'instance' (NICs attached to a guest VM and exposed in the external API) or 'service' (NIC associated with an internal control plane service). The previous `instance_id` column is renamed to `parent_id` and the table to which it refers to as a FK is now dependant on the kind (either `instance` or `service`). Since a lot of the db and authz lookup macros end up relying on the specific column name, this also introduces database views for each kind which can be queried as if they were their own tables. This also allows differentiating the different kinds of NICs as necessary. CockroachDB, unlike Postgres, does not allow inserting or updating into simple view and so we also model the base table itself to execute queries that modify it. From an external perspective, this doesn't change anything (modulo some renaming from `NetworkInterface*` -> `InstanceNetworkInterface*`) in that all external APIs still only deal with instance NICs. There are some basic definitions for service NICs included but those will be more fleshed in subsequent commits. --- common/src/api/external/mod.rs | 9 +- common/src/sql/dbinit.sql | 85 ++- nexus/db-model/src/network_interface.rs | 244 +++++++- nexus/db-model/src/schema.rs | 39 ++ nexus/db-queries/src/authz/api_resources.rs | 2 +- nexus/db-queries/src/authz/oso_generic.rs | 2 +- .../src/authz/policy_test/resources.rs | 2 +- .../src/db/datastore/network_interface.rs | 105 ++-- nexus/db-queries/src/db/datastore/vpc.rs | 51 +- nexus/db-queries/src/db/lookup.rs | 13 +- .../src/db/queries/network_interface.rs | 526 +++++++++++------- nexus/db-queries/tests/output/authz-roles.out | 6 +- nexus/src/app/network_interface.rs | 38 +- nexus/src/app/sagas/instance_create.rs | 21 +- nexus/src/app/vpc_subnet.rs | 10 +- nexus/src/external_api/http_entrypoints.rs | 56 +- nexus/tests/integration_tests/endpoints.rs | 8 +- nexus/tests/integration_tests/instances.rs | 126 +++-- .../integration_tests/subnet_allocation.rs | 10 +- nexus/tests/integration_tests/vpc_subnets.rs | 4 +- nexus/types/src/external_api/params.rs | 22 +- openapi/nexus.json | 344 ++++++------ 22 files changed, 1123 insertions(+), 600 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index d0817761a0..9df34e0fa3 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -698,7 +698,7 @@ pub enum ResourceType { Image, Instance, IpPool, - NetworkInterface, + InstanceNetworkInterface, PhysicalDisk, Rack, Service, @@ -2044,9 +2044,10 @@ impl TryFrom for Vni { } } -/// A `NetworkInterface` represents a virtual network interface device. +/// An `InstanceNetworkInterface` represents a virtual network interface device +/// attached to an instance. #[derive(ObjectIdentity, Clone, Debug, Deserialize, JsonSchema, Serialize)] -pub struct NetworkInterface { +pub struct InstanceNetworkInterface { /// common identifying metadata #[serde(flatten)] pub identity: IdentityMetadata, @@ -2064,9 +2065,9 @@ pub struct NetworkInterface { pub mac: MacAddr, /// The IP address assigned to this interface. - pub ip: IpAddr, // TODO-correctness: We need to split this into an optional V4 and optional // V6 address, at least one of which must be specified. + pub ip: IpAddr, /// True if this interface is the primary for the instance to which it's /// attached. pub primary: bool, diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 4f946fd46c..523f733d8a 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -1036,6 +1036,15 @@ CREATE UNIQUE INDEX ON omicron.public.vpc_subnet ( ) WHERE time_deleted IS NULL; +/* The kind of network interface. */ +CREATE TYPE omicron.public.network_interface_kind AS ENUM ( + /* An interface attached to a guest instance. */ + 'instance', + + /* An interface attached to a service. */ + 'service' +); + CREATE TABLE omicron.public.network_interface ( /* Identity metadata (resource) */ id UUID PRIMARY KEY, @@ -1046,11 +1055,14 @@ CREATE TABLE omicron.public.network_interface ( /* Indicates that the object has been deleted */ time_deleted TIMESTAMPTZ, - /* FK into Instance table. - * Note that interfaces are always attached to a particular instance. - * IP addresses may be reserved, but this is a different resource. + /* The kind of network interface, e.g., instance */ + kind omicron.public.network_interface_kind NOT NULL, + + /* + * FK into the parent resource of this interface (e.g. Instance, Service) + * as determined by the `kind`. */ - instance_id UUID NOT NULL, + parent_id UUID NOT NULL, /* FK into VPC table */ vpc_id UUID NOT NULL, @@ -1065,21 +1077,65 @@ CREATE TABLE omicron.public.network_interface ( */ mac INT8 NOT NULL, + /* The private VPC IP address of the interface. */ ip INET NOT NULL, + /* * Limited to 8 NICs per instance. This value must be kept in sync with * `crate::nexus::MAX_NICS_PER_INSTANCE`. */ slot INT2 NOT NULL CHECK (slot >= 0 AND slot < 8), - /* True if this interface is the primary interface for the instance. + /* True if this interface is the primary interface. * * The primary interface appears in DNS and its address is used for external - * connectivity for the instance. + * connectivity. */ is_primary BOOL NOT NULL ); +/* A view of the network_interface table for just instance-kind records. */ +CREATE VIEW omicron.public.instance_network_interface AS +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + parent_id AS instance_id, + vpc_id, + subnet_id, + mac, + ip, + slot, + is_primary +FROM + omicron.public.network_interface +WHERE + kind = 'instance'; + +/* A view of the network_interface table for just service-kind records. */ +CREATE VIEW omicron.public.service_network_interface AS +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + parent_id AS service_id, + vpc_id, + subnet_id, + mac, + ip, + slot, + is_primary +FROM + omicron.public.network_interface +WHERE + kind = 'service'; + /* TODO-completeness * We currently have a NetworkInterface table with the IP and MAC addresses inline. @@ -1105,17 +1161,18 @@ CREATE UNIQUE INDEX ON omicron.public.network_interface ( time_deleted IS NULL; /* - * Index used to verify that an Instance's networking is contained - * within a single VPC, and that all interfaces are in unique VPC - * Subnets. + * Index used to verify that all interfaces for a resource (e.g. Instance, + * Service) are contained within a single VPC, and that all interfaces are + * in unique VPC Subnets. * - * This is also used to quickly find the primary interface for an - * instance, since we store the `is_primary` column. Such queries are - * mostly used when setting a new primary interface for an instance. + * This is also used to quickly find the primary interface since + * we store the `is_primary` column. Such queries are mostly used + * when setting a new primary interface. */ CREATE UNIQUE INDEX ON omicron.public.network_interface ( - instance_id, - name + parent_id, + name, + kind ) STORING (vpc_id, subnet_id, is_primary) WHERE diff --git a/nexus/db-model/src/network_interface.rs b/nexus/db-model/src/network_interface.rs index 0538f00f85..b7340b561b 100644 --- a/nexus/db-model/src/network_interface.rs +++ b/nexus/db-model/src/network_interface.rs @@ -3,7 +3,10 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{MacAddr, VpcSubnet}; +use crate::impl_enum_type; +use crate::schema::instance_network_interface; use crate::schema::network_interface; +use crate::schema::service_network_interface; use crate::Name; use chrono::DateTime; use chrono::Utc; @@ -14,36 +17,184 @@ use nexus_types::identity::Resource; use omicron_common::api::external; use uuid::Uuid; -#[derive(Selectable, Queryable, Insertable, Clone, Debug, Resource)] +impl_enum_type! { + #[derive(SqlType, QueryId, Debug, Clone, Copy)] + #[diesel(postgres_type(name = "network_interface_kind"))] + pub struct NetworkInterfaceKindEnum; + + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq)] + #[diesel(sql_type = NetworkInterfaceKindEnum)] + pub enum NetworkInterfaceKind; + + Instance => b"instance" + Service => b"service" +} + +/// Generic Network Interface DB model. +#[derive(Selectable, Queryable, Clone, Debug, Resource)] #[diesel(table_name = network_interface)] pub struct NetworkInterface { #[diesel(embed)] pub identity: NetworkInterfaceIdentity, - pub instance_id: Uuid, + pub kind: NetworkInterfaceKind, + pub parent_id: Uuid, + pub vpc_id: Uuid, pub subnet_id: Uuid, + pub mac: MacAddr, // TODO-correctness: We need to split this into an optional V4 and optional V6 address, at - // least one of which will always be specified. + // least one of which will lways be specified. // // If user requests an address of either kind, give exactly that and not the other. // If neither is specified, auto-assign one of each? pub ip: ipnetwork::IpNetwork, + pub slot: i16, #[diesel(column_name = is_primary)] pub primary: bool, } -impl From for external::NetworkInterface { - fn from(iface: NetworkInterface) -> Self { - Self { - identity: iface.identity(), - instance_id: iface.instance_id, +/// Instance Network Interface DB model. +/// +/// The underlying "table" (`instance_network_interface`) is actually a view +/// over the `network_interface` table, that contains only rows with +/// `kind = 'instance'`. +#[derive(Selectable, Queryable, Clone, Debug, Resource)] +#[diesel(table_name = instance_network_interface)] +pub struct InstanceNetworkInterface { + #[diesel(embed)] + pub identity: InstanceNetworkInterfaceIdentity, + + pub instance_id: Uuid, + pub vpc_id: Uuid, + pub subnet_id: Uuid, + + pub mac: MacAddr, + pub ip: ipnetwork::IpNetwork, + + pub slot: i16, + #[diesel(column_name = is_primary)] + pub primary: bool, +} + +/// Service Network Interface DB model. +/// +/// The underlying "table" (`service_network_interface`) is actually a view +/// over the `network_interface` table, that contains only rows with +/// `kind = 'service'`. +#[derive(Selectable, Queryable, Clone, Debug, Resource)] +#[diesel(table_name = service_network_interface)] +pub struct ServiceNetworkInterface { + #[diesel(embed)] + pub identity: ServiceNetworkInterfaceIdentity, + + pub service_id: Uuid, + pub vpc_id: Uuid, + pub subnet_id: Uuid, + + pub mac: MacAddr, + pub ip: ipnetwork::IpNetwork, + + pub slot: i16, + #[diesel(column_name = is_primary)] + pub primary: bool, +} + +impl NetworkInterface { + /// Treat this `NetworkInterface` as an `InstanceNetworkInterface`. + /// + /// # Panics + /// Panics if this isn't an 'instance' kind network interface. + pub fn as_instance(self) -> InstanceNetworkInterface { + assert_eq!(self.kind, NetworkInterfaceKind::Instance); + InstanceNetworkInterface { + identity: InstanceNetworkInterfaceIdentity { + id: self.identity.id, + name: self.identity.name, + description: self.identity.description, + time_created: self.identity.time_created, + time_modified: self.identity.time_modified, + time_deleted: self.identity.time_deleted, + }, + instance_id: self.parent_id, + vpc_id: self.vpc_id, + subnet_id: self.subnet_id, + mac: self.mac, + ip: self.ip, + slot: self.slot, + primary: self.primary, + } + } + + /// Treat this `NetworkInterface` as a `ServiceNetworkInterface`. + /// + /// # Panics + /// Panics if this isn't a 'service' kind network interface. + pub fn as_service(self) -> ServiceNetworkInterface { + assert_eq!(self.kind, NetworkInterfaceKind::Service); + ServiceNetworkInterface { + identity: ServiceNetworkInterfaceIdentity { + id: self.identity.id, + name: self.identity.name, + description: self.identity.description, + time_created: self.identity.time_created, + time_modified: self.identity.time_modified, + time_deleted: self.identity.time_deleted, + }, + service_id: self.parent_id, + vpc_id: self.vpc_id, + subnet_id: self.subnet_id, + mac: self.mac, + ip: self.ip, + slot: self.slot, + primary: self.primary, + } + } +} + +impl From for NetworkInterface { + fn from(iface: InstanceNetworkInterface) -> Self { + NetworkInterface { + identity: NetworkInterfaceIdentity { + id: iface.identity.id, + name: iface.identity.name, + description: iface.identity.description, + time_created: iface.identity.time_created, + time_modified: iface.identity.time_modified, + time_deleted: iface.identity.time_deleted, + }, + kind: NetworkInterfaceKind::Instance, + parent_id: iface.instance_id, vpc_id: iface.vpc_id, subnet_id: iface.subnet_id, - ip: iface.ip.ip(), - mac: *iface.mac, + mac: iface.mac, + ip: iface.ip, + slot: iface.slot, + primary: iface.primary, + } + } +} + +impl From for NetworkInterface { + fn from(iface: ServiceNetworkInterface) -> Self { + NetworkInterface { + identity: NetworkInterfaceIdentity { + id: iface.identity.id, + name: iface.identity.name, + description: iface.identity.description, + time_created: iface.identity.time_created, + time_modified: iface.identity.time_modified, + time_deleted: iface.identity.time_deleted, + }, + kind: NetworkInterfaceKind::Service, + parent_id: iface.service_id, + vpc_id: iface.vpc_id, + subnet_id: iface.subnet_id, + mac: iface.mac, + ip: iface.ip, + slot: iface.slot, primary: iface.primary, } } @@ -54,16 +205,18 @@ impl From for external::NetworkInterface { #[derive(Clone, Debug)] pub struct IncompleteNetworkInterface { pub identity: NetworkInterfaceIdentity, - pub instance_id: Uuid, + pub kind: NetworkInterfaceKind, + pub parent_id: Uuid, pub vpc_id: Uuid, pub subnet: VpcSubnet, pub ip: Option, } impl IncompleteNetworkInterface { - pub fn new( + fn new( interface_id: Uuid, - instance_id: Uuid, + kind: NetworkInterfaceKind, + parent_id: Uuid, vpc_id: Uuid, subnet: VpcSubnet, identity: external::IdentityMetadataCreateParams, @@ -73,7 +226,52 @@ impl IncompleteNetworkInterface { subnet.check_requestable_addr(ip)?; }; let identity = NetworkInterfaceIdentity::new(interface_id, identity); - Ok(Self { identity, instance_id, subnet, vpc_id, ip }) + Ok(IncompleteNetworkInterface { + identity, + kind, + parent_id, + subnet, + vpc_id, + ip, + }) + } + + pub fn new_instance( + interface_id: Uuid, + instance_id: Uuid, + vpc_id: Uuid, + subnet: VpcSubnet, + identity: external::IdentityMetadataCreateParams, + ip: Option, + ) -> Result { + Self::new( + interface_id, + NetworkInterfaceKind::Instance, + instance_id, + vpc_id, + subnet, + identity, + ip, + ) + } + + pub fn new_service( + interface_id: Uuid, + service_id: Uuid, + vpc_id: Uuid, + subnet: VpcSubnet, + identity: external::IdentityMetadataCreateParams, + ip: Option, + ) -> Result { + Self::new( + interface_id, + NetworkInterfaceKind::Service, + service_id, + vpc_id, + subnet, + identity, + ip, + ) } } @@ -88,8 +286,22 @@ pub struct NetworkInterfaceUpdate { pub primary: Option, } -impl From for NetworkInterfaceUpdate { - fn from(params: params::NetworkInterfaceUpdate) -> Self { +impl From for external::InstanceNetworkInterface { + fn from(iface: InstanceNetworkInterface) -> 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, + primary: iface.primary, + } + } +} + +impl From for NetworkInterfaceUpdate { + fn from(params: params::InstanceNetworkInterfaceUpdate) -> Self { let primary = if params.primary { Some(true) } else { None }; Self { name: params.identity.name.map(|n| n.into()), diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index d6448f3376..ff6cc1a3b1 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -127,6 +127,25 @@ table! { table! { network_interface (id) { + id -> Uuid, + name -> Text, + description -> Text, + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + kind -> crate::NetworkInterfaceKindEnum, + parent_id -> Uuid, + vpc_id -> Uuid, + subnet_id -> Uuid, + mac -> Int8, + ip -> Inet, + slot -> Int2, + is_primary -> Bool, + } +} + +table! { + instance_network_interface (id) { id -> Uuid, name -> Text, description -> Text, @@ -143,6 +162,24 @@ table! { } } +table! { + service_network_interface (id) { + id -> Uuid, + name -> Text, + description -> Text, + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + service_id -> Uuid, + vpc_id -> Uuid, + subnet_id -> Uuid, + mac -> Int8, + ip -> Inet, + slot -> Int2, + is_primary -> Bool, + } +} + table! { ip_pool (id) { id -> Uuid, @@ -802,6 +839,8 @@ allow_tables_to_appear_in_same_query!( instance, metric_producer, network_interface, + instance_network_interface, + service_network_interface, oximeter, project, rack, diff --git a/nexus/db-queries/src/authz/api_resources.rs b/nexus/db-queries/src/authz/api_resources.rs index 23dec07960..c5fa77022d 100644 --- a/nexus/db-queries/src/authz/api_resources.rs +++ b/nexus/db-queries/src/authz/api_resources.rs @@ -708,7 +708,7 @@ authz_resource! { } authz_resource! { - name = "NetworkInterface", + name = "InstanceNetworkInterface", parent = "Instance", primary_key = Uuid, roles_allowed = false, diff --git a/nexus/db-queries/src/authz/oso_generic.rs b/nexus/db-queries/src/authz/oso_generic.rs index f6625fd657..2ed0d67bc8 100644 --- a/nexus/db-queries/src/authz/oso_generic.rs +++ b/nexus/db-queries/src/authz/oso_generic.rs @@ -124,7 +124,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { Image::init(), Instance::init(), IpPool::init(), - NetworkInterface::init(), + InstanceNetworkInterface::init(), Vpc::init(), VpcRouter::init(), RouterRoute::init(), diff --git a/nexus/db-queries/src/authz/policy_test/resources.rs b/nexus/db-queries/src/authz/policy_test/resources.rs index 4ee4f388b2..0f3a2ed709 100644 --- a/nexus/db-queries/src/authz/policy_test/resources.rs +++ b/nexus/db-queries/src/authz/policy_test/resources.rs @@ -236,7 +236,7 @@ async fn make_project( LookupType::ByName(disk_name.clone()), )); builder.new_resource(instance.clone()); - builder.new_resource(authz::NetworkInterface::new( + builder.new_resource(authz::InstanceNetworkInterface::new( instance, Uuid::new_v4(), LookupType::ByName(format!("{}-nic1", instance_name)), diff --git a/nexus/db-queries/src/db/datastore/network_interface.rs b/nexus/db-queries/src/db/datastore/network_interface.rs index e8b10373ba..bdb033f0d3 100644 --- a/nexus/db-queries/src/db/datastore/network_interface.rs +++ b/nexus/db-queries/src/db/datastore/network_interface.rs @@ -16,8 +16,10 @@ use crate::db::error::ErrorHandler; use crate::db::error::TransactionError; use crate::db::model::IncompleteNetworkInterface; use crate::db::model::Instance; +use crate::db::model::InstanceNetworkInterface; use crate::db::model::Name; use crate::db::model::NetworkInterface; +use crate::db::model::NetworkInterfaceKind; use crate::db::model::NetworkInterfaceUpdate; use crate::db::model::VpcSubnet; use crate::db::pagination::paginated; @@ -78,7 +80,7 @@ impl DataStore { authz_subnet: &authz::VpcSubnet, authz_instance: &authz::Instance, interface: IncompleteNetworkInterface, - ) -> Result { + ) -> Result { opctx .authorize(authz::Action::CreateChild, authz_instance) .await @@ -94,8 +96,15 @@ impl DataStore { &self, opctx: &OpContext, interface: IncompleteNetworkInterface, - ) -> Result { + ) -> Result { use db::schema::network_interface::dsl; + if interface.kind != NetworkInterfaceKind::Instance { + return Err(network_interface::InsertError::External( + Error::invalid_request( + "expected instance type network interface", + ), + )); + } let subnet_id = interface.subnet.identity.id; let query = network_interface::InsertQuery::new(interface.clone()); VpcSubnet::insert_resource( @@ -108,6 +117,9 @@ impl DataStore { .map_err(network_interface::InsertError::External)?, ) .await + // Convert to `InstanceNetworkInterface` before returning; we know + // this is valid as we've checked the condition on-entry. + .map(NetworkInterface::as_instance) .map_err(|e| match e { AsyncInsertError::CollectionNotFound => { network_interface::InsertError::External( @@ -134,7 +146,8 @@ impl DataStore { use db::schema::network_interface::dsl; let now = Utc::now(); diesel::update(dsl::network_interface) - .filter(dsl::instance_id.eq(authz_instance.id())) + .filter(dsl::parent_id.eq(authz_instance.id())) + .filter(dsl::kind.eq(NetworkInterfaceKind::Instance)) .filter(dsl::time_deleted.is_null()) .set(dsl::time_deleted.eq(now)) .execute_async(self.pool_authorized(opctx).await?) @@ -148,7 +161,7 @@ impl DataStore { Ok(()) } - /// Delete a `NetworkInterface` attached to a provided instance. + /// Delete an instance attached to a provided instance. /// /// Note that the primary interface for an instance cannot be deleted if /// there are any secondary interfaces. @@ -156,13 +169,14 @@ impl DataStore { &self, opctx: &OpContext, authz_instance: &authz::Instance, - authz_interface: &authz::NetworkInterface, + authz_interface: &authz::InstanceNetworkInterface, ) -> Result<(), network_interface::DeleteError> { opctx .authorize(authz::Action::Delete, authz_interface) .await .map_err(network_interface::DeleteError::External)?; let query = network_interface::DeleteQuery::new( + NetworkInterfaceKind::Instance, authz_instance.id(), authz_interface.id(), ); @@ -183,7 +197,7 @@ impl DataStore { /// Return information about network interfaces required for the sled /// agent to instantiate or modify them via OPTE. This function takes /// a partially constructed query over the network interface table so - /// that we can use it for instances, VPCs, and subnets. + /// that we can use it for instances, services, VPCs, and subnets. async fn derive_network_interface_info( &self, opctx: &OpContext, @@ -238,7 +252,10 @@ impl DataStore { self.derive_network_interface_info( opctx, network_interface::table - .filter(network_interface::instance_id.eq(authz_instance.id())) + .filter(network_interface::parent_id.eq(authz_instance.id())) + .filter( + network_interface::kind.eq(NetworkInterfaceKind::Instance), + ) .into_boxed(), ) .await @@ -288,24 +305,26 @@ impl DataStore { opctx: &OpContext, authz_instance: &authz::Instance, pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_instance).await?; - use db::schema::network_interface::dsl; + use db::schema::instance_network_interface::dsl; match pagparams { PaginatedBy::Id(pagparams) => { - paginated(dsl::network_interface, dsl::id, &pagparams) + paginated(dsl::instance_network_interface, dsl::id, &pagparams) } PaginatedBy::Name(pagparams) => paginated( - dsl::network_interface, + dsl::instance_network_interface, dsl::name, &pagparams.map_name(|n| Name::ref_cast(n)), ), } .filter(dsl::time_deleted.is_null()) .filter(dsl::instance_id.eq(authz_instance.id())) - .select(NetworkInterface::as_select()) - .load_async::(self.pool_authorized(opctx).await?) + .select(InstanceNetworkInterface::as_select()) + .load_async::( + self.pool_authorized(opctx).await?, + ) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } @@ -315,11 +334,9 @@ impl DataStore { &self, opctx: &OpContext, authz_instance: &authz::Instance, - authz_interface: &authz::NetworkInterface, + authz_interface: &authz::InstanceNetworkInterface, updates: NetworkInterfaceUpdate, - ) -> UpdateResult { - use crate::db::schema::network_interface::dsl; - + ) -> UpdateResult { // This database operation is surprisingly subtle. It's possible to // express this in a single query, with multiple common-table // expressions for the updated rows. For example, if we're setting a new @@ -334,33 +351,54 @@ impl DataStore { // // See https://github.com/oxidecomputer/omicron/issues/1204 for the // issue tracking the work to move this into a CTE. + // + // It's also further complicated by the fact we're updating a row + // in the `network_interface` table but will return a `InstanceNetworkInterface` + // which represents a row in the `instance_network_interface`. This is + // fine because `instance_network_interface` is just a view over + // `network_interface` constrained to rows of `kind = 'instance'`. // Build up some of the queries first, outside the transaction. // // This selects the existing primary interface. + // Note we consult only `kind = 'instance'` rows by querying from + // `instance_network_interface`. let instance_id = authz_instance.id(); let interface_id = authz_interface.id(); - let find_primary_query = dsl::network_interface - .filter(dsl::instance_id.eq(instance_id)) - .filter(dsl::is_primary.eq(true)) - .filter(dsl::time_deleted.is_null()) - .select(NetworkInterface::as_select()); + let find_primary_query = { + use crate::db::schema::instance_network_interface::dsl; + dsl::instance_network_interface + .filter(dsl::instance_id.eq(instance_id)) + .filter(dsl::is_primary.eq(true)) + .filter(dsl::time_deleted.is_null()) + .select(InstanceNetworkInterface::as_select()) + }; // This returns the state of the associated instance. - let instance_query = db::schema::instance::dsl::instance - .filter(db::schema::instance::dsl::id.eq(instance_id)) - .filter(db::schema::instance::dsl::time_deleted.is_null()) - .select(Instance::as_select()); + let instance_query = { + use crate::db::schema::instance::dsl; + dsl::instance + .filter(dsl::id.eq(instance_id)) + .filter(dsl::time_deleted.is_null()) + .select(Instance::as_select()) + }; let stopped = db::model::InstanceState::new(external::InstanceState::Stopped); // This is the actual query to update the target interface. + // Unlike Postgres, CockroachDB doesn't support inserting or updating a view + // so we need to use the underlying table, taking care to constrain + // the update to rows of `kind = 'instance'`. let primary = matches!(updates.primary, Some(true)); - let update_target_query = diesel::update(dsl::network_interface) - .filter(dsl::id.eq(interface_id)) - .filter(dsl::time_deleted.is_null()) - .set(updates) - .returning(NetworkInterface::as_returning()); + let update_target_query = { + use crate::db::schema::network_interface::dsl; + diesel::update(dsl::network_interface) + .filter(dsl::id.eq(interface_id)) + .filter(dsl::kind.eq(NetworkInterfaceKind::Instance)) + .filter(dsl::time_deleted.is_null()) + .set(updates) + .returning(NetworkInterface::as_returning()) + }; // Errors returned from the below transactions. #[derive(Debug)] @@ -390,8 +428,10 @@ impl DataStore { // If the target and primary are different, we need to toggle // the primary into a secondary. if primary_interface.identity.id != interface_id { + use crate::db::schema::network_interface::dsl; if let Err(e) = diesel::update(dsl::network_interface) .filter(dsl::id.eq(primary_interface.identity.id)) + .filter(dsl::kind.eq(NetworkInterfaceKind::Instance)) .filter(dsl::time_deleted.is_null()) .set(dsl::is_primary.eq(false)) .execute_async(&conn) @@ -429,6 +469,9 @@ impl DataStore { }) } .await + // Convert to `InstanceNetworkInterface` before returning, we know + // this is valid as we've filtered appropriately above. + .map(NetworkInterface::as_instance) .map_err(|e| match e { TxnError::CustomError( NetworkInterfaceUpdateError::InstanceNotStopped, diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index ffce7ca0bb..025f85b942 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -16,8 +16,8 @@ use crate::db::error::ErrorHandler; use crate::db::error::TransactionError; use crate::db::identity::Resource; use crate::db::model::IncompleteVpc; +use crate::db::model::InstanceNetworkInterface; use crate::db::model::Name; -use crate::db::model::NetworkInterface; use crate::db::model::Project; use crate::db::model::RouterRoute; use crate::db::model::RouterRouteUpdate; @@ -352,18 +352,35 @@ impl DataStore { ) -> Result, Error> { // Resolve each VNIC in the VPC to the Sled it's on, so we know which // Sleds to notify when firewall rules change. - use db::schema::{instance, network_interface, sled}; - network_interface::table + use db::schema::{ + instance, instance_network_interface, service, + service_network_interface, sled, + }; + + let instance_query = instance_network_interface::table .inner_join( instance::table - .on(instance::id.eq(network_interface::instance_id)), + .on(instance::id + .eq(instance_network_interface::instance_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_network_interface::vpc_id.eq(vpc_id)) + .filter(instance_network_interface::time_deleted.is_null()) .filter(instance::time_deleted.is_null()) - .select(Sled::as_select()) - .distinct() + .select(Sled::as_select()); + + let service_query = service_network_interface::table + .inner_join( + service::table + .on(service::id.eq(service_network_interface::service_id)), + ) + .inner_join(sled::table.on(sled::id.eq(service::sled_id))) + .filter(service_network_interface::vpc_id.eq(vpc_id)) + .filter(service_network_interface::time_deleted.is_null()) + .select(Sled::as_select()); + + instance_query + .union(service_query) .get_results_async(self.pool()) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) @@ -460,8 +477,8 @@ impl DataStore { { return Err(Error::InvalidRequest { message: String::from( - "VPC Subnet cannot be deleted while instances \ - with network interfaces in the subnet exist", + "VPC Subnet cannot be deleted while \ + network interfaces in the subnet exist", ), }); } @@ -516,30 +533,30 @@ impl DataStore { }) } - pub async fn subnet_list_network_interfaces( + pub async fn subnet_list_instance_network_interfaces( &self, opctx: &OpContext, authz_subnet: &authz::VpcSubnet, pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_subnet).await?; - use db::schema::network_interface::dsl; + use db::schema::instance_network_interface::dsl; match pagparams { PaginatedBy::Id(pagparams) => { - paginated(dsl::network_interface, dsl::id, &pagparams) + paginated(dsl::instance_network_interface, dsl::id, &pagparams) } PaginatedBy::Name(pagparams) => paginated( - dsl::network_interface, + dsl::instance_network_interface, dsl::name, &pagparams.map_name(|n| Name::ref_cast(n)), ), } .filter(dsl::time_deleted.is_null()) .filter(dsl::subnet_id.eq(authz_subnet.id())) - .select(NetworkInterface::as_select()) - .load_async::( + .select(InstanceNetworkInterface::as_select()) + .load_async::( self.pool_authorized(opctx).await?, ) .await diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 6ccb629534..6ed5fc3f3e 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -175,9 +175,12 @@ impl<'a> LookupPath<'a> { Snapshot::PrimaryKey(Root { lookup_root: self }, id) } - /// Select a resource of type NetworkInterface, identified by its id - pub fn network_interface_id(self, id: Uuid) -> NetworkInterface<'a> { - NetworkInterface::PrimaryKey(Root { lookup_root: self }, id) + /// Select a resource of type InstanceNetworkInterface, identified by its id + pub fn instance_network_interface_id( + self, + id: Uuid, + ) -> InstanceNetworkInterface<'a> { + InstanceNetworkInterface::PrimaryKey(Root { lookup_root: self }, id) } /// Select a resource of type Vpc, identified by its id @@ -523,14 +526,14 @@ lookup_resource! { lookup_resource! { name = "Instance", ancestors = [ "Silo", "Project" ], - children = [ "NetworkInterface" ], + children = [ "InstanceNetworkInterface" ], lookup_by_name = true, soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { - name = "NetworkInterface", + name = "InstanceNetworkInterface", ancestors = [ "Silo", "Project", "Instance" ], children = [], lookup_by_name = true, diff --git a/nexus/db-queries/src/db/queries/network_interface.rs b/nexus/db-queries/src/db/queries/network_interface.rs index 9599147e3c..caa2045b5c 100644 --- a/nexus/db-queries/src/db/queries/network_interface.rs +++ b/nexus/db-queries/src/db/queries/network_interface.rs @@ -2,7 +2,7 @@ // 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/. -//! Query for inserting a guest network interface. +//! Queries for inserting and deleting network interfaces. use crate::db; use crate::db::model::IncompleteNetworkInterface; @@ -24,12 +24,19 @@ use diesel::QueryResult; use diesel::RunQueryDsl; use ipnetwork::IpNetwork; use ipnetwork::Ipv4Network; +use nexus_db_model::NetworkInterfaceKind; +use nexus_db_model::NetworkInterfaceKindEnum; use omicron_common::api::external; use omicron_common::nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; use std::net::IpAddr; use uuid::Uuid; -pub(crate) const MAX_NICS_PER_INSTANCE: usize = 8; +/// The max number of interfaces that may be associated with a resource, +/// e.g., instance or service. +/// +/// RFD 135 caps instances at 8 interfaces and we use the same limit for +/// all types of interfaces for simplicity. +pub(crate) const MAX_NICS: usize = 8; // These are sentinel values and other constants used to verify the state of the // system when operating on network interfaces @@ -86,15 +93,16 @@ pub enum InsertError { /// /// Note: An interface with the same name existing is a different error; /// this error matches the case where the UUID already exists. - InterfaceAlreadyExists(String), - /// The instance specified for this interface is already associated with a - /// different VPC from this interface. - InstanceSpansMultipleVpcs(Uuid), + InterfaceAlreadyExists(String, NetworkInterfaceKind), + /// The resource specified for this interface is already associated with a + /// different VPC from this interface, e.g., the instance has a different + /// interface that is associated with another VPC. + ResourceSpansMultipleVpcs(Uuid), /// There are no available IP addresses in the requested subnet NoAvailableIpAddresses, /// An explicitly-requested IP address is already in use IpAddressNotAvailable(std::net::IpAddr), - /// There are no slots available on the instance + /// There are no slots available for a new interface NoSlotsAvailable, /// There are no MAC addresses available NoMacAddrressesAvailable, @@ -109,7 +117,7 @@ pub enum InsertError { } impl InsertError { - /// Construct a `InsertError` from a database error + /// Construct an `InsertError` from a database error /// /// This catches the various errors that the `InsertQuery` /// can generate, especially the intentional errors that indicate either IP @@ -140,21 +148,24 @@ impl InsertError { /// Convert this error into an external one. pub fn into_external(self) -> external::Error { match self { - InsertError::InterfaceAlreadyExists(name) => { + InsertError::InterfaceAlreadyExists(name, NetworkInterfaceKind::Instance) => { external::Error::ObjectAlreadyExists { - type_name: external::ResourceType::NetworkInterface, + type_name: external::ResourceType::InstanceNetworkInterface, object_name: name, } } + InsertError::InterfaceAlreadyExists(_name, NetworkInterfaceKind::Service) => { + unimplemented!("service network interface") + } InsertError::NoAvailableIpAddresses => { external::Error::invalid_request( "No available IP addresses for interface", ) } - InsertError::InstanceSpansMultipleVpcs(_) => { + InsertError::ResourceSpansMultipleVpcs(_) => { external::Error::invalid_request(concat!( "Networking may not span multiple VPCs, but the ", - "requested instance is associated with another VPC" + "requested resource is associated with another VPC" )) } InsertError::IpAddressNotAvailable(ip) => { @@ -165,8 +176,8 @@ impl InsertError { } InsertError::NoSlotsAvailable => { external::Error::invalid_request(&format!( - "Instances may not have more than {} network interfaces", - MAX_NICS_PER_INSTANCE + "May not attach more than {} network interfaces", + MAX_NICS )) } InsertError::NoMacAddrressesAvailable => { @@ -176,7 +187,7 @@ impl InsertError { } InsertError::NonUniqueVpcSubnets => { external::Error::invalid_request( - "Each interface for an instance must be in a distinct VPC Subnet" + "Each interface must be in a distinct VPC Subnet" ) } InsertError::InstanceMustBeStopped(_) => { @@ -216,7 +227,10 @@ fn decode_database_error( // Error message generated when we attempt to insert an interface in a // different VPC from the interface(s) already associated with the instance - const MULTIPLE_VPC_ERROR_MESSAGE: &str = r#"could not parse "multiple-vpcs" as type uuid: uuid: incorrect UUID length: multiple-vpcs"#; + const MULTIPLE_VPC_ERROR_MESSAGE: &str = concat!( + r#"could not parse "multiple-vpcs" as type uuid: uuid: "#, + r#"incorrect UUID length: multiple-vpcs"#, + ); // Error message generated when we attempt to insert NULL in the `ip` // column, which only happens when we run out of IPs in the subnet. @@ -230,16 +244,16 @@ fn decode_database_error( // The name of the index whose uniqueness is violated if we try to assign a // name to an interface that is already used for another interface on the - // same instance. + // same resource. const NAME_CONFLICT_CONSTRAINT: &str = - "network_interface_instance_id_name_key"; + "network_interface_parent_id_name_kind_key"; // The name of the constraint violated if we try to re-insert an already // existing interface with the same UUID. const ID_CONFLICT_CONSTRAINT: &str = "network_interface_pkey"; - // The check violated in the case where we try to insert more that the - // maximum number of NICs (`MAX_NICS_PER_INSTANCE`). + // The check violated in the case where we try to insert more that the + // maximum number of NICs (`MAX_NICS`). const NO_SLOTS_AVAILABLE_ERROR_MESSAGE: &str = concat!( "failed to satisfy CHECK constraint ", "((slot >= 0:::INT8) AND (slot < 8:::INT8))", @@ -255,10 +269,13 @@ fn decode_database_error( r#"null value in column "mac" violates not-null constraint"#; // Error message received when attempting to add an interface in a VPC - // Subnet, where that instance already has an interface in that VPC Subnet. + // Subnet, where that resource already has an interface in that VPC Subnet. // This enforces the constraint that all interfaces are in distinct VPC // Subnets. - const NON_UNIQUE_VPC_SUBNET_ERROR_MESSAGE: &str = r#"could not parse "non-unique-subnets" as type uuid: uuid: incorrect UUID length: non-unique-subnets"#; + const NON_UNIQUE_VPC_SUBNET_ERROR_MESSAGE: &str = concat!( + r#"could not parse "non-unique-subnets" as type uuid: "#, + r#"uuid: incorrect UUID length: non-unique-subnets"#, + ); match err { // If the address allocation subquery fails, we'll attempt to insert @@ -272,16 +289,16 @@ fn decode_database_error( // This catches the error intentionally introduced by the // `push_ensure_unique_vpc_expression` subquery, which generates a - // UUID parsing error if an instance is already associated with - // another VPC. + // UUID parsing error if the resource (e.g. instance) we want to attach + // to is already associated with another VPC. PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), )) if info.message() == MULTIPLE_VPC_ERROR_MESSAGE => { - InsertError::InstanceSpansMultipleVpcs(interface.instance_id) + InsertError::ResourceSpansMultipleVpcs(interface.parent_id) } // This checks the constraint on the interface slot numbers, used to - // limit instances to a maximum number. + // limit total number of interfaces per resource to a maximum number. PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::CheckViolation, ref info), )) if info.message() == NO_SLOTS_AVAILABLE_ERROR_MESSAGE => { @@ -290,7 +307,7 @@ fn decode_database_error( // If the MAC allocation subquery fails, we'll attempt to insert NULL // for the `mac` column. This checks that the non-NULL constraint on - // that colum has been violated. + // that column has been violated. PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::NotNullViolation, ref info), )) if info.message() == MAC_EXHAUSTION_ERROR_MESSAGE => { @@ -299,7 +316,7 @@ fn decode_database_error( // This catches the error intentionally introduced by the // `push_ensure_unique_vpc_subnet_expression` subquery, which generates - // a UUID parsing error if an instance has another interface in the VPC + // a UUID parsing error if the resource has another interface in the VPC // Subnet of the one we're trying to insert. PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), @@ -313,7 +330,8 @@ fn decode_database_error( PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), )) if info.message() == INSTANCE_BAD_STATE_ERROR_MESSAGE => { - InsertError::InstanceMustBeStopped(interface.instance_id) + assert_eq!(interface.kind, NetworkInterfaceKind::Instance); + InsertError::InstanceMustBeStopped(interface.parent_id) } // This catches the UUID-cast failure intentionally introduced by // `push_instance_state_verification_subquery`, which verifies that @@ -321,7 +339,8 @@ fn decode_database_error( PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), )) if info.message() == NO_INSTANCE_ERROR_MESSAGE => { - InsertError::InstanceNotFound(interface.instance_id) + assert_eq!(interface.kind, NetworkInterfaceKind::Instance); + InsertError::InstanceNotFound(interface.parent_id) } // This path looks specifically at constraint names. @@ -337,12 +356,20 @@ fn decode_database_error( InsertError::IpAddressNotAvailable(ip) } // Constraint violated if the user-requested name is already - // assigned to an interface on this instance. + // assigned to an interface on this resource Some(constraint) if constraint == NAME_CONFLICT_CONSTRAINT => { + let resource_type = match interface.kind { + NetworkInterfaceKind::Instance => { + external::ResourceType::InstanceNetworkInterface + } + NetworkInterfaceKind::Service => { + todo!("service network interface") + } + }; InsertError::External(error::public_error_from_diesel_pool( err, error::ErrorHandler::Conflict( - external::ResourceType::NetworkInterface, + resource_type, interface.identity.name.as_str(), ), )) @@ -352,6 +379,7 @@ fn decode_database_error( Some(constraint) if constraint == ID_CONFLICT_CONSTRAINT => { InsertError::InterfaceAlreadyExists( interface.identity.name.to_string(), + interface.kind, ) } // Any other constraint violation is a bug @@ -427,10 +455,10 @@ fn first_available_address(subnet: &IpNetwork) -> IpAddr { } } -/// The `NextGuestIpv4Address` query is a `NextItem` query for choosing the next -/// available IPv4 address for a guest interface. +/// The `NextIpv4Address` query is a `NextItem` query for choosing the next +/// available IPv4 address for an interface. #[derive(Debug, Clone, Copy)] -pub struct NextGuestIpv4Address { +pub struct NextIpv4Address { inner: NextItem< db::schema::network_interface::table, IpNetwork, @@ -440,7 +468,7 @@ pub struct NextGuestIpv4Address { >, } -impl NextGuestIpv4Address { +impl NextIpv4Address { pub fn new(subnet: Ipv4Network, subnet_id: Uuid) -> Self { let subnet = IpNetwork::from(subnet); let net = IpNetwork::from(first_available_address(&subnet)); @@ -451,26 +479,25 @@ impl NextGuestIpv4Address { } } -delegate_query_fragment_impl!(NextGuestIpv4Address); +delegate_query_fragment_impl!(NextIpv4Address); -/// A `[NextItem`] subquery that selects the next empty slot for an interface. +/// A `NextItem` subquery that selects the next empty slot for an interface. /// -/// Instances are limited to 8 interfaces (RFD 135). This pushes a subquery that -/// looks like: +/// This pushes a subquery that looks like: /// /// ```sql /// SELECT COALESCE(( /// SELECT /// next_slot /// FROM -/// generate_series(0, ) +/// generate_series(0, ) /// AS /// next_slot /// LEFT OUTER JOIN /// network_interface /// ON -/// (instance_id, time_deleted IS NULL, slot) = -/// (, TRUE, next_slot) +/// (parent_id, time_deleted IS NULL, slot) = +/// (, TRUE, next_slot) /// WHERE /// slot IS NULL /// LIMIT 1) @@ -480,6 +507,9 @@ delegate_query_fragment_impl!(NextGuestIpv4Address); /// That is, we select the lowest slot that has not yet been claimed by an /// interface on this instance, or zero if there is no such instance at all. /// +/// `parent_id` is the UUID of the parent resource (e.g., `instance_id`) the +/// interface will be associated with. +/// /// Errors /// ------ /// @@ -496,19 +526,19 @@ pub struct NextNicSlot { i16, db::schema::network_interface::dsl::slot, Uuid, - db::schema::network_interface::dsl::instance_id, + db::schema::network_interface::dsl::parent_id, >, } impl NextNicSlot { - pub fn new(instance_id: Uuid) -> Self { + pub fn new(parent_id: Uuid) -> Self { let generator = DefaultShiftGenerator { base: 0, - max_shift: i64::try_from(MAX_NICS_PER_INSTANCE) + max_shift: i64::try_from(MAX_NICS) .expect("Too many network interfaces"), min_shift: 0, }; - Self { inner: NextItem::new_scoped(generator, instance_id) } + Self { inner: NextItem::new_scoped(generator, parent_id) } } } @@ -521,10 +551,10 @@ impl QueryFragment for NextNicSlot { } } -/// A `NextItem` query that selects a random available MAC address for a guest -/// network interface. +/// A `NextItem` query that selects a random available MAC address for +/// a network interface. #[derive(Debug, Clone, Copy)] -pub struct NextGuestMacAddress { +pub struct NextMacAddress { inner: NextItem< db::schema::network_interface::table, MacAddr, @@ -534,34 +564,47 @@ pub struct NextGuestMacAddress { >, } -impl NextGuestMacAddress { - pub fn new(vpc_id: Uuid) -> Self { - let base = MacAddr::random_guest(); - let x = base.to_i64(); - let max_shift = MacAddr::MAX_GUEST_ADDR - x; - let min_shift = x - MacAddr::MIN_GUEST_ADDR; +impl NextMacAddress { + pub fn new(vpc_id: Uuid, kind: NetworkInterfaceKind) -> Self { + let (base, max_shift, min_shift) = match kind { + NetworkInterfaceKind::Instance => { + let base = MacAddr::random_guest(); + let x = base.to_i64(); + let max_shift = MacAddr::MAX_GUEST_ADDR - x; + let min_shift = x - MacAddr::MIN_GUEST_ADDR; + (base, max_shift, min_shift) + } + NetworkInterfaceKind::Service => { + let base = MacAddr::random_system(); + let x = base.to_i64(); + let max_shift = MacAddr::MAX_SYSTEM_ADDR - x; + let min_shift = x - MacAddr::MAX_SYSTEM_ADDR; + (base, max_shift, min_shift) + } + }; let generator = DefaultShiftGenerator { base, max_shift, min_shift }; Self { inner: NextItem::new_scoped(generator, vpc_id) } } } -delegate_query_fragment_impl!(NextGuestMacAddress); +delegate_query_fragment_impl!(NextMacAddress); -/// Add a subquery intended to verify that an Instance's networking does not +/// Add a subquery intended to verify that a resource's networking does not /// span multiple VPCs. /// /// As described in RFD 21, an Instance's networking is confined to a single /// VPC. That is, any NetworkInterfaces attached to an Instance must all have -/// the same VPC ID. This function adds a subquery, shown below, that fails in a +/// the same VPC ID. We apply that same restriction to any interface kind. +/// This function adds a subquery, shown below, that fails in a /// specific way (parsing error) if that invariant is violated. The basic /// structure of the query is: /// /// ```text -/// CAST(IF(, '', 'multiple-vpcs') AS UUID) +/// CAST(IF(, '', 'multiple-vpcs') AS UUID) /// ``` /// /// This selects either the actual VPC UUID (as a string) or the literal string -/// "multiple-vpcs" if any existing VPC IDs for this instance are the same. If +/// "multiple-vpcs" if any existing VPC IDs for this resource are the same. If /// true, we cast the VPC ID string back to a UUID. If false, we try to cast the /// string `"multiple-vpcs"` which fails in a detectable way. /// @@ -578,7 +621,8 @@ delegate_query_fragment_impl!(NextGuestMacAddress); /// FROM network_interface /// WHERE /// time_deleted IS NULL AND -/// instance_id = +/// parent_id = AND +/// kind = /// LIMIT 1 /// ), /// @@ -589,7 +633,7 @@ delegate_query_fragment_impl!(NextGuestMacAddress); /// ``` /// /// This uses a partial index on the `network_interface` table to look up the -/// first record with the provided `instance_id`, if any. It then compares that +/// first record with the provided `parent_id`, if any. It then compares that /// stored `vpc_id` to the one provided to this query. If those IDs match, then /// the ID is returned. If they do _not_ match, the `IF` statement returns the /// string "multiple-vpcs", which it tries to cast as a UUID. That fails, in a @@ -603,7 +647,8 @@ fn push_ensure_unique_vpc_expression<'a>( mut out: AstPass<'_, 'a, Pg>, vpc_id: &'a Uuid, vpc_id_str: &'a String, - instance_id: &'a Uuid, + kind: &'a NetworkInterfaceKind, + parent_id: &'a Uuid, ) -> diesel::QueryResult<()> { out.push_sql("CAST(IF(COALESCE((SELECT "); out.push_identifier(dsl::vpc_id::NAME)?; @@ -612,9 +657,15 @@ fn push_ensure_unique_vpc_expression<'a>( out.push_sql(" WHERE "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL AND "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::parent_id::NAME)?; out.push_sql(" = "); - out.push_bind_param::(instance_id)?; + out.push_bind_param::(parent_id)?; + out.push_sql(" AND "); + out.push_identifier(dsl::kind::NAME)?; + out.push_sql(" = "); + out.push_bind_param::( + kind, + )?; out.push_sql(" LIMIT 1), "); out.push_bind_param::(vpc_id)?; out.push_sql(") = "); @@ -631,7 +682,7 @@ fn push_ensure_unique_vpc_expression<'a>( // To do that, we generate a query like: // // ``` - // CAST(IF(, '', 'multiple-vpcs') AS UUID) + // CAST(IF(, '', 'multiple-vpcs') AS UUID) // ``` // // The string "multiple-vpcs" cannot be cast to a UUID, so we get a parsing @@ -648,7 +699,7 @@ fn push_ensure_unique_vpc_expression<'a>( Ok(()) } -/// Push a subquery that checks that all NICs for an instance are in distinct +/// Push a subquery that checks that all NICs for a resource are in distinct /// VPC Subnets. /// /// This generates a subquery like: @@ -660,7 +711,8 @@ fn push_ensure_unique_vpc_expression<'a>( /// FROM network_interface /// WHERE /// id != AND -/// instance_id = AND +/// parent_id = AND +/// kind = AND /// time_deleted IS NULL AND /// subnet_id = /// ), @@ -678,7 +730,8 @@ fn push_ensure_unique_vpc_subnet_expression<'a>( interface_id: &'a Uuid, subnet_id: &'a Uuid, subnet_id_str: &'a String, - instance_id: &'a Uuid, + kind: &'a NetworkInterfaceKind, + parent_id: &'a Uuid, ) -> diesel::QueryResult<()> { out.push_sql("CAST(IF(EXISTS(SELECT "); out.push_identifier(dsl::subnet_id::NAME)?; @@ -689,9 +742,15 @@ fn push_ensure_unique_vpc_subnet_expression<'a>( out.push_sql(" != "); out.push_bind_param::(interface_id)?; out.push_sql(" AND "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::parent_id::NAME)?; out.push_sql(" = "); - out.push_bind_param::(instance_id)?; + out.push_bind_param::(parent_id)?; + out.push_sql(" AND "); + out.push_identifier(dsl::kind::NAME)?; + out.push_sql(" = "); + out.push_bind_param::( + kind, + )?; out.push_sql(" AND "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL AND "); @@ -704,41 +763,42 @@ fn push_ensure_unique_vpc_subnet_expression<'a>( Ok(()) } -/// Push the main instance-validation common-table expression. +/// Push the main interface-validation common-table expression. /// /// This generates a CTE that looks like: /// /// ```sql -/// WITH validated_instance(vpc_id, subnet_id, slot, is_primary) AS +/// WITH validated_interface(vpc_id, subnet_id, slot, is_primary) AS /// ( /// , /// , -/// , +/// , /// , /// /// ) /// ``` #[allow(clippy::too_many_arguments)] -fn push_instance_validation_cte<'a>( +fn push_interface_validation_cte<'a>( mut out: AstPass<'_, 'a, Pg>, interface_id: &'a Uuid, vpc_id: &'a Uuid, vpc_id_str: &'a String, subnet_id: &'a Uuid, subnet_id_str: &'a String, - instance_id: &'a Uuid, - instance_id_str: &'a String, + kind: &'a NetworkInterfaceKind, + parent_id: &'a Uuid, + parent_id_str: &'a String, next_slot_subquery: &'a NextNicSlot, is_primary_subquery: &'a IsPrimaryNic, ) -> diesel::QueryResult<()> { - // Push the `validated_instance` CTE, which ensures that the VPC and VPC + // Push the `validated_interface` CTE, which ensures that the VPC and VPC // Subnet are valid, and also selects the slot / is_primary. - out.push_sql("WITH validated_instance("); + out.push_sql("WITH validated_interface("); out.push_identifier(dsl::vpc_id::NAME)?; out.push_sql(", "); out.push_identifier(dsl::subnet_id::NAME)?; out.push_sql(", "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::parent_id::NAME)?; out.push_sql(", "); out.push_identifier(dsl::slot::NAME)?; out.push_sql(", "); @@ -748,7 +808,8 @@ fn push_instance_validation_cte<'a>( out.reborrow(), vpc_id, vpc_id_str, - instance_id, + kind, + parent_id, )?; out.push_sql(" AS "); out.push_identifier(dsl::vpc_id::NAME)?; @@ -758,29 +819,35 @@ fn push_instance_validation_cte<'a>( interface_id, subnet_id, subnet_id_str, - instance_id, + kind, + parent_id, )?; out.push_sql(" AS "); out.push_identifier(dsl::subnet_id::NAME)?; - // Push the subquery to ensure the instance state when trying to insert the - // new interface. out.push_sql(", ("); - push_instance_state_verification_subquery( - instance_id, - instance_id_str, - out.reborrow(), - false, - )?; + // Push the subquery to ensure the instance state when trying to insert the + // new interface, if `kind=instance` + if *kind == NetworkInterfaceKind::Instance { + push_instance_state_verification_subquery( + parent_id, + parent_id_str, + out.reborrow(), + false, + )?; + } else { + out.push_sql("SELECT "); + out.push_bind_param::(parent_id)?; + } // Push the subquery used to select and validate the slot number for the // interface, including validating that there are available slots on the - // instance. + // resource. out.push_sql("), ("); next_slot_subquery.walk_ast(out.reborrow())?; // Push the subquery used to detect whether this interface is the primary. - // That's true iff there are zero interfaces for this instance at the time + // That's true iff there are zero interfaces for this resource at the time // this interface is inserted. out.push_sql("), ("); is_primary_subquery.walk_ast(out.reborrow())?; @@ -834,7 +901,7 @@ fn push_instance_validation_cte<'a>( /// /// If the user wants an address allocated, then this generates a subquery that /// tries to find the next available IP address (if any). See -/// [`NextGuestIpv4Address`] for details on that allocation subquery. If that +/// [`NextIpv4Address`] for details on that allocation subquery. If that /// fails, due to address exhaustion, this is detected and forwarded to the /// caller. /// @@ -883,10 +950,10 @@ pub struct InsertQuery { // long as the entire call to [`QueryFragment::walk_ast`]. vpc_id_str: String, subnet_id_str: String, - instance_id_str: String, + parent_id_str: String, ip_sql: Option, - next_mac_subquery: NextGuestMacAddress, - next_ipv4_address_subquery: NextGuestIpv4Address, + next_mac_subquery: NextMacAddress, + next_ipv4_address_subquery: NextIpv4Address, next_slot_subquery: NextNicSlot, is_primary_subquery: IsPrimaryNic, } @@ -895,22 +962,24 @@ impl InsertQuery { pub fn new(interface: IncompleteNetworkInterface) -> Self { let vpc_id_str = interface.vpc_id.to_string(); let subnet_id_str = interface.subnet.identity.id.to_string(); - let instance_id_str = interface.instance_id.to_string(); + let kind = interface.kind; + let parent_id_str = interface.parent_id.to_string(); let ip_sql = interface.ip.map(|ip| ip.into()); - let next_mac_subquery = NextGuestMacAddress::new(interface.vpc_id); - let next_ipv4_address_subquery = NextGuestIpv4Address::new( + let next_mac_subquery = + NextMacAddress::new(interface.vpc_id, interface.kind); + let next_ipv4_address_subquery = NextIpv4Address::new( interface.subnet.ipv4_block.0 .0, interface.subnet.identity.id, ); - let next_slot_subquery = NextNicSlot::new(interface.instance_id); + let next_slot_subquery = NextNicSlot::new(interface.parent_id); let is_primary_subquery = - IsPrimaryNic { instance_id: interface.instance_id }; + IsPrimaryNic { kind, parent_id: interface.parent_id }; Self { interface, now: Utc::now(), vpc_id_str, subnet_id_str, - instance_id_str, + parent_id_str, ip_sql, next_mac_subquery, next_ipv4_address_subquery, @@ -951,15 +1020,16 @@ impl QueryFragment for InsertQuery { // - `subnet_id` // - `slot` // - `is_primary` - push_instance_validation_cte( + push_interface_validation_cte( out.reborrow(), &self.interface.identity.id, &self.interface.vpc_id, &self.vpc_id_str, &self.interface.subnet.identity.id, &self.subnet_id_str, - &self.interface.instance_id, - &self.instance_id_str, + &self.interface.kind, + &self.interface.parent_id, + &self.parent_id_str, &self.next_slot_subquery, &self.is_primary_subquery, )?; @@ -1008,11 +1078,18 @@ impl QueryFragment for InsertQuery { out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(", "); + out.push_bind_param::( + &self.interface.kind, + )?; + out.push_sql(" AS "); + out.push_identifier(dsl::kind::NAME)?; + out.push_sql(", "); + out.push_bind_param::( - &self.interface.instance_id, + &self.interface.parent_id, )?; out.push_sql(" AS "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::parent_id::NAME)?; out.push_sql(", "); // Helper function to push a subquery selecting something from the CTE. @@ -1022,7 +1099,7 @@ impl QueryFragment for InsertQuery { ) -> diesel::QueryResult<()> { out.push_sql("(SELECT "); out.push_identifier(column)?; - out.push_sql(" FROM validated_instance)"); + out.push_sql(" FROM validated_interface)"); Ok(()) } @@ -1094,7 +1171,9 @@ impl QueryFragment for InsertQueryValues { out.push_sql(", "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(", "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::kind::NAME)?; + out.push_sql(", "); + out.push_identifier(dsl::parent_id::NAME)?; out.push_sql(", "); out.push_identifier(dsl::vpc_id::NAME)?; out.push_sql(", "); @@ -1115,13 +1194,13 @@ impl QueryFragment for InsertQueryValues { /// A small helper subquery that automatically assigns the `is_primary` column /// for a new network interface. /// -/// An instance with any network interfaces must have exactly one primary. (An -/// instance may have zero interfaces, however.) This subquery is used to insert -/// the value `true` if there are no extant interfaces on an instance, or -/// `false` if there are. +/// A resource with any network interfaces must have exactly one primary. +/// (It may have zero interfaces, however.) This subquery is used to insert +/// the value `true` if there are no extant interfaces, or `false` if there are. #[derive(Debug, Clone, Copy)] struct IsPrimaryNic { - instance_id: Uuid, + parent_id: Uuid, + kind: NetworkInterfaceKind, } impl QueryId for IsPrimaryNic { @@ -1137,9 +1216,15 @@ impl QueryFragment for IsPrimaryNic { out.push_sql("SELECT NOT EXISTS(SELECT 1 FROM "); NETWORK_INTERFACE_FROM_CLAUSE.walk_ast(out.reborrow())?; out.push_sql(" WHERE "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::parent_id::NAME)?; + out.push_sql(" = "); + out.push_bind_param::(&self.parent_id)?; + out.push_sql(" AND "); + out.push_identifier(dsl::kind::NAME)?; out.push_sql(" = "); - out.push_bind_param::(&self.instance_id)?; + out.push_bind_param::( + &self.kind, + )?; out.push_sql(" AND "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL LIMIT 1)"); @@ -1156,7 +1241,7 @@ const INSTANCE_FROM_CLAUSE: InstanceFromClause = InstanceFromClause::new(); // This pushes a subquery like: // // ```sql -// SELECT CAST( +// CAST( // CASE // COALESCE( // -- Identify the state of the instance @@ -1241,17 +1326,39 @@ fn push_instance_state_verification_subquery<'a>( out.push_sql(" END AS UUID)"); Ok(()) } -/// Delete a network interface from an instance. + +/// Delete a network interface. /// /// There are a few preconditions that need to be checked when deleting a NIC. -/// First, the instance must currently be stopped, though we may relax this in -/// the future. Second, while an instance may have zero or more interfaces, if -/// it has one or more, exactly one of those must be the primary interface. That -/// means we can only delete the primary interface if there are no secondary -/// interfaces. The full query is: +/// First, if it's an instance-kind interface then the instance must currently +/// be stopped, though we may relax this in the future. +/// Second, while an instance may have zero or more interfaces, if it has one +/// or more, exactly one of those must be the primary interface. That means +/// we can only delete the primary interface if there are no secondary interfaces. +/// The full query is: /// /// ```sql /// WITH +/// instance AS MATERIALIZED (SELECT CAST( +/// CASE +/// COALESCE( +/// (SELECT +/// state +/// FROM +/// instance +/// WHERE +/// id = AND +/// time_deleted IS NULL +/// ), +/// 'destroyed' +/// ) +/// WHEN 'stopped' THEN '' +/// WHEN 'creating' THEN '' +/// WHEN 'failed' THEN '' +/// WHEN 'destroyed' THEN 'no-instance' +/// ELSE 'bad-state' +/// END +/// AS UUID)), /// interface AS MATERIALIZED ( /// SELECT CAST(IF( /// ( @@ -1270,30 +1377,14 @@ fn push_instance_state_verification_subquery<'a>( /// FROM /// network_interface /// WHERE -/// instance_id = AND +/// parent_id = AND +/// kind = AND /// time_deleted IS NULL /// ) = 1, /// '', /// 'secondaries' /// ) AS UUID) /// ) -/// instance AS MATERIALIZED ( -/// SELECT CAST(CASE COALESCE(( -/// SELECT -/// state -/// FROM -/// instance -/// WHERE -/// id = AND -/// time_deleted IS NULL -/// )), 'destroyed') -/// WHEN 'stopped' THEN '' -/// WHEN 'creating' THEN '' -/// WHEN 'failed' THEN '' -/// WHEN 'destroyed' THEN 'no-instance' -/// ELSE 'bad-state' -/// ) AS UUID) -/// ) /// UPDATE /// network_interface /// SET @@ -1308,20 +1399,28 @@ fn push_instance_state_verification_subquery<'a>( /// /// As with some of the other queries in this module, this uses some casting /// trickery to learn why the query fails. This is why we store the -/// `instance_id` as a string in this type. +/// `parent_id` as a string in this type. +/// +/// The `instance` CTE is only present if the interface is an instance-kind. #[derive(Debug, Clone)] pub struct DeleteQuery { interface_id: Uuid, - instance_id: Uuid, - instance_id_str: String, + kind: NetworkInterfaceKind, + parent_id: Uuid, + parent_id_str: String, } impl DeleteQuery { - pub fn new(instance_id: Uuid, interface_id: Uuid) -> Self { + pub fn new( + kind: NetworkInterfaceKind, + parent_id: Uuid, + interface_id: Uuid, + ) -> Self { Self { interface_id, - instance_id, - instance_id_str: instance_id.to_string(), + kind, + parent_id, + parent_id_str: parent_id.to_string(), } } } @@ -1336,16 +1435,18 @@ impl QueryFragment for DeleteQuery { &'a self, mut out: AstPass<'_, 'a, Pg>, ) -> diesel::QueryResult<()> { - out.push_sql("WITH instance AS MATERIALIZED (SELECT "); - push_instance_state_verification_subquery( - &self.instance_id, - &self.instance_id_str, - out.reborrow(), - true, - )?; - out.push_sql( - "), interface AS MATERIALIZED (SELECT CAST(IF((SELECT NOT ", - ); + out.push_sql("WITH "); + if self.kind == NetworkInterfaceKind::Instance { + out.push_sql("instance AS MATERIALIZED (SELECT "); + push_instance_state_verification_subquery( + &self.parent_id, + &self.parent_id_str, + out.reborrow(), + true, + )?; + out.push_sql("), "); + } + out.push_sql("interface AS MATERIALIZED (SELECT CAST(IF((SELECT NOT "); out.push_identifier(dsl::is_primary::NAME)?; out.push_sql(" FROM "); NETWORK_INTERFACE_FROM_CLAUSE.walk_ast(out.reborrow())?; @@ -1358,13 +1459,19 @@ impl QueryFragment for DeleteQuery { out.push_sql(" IS NULL) OR (SELECT COUNT(*) FROM "); NETWORK_INTERFACE_FROM_CLAUSE.walk_ast(out.reborrow())?; out.push_sql(" WHERE "); - out.push_identifier(dsl::instance_id::NAME)?; + out.push_identifier(dsl::parent_id::NAME)?; out.push_sql(" = "); - out.push_bind_param::(&self.instance_id)?; + out.push_bind_param::(&self.parent_id)?; + out.push_sql(" AND "); + out.push_identifier(dsl::kind::NAME)?; + out.push_sql(" = "); + out.push_bind_param::( + &self.kind, + )?; out.push_sql(" AND "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL) = 1, "); - out.push_bind_param::(&self.instance_id_str)?; + out.push_bind_param::(&self.parent_id_str)?; out.push_sql(", "); out.push_bind_param::( &DeleteError::HAS_SECONDARIES_SENTINEL, @@ -1386,12 +1493,12 @@ impl QueryFragment for DeleteQuery { impl RunQueryDsl for DeleteQuery {} -/// Errors related to deleting a network interface from an instance +/// Errors related to deleting a network interface #[derive(Debug)] pub enum DeleteError { /// Attempting to delete the primary interface, while there still exist /// secondary interfaces. - InstanceHasSecondaryInterfaces(Uuid), + SecondariesExist(Uuid), /// Instance must be stopped or failed prior to deleting interfaces from it InstanceBadState(Uuid), /// The instance does not exist at all, or is in the destroyed state. @@ -1424,7 +1531,7 @@ impl DeleteError { Error::DatabaseError(_, _), )) => decode_delete_network_interface_database_error( e, - query.instance_id, + query.parent_id, ), // Any other error at all is a bug _ => DeleteError::External(error::public_error_from_diesel_pool( @@ -1437,9 +1544,9 @@ impl DeleteError { /// Convert this error into an external one. pub fn into_external(self) -> external::Error { match self { - DeleteError::InstanceHasSecondaryInterfaces(_) => { + DeleteError::SecondariesExist(_) => { external::Error::invalid_request( - "The primary interface for an instance \ + "The primary interface \ may not be deleted while secondary interfaces \ are still attached", ) @@ -1469,7 +1576,7 @@ impl DeleteError { /// including the software version and our schema. fn decode_delete_network_interface_database_error( err: async_bb8_diesel::PoolError, - instance_id: Uuid, + parent_id: Uuid, ) -> DeleteError { use crate::db::error; use async_bb8_diesel::ConnectionError; @@ -1491,7 +1598,7 @@ fn decode_delete_network_interface_database_error( PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), )) if info.message() == HAS_SECONDARIES_ERROR_MESSAGE => { - DeleteError::InstanceHasSecondaryInterfaces(instance_id) + DeleteError::SecondariesExist(parent_id) } // This catches the UUID-cast failure intentionally introduced by @@ -1500,7 +1607,7 @@ fn decode_delete_network_interface_database_error( PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), )) if info.message() == INSTANCE_BAD_STATE_ERROR_MESSAGE => { - DeleteError::InstanceBadState(instance_id) + DeleteError::InstanceBadState(parent_id) } // This catches the UUID-cast failure intentionally introduced by // `push_instance_state_verification_subquery`, which verifies that @@ -1508,7 +1615,7 @@ fn decode_delete_network_interface_database_error( PoolError::Connection(ConnectionError::Query( Error::DatabaseError(DatabaseErrorKind::Unknown, ref info), )) if info.message() == NO_INSTANCE_ERROR_MESSAGE => { - DeleteError::InstanceNotFound(instance_id) + DeleteError::InstanceNotFound(parent_id) } // Any other error at all is a bug @@ -1524,7 +1631,7 @@ mod tests { use super::first_available_address; use super::last_address_offset; use super::InsertError; - use super::MAX_NICS_PER_INSTANCE; + use super::MAX_NICS; use super::NUM_INITIAL_RESERVED_IP_ADDRESSES; use crate::authz; use crate::context::OpContext; @@ -1543,6 +1650,7 @@ mod tests { use dropshot::test_util::LogContext; use ipnetwork::Ipv4Network; use ipnetwork::Ipv6Network; + use model::NetworkInterfaceKind; use nexus_test_utils::db::test_setup_database; use nexus_types::external_api::params; use nexus_types::external_api::params::InstanceCreate; @@ -1799,7 +1907,7 @@ mod tests { context.create_instance(external::InstanceState::Running).await; let instance_id = instance.id(); let requested_ip = "172.30.0.5".parse().unwrap(); - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance_id, context.net1.vpc_id, @@ -1830,7 +1938,7 @@ mod tests { context.create_instance(external::InstanceState::Stopped).await; let instance_id = instance.id(); let requested_ip = "172.30.0.5".parse().unwrap(); - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance_id, context.net1.vpc_id, @@ -1850,7 +1958,7 @@ mod tests { ) .await .expect("Failed to insert interface with known-good IP address"); - assert_interfaces_eq(&interface, &inserted_interface); + assert_interfaces_eq(&interface, &inserted_interface.clone().into()); assert_eq!( inserted_interface.ip.ip(), requested_ip, @@ -1863,7 +1971,7 @@ mod tests { async fn test_insert_no_instance_fails() { let context = TestContext::new("test_insert_no_instance_fails", 2).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), Uuid::new_v4(), context.net1.vpc_id, @@ -1902,7 +2010,7 @@ mod tests { for (i, expected_address) in addresses.take(2).enumerate() { let instance = context.create_instance(external::InstanceState::Stopped).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -1922,7 +2030,10 @@ mod tests { ) .await .expect("Failed to insert interface"); - assert_interfaces_eq(&interface, &inserted_interface); + assert_interfaces_eq( + &interface, + &inserted_interface.clone().into(), + ); let actual_address = inserted_interface.ip.ip(); assert_eq!( actual_address, expected_address, @@ -1943,7 +2054,7 @@ mod tests { context.create_instance(external::InstanceState::Stopped).await; // Insert an interface on the first instance. - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -1963,7 +2074,7 @@ mod tests { // Inserting an interface with the same IP should fail, even if all // other parameters are valid. - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), new_instance.id(), context.net1.vpc_id, @@ -1992,7 +2103,7 @@ mod tests { TestContext::new("test_insert_with_duplicate_name_fails", 2).await; let instance = context.create_instance(external::InstanceState::Stopped).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2012,7 +2123,7 @@ mod tests { ) .await .expect("Failed to insert interface"); - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2044,7 +2155,7 @@ mod tests { TestContext::new("test_insert_same_vpc_subnet_fails", 2).await; let instance = context.create_instance(external::InstanceState::Stopped).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2061,7 +2172,7 @@ mod tests { .instance_create_network_interface_raw(&context.opctx, interface) .await .expect("Failed to insert interface"); - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2090,7 +2201,7 @@ mod tests { TestContext::new("test_insert_same_interface_fails", 2).await; let instance = context.create_instance(external::InstanceState::Stopped).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2115,7 +2226,13 @@ mod tests { .instance_create_network_interface_raw(&context.opctx, interface) .await; assert!( - matches!(result, Err(InsertError::InterfaceAlreadyExists(_))), + matches!( + result, + Err(InsertError::InterfaceAlreadyExists( + _, + NetworkInterfaceKind::Instance + )), + ), "Expected that interface would already exist", ); context.success().await; @@ -2127,7 +2244,7 @@ mod tests { TestContext::new("test_insert_multiple_vpcs_fails", 2).await; let instance = context.create_instance(external::InstanceState::Stopped).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2146,7 +2263,7 @@ mod tests { .expect("Failed to insert interface"); let expected_address = "172.30.0.5".parse().unwrap(); for addr in [Some(expected_address), None] { - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net2.vpc_id, @@ -2166,8 +2283,8 @@ mod tests { ) .await; assert!( - matches!(result, Err(InsertError::InstanceSpansMultipleVpcs(_))), - "Attaching an interface to an instance which already has one in a different VPC should fail" + matches!(result, Err(InsertError::ResourceSpansMultipleVpcs(_))), + "Attaching an interface to a resource which already has one in a different VPC should fail" ); } context.success().await; @@ -2183,7 +2300,7 @@ mod tests { for _ in 0..n_interfaces { let instance = context.create_instance(external::InstanceState::Stopped).await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2212,7 +2329,7 @@ mod tests { &context.db_datastore, ) .await; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2245,7 +2362,7 @@ mod tests { let instance = context.create_instance(external::InstanceState::Stopped).await; for (i, subnet) in context.net1.subnets.iter().enumerate() { - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2281,14 +2398,23 @@ mod tests { assert_eq!(inserted.id(), incomplete.identity.id); assert_eq!(inserted.name(), &incomplete.identity.name.0); assert_eq!(inserted.description(), incomplete.identity.description); - assert_eq!(inserted.instance_id, incomplete.instance_id); + assert_eq!(inserted.kind, incomplete.kind); + assert_eq!(inserted.parent_id, incomplete.parent_id); assert_eq!(inserted.vpc_id, incomplete.vpc_id); assert_eq!(inserted.subnet_id, incomplete.subnet.id()); + let (lo, hi, kind) = match incomplete.kind { + NetworkInterfaceKind::Instance => { + (MacAddr::MIN_GUEST_ADDR, MacAddr::MAX_GUEST_ADDR, "guest") + } + NetworkInterfaceKind::Service => { + (MacAddr::MIN_SYSTEM_ADDR, MacAddr::MAX_SYSTEM_ADDR, "system") + } + }; assert!( - inserted.mac.to_i64() >= MacAddr::MIN_GUEST_ADDR - && inserted.mac.to_i64() <= MacAddr::MAX_GUEST_ADDR, - "The random MAC address {:?} is not a valid guest address", + inserted.mac.to_i64() >= lo && inserted.mac.to_i64() <= hi, + "The random MAC address {:?} is not a valid {} address", inserted.mac, + kind, ); } @@ -2298,14 +2424,14 @@ mod tests { async fn test_limit_number_of_interfaces_per_instance_query() { let context = TestContext::new( "test_limit_number_of_interfaces_per_instance_query", - MAX_NICS_PER_INSTANCE as u8 + 1, + MAX_NICS as u8 + 1, ) .await; let instance = context.create_instance(external::InstanceState::Stopped).await; - for slot in 0..MAX_NICS_PER_INSTANCE { + for slot in 0..MAX_NICS { let subnet = &context.net1.subnets[slot]; - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, @@ -2342,7 +2468,7 @@ mod tests { } // The next one should fail - let interface = IncompleteNetworkInterface::new( + let interface = IncompleteNetworkInterface::new_instance( Uuid::new_v4(), instance.id(), context.net1.vpc_id, diff --git a/nexus/db-queries/tests/output/authz-roles.out b/nexus/db-queries/tests/output/authz-roles.out index 888506095a..bab2a424ac 100644 --- a/nexus/db-queries/tests/output/authz-roles.out +++ b/nexus/db-queries/tests/output/authz-roles.out @@ -236,7 +236,7 @@ resource: Instance "silo1-proj1-instance1" silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! -resource: NetworkInterface "silo1-proj1-instance1-nic1" +resource: InstanceNetworkInterface "silo1-proj1-instance1-nic1" USER Q R LC RP M MP CC D fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ @@ -348,7 +348,7 @@ resource: Instance "silo1-proj2-instance1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! -resource: NetworkInterface "silo1-proj2-instance1-nic1" +resource: InstanceNetworkInterface "silo1-proj2-instance1-nic1" USER Q R LC RP M MP CC D fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ @@ -572,7 +572,7 @@ resource: Instance "silo2-proj1-instance1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! -resource: NetworkInterface "silo2-proj1-instance1-nic1" +resource: InstanceNetworkInterface "silo2-proj1-instance1-nic1" USER Q R LC RP M MP CC D fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ diff --git a/nexus/src/app/network_interface.rs b/nexus/src/app/network_interface.rs index d1940009a8..ea6f1d9483 100644 --- a/nexus/src/app/network_interface.rs +++ b/nexus/src/app/network_interface.rs @@ -20,31 +20,31 @@ use crate::db; use crate::db::lookup::{self, LookupPath}; impl super::Nexus { - pub fn network_interface_lookup<'a>( + pub fn instance_network_interface_lookup<'a>( &'a self, opctx: &'a OpContext, - network_interface_selector: &'a params::NetworkInterfaceSelector, - ) -> LookupResult> { + network_interface_selector: &'a params::InstanceNetworkInterfaceSelector, + ) -> LookupResult> { match network_interface_selector { - params::NetworkInterfaceSelector { + params::InstanceNetworkInterfaceSelector { instance_selector: None, network_interface: NameOrId::Id(id), } => { let network_interface = LookupPath::new(opctx, &self.db_datastore) - .network_interface_id(*id); + .instance_network_interface_id(*id); Ok(network_interface) } - params::NetworkInterfaceSelector { + params::InstanceNetworkInterfaceSelector { instance_selector: Some(instance_selector), network_interface: NameOrId::Name(name), } => { let network_interface = self .instance_lookup(opctx, instance_selector)? - .network_interface_name(Name::ref_cast(name)); + .instance_network_interface_name(Name::ref_cast(name)); Ok(network_interface) } - params::NetworkInterfaceSelector { + params::InstanceNetworkInterfaceSelector { instance_selector: Some(_), network_interface: NameOrId::Id(_), } => { @@ -68,8 +68,8 @@ impl super::Nexus { &self, opctx: &OpContext, instance_lookup: &lookup::Instance<'_>, - params: ¶ms::NetworkInterfaceCreate, - ) -> CreateResult { + params: ¶ms::InstanceNetworkInterfaceCreate, + ) -> CreateResult { let (.., authz_project, authz_instance) = instance_lookup.lookup_for(authz::Action::Modify).await?; @@ -85,7 +85,7 @@ impl super::Nexus { .fetch() .await?; let interface_id = Uuid::new_v4(); - let interface = db::model::IncompleteNetworkInterface::new( + let interface = db::model::IncompleteNetworkInterface::new_instance( interface_id, authz_instance.id(), authz_vpc.id(), @@ -125,12 +125,12 @@ impl super::Nexus { } /// Lists network interfaces attached to the instance. - pub async fn network_interface_list( + pub async fn instance_network_interface_list( &self, opctx: &OpContext, instance_lookup: &lookup::Instance<'_>, pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec { let (.., authz_instance) = instance_lookup.lookup_for(authz::Action::ListChildren).await?; self.db_datastore @@ -139,12 +139,12 @@ impl super::Nexus { } /// Update a network interface for the given instance. - pub async fn network_interface_update( + pub async fn instance_network_interface_update( &self, opctx: &OpContext, - network_interface_lookup: &lookup::NetworkInterface<'_>, - updates: params::NetworkInterfaceUpdate, - ) -> UpdateResult { + network_interface_lookup: &lookup::InstanceNetworkInterface<'_>, + updates: params::InstanceNetworkInterfaceUpdate, + ) -> UpdateResult { let (.., authz_instance, authz_interface) = network_interface_lookup.lookup_for(authz::Action::Modify).await?; self.db_datastore @@ -161,10 +161,10 @@ impl super::Nexus { /// /// Note that the primary interface for an instance cannot be deleted if /// there are any secondary interfaces. - pub async fn network_interface_delete( + pub async fn instance_network_interface_delete( &self, opctx: &OpContext, - network_interface_lookup: &lookup::NetworkInterface<'_>, + network_interface_lookup: &lookup::InstanceNetworkInterface<'_>, ) -> DeleteResult { let (.., authz_instance, authz_interface) = network_interface_lookup.lookup_for(authz::Action::Delete).await?; diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 755e943423..aa88341a85 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -16,6 +16,7 @@ use crate::db::queries::network_interface::InsertError as InsertNicError; use crate::external_api::params; use crate::{authn, authz, db}; use chrono::Utc; +use nexus_db_model::NetworkInterfaceKind; use nexus_db_queries::context::OpContext; use nexus_defaults::DEFAULT_PRIMARY_NIC_NAME; use nexus_types::external_api::params::InstanceDiskAttachment; @@ -667,7 +668,7 @@ async fn sic_create_network_interface_undo( .await .map_err(ActionError::action_failed)?; match LookupPath::new(&opctx, &datastore) - .network_interface_id(interface_id) + .instance_network_interface_id(interface_id) .lookup_for(authz::Action::Delete) .await { @@ -706,7 +707,7 @@ async fn create_custom_network_interface( saga_params: &Params, instance_id: Uuid, interface_id: Uuid, - interface_params: ¶ms::NetworkInterfaceCreate, + interface_params: ¶ms::InstanceNetworkInterfaceCreate, ) -> Result<(), ActionError> { let osagactx = sagactx.user_data(); let datastore = osagactx.datastore(); @@ -741,7 +742,7 @@ async fn create_custom_network_interface( .fetch() .await .map_err(ActionError::action_failed)?; - let interface = db::model::IncompleteNetworkInterface::new( + let interface = db::model::IncompleteNetworkInterface::new_instance( interface_id, instance_id, authz_vpc.id(), @@ -762,7 +763,10 @@ async fn create_custom_network_interface( .or_else(|err| { match err { // Necessary for idempotency - InsertNicError::InterfaceAlreadyExists(_) => Ok(()), + InsertNicError::InterfaceAlreadyExists( + _, + NetworkInterfaceKind::Instance, + ) => Ok(()), _ => Err(err), } }) @@ -806,7 +810,7 @@ async fn create_default_primary_network_interface( let iface_name = Name::try_from(DEFAULT_PRIMARY_NIC_NAME.to_string()).unwrap(); - let interface_params = params::NetworkInterfaceCreate { + let interface_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: iface_name.clone(), description: format!( @@ -834,7 +838,7 @@ async fn create_default_primary_network_interface( .await .map_err(ActionError::action_failed)?; - let interface = db::model::IncompleteNetworkInterface::new( + let interface = db::model::IncompleteNetworkInterface::new_instance( interface_id, instance_id, authz_vpc.id(), @@ -855,7 +859,10 @@ async fn create_default_primary_network_interface( .or_else(|err| { match err { // Necessary for idempotency - InsertNicError::InterfaceAlreadyExists(_) => Ok(()), + InsertNicError::InterfaceAlreadyExists( + _, + NetworkInterfaceKind::Instance, + ) => Ok(()), _ => Err(err), } }) diff --git a/nexus/src/app/vpc_subnet.rs b/nexus/src/app/vpc_subnet.rs index 6c8f98d643..6f8fa5d8f1 100644 --- a/nexus/src/app/vpc_subnet.rs +++ b/nexus/src/app/vpc_subnet.rs @@ -252,16 +252,20 @@ impl super::Nexus { .await } - pub async fn subnet_list_network_interfaces( + pub async fn subnet_list_instance_network_interfaces( &self, opctx: &OpContext, subnet_lookup: &lookup::VpcSubnet<'_>, pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec { let (.., authz_subnet) = subnet_lookup.lookup_for(authz::Action::ListChildren).await?; self.db_datastore - .subnet_list_network_interfaces(opctx, &authz_subnet, pagparams) + .subnet_list_instance_network_interfaces( + opctx, + &authz_subnet, + pagparams, + ) .await } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 940827cffe..3f73e45f93 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -56,9 +56,9 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Disk; use omicron_common::api::external::Error; use omicron_common::api::external::Instance; +use omicron_common::api::external::InstanceNetworkInterface; use omicron_common::api::external::InternalContext; use omicron_common::api::external::NameOrId; -use omicron_common::api::external::NetworkInterface; use omicron_common::api::external::RouterRoute; use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::Saga; @@ -2522,7 +2522,7 @@ async fn image_delete( async fn instance_network_interface_list( rqctx: RequestContext>, query_params: Query>, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; @@ -2534,7 +2534,11 @@ async fn instance_network_interface_list( let instance_lookup = nexus.instance_lookup(&opctx, &scan_params.selector)?; let interfaces = nexus - .network_interface_list(&opctx, &instance_lookup, &paginated_by) + .instance_network_interface_list( + &opctx, + &instance_lookup, + &paginated_by, + ) .await? .into_iter() .map(|d| d.into()) @@ -2557,8 +2561,8 @@ async fn instance_network_interface_list( async fn instance_network_interface_create( rqctx: RequestContext>, query_params: Query, - interface_params: TypedBody, -) -> Result, HttpError> { + interface_params: TypedBody, +) -> Result, HttpError> { let apictx = rqctx.context(); let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; @@ -2599,13 +2603,15 @@ async fn instance_network_interface_delete( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); - let interface_selector = params::NetworkInterfaceSelector { + let interface_selector = params::InstanceNetworkInterfaceSelector { instance_selector: query.instance_selector, network_interface: path.interface, }; - let interface_lookup = - nexus.network_interface_lookup(&opctx, &interface_selector)?; - nexus.network_interface_delete(&opctx, &interface_lookup).await?; + let interface_lookup = nexus + .instance_network_interface_lookup(&opctx, &interface_selector)?; + nexus + .instance_network_interface_delete(&opctx, &interface_lookup) + .await?; Ok(HttpResponseDeleted()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2621,19 +2627,19 @@ async fn instance_network_interface_view( rqctx: RequestContext>, path_params: Path, query_params: Query, -) -> Result, HttpError> { +) -> Result, HttpError> { let apictx = rqctx.context(); let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); - let interface_selector = params::NetworkInterfaceSelector { + let interface_selector = params::InstanceNetworkInterfaceSelector { instance_selector: query.instance_selector, network_interface: path.interface, }; let (.., interface) = nexus - .network_interface_lookup(&opctx, &interface_selector)? + .instance_network_interface_lookup(&opctx, &interface_selector)? .fetch() .await?; Ok(HttpResponseOk(interface.into())) @@ -2651,8 +2657,8 @@ async fn instance_network_interface_update( rqctx: RequestContext>, path_params: Path, query_params: Query, - updated_iface: TypedBody, -) -> Result, HttpError> { + updated_iface: TypedBody, +) -> Result, HttpError> { let apictx = rqctx.context(); let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; @@ -2660,20 +2666,24 @@ async fn instance_network_interface_update( let path = path_params.into_inner(); let query = query_params.into_inner(); let updated_iface = updated_iface.into_inner(); - let network_interface_selector = params::NetworkInterfaceSelector { - instance_selector: query.instance_selector, - network_interface: path.interface, - }; + let network_interface_selector = + params::InstanceNetworkInterfaceSelector { + instance_selector: query.instance_selector, + network_interface: path.interface, + }; let network_interface_lookup = nexus - .network_interface_lookup(&opctx, &network_interface_selector)?; + .instance_network_interface_lookup( + &opctx, + &network_interface_selector, + )?; let interface = nexus - .network_interface_update( + .instance_network_interface_update( &opctx, &network_interface_lookup, updated_iface, ) .await?; - Ok(HttpResponseOk(NetworkInterface::from(interface))) + Ok(HttpResponseOk(InstanceNetworkInterface::from(interface))) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -3147,7 +3157,7 @@ async fn vpc_subnet_list_network_interfaces( rqctx: RequestContext>, path_params: Path, query_params: Query>, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { let nexus = &apictx.nexus; @@ -3164,7 +3174,7 @@ async fn vpc_subnet_list_network_interfaces( let subnet_lookup = nexus.vpc_subnet_lookup(&opctx, &subnet_selector)?; let interfaces = nexus - .subnet_list_network_interfaces( + .subnet_list_instance_network_interfaces( &opctx, &subnet_lookup, &paginated_by, diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 300a9eb1b4..617077558e 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -298,8 +298,8 @@ lazy_static! { nexus_defaults::DEFAULT_PRIMARY_NIC_NAME.parse().unwrap(); pub static ref DEMO_INSTANCE_NIC_URL: String = format!("/v1/network-interfaces/{}?project={}&instance={}", *DEMO_INSTANCE_NIC_NAME, *DEMO_PROJECT_NAME, *DEMO_INSTANCE_NAME); - pub static ref DEMO_INSTANCE_NIC_CREATE: params::NetworkInterfaceCreate = - params::NetworkInterfaceCreate { + pub static ref DEMO_INSTANCE_NIC_CREATE: params::InstanceNetworkInterfaceCreate = + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: DEMO_INSTANCE_NIC_NAME.clone(), description: String::from(""), @@ -308,8 +308,8 @@ lazy_static! { subnet_name: DEMO_VPC_SUBNET_NAME.clone(), ip: None, }; - pub static ref DEMO_INSTANCE_NIC_PUT: params::NetworkInterfaceUpdate = { - params::NetworkInterfaceUpdate { + pub static ref DEMO_INSTANCE_NIC_PUT: params::InstanceNetworkInterfaceUpdate = { + params::InstanceNetworkInterfaceUpdate { identity: IdentityMetadataUpdateParams { name: None, description: Some(String::from("an updated description")), diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index f073590a2f..ea3dd7380b 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -29,10 +29,10 @@ use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::IdentityMetadataUpdateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; +use omicron_common::api::external::InstanceNetworkInterface; use omicron_common::api::external::InstanceState; use omicron_common::api::external::Ipv4Net; use omicron_common::api::external::Name; -use omicron_common::api::external::NetworkInterface; use omicron_nexus::authz::SiloRole; use omicron_nexus::db::fixed_data::silo::SILO_ID; use omicron_nexus::external_api::shared::IpKind; @@ -256,7 +256,7 @@ async fn test_instances_create_reboot_halt( PROJECT_NAME ); let network_interfaces = - objects_list_page_authz::(client, &nics_url) + objects_list_page_authz::(client, &nics_url) .await .items; assert_eq!(network_interfaces.len(), 1); @@ -433,10 +433,12 @@ async fn test_instances_create_reboot_halt( "/v1/vpc-subnets/default/network-interfaces?project={}&vpc=default", PROJECT_NAME, ); - let interfaces = - objects_list_page_authz::(client, &url_interfaces) - .await - .items; + let interfaces = objects_list_page_authz::( + client, + &url_interfaces, + ) + .await + .items; assert!( interfaces.is_empty(), "Expected all network interfaces for the instance to be deleted" @@ -825,7 +827,7 @@ async fn test_instance_create_saga_removes_instance_database_record( // The network interface parameters. let default_name = "default".parse::().unwrap(); let requested_address = "172.30.0.10".parse::().unwrap(); - let if0_params = params::NetworkInterfaceCreate { + let if0_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if0")).unwrap(), description: String::from("first custom interface"), @@ -898,7 +900,7 @@ async fn test_instance_create_saga_removes_instance_database_record( // as-is. This would fail with a conflict on the instance name, if we don't // fully unwind the saga and delete the instance database record. let requested_address = "172.30.0.11".parse::().unwrap(); - let if0_params = params::NetworkInterfaceCreate { + let if0_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if0")).unwrap(), description: String::from("first custom interface"), @@ -940,7 +942,7 @@ async fn test_instance_with_single_explicit_ip_address( // Create the parameters for the interface. let default_name = "default".parse::().unwrap(); let requested_address = "172.30.0.10".parse::().unwrap(); - let if0_params = params::NetworkInterfaceCreate { + let if0_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if0")).unwrap(), description: String::from("first custom interface"), @@ -992,7 +994,7 @@ async fn test_instance_with_single_explicit_ip_address( .execute() .await .expect("Failed to get network interface for new instance") - .parsed_body::() + .parsed_body::() .expect("Failed to parse a network interface"); assert_eq!(interface.instance_id, instance.identity.id); assert_eq!(interface.identity.name, if0_params.identity.name); @@ -1043,7 +1045,7 @@ async fn test_instance_with_new_custom_network_interfaces( // Create the parameters for the interfaces. These will be created during // the saga for instance creation. - let if0_params = params::NetworkInterfaceCreate { + let if0_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if0")).unwrap(), description: String::from("first custom interface"), @@ -1052,7 +1054,7 @@ async fn test_instance_with_new_custom_network_interfaces( subnet_name: default_name.clone(), ip: None, }; - let if1_params = params::NetworkInterfaceCreate { + let if1_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if1")).unwrap(), description: String::from("second custom interface"), @@ -1102,11 +1104,10 @@ async fn test_instance_with_new_custom_network_interfaces( }; // The first interface is in the default VPC Subnet - let interfaces = NexusRequest::iter_collection_authn::( - client, - nics_url(&default_name).as_str(), - "", - Some(100), + let interfaces = NexusRequest::iter_collection_authn::< + InstanceNetworkInterface, + >( + client, nics_url(&default_name).as_str(), "", Some(100) ) .await .expect("Failed to get interfaces in default VPC Subnet"); @@ -1121,14 +1122,15 @@ async fn test_instance_with_new_custom_network_interfaces( assert_eq!(if0.instance_id, instance.identity.id); assert_eq!(if0.ip, std::net::IpAddr::V4("172.30.0.5".parse().unwrap())); - let interfaces1 = NexusRequest::iter_collection_authn::( - client, - nics_url(&non_default_subnet_name).as_str(), - "", - Some(100), - ) - .await - .expect("Failed to get interfaces in non-default VPC Subnet"); + let interfaces1 = + NexusRequest::iter_collection_authn::( + client, + nics_url(&non_default_subnet_name).as_str(), + "", + Some(100), + ) + .await + .expect("Failed to get interfaces in non-default VPC Subnet"); assert_eq!( interfaces1.all_items.len(), 1, @@ -1211,12 +1213,9 @@ async fn test_instance_create_delete_network_interface( "/v1/network-interfaces?project={}&instance={}", PROJECT_NAME, instance.identity.name, ); - let interfaces = NexusRequest::iter_collection_authn::( - client, - url_interfaces.as_str(), - "", - Some(100), - ) + let interfaces = NexusRequest::iter_collection_authn::< + InstanceNetworkInterface, + >(client, url_interfaces.as_str(), "", Some(100)) .await .expect("Failed to get interfaces for instance"); assert!( @@ -1226,7 +1225,7 @@ async fn test_instance_create_delete_network_interface( // Parameters for the interfaces to create/attach let if_params = vec![ - params::NetworkInterfaceCreate { + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: "if0".parse().unwrap(), description: String::from("a new nic"), @@ -1235,7 +1234,7 @@ async fn test_instance_create_delete_network_interface( subnet_name: "default".parse().unwrap(), ip: Some("172.30.0.10".parse().unwrap()), }, - params::NetworkInterfaceCreate { + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: "if1".parse().unwrap(), description: String::from("a new nic"), @@ -1286,7 +1285,7 @@ async fn test_instance_create_delete_network_interface( .execute() .await .expect("Failed to create network interface on stopped instance"); - let iface = response.parsed_body::().unwrap(); + let iface = response.parsed_body::().unwrap(); assert_eq!(iface.identity.name, params.identity.name); assert_eq!(iface.ip, params.ip.unwrap()); assert_eq!( @@ -1303,10 +1302,12 @@ async fn test_instance_create_delete_network_interface( instance_simulate(nexus, &instance.identity.id).await; // Get all interfaces in one request. - let other_interfaces = - objects_list_page_authz::(client, &url_interfaces) - .await - .items; + let other_interfaces = objects_list_page_authz::( + client, + &url_interfaces, + ) + .await + .items; for (iface0, iface1) in interfaces.iter().zip(other_interfaces) { assert_eq!(iface0.identity.id, iface1.identity.id); assert_eq!(iface0.vpc_id, iface1.vpc_id); @@ -1363,7 +1364,7 @@ async fn test_instance_create_delete_network_interface( .expect("Failed to parse error response body"); assert_eq!( err.message, - "The primary interface for an instance \ + "The primary interface \ may not be deleted while secondary interfaces \ are still attached", "Expected an InvalidRequest response when detaching \ @@ -1453,7 +1454,7 @@ async fn test_instance_update_network_interfaces( // Parameters for each interface to try to modify. let if_params = vec![ - params::NetworkInterfaceCreate { + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: "if0".parse().unwrap(), description: String::from("a new nic"), @@ -1462,7 +1463,7 @@ async fn test_instance_update_network_interfaces( subnet_name: "default".parse().unwrap(), ip: Some("172.30.0.10".parse().unwrap()), }, - params::NetworkInterfaceCreate { + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: "if1".parse().unwrap(), description: String::from("a new nic"), @@ -1487,7 +1488,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to create network interface on stopped instance") - .parsed_body::() + .parsed_body::() .unwrap(); assert_eq!(primary_iface.identity.name, if_params[0].identity.name); assert_eq!(primary_iface.ip, if_params[0].ip.unwrap()); @@ -1502,7 +1503,7 @@ async fn test_instance_update_network_interfaces( // We'll change the interface's name and description let new_name = Name::try_from(String::from("new-if0")).unwrap(); let new_description = String::from("new description"); - let updates = params::NetworkInterfaceUpdate { + let updates = params::InstanceNetworkInterfaceUpdate { identity: IdentityMetadataUpdateParams { name: Some(new_name.clone()), description: Some(new_description.clone()), @@ -1545,7 +1546,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to update an interface") - .parsed_body::() + .parsed_body::() .unwrap(); // Verify the modifications have taken effect, updating the name, @@ -1558,7 +1559,8 @@ async fn test_instance_update_network_interfaces( // interface, and that the modification time for the new is later than the // old. let verify_unchanged_attributes = - |original_iface: &NetworkInterface, new_iface: &NetworkInterface| { + |original_iface: &InstanceNetworkInterface, + new_iface: &InstanceNetworkInterface| { assert_eq!( original_iface.identity.time_created, new_iface.identity.time_created @@ -1577,7 +1579,7 @@ async fn test_instance_update_network_interfaces( // Try with the same request again, but this time only changing // `primary`. This should have no effect. - let updates = params::NetworkInterfaceUpdate { + let updates = params::InstanceNetworkInterfaceUpdate { identity: IdentityMetadataUpdateParams { name: None, description: None, @@ -1596,7 +1598,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to update an interface") - .parsed_body::() + .parsed_body::() .unwrap(); // Everything should still be the same, except the modification time. @@ -1626,7 +1628,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to create network interface on stopped instance") - .parsed_body::() + .parsed_body::() .unwrap(); assert_eq!(secondary_iface.identity.name, if_params[1].identity.name); assert_eq!(secondary_iface.ip, if_params[1].ip.unwrap()); @@ -1671,7 +1673,7 @@ async fn test_instance_update_network_interfaces( // Verify that we can set the secondary as the new primary, and that nothing // else changes about the NICs. - let updates = params::NetworkInterfaceUpdate { + let updates = params::InstanceNetworkInterfaceUpdate { identity: IdentityMetadataUpdateParams { name: None, description: None, @@ -1687,7 +1689,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to update an interface") - .parsed_body::() + .parsed_body::() .unwrap(); // It should now be the primary and have an updated modification time @@ -1713,7 +1715,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to get the old primary / new secondary interface") - .parsed_body::() + .parsed_body::() .unwrap(); // The now-secondary interface should be, well, secondary @@ -1742,10 +1744,12 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to delete original secondary interface"); - let all_interfaces = - objects_list_page_authz::(client, &url_interfaces) - .await - .items; + let all_interfaces = objects_list_page_authz::( + client, + &url_interfaces, + ) + .await + .items; assert_eq!( all_interfaces.len(), 1, @@ -1764,7 +1768,7 @@ async fn test_instance_update_network_interfaces( .execute() .await .expect("Failed to create network interface on stopped instance") - .parsed_body::() + .parsed_body::() .unwrap(); assert!(!iface.primary); assert_eq!(iface.identity.name, if_params[0].identity.name); @@ -1786,7 +1790,7 @@ async fn test_instance_with_multiple_nics_unwinds_completely( // error on creation of the second NIC, and we'll make sure that both are // deleted. let default_name = "default".parse::().unwrap(); - let if0_params = params::NetworkInterfaceCreate { + let if0_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if0")).unwrap(), description: String::from("first custom interface"), @@ -1795,7 +1799,7 @@ async fn test_instance_with_multiple_nics_unwinds_completely( subnet_name: default_name.clone(), ip: Some("172.30.0.6".parse().unwrap()), }; - let if1_params = params::NetworkInterfaceCreate { + let if1_params = params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: Name::try_from(String::from("if1")).unwrap(), description: String::from("second custom interface"), @@ -1841,9 +1845,9 @@ async fn test_instance_with_multiple_nics_unwinds_completely( "/v1/vpc-subnets/default/network-interfaces?project={}&vpc=default", PROJECT_NAME, ); - let interfaces = NexusRequest::iter_collection_authn::( - client, &url_nics, "", None, - ) + let interfaces = NexusRequest::iter_collection_authn::< + InstanceNetworkInterface, + >(client, &url_nics, "", None) .await .expect("failed to list network interfaces") .all_items; diff --git a/nexus/tests/integration_tests/subnet_allocation.rs b/nexus/tests/integration_tests/subnet_allocation.rs index db85057c0c..afb41c163a 100644 --- a/nexus/tests/integration_tests/subnet_allocation.rs +++ b/nexus/tests/integration_tests/subnet_allocation.rs @@ -19,8 +19,8 @@ use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::resource_helpers::populate_ip_pool; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::{ - ByteCount, IdentityMetadataCreateParams, InstanceCpuCount, Ipv4Net, - NetworkInterface, + ByteCount, IdentityMetadataCreateParams, InstanceCpuCount, + InstanceNetworkInterface, Ipv4Net, }; use omicron_common::nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; use omicron_nexus::external_api::params; @@ -37,7 +37,7 @@ async fn create_instance_expect_failure( ) -> HttpErrorResponseBody { let network_interfaces = params::InstanceNetworkInterfaceAttachment::Create(vec![ - params::NetworkInterfaceCreate { + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { // We're using the name of the instance purposefully, to // avoid any naming conflicts on the interface. @@ -116,7 +116,7 @@ async fn test_subnet_allocation(cptestctx: &ControlPlaneTestContext) { // 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. let nic = params::InstanceNetworkInterfaceAttachment::Create(vec![ - params::NetworkInterfaceCreate { + params::InstanceNetworkInterfaceCreate { identity: IdentityMetadataCreateParams { name: "eth0".parse().unwrap(), description: String::from("some iface"), @@ -161,7 +161,7 @@ async fn test_subnet_allocation(cptestctx: &ControlPlaneTestContext) { subnet_name, vpc_selector ); let mut network_interfaces = - objects_list_page_authz::(client, &url_ips) + objects_list_page_authz::(client, &url_ips) .await .items; assert_eq!(network_interfaces.len(), subnet_size); diff --git a/nexus/tests/integration_tests/vpc_subnets.rs b/nexus/tests/integration_tests/vpc_subnets.rs index 12a2b6762a..0a9c60b75d 100644 --- a/nexus/tests/integration_tests/vpc_subnets.rs +++ b/nexus/tests/integration_tests/vpc_subnets.rs @@ -70,8 +70,8 @@ async fn test_delete_vpc_subnet_with_interfaces_fails( .unwrap(); assert_eq!( err.message, - "VPC Subnet cannot be deleted while instances \ - with network interfaces in the subnet exist", + "VPC Subnet cannot be deleted while \ + network interfaces in the subnet exist", ); // Stop and then delete the instance diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index c3b6f228d1..ce1f4a56e1 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -139,7 +139,7 @@ pub struct OptionalInstanceSelector { } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] -pub struct NetworkInterfaceSelector { +pub struct InstanceNetworkInterfaceSelector { #[serde(flatten)] pub instance_selector: Option, /// Name or ID of the network interface @@ -602,10 +602,10 @@ pub struct ProjectUpdate { // NETWORK INTERFACES -/// Create-time parameters for a -/// [`NetworkInterface`](omicron_common::api::external::NetworkInterface) +/// Create-time parameters for an +/// [`InstanceNetworkInterface`](omicron_common::api::external::InstanceNetworkInterface). #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct NetworkInterfaceCreate { +pub struct InstanceNetworkInterfaceCreate { #[serde(flatten)] pub identity: IdentityMetadataCreateParams, /// The VPC in which to create the interface. @@ -616,13 +616,13 @@ pub struct NetworkInterfaceCreate { pub ip: Option, } -/// Parameters for updating a -/// [`NetworkInterface`](omicron_common::api::external::NetworkInterface). +/// Parameters for updating an +/// [`InstanceNetworkInterface`](omicron_common::api::external::InstanceNetworkInterface). /// /// Note that modifying IP addresses for an interface is not yet supported, a /// new interface must be created instead. #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct NetworkInterfaceUpdate { +pub struct InstanceNetworkInterfaceUpdate { #[serde(flatten)] pub identity: IdentityMetadataUpdateParams, @@ -692,8 +692,8 @@ pub struct IpPoolUpdate { pub const MIN_MEMORY_SIZE_BYTES: u32 = 1 << 30; // 1 GiB -/// Describes an attachment of a `NetworkInterface` to an `Instance`, at the -/// time the instance is created. +/// Describes an attachment of an `InstanceNetworkInterface` to an `Instance`, +/// at the time the instance is created. // NOTE: VPC's are an organizing concept for networking resources, not for // instances. It's true that all networking resources for an instance must // belong to a single VPC, but we don't consider instances to be "scoped" to a @@ -711,11 +711,11 @@ pub const MIN_MEMORY_SIZE_BYTES: u32 = 1 << 30; // 1 GiB #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(tag = "type", content = "params", rename_all = "snake_case")] pub enum InstanceNetworkInterfaceAttachment { - /// Create one or more `NetworkInterface`s for the `Instance`. + /// Create one or more `InstanceNetworkInterface`s for the `Instance`. /// /// If more than one interface is provided, then the first will be /// designated the primary interface for the instance. - Create(Vec), + Create(Vec), /// The default networking configuration for an instance is to create a /// single primary interface with an automatically-assigned IP address. The diff --git a/openapi/nexus.json b/openapi/nexus.json index dc4bc484f4..e409984e3c 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -2388,7 +2388,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterfaceResultsPage" + "$ref": "#/components/schemas/InstanceNetworkInterfaceResultsPage" } } } @@ -2431,7 +2431,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterfaceCreate" + "$ref": "#/components/schemas/InstanceNetworkInterfaceCreate" } } }, @@ -2443,7 +2443,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterface" + "$ref": "#/components/schemas/InstanceNetworkInterface" } } } @@ -2497,7 +2497,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterface" + "$ref": "#/components/schemas/InstanceNetworkInterface" } } } @@ -2547,7 +2547,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterfaceUpdate" + "$ref": "#/components/schemas/InstanceNetworkInterfaceUpdate" } } }, @@ -2559,7 +2559,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterface" + "$ref": "#/components/schemas/InstanceNetworkInterface" } } } @@ -6754,7 +6754,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/NetworkInterfaceResultsPage" + "$ref": "#/components/schemas/InstanceNetworkInterfaceResultsPage" } } } @@ -9218,17 +9218,95 @@ "dst_sled_id" ] }, + "InstanceNetworkInterface": { + "description": "An `InstanceNetworkInterface` represents a virtual network interface device attached to an instance.", + "type": "object", + "properties": { + "description": { + "description": "human-readable free-form text about a resource", + "type": "string" + }, + "id": { + "description": "unique, immutable, system-controlled identifier for each resource", + "type": "string", + "format": "uuid" + }, + "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" + } + ] + }, + "name": { + "description": "unique, mutable, user-controlled identifier for each resource", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + }, + "primary": { + "description": "True if this interface is the primary for the instance to which it's attached.", + "type": "boolean" + }, + "subnet_id": { + "description": "The subnet to which the interface belongs.", + "type": "string", + "format": "uuid" + }, + "time_created": { + "description": "timestamp when this resource was created", + "type": "string", + "format": "date-time" + }, + "time_modified": { + "description": "timestamp when this resource was last modified", + "type": "string", + "format": "date-time" + }, + "vpc_id": { + "description": "The VPC to which the interface belongs.", + "type": "string", + "format": "uuid" + } + }, + "required": [ + "description", + "id", + "instance_id", + "ip", + "mac", + "name", + "primary", + "subnet_id", + "time_created", + "time_modified", + "vpc_id" + ] + }, "InstanceNetworkInterfaceAttachment": { - "description": "Describes an attachment of a `NetworkInterface` to an `Instance`, at the time the instance is created.", + "description": "Describes an attachment of an `InstanceNetworkInterface` to an `Instance`, at the time the instance is created.", "oneOf": [ { - "description": "Create one or more `NetworkInterface`s for the `Instance`.\n\nIf more than one interface is provided, then the first will be designated the primary interface for the instance.", + "description": "Create one or more `InstanceNetworkInterface`s for the `Instance`.\n\nIf more than one interface is provided, then the first will be designated the primary interface for the instance.", "type": "object", "properties": { "params": { "type": "array", "items": { - "$ref": "#/components/schemas/NetworkInterfaceCreate" + "$ref": "#/components/schemas/InstanceNetworkInterfaceCreate" } }, "type": { @@ -9275,6 +9353,90 @@ } ] }, + "InstanceNetworkInterfaceCreate": { + "description": "Create-time parameters for an [`InstanceNetworkInterface`](omicron_common::api::external::InstanceNetworkInterface).", + "type": "object", + "properties": { + "description": { + "type": "string" + }, + "ip": { + "nullable": true, + "description": "The IP address for the interface. One will be auto-assigned if not provided.", + "type": "string", + "format": "ip" + }, + "name": { + "$ref": "#/components/schemas/Name" + }, + "subnet_name": { + "description": "The VPC Subnet in which to create the interface.", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + }, + "vpc_name": { + "description": "The VPC in which to create the interface.", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + } + }, + "required": [ + "description", + "name", + "subnet_name", + "vpc_name" + ] + }, + "InstanceNetworkInterfaceResultsPage": { + "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/InstanceNetworkInterface" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, + "InstanceNetworkInterfaceUpdate": { + "description": "Parameters for updating an [`InstanceNetworkInterface`](omicron_common::api::external::InstanceNetworkInterface).\n\nNote that modifying IP addresses for an interface is not yet supported, a new interface must be created instead.", + "type": "object", + "properties": { + "description": { + "nullable": true, + "type": "string" + }, + "name": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + }, + "primary": { + "description": "Make a secondary interface the instance's primary interface.\n\nIf applied to a secondary interface, that interface will become the primary on the next reboot of the instance. Note that this may have implications for routing between instances, as the new primary interface will be on a distinct subnet from the previous primary interface.\n\nNote that this can only be used to select a new primary interface for an instance. Requests to change the primary interface into a secondary will return an error.", + "default": false, + "type": "boolean" + } + } + }, "InstanceResultsPage": { "description": "A single page of results", "type": "object", @@ -9716,168 +9878,6 @@ } ] }, - "NetworkInterface": { - "description": "A `NetworkInterface` represents a virtual network interface device.", - "type": "object", - "properties": { - "description": { - "description": "human-readable free-form text about a resource", - "type": "string" - }, - "id": { - "description": "unique, immutable, system-controlled identifier for each resource", - "type": "string", - "format": "uuid" - }, - "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" - } - ] - }, - "name": { - "description": "unique, mutable, user-controlled identifier for each resource", - "allOf": [ - { - "$ref": "#/components/schemas/Name" - } - ] - }, - "primary": { - "description": "True if this interface is the primary for the instance to which it's attached.", - "type": "boolean" - }, - "subnet_id": { - "description": "The subnet to which the interface belongs.", - "type": "string", - "format": "uuid" - }, - "time_created": { - "description": "timestamp when this resource was created", - "type": "string", - "format": "date-time" - }, - "time_modified": { - "description": "timestamp when this resource was last modified", - "type": "string", - "format": "date-time" - }, - "vpc_id": { - "description": "The VPC to which the interface belongs.", - "type": "string", - "format": "uuid" - } - }, - "required": [ - "description", - "id", - "instance_id", - "ip", - "mac", - "name", - "primary", - "subnet_id", - "time_created", - "time_modified", - "vpc_id" - ] - }, - "NetworkInterfaceCreate": { - "description": "Create-time parameters for a [`NetworkInterface`](omicron_common::api::external::NetworkInterface)", - "type": "object", - "properties": { - "description": { - "type": "string" - }, - "ip": { - "nullable": true, - "description": "The IP address for the interface. One will be auto-assigned if not provided.", - "type": "string", - "format": "ip" - }, - "name": { - "$ref": "#/components/schemas/Name" - }, - "subnet_name": { - "description": "The VPC Subnet in which to create the interface.", - "allOf": [ - { - "$ref": "#/components/schemas/Name" - } - ] - }, - "vpc_name": { - "description": "The VPC in which to create the interface.", - "allOf": [ - { - "$ref": "#/components/schemas/Name" - } - ] - } - }, - "required": [ - "description", - "name", - "subnet_name", - "vpc_name" - ] - }, - "NetworkInterfaceResultsPage": { - "description": "A single page of results", - "type": "object", - "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" - ] - }, - "NetworkInterfaceUpdate": { - "description": "Parameters for updating a [`NetworkInterface`](omicron_common::api::external::NetworkInterface).\n\nNote that modifying IP addresses for an interface is not yet supported, a new interface must be created instead.", - "type": "object", - "properties": { - "description": { - "nullable": true, - "type": "string" - }, - "name": { - "nullable": true, - "allOf": [ - { - "$ref": "#/components/schemas/Name" - } - ] - }, - "primary": { - "description": "Make a secondary interface the instance's primary interface.\n\nIf applied to a secondary interface, that interface will become the primary on the next reboot of the instance. Note that this may have implications for routing between instances, as the new primary interface will be on a distinct subnet from the previous primary interface.\n\nNote that this can only be used to select a new primary interface for an instance. Requests to change the primary interface into a secondary will return an error.", - "default": false, - "type": "boolean" - } - } - }, "NodeName": { "description": "Unique name for a saga [`Node`]\n\nEach node requires a string name that's unique within its DAG. The name is used to identify its output. Nodes that depend on a given node (either directly or indirectly) can access the node's output using its name.", "type": "string" From 374027d75b6e1a4d576a65f501c9ae924e28727c Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Wed, 5 Apr 2023 11:21:16 -0700 Subject: [PATCH 4/6] Fix typo in comment. Co-authored-by: Justin Bennett --- nexus/db-model/src/network_interface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-model/src/network_interface.rs b/nexus/db-model/src/network_interface.rs index b7340b561b..4e1839109f 100644 --- a/nexus/db-model/src/network_interface.rs +++ b/nexus/db-model/src/network_interface.rs @@ -45,7 +45,7 @@ pub struct NetworkInterface { pub mac: MacAddr, // TODO-correctness: We need to split this into an optional V4 and optional V6 address, at - // least one of which will lways be specified. + // least one of which will always be specified. // // If user requests an address of either kind, give exactly that and not the other. // If neither is specified, auto-assign one of each? From 54a021dab9ceb6461514c363e09777dab0a4d8db Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Thu, 6 Apr 2023 01:21:51 +0000 Subject: [PATCH 5/6] Fix test post merge. --- nexus/tests/integration_tests/instances.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index aaf701bd65..2f2ee21fef 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -2973,9 +2973,10 @@ async fn test_instance_v2p_mappings(cptestctx: &ControlPlaneTestContext) { let nics_url = format!("/v1/network-interfaces?instance={}", instance.identity.id,); - let nics = objects_list_page_authz::(client, &nics_url) - .await - .items; + let nics = + objects_list_page_authz::(client, &nics_url) + .await + .items; // The default config is one NIC assert_eq!(nics.len(), 1); From 249149b4651a13a723572d67abd3d85d6ac04495 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Thu, 6 Apr 2023 10:22:48 -0700 Subject: [PATCH 6/6] Fix comment --- nexus/db-queries/src/db/datastore/network_interface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/network_interface.rs b/nexus/db-queries/src/db/datastore/network_interface.rs index ed7f595c8c..e81b411346 100644 --- a/nexus/db-queries/src/db/datastore/network_interface.rs +++ b/nexus/db-queries/src/db/datastore/network_interface.rs @@ -163,7 +163,7 @@ impl DataStore { Ok(()) } - /// Delete an instance attached to a provided instance. + /// Delete an `InstanceNetworkInterface` attached to a provided instance. /// /// Note that the primary interface for an instance cannot be deleted if /// there are any secondary interfaces.