From e5d5317adbbd13879b38ee2e761b8298fa320192 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Wed, 14 Jul 2021 14:05:42 -0700 Subject: [PATCH 1/6] Adds basic types to represent VPCs, subnets, and NICs. - Adds the `VPC`, `VPCSubnet` and `VNIC` types, including internal control plane representations, client views, and database types. - Implements some required traits for generating JSON schema and serializing to/from PostgreSQL wire formats. Note that the MAC address type is serialized in the database as a string, as CockroachDB doesn't currently support the PostgreSQL MACADDR type. --- Cargo.lock | 19 ++- omicron-common/Cargo.toml | 9 + omicron-common/src/api/model.rs | 275 +++++++++++++++++++++++++++++- omicron-common/src/model_db.rs | 160 +++++++++++++++++ omicron-common/src/sql/dbinit.sql | 67 ++++++++ omicron-nexus/src/db/schema.rs | 59 +++++++ 6 files changed, 586 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d5528109e62..2e8f3113913 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -914,9 +914,12 @@ dependencies = [ [[package]] name = "ipnet" -version = "2.3.0" +version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47be2f14c678be2fdcab04ab1171db51b2762ce6f0a8ee87c8dd4a04ed216135" +checksum = "68f2d64f2edebec4ce84ad108148e67e1064789bee435edc5b60ad398714a3a9" +dependencies = [ + "serde", +] [[package]] name = "itertools" @@ -978,6 +981,15 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "macaddr" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baee0bbc17ce759db233beb01648088061bf678383130602a298e6998eedb2d8" +dependencies = [ + "serde", +] + [[package]] name = "maplit" version = "1.0.2" @@ -1210,7 +1222,10 @@ dependencies = [ "futures", "http", "hyper", + "ipnet", "libc", + "macaddr", + "postgres-protocol", "propolis-server", "rayon", "reqwest", diff --git a/omicron-common/Cargo.toml b/omicron-common/Cargo.toml index 97d28d175a7..e0cc6b74e27 100644 --- a/omicron-common/Cargo.toml +++ b/omicron-common/Cargo.toml @@ -11,6 +11,7 @@ http = "0.2.0" hyper = "0.14" libc = "0.2.98" propolis-server = { git = "https://github.com/oxidecomputer/propolis", rev = "b6da043d" } +postgres-protocol = "0.6.1" rayon = "1.5" reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] } ring = "0.16" @@ -41,6 +42,14 @@ features = [ "serde" ] git = "https://github.com/oxidecomputer/dropshot" branch = "main" +[dependencies.ipnet] +version = "2.3.1" +features = [ "serde" ] + +[dependencies.macaddr] +version = "1.0.1" +features = [ "serde_std" ] + [dependencies.schemars] version = "0.8" features = [ "chrono", "uuid" ] diff --git a/omicron-common/src/api/model.rs b/omicron-common/src/api/model.rs index d974748f308..cceb3b89928 100644 --- a/omicron-common/src/api/model.rs +++ b/omicron-common/src/api/model.rs @@ -24,7 +24,7 @@ use std::fmt::Debug; use std::fmt::Display; use std::fmt::Formatter; use std::fmt::Result as FormatResult; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use std::num::NonZeroU32; use std::time::Duration; use uuid::Uuid; @@ -1313,6 +1313,279 @@ impl From for SagaStateView { } } +/// A Virtual Private Cloud (VPC) object. +#[derive(Clone, Debug)] +pub struct VPC { + /** common identifying metadata */ + pub identity: IdentityMetadata, + // TODO: Implement project-scoping + // /** id for the project containing this Instance */ + // pub project_id: Uuid, +} + +/// Client view onto a `VPC` object. +#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct VPCView { + #[serde(flatten)] + pub identity: IdentityMetadata, + // TODO: Implement project-scoping + // /** id for the project containing this Instance */ + // pub project_id: Uuid, +} + +impl Object for VPC { + type View = VPCView; + fn to_view(&self) -> Self::View { + VPCView { identity: self.identity.clone() } + } +} + +/// An `IpNet` represents a IP subnetwork (v4 or v6), including the address and network mask. +// NOTE: We're using the `ipnet` crate's implementation, but we wrap it in a newtype because that +// crate does not implement `JsonSchema` and Rust's orphan rules prevent us from implementing the +// trait directly on the type. +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] +pub struct IpNet(pub ipnet::IpNet); + +impl std::ops::Deref for IpNet { + type Target = ipnet::IpNet; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::fmt::Display for IpNet { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl JsonSchema for IpNet { + fn schema_name() -> String { + "IpNet".to_string() + } + + fn json_schema( + _: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + schemars::schema::Schema::Object( + schemars::schema::SchemaObject { + metadata: Some(Box::new(schemars::schema::Metadata { + id: None, + title: Some("An IP subnet".to_string()), + description: Some("An IPv4 or IPv6 subnet, including prefix and subnet mask".to_string()), + default: None, + deprecated: false, + read_only: false, + write_only: false, + examples: vec!["192.168.1.0/24".into(), "fd12:3456::/64".into()], + })), + instance_type: Some(schemars::schema::SingleOrVec::Single(Box::new(schemars::schema::InstanceType::String))), + format: None, + enum_values: None, + const_value: None, + subschemas: None, + number: None, + string: Some(Box::new(schemars::schema::StringValidation { + max_length: Some(23), // fully-specified IPv6, slash and 3-digit mask + min_length: None, + // This regex validator is inspired by https://ihateregex.io/expr/ipv{4,6} + // Note that the regex is more permissive than we intend to be in the final + // API. This passes _any_ IPs, while we're likely going to restrict them to the + // private address space for v4 for example (RFD 21 sec 2.2). + pattern: Some( + r#"(^(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$)|(^(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))$)"#.to_string() + ), + })), + array: None, + object: None, + reference: None, + extensions: BTreeMap::new(), + } + ) + } +} + +/// An IP subnet within a VPC. +#[derive(Clone, Debug)] +pub struct VPCSubnet { + /** common identifying metadata */ + pub identity: IdentityMetadata, + + // TODO: Implement project-scoping + // /** id for the project containing this Instance */ + // pub project_id: Uuid, + /** The VPC to which the subnet belongs. */ + pub vpc_id: Uuid, + + // TODO-correctness: RFD 21 sec 3.5 indicates that users may specify both v4 and v6 subnets in + // a single API call, but does not explicitly say if either is required. Clearly, one of them + // has to be specified, and most cloud providers require v4 and make v6 optional. However, RFD + // hints that users _might_ be able to specify v6 and not v4 (and maybe that the omitted block + // will be added by default). + /** The IPv4 subnet CIDR block. */ + pub ipv4_block: Option, + + /** The IPv4 subnet CIDR block. */ + pub ipv6_block: Option, +} + +impl Object for VPCSubnet { + type View = VPCSubnetView; + fn to_view(&self) -> Self::View { + VPCSubnetView { + identity: self.identity.clone(), + vpc_id: self.vpc_id, + ipv4_block: self.ipv4_block, + ipv6_block: self.ipv6_block, + } + } +} + +/// Client view onto a `VPCSubnet` object. +#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct VPCSubnetView { + #[serde(flatten)] + pub identity: IdentityMetadata, + + // TODO: Implement project-scoping + // /** id for the project containing this Instance */ + // pub project_id: Uuid, + /** The VPC to which the subnet belongs. */ + pub vpc_id: Uuid, + + /** The IPv4 subnetwork. */ + pub ipv4_block: Option, + + /** The IPv6 subnetwork. */ + pub ipv6_block: Option, +} + +/// The `MacAddr` represents a Media Access Control (MAC) address, used to uniquely identify +/// hardware devices on a network. +// NOTE: We're using the `macaddr` crate for the internal representation. But as with the `ipnet`, +// this crate does not implement `JsonSchema`, nor the the SQL conversion traits `FromSql` and +// `ToSql`. +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] +pub struct MacAddr(pub macaddr::MacAddr6); + +impl std::ops::Deref for MacAddr { + type Target = macaddr::MacAddr6; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl std::fmt::Display for MacAddr { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl JsonSchema for MacAddr { + fn schema_name() -> String { + "MacAddr".to_string() + } + + fn json_schema( + _: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + metadata: Some(Box::new(schemars::schema::Metadata { + id: None, + title: Some("A MAC address".to_string()), + description: Some( + "A Media Access Control address, in EUI-48 format" + .to_string(), + ), + default: None, + deprecated: false, + read_only: false, + write_only: false, + examples: vec!["ff:ff:ff:ff:ff:ff".into()], + })), + instance_type: Some(schemars::schema::SingleOrVec::Single( + Box::new(schemars::schema::InstanceType::String), + )), + format: None, + enum_values: None, + const_value: None, + subschemas: None, + number: None, + string: Some(Box::new(schemars::schema::StringValidation { + max_length: Some(17), // 12 hex characters and 5 ":"-separators + min_length: Some(17), + pattern: Some( + r#"^([0-8a-fA-F]{2}:){5}[0-8a-fA-F]{2}$"#.to_string(), + ), + })), + array: None, + object: None, + reference: None, + extensions: BTreeMap::new(), + }) + } +} + +/// A `VNIC` represents a virtual network interface device. +#[derive(Clone, Debug)] +pub struct VNIC { + /** common identifying metadata */ + pub identity: IdentityMetadata, + + // TODO: Implement project-scoping + // /** id for the project containing this Instance */ + // pub project_id: Uuid, + /** The VPC to which the NIC belongs. */ + pub vpc_id: Uuid, + + /** The subnet to which the NIC belongs. */ + pub subnet_id: Uuid, + + /** The MAC address assigned to this NIC. */ + pub mac: MacAddr, + + /** The IP address assigned to this NIC. */ + pub ip: IpAddr, +} + +impl Object for VNIC { + type View = VNICView; + fn to_view(&self) -> Self::View { + VNICView { + identity: self.identity.clone(), + vpc_id: self.vpc_id, + subnet_id: self.subnet_id, + mac: self.mac, + ip: self.ip, + } + } +} + +/// Client view onto a `VNIC` object. +#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct VNICView { + #[serde(flatten)] + pub identity: IdentityMetadata, + + // TODO: Implement project-scoping + // pub project_id: Uuid, + /** The VPC to which the NIC belongs. */ + pub vpc_id: Uuid, + + /** The subnet to which the NIC belongs. */ + pub subnet_id: Uuid, + + /** The MAC address assigned to this NIC. */ + pub mac: MacAddr, + + /** The IP address assigned to this NIC. */ + pub ip: IpAddr, +} + /* * Internal Control Plane API objects */ diff --git a/omicron-common/src/model_db.rs b/omicron-common/src/model_db.rs index fd35dedc922..7da347e9a5e 100644 --- a/omicron-common/src/model_db.rs +++ b/omicron-common/src/model_db.rs @@ -65,11 +65,16 @@ use crate::api::Instance; use crate::api::InstanceCpuCount; use crate::api::InstanceRuntimeState; use crate::api::InstanceState; +use crate::api::IpNet; +use crate::api::MacAddr; use crate::api::Name; use crate::api::OximeterAssignment; use crate::api::OximeterInfo; use crate::api::ProducerEndpoint; use crate::api::Project; +use crate::api::VPCSubnet; +use crate::api::VNIC; +use crate::api::VPC; use crate::bail_unless; use chrono::DateTime; use chrono::Utc; @@ -130,6 +135,82 @@ impl_sql_wrapping!(Generation, i64); impl_sql_wrapping!(InstanceCpuCount, i64); impl_sql_wrapping!(Name, &str); +// Conversion to/from SQL types for IpNet. +// +// Although there are implementations that convert an IP _address_ to and from PostgreSQL types, +// there does not appear to be any existing implementation that converts a _subnet_, including the +// address and mask, to and from SQL types. At least, not for the 3rd-party crate `ipnet` we're +// using to actually represent a subnet. +impl tokio_postgres::types::ToSql for IpNet { + fn to_sql( + &self, + _ty: &tokio_postgres::types::Type, + out: &mut tokio_postgres::types::private::BytesMut, + ) -> Result< + tokio_postgres::types::IsNull, + Box, + > { + postgres_protocol::types::inet_to_sql( + self.addr(), + self.prefix_len(), + out, + ); + Ok(tokio_postgres::types::IsNull::No) + } + + fn accepts(ty: &tokio_postgres::types::Type) -> bool { + matches!(ty, &tokio_postgres::types::Type::INET) + } + + tokio_postgres::types::to_sql_checked!(); +} + +impl<'a> tokio_postgres::types::FromSql<'a> for IpNet { + fn from_sql( + _ty: &tokio_postgres::types::Type, + raw: &'a [u8], + ) -> Result> { + let value = postgres_protocol::types::inet_from_sql(raw)?; + let (addr, mask) = (value.addr(), value.netmask()); + let subnet = match addr { + std::net::IpAddr::V4(addr) => { + ipnet::Ipv4Net::new(addr, mask)?.into() + } + std::net::IpAddr::V6(addr) => { + ipnet::Ipv6Net::new(addr, mask)?.into() + } + }; + Ok(IpNet(subnet)) + } + + fn accepts(ty: &tokio_postgres::types::Type) -> bool { + matches!(ty, &tokio_postgres::types::Type::INET) + } +} + +// Conversion to/from SQL types for MacAddr. +// +// As with the IP subnet above, there's no existing crate for MAC addresses that satisfies all our +// needs. The `eui48` crate implements SQL conversions, but not `Serialize`/`Deserialize` or +// `JsonSchema`. The `macaddr` crate implements serialization, but not the other two. Since +// serialization is more annoying that the others, we choose the latter, so we're implementing the +// SQL serialization here. CockroachDB does not currently support the Postgres MACADDR type, so +// we're storing it as a CHAR(17), 12 characters for the hexadecimal and 5 ":"-separators. +impl From<&MacAddr> for String { + fn from(addr: &MacAddr) -> String { + format!("{}", addr) + } +} + +impl TryFrom for MacAddr { + type Error = macaddr::ParseError; + + fn try_from(s: String) -> Result { + s.parse().map(|addr| MacAddr(addr)) + } +} +impl_sql_wrapping!(MacAddr, String); + /* * TryFrom impls used for more complex Rust types */ @@ -317,3 +398,82 @@ impl TryFrom<&tokio_postgres::Row> for OximeterAssignment { Ok(Self { oximeter_id, producer_id }) } } + +/// Load an [`VPC`] from a row in the `VPC` table. +impl TryFrom<&tokio_postgres::Row> for VPC { + type Error = Error; + + fn try_from(value: &tokio_postgres::Row) -> Result { + Ok(Self { identity: IdentityMetadata::try_from(value)? }) + } +} + +/// Load an [`VPCSubnet`] from a row in the `VPCSubnet` table. +impl TryFrom<&tokio_postgres::Row> for VPCSubnet { + type Error = Error; + + fn try_from(value: &tokio_postgres::Row) -> Result { + Ok(Self { + identity: IdentityMetadata::try_from(value)?, + vpc_id: sql_row_value(value, "vpc_id")?, + ipv4_block: sql_row_value(value, "ipv4_block")?, + ipv6_block: sql_row_value(value, "ipv6_block")?, + }) + } +} + +/// Load an [`VPC`] from a row in the `VNIC` table. +impl TryFrom<&tokio_postgres::Row> for VNIC { + type Error = Error; + + fn try_from(value: &tokio_postgres::Row) -> Result { + Ok(Self { + identity: IdentityMetadata::try_from(value)?, + vpc_id: sql_row_value(value, "vpc_id")?, + subnet_id: sql_row_value(value, "subnet_id")?, + mac: sql_row_value(value, "mac")?, + ip: sql_row_value(value, "ip")?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tokio_postgres::types::private::BytesMut; + use tokio_postgres::types::{FromSql, ToSql}; + + #[test] + fn test_ipnet_conversion() { + let subnet = IpNet("192.168.1.0/24".parse().unwrap()); + let ty = tokio_postgres::types::Type::INET; + let mut buf = BytesMut::with_capacity(128); + let is_null = subnet.to_sql(&ty, &mut buf).unwrap(); + assert!(matches!(is_null, tokio_postgres::types::IsNull::No)); + let from_sql = IpNet::from_sql(&ty, &buf).unwrap(); + assert_eq!(subnet, from_sql); + assert_eq!( + from_sql.addr(), + "192.168.1.0".parse::().unwrap() + ); + assert_eq!(from_sql.prefix_len(), 24); + + let bad = b"some-bad-net"; + assert!(IpNet::from_sql(&ty, &bad[..]).is_err()); + } + + #[test] + fn test_macaddr_conversion() { + let mac = MacAddr([0xAFu8; 6].into()); + let ty = tokio_postgres::types::Type::MACADDR; + let mut buf = BytesMut::with_capacity(128); + let is_null = mac.to_sql(&ty, &mut buf).unwrap(); + assert!(matches!(is_null, tokio_postgres::types::IsNull::No)); + let from_sql = MacAddr::from_sql(&ty, &buf).unwrap(); + assert_eq!(mac, from_sql); + assert_eq!(from_sql.into_array(), [0xaf; 6]); + + let bad = b"not:a:mac:addr"; + assert!(MacAddr::from_sql(&ty, &bad[..]).is_err()); + } +} diff --git a/omicron-common/src/sql/dbinit.sql b/omicron-common/src/sql/dbinit.sql index d7693711d11..8ef61f628c2 100644 --- a/omicron-common/src/sql/dbinit.sql +++ b/omicron-common/src/sql/dbinit.sql @@ -245,6 +245,73 @@ CREATE TABLE omicron.public.OximeterAssignment ( CONSTRAINT "primary" PRIMARY KEY (oximeter_id, producer_id) ); +/* + * VPCs and networking primitives + */ + +CREATE TABLE omicron.public.VPC ( + /* Identity metadata */ + id UUID PRIMARY KEY, + name STRING(63) NOT NULL, + description STRING(512) NOT NULL, + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + /* Indicates that the object has been deleted */ + time_deleted TIMESTAMPTZ + /* TODO: Add project-scoping. + * project_id UUID NOT NULL REFERENCES omicron.public.Project (id), + */ +); + +-- TODO: add project_id to index +CREATE UNIQUE INDEX ON omicron.public.VPC ( + name +) WHERE + time_deleted IS NULL; + +CREATE TABLE omicron.public.VPCSubnet ( + /* Identity metadata */ + id UUID PRIMARY KEY, + name STRING(63) NOT NULL, + description STRING(512) NOT NULL, + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + /* Indicates that the object has been deleted */ + time_deleted TIMESTAMPTZ, + vpc_id UUID NOT NULL, + ipv4_block INET, + ipv6_block INET +); + +-- TODO: add project_id to index +CREATE UNIQUE INDEX ON omicron.public.VPCSubnet ( + name +) WHERE + time_deleted IS NULL; + +CREATE TABLE omicron.public.VNIC ( + /* Identity metadata */ + id UUID PRIMARY KEY, + name STRING(63) NOT NULL, + description STRING(512) NOT NULL, + time_created TIMESTAMPTZ NOT NULL, + time_modified TIMESTAMPTZ NOT NULL, + /* Indicates that the object has been deleted */ + time_deleted TIMESTAMPTZ, + /* FK into VPC table */ + vpc_id UUID NOT NULL, + /* FK into VPCSubnet table. */ + subnet_id UUID NOT NULL, + mac CHAR(17) NOT NULL, -- 6 octets in hex -> 12 characters, plus 5 ":" separators + ip INET NOT NULL +); + +-- TODO: add project_id to index +CREATE UNIQUE INDEX ON omicron.public.VNIC ( + name +) WHERE + time_deleted IS NULL; + /*******************************************************************/ /* diff --git a/omicron-nexus/src/db/schema.rs b/omicron-nexus/src/db/schema.rs index b289c03a912..13ca4342bb1 100644 --- a/omicron-nexus/src/db/schema.rs +++ b/omicron-nexus/src/db/schema.rs @@ -152,6 +152,61 @@ impl Table for OximeterAssignment { &["oximeter_id", "producer_id", "time_created"]; } +/** Describes the "VPC" table */ +pub struct VPC; +impl Table for VPC { + type ModelType = api::VPC; + const TABLE_NAME: &'static str = "VPC"; + const ALL_COLUMNS: &'static [&'static str] = &[ + "id", + "name", + "description", + "time_created", + "time_modified", + "time_deleted", + // TODO: Add project-scoping: "project_id", + ]; +} + +/** Describes the "VPCSubnet" table */ +pub struct VPCSubnet; +impl Table for VPCSubnet { + type ModelType = api::VPCSubnet; + const TABLE_NAME: &'static str = "VPCSubnet"; + const ALL_COLUMNS: &'static [&'static str] = &[ + "id", + "name", + "description", + "time_created", + "time_modified", + "time_deleted", + // TODO: Add project-scoping: "project_id", + "vpc_id", + "ipv4_block", + "ipv6_block", + ]; +} + +/** Describes the "VNIC" table */ +pub struct VNIC; +impl Table for VNIC { + type ModelType = api::VNIC; + const TABLE_NAME: &'static str = "VNIC"; + const ALL_COLUMNS: &'static [&'static str] = &[ + "id", + "name", + "description", + "time_created", + "time_modified", + "time_deleted", + // TODO: Add project-scoping: "project_id", + "vpc_id", + "subnet_id", + "mac", + "ip", + ]; +} + #[cfg(test)] mod test { use super::Disk; @@ -161,6 +216,7 @@ mod test { use super::SagaNodeEvent; use super::Table; use super::{MetricProducer, Oximeter, OximeterAssignment}; + use super::{VPCSubnet, VNIC, VPC}; use omicron_common::dev; use std::collections::BTreeSet; use tokio_postgres::types::ToSql; @@ -189,6 +245,9 @@ mod test { check_table_schema::(&client).await; check_table_schema::(&client).await; check_table_schema::(&client).await; + check_table_schema::(&client).await; + check_table_schema::(&client).await; + check_table_schema::(&client).await; database.cleanup().await.expect("failed to clean up database"); logctx.cleanup_successful(); From 8e3dc47532aacbf4a81d191fa91c9b3eb1c16320 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Mon, 19 Jul 2021 16:01:01 -0700 Subject: [PATCH 2/6] Splits IP v4 and v6 subnet types --- omicron-common/src/api/model.rs | 220 ++++++++++++-------------------- omicron-common/src/model_db.rs | 100 +++++++++++---- 2 files changed, 161 insertions(+), 159 deletions(-) diff --git a/omicron-common/src/api/model.rs b/omicron-common/src/api/model.rs index cceb3b89928..1e59fc5065c 100644 --- a/omicron-common/src/api/model.rs +++ b/omicron-common/src/api/model.rs @@ -1323,47 +1323,82 @@ pub struct VPC { // pub project_id: Uuid, } -/// Client view onto a `VPC` object. -#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "camelCase")] -pub struct VPCView { - #[serde(flatten)] - pub identity: IdentityMetadata, - // TODO: Implement project-scoping - // /** id for the project containing this Instance */ - // pub project_id: Uuid, +/// An `Ipv4Net` represents a IPv4 subnetwork, including the address and network mask. +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] +pub struct Ipv4Net(pub ipnet::Ipv4Net); + +impl std::ops::Deref for Ipv4Net { + type Target = ipnet::Ipv4Net; + fn deref(&self) -> &Self::Target { + &self.0 + } } -impl Object for VPC { - type View = VPCView; - fn to_view(&self) -> Self::View { - VPCView { identity: self.identity.clone() } +impl std::fmt::Display for Ipv4Net { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self.0) } } -/// An `IpNet` represents a IP subnetwork (v4 or v6), including the address and network mask. -// NOTE: We're using the `ipnet` crate's implementation, but we wrap it in a newtype because that -// crate does not implement `JsonSchema` and Rust's orphan rules prevent us from implementing the -// trait directly on the type. +impl JsonSchema for Ipv4Net { + fn schema_name() -> String { + "Ipv4Net".to_string() + } + + fn json_schema( + _: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + schemars::schema::Schema::Object( + schemars::schema::SchemaObject { + metadata: Some(Box::new(schemars::schema::Metadata { + title: Some("An IPv4 subnet".to_string()), + description: Some("An IPv4 subnet, including prefix and subnet mask".to_string()), + examples: vec!["192.168.1.0/24".into()], + ..Default::default() + })), + instance_type: Some(schemars::schema::SingleOrVec::Single(Box::new(schemars::schema::InstanceType::String))), + string: Some(Box::new(schemars::schema::StringValidation { + // Fully-specified IPv4 address. Up to 15 chars for address, plus slash and up to 2 subnet digits. + max_length: Some(18), + min_length: None, + // Addresses must be from an RFC 1918 private address space + pattern: Some( + concat!( + // 10.x.x.x/8 + r#"^(10\.(25[0-5]|[1-2][0-4][0-9]|[1-9][0-9]|[0-9]\.){2}(25[0-5]|[1-2][0-4][0-9]|[1-9][0-9]|[0-9])/(1[0-9]|2[0-8]|[8-9]))$"#, + // 172.16.x.x/12 + r#"^(172\.16\.(25[0-5]|[1-2][0-4][0-9]|[1-9][0-9]|[0-9])\.(25[0-5]|[1-2][0-4][0-9]|[1-9][0-9]|[0-9])/(1[2-9]|2[0-8]))$"#, + // 192.168.x.x/16 + r#"^(192\.168\.(25[0-5]|[1-2][0-4][0-9]|[1-9][0-9]|[0-9])\.(25[0-5]|[1-2][0-4][0-9]|[1-9][0-9]|[0-9])/(1[6-9]|2[0-8]))$"#, + ).to_string(), + ), + })), + ..Default::default() + } + ) + } +} + +/// An `Ipv6Net` represents a IPv6 subnetwork, including the address and network mask. #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)] -pub struct IpNet(pub ipnet::IpNet); +pub struct Ipv6Net(pub ipnet::Ipv6Net); -impl std::ops::Deref for IpNet { - type Target = ipnet::IpNet; +impl std::ops::Deref for Ipv6Net { + type Target = ipnet::Ipv6Net; fn deref(&self) -> &Self::Target { &self.0 } } -impl std::fmt::Display for IpNet { +impl std::fmt::Display for Ipv6Net { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "{}", self.0) } } -impl JsonSchema for IpNet { +impl JsonSchema for Ipv6Net { fn schema_name() -> String { - "IpNet".to_string() + "Ipv6Net".to_string() } fn json_schema( @@ -1372,42 +1407,32 @@ impl JsonSchema for IpNet { schemars::schema::Schema::Object( schemars::schema::SchemaObject { metadata: Some(Box::new(schemars::schema::Metadata { - id: None, - title: Some("An IP subnet".to_string()), - description: Some("An IPv4 or IPv6 subnet, including prefix and subnet mask".to_string()), - default: None, - deprecated: false, - read_only: false, - write_only: false, - examples: vec!["192.168.1.0/24".into(), "fd12:3456::/64".into()], + title: Some("An IPv6 subnet".to_string()), + description: Some("An IPv6 subnet, including prefix and subnet mask".to_string()), + examples: vec!["fd12:3456::/64".into()], + ..Default::default() })), instance_type: Some(schemars::schema::SingleOrVec::Single(Box::new(schemars::schema::InstanceType::String))), - format: None, - enum_values: None, - const_value: None, - subschemas: None, - number: None, string: Some(Box::new(schemars::schema::StringValidation { - max_length: Some(23), // fully-specified IPv6, slash and 3-digit mask + // Fully-specified IPv6 address. 4 hex chars per segment, 8 segments, 7 + // ":"-separators, slash and up to 3 subnet digits + max_length: Some(43), min_length: None, - // This regex validator is inspired by https://ihateregex.io/expr/ipv{4,6} - // Note that the regex is more permissive than we intend to be in the final - // API. This passes _any_ IPs, while we're likely going to restrict them to the - // private address space for v4 for example (RFD 21 sec 2.2). pattern: Some( - r#"(^(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$)|(^(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))$)"#.to_string() + // Conforming to unique local addressing scheme, `fd00::/8` + concat!( + r#"^(fd|FD)00:((([0-8a-fA-F]{1,4}\:){6}[0-8a-fA-F]{1,4})|(([0-8a-fA-F]{1,4}:){1,6}:))/(6[4-9]|[7-9][0-9]|1[0-1][0-9]|12[0-6])$"#, + ).to_string(), ), })), - array: None, - object: None, - reference: None, - extensions: BTreeMap::new(), + ..Default::default() } ) } } -/// An IP subnet within a VPC. +/// A VPC subnet represents a logical grouping for instances that allows network traffic between +/// them, within a IPv4 subnetwork or optionall an IPv6 subnetwork. #[derive(Clone, Debug)] pub struct VPCSubnet { /** common identifying metadata */ @@ -1419,48 +1444,18 @@ pub struct VPCSubnet { /** The VPC to which the subnet belongs. */ pub vpc_id: Uuid, - // TODO-correctness: RFD 21 sec 3.5 indicates that users may specify both v4 and v6 subnets in - // a single API call, but does not explicitly say if either is required. Clearly, one of them - // has to be specified, and most cloud providers require v4 and make v6 optional. However, RFD - // hints that users _might_ be able to specify v6 and not v4 (and maybe that the omitted block - // will be added by default). - /** The IPv4 subnet CIDR block. */ - pub ipv4_block: Option, - + // TODO-design: RFD 21 says that V4 subnets are currently required, and V6 are optional. If a + // V6 address is _not_ specified, one is created with a prefix that depends on the VPC and a + // unique subnet-specific portion of the prefix (40 and 16 bits for each, respectively). + // + // We're leaving out the "view" types here for the external HTTP API for now, so it's not clear + // how to do the validation of user-specified CIDR blocks, or how to create a block if one is + // not given. /** The IPv4 subnet CIDR block. */ - pub ipv6_block: Option, -} + pub ipv4_block: Option, -impl Object for VPCSubnet { - type View = VPCSubnetView; - fn to_view(&self) -> Self::View { - VPCSubnetView { - identity: self.identity.clone(), - vpc_id: self.vpc_id, - ipv4_block: self.ipv4_block, - ipv6_block: self.ipv6_block, - } - } -} - -/// Client view onto a `VPCSubnet` object. -#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "camelCase")] -pub struct VPCSubnetView { - #[serde(flatten)] - pub identity: IdentityMetadata, - - // TODO: Implement project-scoping - // /** id for the project containing this Instance */ - // pub project_id: Uuid, - /** The VPC to which the subnet belongs. */ - pub vpc_id: Uuid, - - /** The IPv4 subnetwork. */ - pub ipv4_block: Option, - - /** The IPv6 subnetwork. */ - pub ipv6_block: Option, + /** The IPv6 subnet CIDR block. */ + pub ipv6_block: Option, } /// The `MacAddr` represents a Media Access Control (MAC) address, used to uniquely identify @@ -1494,26 +1489,17 @@ impl JsonSchema for MacAddr { ) -> schemars::schema::Schema { schemars::schema::Schema::Object(schemars::schema::SchemaObject { metadata: Some(Box::new(schemars::schema::Metadata { - id: None, title: Some("A MAC address".to_string()), description: Some( "A Media Access Control address, in EUI-48 format" .to_string(), ), - default: None, - deprecated: false, - read_only: false, - write_only: false, examples: vec!["ff:ff:ff:ff:ff:ff".into()], + ..Default::default() })), instance_type: Some(schemars::schema::SingleOrVec::Single( Box::new(schemars::schema::InstanceType::String), )), - format: None, - enum_values: None, - const_value: None, - subschemas: None, - number: None, string: Some(Box::new(schemars::schema::StringValidation { max_length: Some(17), // 12 hex characters and 5 ":"-separators min_length: Some(17), @@ -1521,10 +1507,7 @@ impl JsonSchema for MacAddr { r#"^([0-8a-fA-F]{2}:){5}[0-8a-fA-F]{2}$"#.to_string(), ), })), - array: None, - object: None, - reference: None, - extensions: BTreeMap::new(), + ..Default::default() }) } } @@ -1551,41 +1534,6 @@ pub struct VNIC { pub ip: IpAddr, } -impl Object for VNIC { - type View = VNICView; - fn to_view(&self) -> Self::View { - VNICView { - identity: self.identity.clone(), - vpc_id: self.vpc_id, - subnet_id: self.subnet_id, - mac: self.mac, - ip: self.ip, - } - } -} - -/// Client view onto a `VNIC` object. -#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "camelCase")] -pub struct VNICView { - #[serde(flatten)] - pub identity: IdentityMetadata, - - // TODO: Implement project-scoping - // pub project_id: Uuid, - /** The VPC to which the NIC belongs. */ - pub vpc_id: Uuid, - - /** The subnet to which the NIC belongs. */ - pub subnet_id: Uuid, - - /** The MAC address assigned to this NIC. */ - pub mac: MacAddr, - - /** The IP address assigned to this NIC. */ - pub ip: IpAddr, -} - /* * Internal Control Plane API objects */ diff --git a/omicron-common/src/model_db.rs b/omicron-common/src/model_db.rs index 7da347e9a5e..ddfcb630055 100644 --- a/omicron-common/src/model_db.rs +++ b/omicron-common/src/model_db.rs @@ -65,7 +65,6 @@ use crate::api::Instance; use crate::api::InstanceCpuCount; use crate::api::InstanceRuntimeState; use crate::api::InstanceState; -use crate::api::IpNet; use crate::api::MacAddr; use crate::api::Name; use crate::api::OximeterAssignment; @@ -75,6 +74,7 @@ use crate::api::Project; use crate::api::VPCSubnet; use crate::api::VNIC; use crate::api::VPC; +use crate::api::{Ipv4Net, Ipv6Net}; use crate::bail_unless; use chrono::DateTime; use chrono::Utc; @@ -135,13 +135,8 @@ impl_sql_wrapping!(Generation, i64); impl_sql_wrapping!(InstanceCpuCount, i64); impl_sql_wrapping!(Name, &str); -// Conversion to/from SQL types for IpNet. -// -// Although there are implementations that convert an IP _address_ to and from PostgreSQL types, -// there does not appear to be any existing implementation that converts a _subnet_, including the -// address and mask, to and from SQL types. At least, not for the 3rd-party crate `ipnet` we're -// using to actually represent a subnet. -impl tokio_postgres::types::ToSql for IpNet { +// Conversion to/from SQL types for Ipv4Net. +impl tokio_postgres::types::ToSql for Ipv4Net { fn to_sql( &self, _ty: &tokio_postgres::types::Type, @@ -151,7 +146,7 @@ impl tokio_postgres::types::ToSql for IpNet { Box, > { postgres_protocol::types::inet_to_sql( - self.addr(), + self.addr().into(), self.prefix_len(), out, ); @@ -165,22 +160,62 @@ impl tokio_postgres::types::ToSql for IpNet { tokio_postgres::types::to_sql_checked!(); } -impl<'a> tokio_postgres::types::FromSql<'a> for IpNet { +impl<'a> tokio_postgres::types::FromSql<'a> for Ipv4Net { fn from_sql( _ty: &tokio_postgres::types::Type, raw: &'a [u8], ) -> Result> { let value = postgres_protocol::types::inet_from_sql(raw)?; let (addr, mask) = (value.addr(), value.netmask()); - let subnet = match addr { - std::net::IpAddr::V4(addr) => { - ipnet::Ipv4Net::new(addr, mask)?.into() - } - std::net::IpAddr::V6(addr) => { - ipnet::Ipv6Net::new(addr, mask)?.into() - } - }; - Ok(IpNet(subnet)) + if let std::net::IpAddr::V4(addr) = addr { + Ok(Ipv4Net(ipnet::Ipv4Net::new(addr, mask)?)) + } else { + panic!("Attempted to deserialize IPv6 subnet from database, expected IPv4 subnet"); + } + } + + fn accepts(ty: &tokio_postgres::types::Type) -> bool { + matches!(ty, &tokio_postgres::types::Type::INET) + } +} + +// Conversion to/from SQL types for Ipv6Net. +impl tokio_postgres::types::ToSql for Ipv6Net { + fn to_sql( + &self, + _ty: &tokio_postgres::types::Type, + out: &mut tokio_postgres::types::private::BytesMut, + ) -> Result< + tokio_postgres::types::IsNull, + Box, + > { + postgres_protocol::types::inet_to_sql( + self.addr().into(), + self.prefix_len(), + out, + ); + Ok(tokio_postgres::types::IsNull::No) + } + + fn accepts(ty: &tokio_postgres::types::Type) -> bool { + matches!(ty, &tokio_postgres::types::Type::INET) + } + + tokio_postgres::types::to_sql_checked!(); +} + +impl<'a> tokio_postgres::types::FromSql<'a> for Ipv6Net { + fn from_sql( + _ty: &tokio_postgres::types::Type, + raw: &'a [u8], + ) -> Result> { + let value = postgres_protocol::types::inet_from_sql(raw)?; + let (addr, mask) = (value.addr(), value.netmask()); + if let std::net::IpAddr::V6(addr) = addr { + Ok(Ipv6Net(ipnet::Ipv6Net::new(addr, mask)?)) + } else { + panic!("Attempted to deserialize IPv4 subnet from database, expected IPv6 subnet"); + } } fn accepts(ty: &tokio_postgres::types::Type) -> bool { @@ -444,13 +479,13 @@ mod tests { use tokio_postgres::types::{FromSql, ToSql}; #[test] - fn test_ipnet_conversion() { - let subnet = IpNet("192.168.1.0/24".parse().unwrap()); + fn test_ipv4net_conversion() { + let subnet = Ipv4Net("192.168.1.0/24".parse().unwrap()); let ty = tokio_postgres::types::Type::INET; let mut buf = BytesMut::with_capacity(128); let is_null = subnet.to_sql(&ty, &mut buf).unwrap(); assert!(matches!(is_null, tokio_postgres::types::IsNull::No)); - let from_sql = IpNet::from_sql(&ty, &buf).unwrap(); + let from_sql = Ipv4Net::from_sql(&ty, &buf).unwrap(); assert_eq!(subnet, from_sql); assert_eq!( from_sql.addr(), @@ -459,7 +494,26 @@ mod tests { assert_eq!(from_sql.prefix_len(), 24); let bad = b"some-bad-net"; - assert!(IpNet::from_sql(&ty, &bad[..]).is_err()); + assert!(Ipv4Net::from_sql(&ty, &bad[..]).is_err()); + } + + #[test] + fn test_ipv6net_conversion() { + let subnet = Ipv6Net("fd00:1234::/24".parse().unwrap()); + let ty = tokio_postgres::types::Type::INET; + let mut buf = BytesMut::with_capacity(256); + let is_null = subnet.to_sql(&ty, &mut buf).unwrap(); + assert!(matches!(is_null, tokio_postgres::types::IsNull::No)); + let from_sql = Ipv6Net::from_sql(&ty, &buf).unwrap(); + assert_eq!(subnet, from_sql); + assert_eq!( + from_sql.addr(), + "fd00:1234::".parse::().unwrap() + ); + assert_eq!(from_sql.prefix_len(), 24); + + let bad = b"some-bad-net"; + assert!(Ipv6Net::from_sql(&ty, &bad[..]).is_err()); } #[test] From 63b51d4b54c7216fcac852f13367854e816896ea Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Tue, 20 Jul 2021 09:44:28 -0700 Subject: [PATCH 3/6] Use STRING in MAC addr DB type for uniformity, and add TODO about NIC/IP table design --- omicron-common/src/sql/dbinit.sql | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/omicron-common/src/sql/dbinit.sql b/omicron-common/src/sql/dbinit.sql index 8ef61f628c2..c88df61cbd0 100644 --- a/omicron-common/src/sql/dbinit.sql +++ b/omicron-common/src/sql/dbinit.sql @@ -302,10 +302,18 @@ CREATE TABLE omicron.public.VNIC ( vpc_id UUID NOT NULL, /* FK into VPCSubnet table. */ subnet_id UUID NOT NULL, - mac CHAR(17) NOT NULL, -- 6 octets in hex -> 12 characters, plus 5 ":" separators + mac STRING(17) NOT NULL, -- e.g., "ff:ff:ff:ff:ff:ff" ip INET NOT NULL ); +/* TODO-completeness + + * We currently have a VNIC table with the IP and MAC addresses inline. + * Eventually, we'll probably want to move these to their own tables, and + * refer to them here, most notably to support multiple IPs per NIC, as well + * as moving IPs between NICs on different instances, etc. + */ + -- TODO: add project_id to index CREATE UNIQUE INDEX ON omicron.public.VNIC ( name From cef9eccdeb3969176ba917127a8fc30306ac5167 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Wed, 21 Jul 2021 08:03:42 -0700 Subject: [PATCH 4/6] Rename VNIC -> NetworkInterface --- omicron-common/src/api/model.rs | 12 ++++++------ omicron-common/src/model_db.rs | 6 +++--- omicron-common/src/sql/dbinit.sql | 6 +++--- omicron-nexus/src/db/schema.rs | 14 +++++++------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/omicron-common/src/api/model.rs b/omicron-common/src/api/model.rs index 1e59fc5065c..f03ab8b3bf4 100644 --- a/omicron-common/src/api/model.rs +++ b/omicron-common/src/api/model.rs @@ -1512,25 +1512,25 @@ impl JsonSchema for MacAddr { } } -/// A `VNIC` represents a virtual network interface device. +/// A `NetworkInterface` represents a virtual network interface device. #[derive(Clone, Debug)] -pub struct VNIC { +pub struct NetworkInterface { /** common identifying metadata */ pub identity: IdentityMetadata, // TODO: Implement project-scoping // /** id for the project containing this Instance */ // pub project_id: Uuid, - /** The VPC to which the NIC belongs. */ + /** The VPC to which the interface belongs. */ pub vpc_id: Uuid, - /** The subnet to which the NIC belongs. */ + /** The subnet to which the interface belongs. */ pub subnet_id: Uuid, - /** The MAC address assigned to this NIC. */ + /** The MAC address assigned to this interface. */ pub mac: MacAddr, - /** The IP address assigned to this NIC. */ + /** The IP address assigned to this interface. */ pub ip: IpAddr, } diff --git a/omicron-common/src/model_db.rs b/omicron-common/src/model_db.rs index ddfcb630055..1674dd3a637 100644 --- a/omicron-common/src/model_db.rs +++ b/omicron-common/src/model_db.rs @@ -67,12 +67,12 @@ use crate::api::InstanceRuntimeState; use crate::api::InstanceState; use crate::api::MacAddr; use crate::api::Name; +use crate::api::NetworkInterface; use crate::api::OximeterAssignment; use crate::api::OximeterInfo; use crate::api::ProducerEndpoint; use crate::api::Project; use crate::api::VPCSubnet; -use crate::api::VNIC; use crate::api::VPC; use crate::api::{Ipv4Net, Ipv6Net}; use crate::bail_unless; @@ -457,8 +457,8 @@ impl TryFrom<&tokio_postgres::Row> for VPCSubnet { } } -/// Load an [`VPC`] from a row in the `VNIC` table. -impl TryFrom<&tokio_postgres::Row> for VNIC { +/// Load a [`NetworkInterface`] from a row in the `NetworkInterface` table. +impl TryFrom<&tokio_postgres::Row> for NetworkInterface { type Error = Error; fn try_from(value: &tokio_postgres::Row) -> Result { diff --git a/omicron-common/src/sql/dbinit.sql b/omicron-common/src/sql/dbinit.sql index c88df61cbd0..54b02890d7d 100644 --- a/omicron-common/src/sql/dbinit.sql +++ b/omicron-common/src/sql/dbinit.sql @@ -289,7 +289,7 @@ CREATE UNIQUE INDEX ON omicron.public.VPCSubnet ( ) WHERE time_deleted IS NULL; -CREATE TABLE omicron.public.VNIC ( +CREATE TABLE omicron.public.NetworkInterface ( /* Identity metadata */ id UUID PRIMARY KEY, name STRING(63) NOT NULL, @@ -308,14 +308,14 @@ CREATE TABLE omicron.public.VNIC ( /* TODO-completeness - * We currently have a VNIC table with the IP and MAC addresses inline. + * We currently have a NetworkInterface table with the IP and MAC addresses inline. * Eventually, we'll probably want to move these to their own tables, and * refer to them here, most notably to support multiple IPs per NIC, as well * as moving IPs between NICs on different instances, etc. */ -- TODO: add project_id to index -CREATE UNIQUE INDEX ON omicron.public.VNIC ( +CREATE UNIQUE INDEX ON omicron.public.NetworkInterface ( name ) WHERE time_deleted IS NULL; diff --git a/omicron-nexus/src/db/schema.rs b/omicron-nexus/src/db/schema.rs index 13ca4342bb1..b3b9893aa41 100644 --- a/omicron-nexus/src/db/schema.rs +++ b/omicron-nexus/src/db/schema.rs @@ -187,11 +187,11 @@ impl Table for VPCSubnet { ]; } -/** Describes the "VNIC" table */ -pub struct VNIC; -impl Table for VNIC { - type ModelType = api::VNIC; - const TABLE_NAME: &'static str = "VNIC"; +/** Describes the "NetworkInterface" table */ +pub struct NetworkInterface; +impl Table for NetworkInterface { + type ModelType = api::NetworkInterface; + const TABLE_NAME: &'static str = "NetworkInterface"; const ALL_COLUMNS: &'static [&'static str] = &[ "id", "name", @@ -216,7 +216,7 @@ mod test { use super::SagaNodeEvent; use super::Table; use super::{MetricProducer, Oximeter, OximeterAssignment}; - use super::{VPCSubnet, VNIC, VPC}; + use super::{NetworkInterface, VPCSubnet, VPC}; use omicron_common::dev; use std::collections::BTreeSet; use tokio_postgres::types::ToSql; @@ -247,7 +247,7 @@ mod test { check_table_schema::(&client).await; check_table_schema::(&client).await; check_table_schema::(&client).await; - check_table_schema::(&client).await; + check_table_schema::(&client).await; database.cleanup().await.expect("failed to clean up database"); logctx.cleanup_successful(); From 7a4021bd306d1d925431f15d8aa33ab32d234780 Mon Sep 17 00:00:00 2001 From: Benjamin Naecker Date: Wed, 21 Jul 2021 08:39:53 -0700 Subject: [PATCH 5/6] Add serialization derives to NetworkInterface --- omicron-common/src/api/model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omicron-common/src/api/model.rs b/omicron-common/src/api/model.rs index f03ab8b3bf4..816686f6a30 100644 --- a/omicron-common/src/api/model.rs +++ b/omicron-common/src/api/model.rs @@ -1513,7 +1513,7 @@ impl JsonSchema for MacAddr { } /// A `NetworkInterface` represents a virtual network interface device. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct NetworkInterface { /** common identifying metadata */ pub identity: IdentityMetadata, From 22d12f881783a7ccefb8b6eac8702865b496d7c1 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 27 Jul 2021 13:55:40 -0500 Subject: [PATCH 6/6] put project_id on VPC model, remove TODO on subnet and NIC --- omicron-common/src/api/model.rs | 11 ++--------- omicron-common/src/model_db.rs | 5 ++++- omicron-common/src/sql/dbinit.sql | 6 ++---- omicron-nexus/src/db/schema.rs | 4 +--- 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/omicron-common/src/api/model.rs b/omicron-common/src/api/model.rs index 816686f6a30..874d6c692a2 100644 --- a/omicron-common/src/api/model.rs +++ b/omicron-common/src/api/model.rs @@ -1318,9 +1318,8 @@ impl From for SagaStateView { pub struct VPC { /** common identifying metadata */ pub identity: IdentityMetadata, - // TODO: Implement project-scoping - // /** id for the project containing this Instance */ - // pub project_id: Uuid, + /** id for the project containing this Instance */ + pub project_id: Uuid, } /// An `Ipv4Net` represents a IPv4 subnetwork, including the address and network mask. @@ -1438,9 +1437,6 @@ pub struct VPCSubnet { /** common identifying metadata */ pub identity: IdentityMetadata, - // TODO: Implement project-scoping - // /** id for the project containing this Instance */ - // pub project_id: Uuid, /** The VPC to which the subnet belongs. */ pub vpc_id: Uuid, @@ -1518,9 +1514,6 @@ pub struct NetworkInterface { /** common identifying metadata */ pub identity: IdentityMetadata, - // TODO: Implement project-scoping - // /** id for the project containing this Instance */ - // pub project_id: Uuid, /** The VPC to which the interface belongs. */ pub vpc_id: Uuid, diff --git a/omicron-common/src/model_db.rs b/omicron-common/src/model_db.rs index 1674dd3a637..638630a2f75 100644 --- a/omicron-common/src/model_db.rs +++ b/omicron-common/src/model_db.rs @@ -439,7 +439,10 @@ impl TryFrom<&tokio_postgres::Row> for VPC { type Error = Error; fn try_from(value: &tokio_postgres::Row) -> Result { - Ok(Self { identity: IdentityMetadata::try_from(value)? }) + Ok(Self { + identity: IdentityMetadata::try_from(value)?, + project_id: sql_row_value(value, "project_id")?, + }) } } diff --git a/omicron-common/src/sql/dbinit.sql b/omicron-common/src/sql/dbinit.sql index 54b02890d7d..12f842fd603 100644 --- a/omicron-common/src/sql/dbinit.sql +++ b/omicron-common/src/sql/dbinit.sql @@ -257,10 +257,8 @@ CREATE TABLE omicron.public.VPC ( time_created TIMESTAMPTZ NOT NULL, time_modified TIMESTAMPTZ NOT NULL, /* Indicates that the object has been deleted */ - time_deleted TIMESTAMPTZ - /* TODO: Add project-scoping. - * project_id UUID NOT NULL REFERENCES omicron.public.Project (id), - */ + time_deleted TIMESTAMPTZ, + project_id UUID NOT NULL ); -- TODO: add project_id to index diff --git a/omicron-nexus/src/db/schema.rs b/omicron-nexus/src/db/schema.rs index b3b9893aa41..79fba192cac 100644 --- a/omicron-nexus/src/db/schema.rs +++ b/omicron-nexus/src/db/schema.rs @@ -164,7 +164,7 @@ impl Table for VPC { "time_created", "time_modified", "time_deleted", - // TODO: Add project-scoping: "project_id", + "project_id", ]; } @@ -180,7 +180,6 @@ impl Table for VPCSubnet { "time_created", "time_modified", "time_deleted", - // TODO: Add project-scoping: "project_id", "vpc_id", "ipv4_block", "ipv6_block", @@ -199,7 +198,6 @@ impl Table for NetworkInterface { "time_created", "time_modified", "time_deleted", - // TODO: Add project-scoping: "project_id", "vpc_id", "subnet_id", "mac",