diff --git a/Cargo.lock b/Cargo.lock index 8cc2a8d5c4..3e5f90dfcd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -817,6 +817,7 @@ dependencies = [ "heck 0.4.0", "proc-macro2", "quote", + "rustfmt-wrapper", "serde", "serde_tokenstream", "syn", diff --git a/common/src/api/external/error.rs b/common/src/api/external/error.rs index d641020bd3..36a1f33eee 100644 --- a/common/src/api/external/error.rs +++ b/common/src/api/external/error.rs @@ -68,6 +68,9 @@ pub enum LookupType { ById(Uuid), /// a session token was requested BySessionToken(String), + /// a specific id was requested with some composite type + /// (caller summarizes it) + ByCompositeId(String), } impl LookupType { @@ -160,6 +163,7 @@ impl From for HttpError { let (lookup_field, lookup_value) = match lt { LookupType::ByName(name) => ("name", name), LookupType::ById(id) => ("id", id.to_string()), + LookupType::ByCompositeId(label) => ("id", label), LookupType::BySessionToken(token) => { ("session token", token) } diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index abd175db9f..1340684836 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -537,6 +537,7 @@ pub enum ResourceType { Oximeter, MetricProducer, Role, + UpdateAvailableArtifact, User, Zpool, } diff --git a/common/src/api/internal/nexus.rs b/common/src/api/internal/nexus.rs index efba6245a8..4572e1f876 100644 --- a/common/src/api/internal/nexus.rs +++ b/common/src/api/internal/nexus.rs @@ -88,7 +88,15 @@ pub struct UpdateArtifact { /// Kinds of update artifacts, as used by Nexus to determine what updates are available and by /// sled-agent to determine how to apply an update when asked. #[derive( - Clone, Copy, Debug, PartialEq, Display, Deserialize, Serialize, JsonSchema, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Display, + Deserialize, + Serialize, + JsonSchema, )] #[display(style = "kebab-case")] #[serde(rename_all = "kebab-case")] diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index df09dedac5..c9715b26ab 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -38,6 +38,7 @@ use crate::authn; use crate::context::OpContext; use crate::db::fixed_data::FLEET_ID; use crate::db::model::Name; +use crate::db::model::UpdateArtifactKind; use crate::db::DataStore; use authz_macros::authz_resource; use futures::future::BoxFuture; @@ -262,6 +263,14 @@ authz_resource! { // Miscellaneous resources nested directly below "Fleet" +authz_resource! { + name = "SiloUser", + parent = "Fleet", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = FleetChild, +} + authz_resource! { name = "Role", parent = "Fleet", @@ -293,3 +302,11 @@ authz_resource! { roles_allowed = false, polar_snippet = FleetChild, } + +authz_resource! { + name = "UpdateAvailableArtifact", + parent = "Fleet", + primary_key = (String, i64, UpdateArtifactKind), + roles_allowed = false, + polar_snippet = FleetChild, +} diff --git a/nexus/src/authz/mod.rs b/nexus/src/authz/mod.rs index 44a5705579..465c88f868 100644 --- a/nexus/src/authz/mod.rs +++ b/nexus/src/authz/mod.rs @@ -162,22 +162,7 @@ mod actor; mod api_resources; -pub use api_resources::ApiResourceError; -pub use api_resources::Disk; -pub use api_resources::Fleet; -pub use api_resources::Instance; -pub use api_resources::NetworkInterface; -pub use api_resources::Organization; -pub use api_resources::Project; -pub use api_resources::Rack; -pub use api_resources::Role; -pub use api_resources::RouterRoute; -pub use api_resources::Sled; -pub use api_resources::User; -pub use api_resources::Vpc; -pub use api_resources::VpcRouter; -pub use api_resources::VpcSubnet; -pub use api_resources::FLEET; +pub use api_resources::*; mod context; pub use context::AuthorizedResource; diff --git a/nexus/src/authz/oso_generic.rs b/nexus/src/authz/oso_generic.rs index bf45203faf..9b260abc31 100644 --- a/nexus/src/authz/oso_generic.rs +++ b/nexus/src/authz/oso_generic.rs @@ -60,6 +60,7 @@ pub fn make_omicron_oso() -> Result { RouterRoute::init(), VpcSubnet::init(), Role::init(), + UpdateAvailableArtifact::init(), User::init(), Rack::init(), Sled::init(), diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index cc26e7d288..1511508421 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -53,7 +53,6 @@ use omicron_common::api::external::UpdateResult; use omicron_common::api::external::{ CreateResult, IdentityMetadataCreateParams, }; -use omicron_common::api::internal::nexus::UpdateArtifact; use omicron_common::bail_unless; use std::convert::{TryFrom, TryInto}; use std::net::Ipv6Addr; @@ -70,9 +69,9 @@ use crate::db::{ Name, NetworkInterface, Organization, OrganizationUpdate, OximeterInfo, ProducerEndpoint, Project, ProjectUpdate, Region, RoleAssignmentBuiltin, RoleBuiltin, RouterRoute, RouterRouteUpdate, - Silo, SiloUser, Sled, UpdateArtifactKind, UpdateAvailableArtifact, - UserBuiltin, Volume, Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, - VpcSubnet, VpcSubnetUpdate, VpcUpdate, Zpool, + Silo, SiloUser, Sled, UpdateAvailableArtifact, UserBuiltin, Volume, + Vpc, VpcFirewallRule, VpcRouter, VpcRouterUpdate, VpcSubnet, + VpcSubnetUpdate, VpcUpdate, Zpool, }, pagination::paginated, pagination::paginated_multicolumn, @@ -2554,32 +2553,6 @@ impl DataStore { }) } - pub async fn update_available_artifact_fetch( - &self, - opctx: &OpContext, - artifact: &UpdateArtifact, - ) -> LookupResult { - opctx.authorize(authz::Action::Read, &authz::FLEET).await?; - - use db::schema::update_available_artifact::dsl; - dsl::update_available_artifact - .filter( - dsl::name - .eq(artifact.name.clone()) - .and(dsl::version.eq(artifact.version)) - .and(dsl::kind.eq(UpdateArtifactKind(artifact.kind))), - ) - .select(UpdateAvailableArtifact::as_select()) - .first_async(self.pool_authorized(opctx).await?) - .await - .map_err(|e| { - Error::internal_error(&format!( - "error fetching artifact: {:?}", - e - )) - }) - } - pub async fn silo_user_fetch( &self, silo_user_id: Uuid, diff --git a/nexus/src/db/db-macros/Cargo.toml b/nexus/src/db/db-macros/Cargo.toml index 0031ff2f46..afd4c67def 100644 --- a/nexus/src/db/db-macros/Cargo.toml +++ b/nexus/src/db/db-macros/Cargo.toml @@ -15,3 +15,6 @@ quote = { version = "1.0" } serde = { version = "1.0", features = [ "derive" ] } serde_tokenstream = "0.1" syn = { version = "1.0", features = [ "full", "derive", "extra-traits" ] } + +[dev-dependencies] +rustfmt-wrapper = "0.1" diff --git a/nexus/src/db/db-macros/src/lib.rs b/nexus/src/db/db-macros/src/lib.rs index c901e9f0ca..fed2c8f488 100644 --- a/nexus/src/db/db-macros/src/lib.rs +++ b/nexus/src/db/db-macros/src/lib.rs @@ -29,6 +29,9 @@ mod lookup; /// name = "Organization", /// ancestors = [], /// children = [ "Project" ], +/// lookup_by_name = true, +/// soft_deletes = true, +/// primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] /// } /// ``` /// @@ -46,12 +49,18 @@ mod lookup; /// name = "Organization", /// ancestors = [], /// children = [ "Project" ], +/// lookup_by_name = true, +/// soft_deletes = true, +/// primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] /// } /// /// lookup_resource! { /// name = "Instance", /// ancestors = [ "Organization", "Project" ], /// children = [], +/// lookup_by_name = true, +/// soft_deletes = true, +/// primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] /// } /// ``` /// diff --git a/nexus/src/db/db-macros/src/lookup.rs b/nexus/src/db/db-macros/src/lookup.rs index 2ad68912be..973c48f36e 100644 --- a/nexus/src/db/db-macros/src/lookup.rs +++ b/nexus/src/db/db-macros/src/lookup.rs @@ -8,6 +8,8 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; +use serde_tokenstream::ParseWrapper; +use std::ops::Deref; // // INPUT (arguments to the macro) @@ -34,6 +36,18 @@ pub struct Input { /// unordered list of resources that are direct children of this resource /// (e.g., for a Project, these would include "Instance" and "Disk") children: Vec, + /// whether lookup by name is supported (usually within the parent collection) + lookup_by_name: bool, + /// Description of the primary key columns + primary_key_columns: Vec, + /// This resources supports soft-deletes + soft_deletes: bool, +} + +#[derive(serde::Deserialize)] +struct PrimaryKeyColumn { + column_name: String, + rust_type: ParseWrapper, } // @@ -50,14 +64,20 @@ pub struct Config { /// Basic information about the resource we're generating resource: Resource, + /// This resources supports soft-deletes + soft_deletes: bool, + + /// Name of the enum describing how we're looking up this resource + lookup_enum: syn::Ident, + // The path to the resource /// list of type names for this resource and its parents /// (e.g., [`Organization`, `Project`]) path_types: Vec, /// list of identifiers used for the authz objects for this resource and its - /// parents, in the same order as `authz_path_types` - /// (e.g., [`authz_organization`, `authz_project`]) + /// parents, in the same order as `authz_path_types` (e.g., + /// [`authz_organization`, `authz_project`]) path_authz_names: Vec, // Child resources @@ -68,11 +88,18 @@ pub struct Config { // Parent resource, if any /// Information about the parent resource, if any parent: Option, + + /// whether lookup by name is supported + lookup_by_name: bool, + + /// Description of the primary key columns + primary_key_columns: Vec, } impl Config { fn for_input(input: Input) -> Config { let resource = Resource::for_name(&input.name); + let lookup_enum = format_ident!("{}Key", resource.name); let mut path_types: Vec<_> = input.ancestors.iter().map(|a| format_ident!("{}", a)).collect(); @@ -87,12 +114,19 @@ impl Config { .collect(); path_authz_names.push(resource.authz_name.clone()); + let child_resources = input.children; + let parent = input.ancestors.last().map(|s| Resource::for_name(&s)); + Config { resource, + lookup_enum, path_types, path_authz_names, - parent: input.ancestors.last().map(|s| Resource::for_name(&s)), - child_resources: input.children, + parent, + child_resources, + lookup_by_name: input.lookup_by_name, + primary_key_columns: input.primary_key_columns, + soft_deletes: input.soft_deletes, } } } @@ -131,14 +165,14 @@ pub fn lookup_resource( let config = Config::for_input(input); let resource_name = &config.resource.name; - let the_struct = generate_struct(&config); + let the_basics = generate_struct(&config); let misc_helpers = generate_misc_helpers(&config); let child_selectors = generate_child_selectors(&config); let lookup_methods = generate_lookup_methods(&config); let database_functions = generate_database_functions(&config); Ok(quote! { - #the_struct + #the_basics impl<'a> #resource_name<'a> { #child_selectors @@ -154,20 +188,47 @@ pub fn lookup_resource( /// Generates the struct definition for this resource fn generate_struct(config: &Config) -> TokenStream { - let root_sym = format_ident!("Root"); let resource_name = &config.resource.name; - let parent_resource_name = - config.parent.as_ref().map(|p| &p.name).unwrap_or_else(|| &root_sym); let doc_struct = format!( "Selects a resource of type {} (or any of its children, using the \ functions on this struct) for lookup or fetch", resource_name ); + let pkey_types = + config.primary_key_columns.iter().map(|c| c.rust_type.deref()); + + /* configure the lookup enum */ + let lookup_enum = &config.lookup_enum; + let name_variant = if config.lookup_by_name { + let root_sym = format_ident!("Root"); + let parent_resource_name = config + .parent + .as_ref() + .map(|p| &p.name) + .unwrap_or_else(|| &root_sym); + quote! { + /// We're looking for a resource with the given name in the given + /// parent collection + Name(#parent_resource_name<'a>, &'a Name), + } + } else { + quote! {} + }; quote! { #[doc = #doc_struct] pub struct #resource_name<'a> { - key: Key<'a, #parent_resource_name<'a>> + key: #lookup_enum<'a> + } + + /// Describes how we're looking up this resource + enum #lookup_enum<'a>{ + #name_variant + + /// We're looking for a resource with the given primary key + /// + /// This has no parent container -- a by-id lookup is always global + PrimaryKey(Root<'a> #(,#pkey_types)*), } } } @@ -180,6 +241,11 @@ fn generate_struct(config: &Config) -> TokenStream { fn generate_child_selectors(config: &Config) -> TokenStream { let child_resource_types: Vec<_> = config.child_resources.iter().map(|c| format_ident!("{}", c)).collect(); + let child_resource_key_types: Vec<_> = config + .child_resources + .iter() + .map(|c| format_ident!("{}Key", c)) + .collect(); let child_selector_fn_names: Vec<_> = config .child_resources .iter() @@ -209,7 +275,7 @@ fn generate_child_selectors(config: &Config) -> TokenStream { 'b: 'c, { #child_resource_types { - key: Key::Name(self, name), + key: #child_resource_key_types::Name(self, name), } } )* @@ -222,6 +288,13 @@ fn generate_misc_helpers(config: &Config) -> TokenStream { let resource_name = &config.resource.name; let parent_resource_name = config.parent.as_ref().map(|p| &p.name).unwrap_or(&fleet_name); + let lookup_enum = &config.lookup_enum; + + let name_variant = if config.lookup_by_name { + quote! { #lookup_enum::Name(parent, _) => parent.lookup_root(), } + } else { + quote! {} + }; quote! { /// Build the `authz` object for this resource @@ -244,8 +317,9 @@ fn generate_misc_helpers(config: &Config) -> TokenStream { /// used for this lookup. fn lookup_root(&self) -> &LookupPath<'a> { match &self.key { - Key::Name(parent, _) => parent.lookup_root(), - Key::Id(root, _) => root.lookup_root(), + #name_variant + + #lookup_enum::PrimaryKey(root, ..) => root.lookup_root(), } } } @@ -258,6 +332,13 @@ fn generate_lookup_methods(config: &Config) -> TokenStream { let path_authz_names = &config.path_authz_names; let resource_name = &config.resource.name; let resource_authz_name = &config.resource.authz_name; + let lookup_enum = &config.lookup_enum; + let pkey_names: Vec<_> = config + .primary_key_columns + .iter() + .enumerate() + .map(|(i, _)| format_ident!("v{}", i)) + .collect(); let (ancestors_authz_names_assign, parent_lookup_arg_actual) = if let Some(p) = &config.parent { let nancestors = config.path_authz_names.len() - 1; @@ -273,6 +354,53 @@ fn generate_lookup_methods(config: &Config) -> TokenStream { (quote! {}, quote! {}) }; + // Generate the by-name branch of the match arm in "fetch_for()" + let fetch_for_name_variant = if config.lookup_by_name { + quote! { + #lookup_enum::Name(parent, name) => { + #ancestors_authz_names_assign + let (#resource_authz_name, db_row) = Self::fetch_by_name_for( + opctx, + datastore, + #parent_lookup_arg_actual + *name, + action, + ).await?; + Ok((#(#path_authz_names,)* db_row)) + } + } + } else { + quote! {} + }; + + // Generate the by-name branch of the match arm in "lookup()" + let lookup_name_variant = if config.lookup_by_name { + quote! { + #lookup_enum::Name(parent, name) => { + // When doing a by-name lookup, we have to look up the + // parent first. Since this is recursive, we wind up + // hitting the database once for each item in the path, + // in order descending from the root of the tree. (So + // we'll look up Organization, then Project, then + // Instance, etc.) + // TODO-performance Instead of doing database queries at + // each level of recursion, we could be building up one + // big "join" query and hit the database just once. + #ancestors_authz_names_assign + let (#resource_authz_name, _) = + Self::lookup_by_name_no_authz( + opctx, + datastore, + #parent_lookup_arg_actual + *name + ).await?; + Ok((#(#path_authz_names,)*)) + } + } + } else { + quote! {} + }; + quote! { /// Fetch the record corresponding to the selected resource /// @@ -301,23 +429,14 @@ fn generate_lookup_methods(config: &Config) -> TokenStream { let datastore = &lookup.datastore; match &self.key { - Key::Name(parent, name) => { - #ancestors_authz_names_assign - let (#resource_authz_name, db_row) = Self::fetch_by_name_for( - opctx, - datastore, - #parent_lookup_arg_actual - *name, - action, - ).await?; - Ok((#(#path_authz_names,)* db_row)) - } - Key::Id(_, id) => { + #fetch_for_name_variant + + #lookup_enum::PrimaryKey(_, #(#pkey_names,)*) => { Self::fetch_by_id_for( opctx, datastore, - *id, - action, + #(#pkey_names,)* + action ).await } } @@ -359,27 +478,9 @@ fn generate_lookup_methods(config: &Config) -> TokenStream { let datastore = &lookup.datastore; match &self.key { - Key::Name(parent, name) => { - // When doing a by-name lookup, we have to look up the - // parent first. Since this is recursive, we wind up - // hitting the database once for each item in the path, - // in order descending from the root of the tree. (So - // we'll look up Organization, then Project, then - // Instance, etc.) - // TODO-performance Instead of doing database queries at - // each level of recursion, we could be building up one - // big "join" query and hit the database just once. - #ancestors_authz_names_assign - let (#resource_authz_name, _) = - Self::lookup_by_name_no_authz( - opctx, - datastore, - #parent_lookup_arg_actual - *name - ).await?; - Ok((#(#path_authz_names,)*)) - } - Key::Id(_, id) => { + #lookup_name_variant + + #lookup_enum::PrimaryKey(_, #(#pkey_names,)*) => { // When doing a by-id lookup, we start directly with the // resource we're looking up. But we still want to // return a full path of authz objects. So we look up @@ -395,7 +496,7 @@ fn generate_lookup_methods(config: &Config) -> TokenStream { Self::lookup_by_id_no_authz( opctx, datastore, - *id + #(#pkey_names,)* ).await?; Ok((#(#path_authz_names,)*)) } @@ -415,6 +516,21 @@ fn generate_database_functions(config: &Config) -> TokenStream { let resource_as_snake = format_ident!("{}", &config.resource.name_as_snake); let path_types = &config.path_types; let path_authz_names = &config.path_authz_names; + let pkey_types: Vec<_> = config + .primary_key_columns + .iter() + .map(|c| c.rust_type.deref()) + .collect(); + let pkey_column_names = config + .primary_key_columns + .iter() + .map(|c| format_ident!("{}", c.column_name)); + let pkey_names: Vec<_> = config + .primary_key_columns + .iter() + .enumerate() + .map(|(i, _)| format_ident!("v{}", i)) + .collect(); let ( parent_lookup_arg_formal, parent_lookup_arg_actual, @@ -433,7 +549,7 @@ fn generate_database_functions(config: &Config) -> TokenStream { quote! { let (#(#ancestors_authz_names,)* _) = #parent_resource_name::lookup_by_id_no_authz( - opctx, datastore, db_row.#parent_id + opctx, datastore, &db_row.#parent_id ).await?; }, quote! { .filter(dsl::#parent_id.eq(#parent_authz_name.id())) }, @@ -443,71 +559,107 @@ fn generate_database_functions(config: &Config) -> TokenStream { (quote! {}, quote! {}, quote! {}, quote! {}, quote! { &authz::FLEET }) }; - quote! { - /// Fetch the database row for a resource by doing a lookup by name, - /// possibly within a collection - /// - /// This function checks whether the caller has permissions to read - /// the requested data. However, it's not intended to be used - /// outside this module. See `fetch_for(authz::Action)`. - // Do NOT make this function public. - async fn fetch_by_name_for( - opctx: &OpContext, - datastore: &DataStore, - #parent_lookup_arg_formal - name: &Name, - action: authz::Action, - ) -> LookupResult<(authz::#resource_name, model::#resource_name)> { - let (#resource_authz_name, db_row) = Self::lookup_by_name_no_authz( - opctx, - datastore, - #parent_lookup_arg_actual - name - ).await?; - opctx.authorize(action, &#resource_authz_name).await?; - Ok((#resource_authz_name, db_row)) - } + let soft_delete_filter = if config.soft_deletes { + quote! { .filter(dsl::time_deleted.is_null()) } + } else { + quote! {} + }; - /// Lowest-level function for looking up a resource in the database - /// by name, possibly within a collection - /// - /// This function does not check whether the caller has permission - /// to read this information. That's why it's not `pub`. Outside - /// this module, you want `fetch()` or `lookup_for(authz::Action)`. - // Do NOT make this function public. - async fn lookup_by_name_no_authz( - opctx: &OpContext, - datastore: &DataStore, - #parent_lookup_arg_formal - name: &Name, - ) -> LookupResult<(authz::#resource_name, model::#resource_name)> { - use db::schema::#resource_as_snake::dsl; + let by_name_funcs = if config.lookup_by_name { + quote! { + /// Fetch the database row for a resource by doing a lookup by + /// name, possibly within a collection + /// + /// This function checks whether the caller has permissions to + /// read the requested data. However, it's not intended to be + /// used outside this module. See `fetch_for(authz::Action)`. + // Do NOT make this function public. + async fn fetch_by_name_for( + opctx: &OpContext, + datastore: &DataStore, + #parent_lookup_arg_formal + name: &Name, + action: authz::Action, + ) -> LookupResult<(authz::#resource_name, model::#resource_name)> { + let (#resource_authz_name, db_row) = + Self::lookup_by_name_no_authz( + opctx, + datastore, + #parent_lookup_arg_actual + name + ).await?; + opctx.authorize(action, &#resource_authz_name).await?; + Ok((#resource_authz_name, db_row)) + } - dsl::#resource_as_snake - .filter(dsl::time_deleted.is_null()) - .filter(dsl::name.eq(name.clone())) - #lookup_filter - .select(model::#resource_name::as_select()) - .get_result_async(datastore.pool_authorized(opctx).await?) - .await - .map_err(|e| { - public_error_from_diesel_pool( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::#resource_name, - LookupType::ByName(name.as_str().to_string()) - ) + /// Lowest-level function for looking up a resource in the + /// database by name, possibly within a collection + /// + /// This function does not check whether the caller has + /// permission to read this information. That's why it's not + /// `pub`. Outside this module, you want `fetch()` or + /// `lookup_for(authz::Action)`. + // Do NOT make this function public. + async fn lookup_by_name_no_authz( + opctx: &OpContext, + datastore: &DataStore, + #parent_lookup_arg_formal + name: &Name, + ) -> LookupResult< + (authz::#resource_name, model::#resource_name) + > { + use db::schema::#resource_as_snake::dsl; + + dsl::#resource_as_snake + #soft_delete_filter + .filter(dsl::name.eq(name.clone())) + #lookup_filter + .select(model::#resource_name::as_select()) + .get_result_async( + datastore.pool_authorized(opctx).await? ) - }) - .map(|db_row| {( - Self::make_authz( - #parent_authz_value, - &db_row, - LookupType::ByName(name.as_str().to_string()) - ), - db_row - )}) + .await + .map_err(|e| { + public_error_from_diesel_pool( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::#resource_name, + LookupType::ByName( + name.as_str().to_string() + ) + ) + ) + }) + .map(|db_row| {( + Self::make_authz( + #parent_authz_value, + &db_row, + LookupType::ByName(name.as_str().to_string()) + ), + db_row + )}) + } } + } else { + quote! {} + }; + + let lookup_type = if config.primary_key_columns.len() == 1 + && config.primary_key_columns[0].column_name == "id" + { + quote! { LookupType::ById(#(#pkey_names.clone())*) } + } else { + let fmtstr = config + .primary_key_columns + .iter() + .map(|c| format!("{} = {{:?}}", c.column_name)) + .collect::>() + .join(", "); + quote! { LookupType::ByCompositeId(format!(#fmtstr, #(#pkey_names,)*)) } + }; + + quote! { + #by_name_funcs /// Fetch the database row for a resource by doing a lookup by id /// @@ -518,14 +670,14 @@ fn generate_database_functions(config: &Config) -> TokenStream { async fn fetch_by_id_for( opctx: &OpContext, datastore: &DataStore, - id: Uuid, + #(#pkey_names: &#pkey_types,)* action: authz::Action, ) -> LookupResult<(#(authz::#path_types,)* model::#resource_name)> { let (#(#path_authz_names,)* db_row) = Self::lookup_by_id_no_authz( opctx, datastore, - id + #(#pkey_names,)* ).await?; opctx.authorize(action, &#resource_authz_name).await?; Ok((#(#path_authz_names,)* db_row)) @@ -541,13 +693,13 @@ fn generate_database_functions(config: &Config) -> TokenStream { async fn lookup_by_id_no_authz( opctx: &OpContext, datastore: &DataStore, - id: Uuid, + #(#pkey_names: &#pkey_types,)* ) -> LookupResult<(#(authz::#path_types,)* model::#resource_name)> { use db::schema::#resource_as_snake::dsl; let db_row = dsl::#resource_as_snake - .filter(dsl::time_deleted.is_null()) - .filter(dsl::id.eq(id)) + #soft_delete_filter + #(.filter(dsl::#pkey_column_names.eq(#pkey_names.clone())))* .select(model::#resource_name::as_select()) .get_result_async(datastore.pool_authorized(opctx).await?) .await @@ -556,7 +708,7 @@ fn generate_database_functions(config: &Config) -> TokenStream { e, ErrorHandler::NotFoundByLookup( ResourceType::#resource_name, - LookupType::ById(id) + #lookup_type ) ) })?; @@ -564,7 +716,7 @@ fn generate_database_functions(config: &Config) -> TokenStream { let #resource_authz_name = Self::make_authz( &#parent_authz_value, &db_row, - LookupType::ById(id) + #lookup_type ); Ok((#(#path_authz_names,)* db_row)) } @@ -579,27 +731,72 @@ fn generate_database_functions(config: &Config) -> TokenStream { // lookup.rs, replacing the call to the macro, then reformat the file, and then // build it in order to see the compiler error in context. #[cfg(test)] -#[test] -fn test_lookup_dump() { - let output = lookup_resource( - quote! { - name = "Organization", - ancestors = [], - children = [ "Project" ], - } - .into(), - ) - .unwrap(); - println!("{}", output); +mod test { + use super::lookup_resource; + use quote::quote; + use rustfmt_wrapper::rustfmt; + + #[test] + #[ignore] + fn test_lookup_dump() { + let output = lookup_resource( + quote! { + name = "Organization", + ancestors = [], + children = [ "Project" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] + } + .into(), + ) + .unwrap(); + println!("{}", rustfmt(output).unwrap()); - let output = lookup_resource( - quote! { - name = "Project", - ancestors = ["Organization"], - children = [ "Disk", "Instance" ], - } - .into(), - ) - .unwrap(); - println!("{}", output); + let output = lookup_resource( + quote! { + name = "Project", + ancestors = ["Organization"], + children = [ "Disk", "Instance" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] + } + .into(), + ) + .unwrap(); + println!("{}", rustfmt(output).unwrap()); + + let output = lookup_resource( + quote! { + name = "SiloUser", + ancestors = [], + children = [], + lookup_by_name = false, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] + } + .into(), + ) + .unwrap(); + println!("{}", rustfmt(output).unwrap()); + + let output = lookup_resource( + quote! { + name = "UpdateAvailableArtifact", + ancestors = [], + children = [], + lookup_by_name = false, + soft_deletes = false, + primary_key_columns = [ + { column_name = "name", rust_type = String }, + { column_name = "version", rust_type = i64 }, + { column_name = "kind", rust_type = UpdateArtifactKind } + ] + } + .into(), + ) + .unwrap(); + println!("{}", rustfmt(output).unwrap()); + } } diff --git a/nexus/src/db/lookup.rs b/nexus/src/db/lookup.rs index 25d5748f9b..23f936f3c0 100644 --- a/nexus/src/db/lookup.rs +++ b/nexus/src/db/lookup.rs @@ -5,6 +5,7 @@ //! Look up API resources from the database use super::datastore::DataStore; +use super::identity::Asset; use super::identity::Resource; use super::model; use crate::{ @@ -13,6 +14,7 @@ use crate::{ db, db::error::{public_error_from_diesel_pool, ErrorHandler}, db::model::Name, + db::model::UpdateArtifactKind, }; use async_bb8_diesel::AsyncRunQueryDsl; use db_macros::lookup_resource; @@ -182,60 +184,78 @@ impl<'a> LookupPath<'a> { 'a: 'c, 'b: 'c, { - Organization { key: Key::Name(Root { lookup_root: self }, name) } + Organization { + key: OrganizationKey::Name(Root { lookup_root: self }, name), + } } /// Select a resource of type Organization, identified by its id pub fn organization_id(self, id: Uuid) -> Organization<'a> { - Organization { key: Key::Id(Root { lookup_root: self }, id) } + Organization { + key: OrganizationKey::PrimaryKey(Root { lookup_root: self }, id), + } } /// Select a resource of type Project, identified by its id pub fn project_id(self, id: Uuid) -> Project<'a> { - Project { key: Key::Id(Root { lookup_root: self }, id) } + Project { key: ProjectKey::PrimaryKey(Root { lookup_root: self }, id) } } /// Select a resource of type Instance, identified by its id pub fn instance_id(self, id: Uuid) -> Instance<'a> { - Instance { key: Key::Id(Root { lookup_root: self }, id) } + Instance { + key: InstanceKey::PrimaryKey(Root { lookup_root: self }, id), + } } /// Select a resource of type Disk, identified by its id pub fn disk_id(self, id: Uuid) -> Disk<'a> { - Disk { key: Key::Id(Root { lookup_root: self }, id) } + Disk { key: DiskKey::PrimaryKey(Root { lookup_root: self }, id) } } /// Select a resource of type Vpc, identified by its id pub fn vpc_id(self, id: Uuid) -> Vpc<'a> { - Vpc { key: Key::Id(Root { lookup_root: self }, id) } + Vpc { key: VpcKey::PrimaryKey(Root { lookup_root: self }, id) } } /// Select a resource of type VpcSubnet, identified by its id pub fn vpc_subnet_id(self, id: Uuid) -> VpcSubnet<'a> { - VpcSubnet { key: Key::Id(Root { lookup_root: self }, id) } + VpcSubnet { + key: VpcSubnetKey::PrimaryKey(Root { lookup_root: self }, id), + } } /// Select a resource of type VpcRouter, identified by its id pub fn vpc_router_id(self, id: Uuid) -> VpcRouter<'a> { - VpcRouter { key: Key::Id(Root { lookup_root: self }, id) } + VpcRouter { + key: VpcRouterKey::PrimaryKey(Root { lookup_root: self }, id), + } } /// Select a resource of type RouterRoute, identified by its id pub fn router_route_id(self, id: Uuid) -> RouterRoute<'a> { - RouterRoute { key: Key::Id(Root { lookup_root: self }, id) } + RouterRoute { + key: RouterRouteKey::PrimaryKey(Root { lookup_root: self }, id), + } } -} - -/// Describes a node along the selection path of a resource -enum Key<'a, P> { - /// We're looking for a resource with the given name within the given parent - /// collection - Name(P, &'a Name), - /// We're looking for a resource with the given id - /// - /// This has no parent container -- a by-id lookup is always global. - Id(Root<'a>, Uuid), + /// Select a resource of type UpdateAvailableArtifact, identified by its + /// `(name, version, kind)` tuple + pub fn update_available_artifact_tuple( + self, + name: &str, + version: i64, + kind: UpdateArtifactKind, + ) -> UpdateAvailableArtifact<'a> { + UpdateAvailableArtifact { + key: UpdateAvailableArtifactKey::PrimaryKey( + Root { lookup_root: self }, + name.to_string(), + version, + kind, + ), + } + } } /// Represents the head of the selection path for a resource @@ -258,60 +278,108 @@ lookup_resource! { name = "Organization", ancestors = [], children = [ "Project" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "Project", ancestors = [ "Organization" ], children = [ "Disk", "Instance", "Vpc" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "Instance", ancestors = [ "Organization", "Project" ], children = [ "NetworkInterface" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "NetworkInterface", ancestors = [ "Organization", "Project", "Instance" ], children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "Disk", ancestors = [ "Organization", "Project" ], children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "Vpc", ancestors = [ "Organization", "Project" ], children = [ "VpcRouter", "VpcSubnet" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "VpcSubnet", ancestors = [ "Organization", "Project", "Vpc" ], children = [ ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "VpcRouter", ancestors = [ "Organization", "Project", "Vpc" ], children = [ "RouterRoute" ], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } lookup_resource! { name = "RouterRoute", ancestors = [ "Organization", "Project", "Vpc", "VpcRouter" ], children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} + +lookup_resource! { + name = "SiloUser", + ancestors = [], + children = [], + lookup_by_name = false, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} + +lookup_resource! { + name = "UpdateAvailableArtifact", + ancestors = [], + children = [], + lookup_by_name = false, + soft_deletes = false, + primary_key_columns = [ + { column_name = "name", rust_type = String }, + { column_name = "version", rust_type = i64 }, + { column_name = "kind", rust_type = UpdateArtifactKind } + ] } #[cfg(test)] mod test { use super::Instance; - use super::Key; use super::LookupPath; use super::Organization; use super::Project; @@ -340,9 +408,9 @@ mod test { .instance_name(&instance_name); assert!(matches!(&leaf, Instance { - key: Key::Name(Project { - key: Key::Name(Organization { - key: Key::Name(_, o) + key: super::InstanceKey::Name(Project { + key: super::ProjectKey::Name(Organization { + key: super::OrganizationKey::Name(_, o) }, p) }, i) } @@ -353,8 +421,8 @@ mod test { .organization_id(org_id) .project_name(&project_name); assert!(matches!(&leaf, Project { - key: Key::Name(Organization { - key: Key::Id(_, o) + key: super::ProjectKey::Name(Organization { + key: super::OrganizationKey::PrimaryKey(_, o) }, p) } if *o == org_id && **p == project_name)); diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index 676569f465..70ef84a685 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -989,26 +989,20 @@ impl DatastoreCollection for Silo { } /// Describes a silo user within the database. -#[derive(Queryable, Insertable, Debug, Selectable)] +#[derive(Asset, Queryable, Insertable, Debug, Selectable)] #[table_name = "silo_user"] pub struct SiloUser { - pub id: Uuid, + #[diesel(embed)] + identity: SiloUserIdentity, pub silo_id: Uuid, - - pub time_created: DateTime, - pub time_modified: DateTime, pub time_deleted: Option>, } impl SiloUser { pub fn new(silo_id: Uuid, user_id: Uuid) -> Self { - let now = Utc::now(); Self { - id: user_id, + identity: SiloUserIdentity::new(user_id), silo_id, - - time_created: now, - time_modified: now, time_deleted: None, } } @@ -2553,7 +2547,7 @@ impl_enum_wrapper!( #[postgres(type_name = "update_artifact_kind", type_schema = "public")] pub struct UpdateArtifactKindEnum; - #[derive(Clone, Debug, Display, AsExpression, FromSqlRow)] + #[derive(Clone, Copy, Debug, Display, AsExpression, FromSqlRow, PartialEq, Eq)] #[display("{0}")] #[sql_type = "UpdateArtifactKindEnum"] pub struct UpdateArtifactKind(pub internal::nexus::UpdateArtifactKind); @@ -2583,6 +2577,12 @@ pub struct UpdateAvailableArtifact { pub target_length: i64, } +impl UpdateAvailableArtifact { + pub fn id(&self) -> (String, i64, UpdateArtifactKind) { + (self.name.clone(), self.version, self.kind) + } +} + #[cfg(test)] mod tests { use super::Uuid; diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index a3e3cfc4dd..0f8740c3da 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -15,6 +15,7 @@ use crate::db::model::DatasetKind; use crate::db::model::Name; use crate::db::model::RouterRoute; use crate::db::model::SiloUser; +use crate::db::model::UpdateArtifactKind; use crate::db::model::VpcRouter; use crate::db::model::VpcRouterKind; use crate::db::model::VpcSubnet; @@ -3428,9 +3429,13 @@ impl Nexus { // We cache the artifact based on its checksum, so fetch that from the // database. - let artifact_entry = self - .db_datastore - .update_available_artifact_fetch(opctx, &artifact) + let (.., artifact_entry) = LookupPath::new(opctx, &self.db_datastore) + .update_available_artifact_tuple( + &artifact.name, + artifact.version, + UpdateArtifactKind(artifact.kind), + ) + .fetch() .await?; let filename = format!( "{}.{}.{}-{}", diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index 9dba23461d..0812b0485b 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -16,6 +16,7 @@ use nexus_test_utils::resource_helpers::{ use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; +use omicron_nexus::db::identity::Asset; #[nexus_test] async fn test_silos(cptestctx: &ControlPlaneTestContext) { @@ -74,7 +75,7 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { .await .unwrap(); - let session = nexus.session_create(new_silo_user.id).await.unwrap(); + let session = nexus.session_create(new_silo_user.id()).await.unwrap(); // Create organization with built-in user auth // Note: this currently goes to the built-in silo! @@ -130,7 +131,7 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { // Verify silo user was also deleted nexus - .silo_user_fetch(new_silo_user.id) + .silo_user_fetch(new_silo_user.id()) .await .expect_err("unexpected success");