From f3c30a850570e7a75d6ab2a3160f2bd8dac6a4e6 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 5 Apr 2023 11:51:18 -0400 Subject: [PATCH 01/27] Add image views --- common/src/sql/dbinit.sql | 59 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 4f946fd46c..9577b60fc7 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -838,6 +838,12 @@ CREATE INDEX ON omicron.public.disk ( ) WHERE time_deleted IS NULL AND attach_instance_id IS NOT NULL; +/* The kind of the image */ +CREATE TYPE omicron.public.image_kind AS ENUM ( + 'project', + 'silo' +); + CREATE TABLE omicron.public.image ( /* Identity metadata (resource) */ id UUID PRIMARY KEY, @@ -848,7 +854,11 @@ CREATE TABLE omicron.public.image ( /* Indicates that the object has been deleted */ time_deleted TIMESTAMPTZ, - project_id UUID NOT NULL, + kind omicron.public.image_kind NOT NULL, + + /* Reference to the parent resource whose type is determined by kind */ + parent_id UUID NOT NULL, + volume_id UUID NOT NULL, url STRING(8192), @@ -859,9 +869,52 @@ CREATE TABLE omicron.public.image ( size_bytes INT NOT NULL ); +CREATE VIEW omicron.public.project_image AS +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + parent_id AS project_id, + volume_id, + url, + os, + version, + digest, + block_size, + size_bytes +FROM + omicron.public.image +WHERE + kind = 'project'; + +CREATE VIEW omicron.public.silo_image AS +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + parent_id AS silo_id, + volume_id, + url, + os, + version, + digest, + block_size, + size_bytes +FROM + omicron.public.image +WHERE + kind = 'silo'; + CREATE UNIQUE INDEX on omicron.public.image ( - project_id, - name + parent_id, + name, + kind ) WHERE time_deleted is NULL; From fa120f87dbc4840072b771d02c2449ddf1a37c38 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 5 Apr 2023 17:19:33 -0400 Subject: [PATCH 02/27] WIP impl project/silo images --- common/src/api/external/mod.rs | 2 + nexus/authz-macros/src/lib.rs | 28 +++ nexus/db-model/src/image.rs | 191 +++++++++++++++++++- nexus/db-model/src/project.rs | 5 +- nexus/db-model/src/schema.rs | 42 +++++ nexus/db-model/src/silo.rs | 11 +- nexus/db-queries/src/authz/api_resources.rs | 10 +- nexus/db-queries/src/db/datastore/image.rs | 85 ++++++++- nexus/db-queries/src/db/lookup.rs | 18 +- nexus/types/src/external_api/views.rs | 12 +- 10 files changed, 385 insertions(+), 19 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index d0817761a0..6725297845 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -696,6 +696,8 @@ pub enum ResourceType { Dataset, Disk, Image, + SiloImage, + ProjectImage, Instance, IpPool, NetworkInterface, diff --git a/nexus/authz-macros/src/lib.rs b/nexus/authz-macros/src/lib.rs index ed6af12893..f8d071be0b 100644 --- a/nexus/authz-macros/src/lib.rs +++ b/nexus/authz-macros/src/lib.rs @@ -136,6 +136,9 @@ enum PolarSnippet { /// Generate it as a global resource, manipulable only to administrators FleetChild, + /// Generate it as resource nested under the Silo + InSilo, + /// Generate it as a resource nested within a Project (either directly or /// indirectly) InProject, @@ -200,6 +203,31 @@ fn do_authz_resource( resource_name, resource_name, ), + // If this resource is directly inside a Silo, we only need to define + // permissions that are contingent on having roles on that Silo. + (PolarSnippet::InSilo, _) => format!( + r#" + resource {} {{ + permissions = [ + "list_children", + "modify", + "read", + "create_child", + ]; + + relations = {{ containing_silo: Silo }}; + "list_children" if "viewer" on "containing_silo"; + "read" if "viewer" on "containing_silo"; + "modify" if "collaborator" on "containing_silo"; + "create_child" if "collaborator" on "containing_silo"; + }} + + has_relation(parent: Silo, "containing_silo", child: {}) + if child.silo = parent; + "#, + resource_name, resource_name, + ), + // If this resource is directly inside a Project, we only need to define // permissions that are contingent on having roles on that Project. (PolarSnippet::InProject, "Project") => format!( diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index e66f1adf6f..f010f706c0 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -3,14 +3,29 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{BlockSize, ByteCount, Digest}; -use crate::schema::image; +use crate::impl_enum_type; +use crate::schema::{image, project_image, silo_image}; use db_macros::Resource; use nexus_types::external_api::views; use nexus_types::identity::Resource; +use omicron_common::api::external::Error; use serde::{Deserialize, Serialize}; use uuid::Uuid; -// Project images +impl_enum_type! { + #[derive(SqlType, QueryId, Debug, Clone, Copy)] + #[diesel(postgres_type(name = "image_kind"))] + pub struct ImageKindEnum; + + #[derive(Clone, Copy, Serialize, Deserialize, Debug, AsExpression, FromSqlRow, PartialEq)] + #[diesel(sql_type = ImageKindEnum)] + pub enum ImageKind; + + Silo => b"silo" + Project => b"project" +} + +// Shared image definition #[derive( Queryable, Insertable, @@ -26,6 +41,34 @@ pub struct Image { #[diesel(embed)] pub identity: ImageIdentity, + pub kind: ImageKind, + pub parent_id: Uuid, + pub volume_id: Uuid, + pub url: Option, + pub os: String, + pub version: String, + pub digest: Option, + pub block_size: BlockSize, + + #[diesel(column_name = size_bytes)] + pub size: ByteCount, +} + +#[derive( + Queryable, + Insertable, + Selectable, + Clone, + Debug, + Resource, + Serialize, + Deserialize, +)] +#[diesel(table_name = project_image)] +pub struct ProjectImage { + #[diesel(embed)] + pub identity: ProjectImageIdentity, + pub project_id: Uuid, pub volume_id: Uuid, pub url: Option, @@ -38,11 +81,153 @@ pub struct Image { pub size: ByteCount, } +#[derive( + Queryable, + Insertable, + Selectable, + Clone, + Debug, + Resource, + Serialize, + Deserialize, +)] +#[diesel(table_name = silo_image)] +pub struct SiloImage { + #[diesel(embed)] + pub identity: SiloImageIdentity, + + pub silo_id: Uuid, + pub volume_id: Uuid, + pub url: Option, + pub os: String, + pub version: String, + pub digest: Option, + pub block_size: BlockSize, + + #[diesel(column_name = size_bytes)] + pub size: ByteCount, +} + +impl TryFrom for ProjectImage { + type Error = Error; + + fn try_from(image: Image) -> Result { + match image.kind { + ImageKind::Project => Ok(Self { + identity: ProjectImageIdentity { + id: image.id(), + name: image.name().clone().into(), + description: image.description().to_string(), + time_created: image.time_created(), + time_modified: image.time_modified(), + time_deleted: image.time_deleted(), + }, + project_id: image.parent_id, + volume_id: image.volume_id, + url: image.url, + os: image.os, + version: image.version, + digest: image.digest, + block_size: image.block_size, + size: image.size, + }), + _ => Err(Error::internal_error( + "tried to convert non-project image to project image", + )), + } + } +} + +impl TryFrom for SiloImage { + type Error = Error; + + fn try_from(image: Image) -> Result { + match image.kind { + ImageKind::Silo => Ok(Self { + identity: SiloImageIdentity { + id: image.id(), + name: image.name().clone().into(), + description: image.description().to_string(), + time_created: image.time_created(), + time_modified: image.time_modified(), + time_deleted: image.time_deleted(), + }, + silo_id: image.parent_id, + volume_id: image.volume_id, + url: image.url, + os: image.os, + version: image.version, + digest: image.digest, + block_size: image.block_size, + size: image.size, + }), + _ => Err(Error::internal_error( + "tried to convert non-silo image to silo image", + )), + } + } +} + +impl From for Image { + fn from(image: ProjectImage) -> Self { + Self { + identity: ImageIdentity { + id: image.id(), + name: image.name().clone().into(), + description: image.description().to_string(), + time_created: image.time_created(), + time_modified: image.time_modified(), + time_deleted: image.time_deleted(), + }, + kind: ImageKind::Project, + parent_id: image.project_id, + volume_id: image.volume_id, + url: image.url, + os: image.os, + version: image.version, + digest: image.digest, + block_size: image.block_size, + size: image.size, + } + } +} + +impl From for Image { + fn from(image: SiloImage) -> Self { + Self { + identity: ImageIdentity { + id: image.id(), + name: image.name().clone().into(), + description: image.description().to_string(), + time_created: image.time_created(), + time_modified: image.time_modified(), + time_deleted: image.time_deleted(), + }, + kind: ImageKind::Silo, + parent_id: image.silo_id, + volume_id: image.volume_id, + url: image.url, + os: image.os, + version: image.version, + digest: image.digest, + block_size: image.block_size, + size: image.size, + } + } +} + impl From for views::Image { fn from(image: Image) -> Self { Self { identity: image.identity(), - project_id: image.project_id, + parent_id: match image.kind { + ImageKind::Project => { + views::SiloOrProjectId::ProjectId(image.parent_id) + } + ImageKind::Silo => { + views::SiloOrProjectId::SiloId(image.parent_id) + } + }, url: image.url, os: image.os, version: image.version, diff --git a/nexus/db-model/src/project.rs b/nexus/db-model/src/project.rs index a5f968a89b..62ea594fa5 100644 --- a/nexus/db-model/src/project.rs +++ b/nexus/db-model/src/project.rs @@ -2,9 +2,10 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use super::{Disk, Generation, Image, Instance, Name, Snapshot, Vpc}; +use super::{Disk, Generation, Instance, Name, Snapshot, Vpc}; use crate::collection::DatastoreCollectionConfig; use crate::schema::{disk, image, instance, project, snapshot, vpc}; +use crate::Image; use chrono::{DateTime, Utc}; use db_macros::Resource; use nexus_types::external_api::params; @@ -63,7 +64,7 @@ impl DatastoreCollectionConfig for Project { type CollectionId = Uuid; type GenerationNumberColumn = project::dsl::rcgen; type CollectionTimeDeletedColumn = project::dsl::time_deleted; - type CollectionIdColumn = image::dsl::project_id; + type CollectionIdColumn = image::dsl::parent_id; } impl DatastoreCollectionConfig for Project { diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index d6448f3376..92f374f4a3 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -31,6 +31,26 @@ table! { table! { image (id) { + id -> Uuid, + name -> Text, + description -> Text, + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + kind -> crate::ImageKindEnum, + parent_id -> Uuid, + volume_id -> Uuid, + url -> Nullable, + os -> Text, + version -> Text, + digest -> Nullable, + block_size -> crate::BlockSizeEnum, + size_bytes -> Int8, + } +} + +table! { + project_image (id) { id -> Uuid, name -> Text, description -> Text, @@ -48,6 +68,25 @@ table! { } } +table! { + silo_image (id) { + id -> Uuid, + name -> Text, + description -> Text, + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + silo_id -> Uuid, + volume_id -> Uuid, + url -> Nullable, + os -> Text, + version -> Text, + digest -> Nullable, + block_size -> crate::BlockSizeEnum, + size_bytes -> Int8, + } +} + table! { global_image (id) { id -> Uuid, @@ -799,6 +838,9 @@ joinable!(ip_pool_range -> ip_pool (ip_pool_id)); allow_tables_to_appear_in_same_query!( dataset, disk, + image, + project_image, + silo_image, instance, metric_producer, network_interface, diff --git a/nexus/db-model/src/silo.rs b/nexus/db-model/src/silo.rs index 14fb5376c9..0d18a35d74 100644 --- a/nexus/db-model/src/silo.rs +++ b/nexus/db-model/src/silo.rs @@ -4,8 +4,8 @@ use super::{Generation, Project}; use crate::collection::DatastoreCollectionConfig; -use crate::impl_enum_type; -use crate::schema::{project, silo}; +use crate::schema::{image, project, silo, silo_image}; +use crate::{impl_enum_type, Image, SiloImage}; use db_macros::Resource; use nexus_types::external_api::shared::SiloIdentityMode; use nexus_types::external_api::views; @@ -154,3 +154,10 @@ impl DatastoreCollectionConfig for Silo { type CollectionTimeDeletedColumn = silo::dsl::time_deleted; type CollectionIdColumn = project::dsl::silo_id; } + +impl DatastoreCollectionConfig for Silo { + type CollectionId = Uuid; + type GenerationNumberColumn = silo::dsl::rcgen; + type CollectionTimeDeletedColumn = silo::dsl::time_deleted; + type CollectionIdColumn = image::dsl::parent_id; +} diff --git a/nexus/db-queries/src/authz/api_resources.rs b/nexus/db-queries/src/authz/api_resources.rs index 23dec07960..b48f994eff 100644 --- a/nexus/db-queries/src/authz/api_resources.rs +++ b/nexus/db-queries/src/authz/api_resources.rs @@ -684,7 +684,7 @@ authz_resource! { } authz_resource! { - name = "Image", + name = "ProjectImage", parent = "Project", primary_key = Uuid, roles_allowed = false, @@ -867,6 +867,14 @@ authz_resource! { polar_snippet = Custom, } +authz_resource! { + name = "SiloImage", + parent = "Silo", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = InSilo, +} + authz_resource! { name = "IdentityProvider", parent = "Silo", diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index aede319649..6da6924eed 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -1,4 +1,5 @@ use diesel::prelude::*; +use nexus_db_model::ImageKind; use nexus_db_model::Name; use nexus_types::identity::Resource; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -17,6 +18,9 @@ use crate::db::error::public_error_from_diesel_pool; use crate::db::error::ErrorHandler; use crate::db::model::Image; use crate::db::model::Project; +use crate::db::model::ProjectImage; +use crate::db::model::Silo; +use crate::db::model::SiloImage; use crate::db::pagination::paginated; use async_bb8_diesel::AsyncRunQueryDsl; @@ -24,7 +28,7 @@ use async_bb8_diesel::AsyncRunQueryDsl; use super::DataStore; impl DataStore { - pub async fn image_list( + pub async fn project_image_list( &self, opctx: &OpContext, authz_project: &authz::Project, @@ -44,23 +48,89 @@ impl DataStore { ), } .filter(dsl::time_deleted.is_null()) - .filter(dsl::project_id.eq(authz_project.id())) + .filter(dsl::kind.eq(ImageKind::Project)) + .filter(dsl::parent_id.eq(authz_project.id())) .select(Image::as_select()) .load_async::(self.pool_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } - pub async fn image_create( + pub async fn silo_image_list( + &self, + opctx: &OpContext, + authz_silo: &authz::Silo, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, authz_silo).await?; + + use db::schema::image::dsl; + match pagparams { + PaginatedBy::Id(pagparams) => { + paginated(dsl::image, dsl::id, &pagparams) + } + PaginatedBy::Name(pagparams) => paginated( + dsl::image, + dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), + } + .filter(dsl::time_deleted.is_null()) + .filter(dsl::parent_id.eq(authz_silo.id())) + .select(Image::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + + pub async fn silo_image_create( + &self, + opctx: &OpContext, + authz_silo: &authz::Silo, + silo_image: SiloImage, + ) -> CreateResult { + let image: Image = silo_image.into(); + opctx.authorize(authz::Action::CreateChild, authz_silo).await?; + + let name = image.name().clone(); + let silo_id = image.parent_id; + + use db::schema::image::dsl; + let image: Image = Silo::insert_resource( + silo_id, + diesel::insert_into(dsl::image) + .values(image) + .on_conflict(dsl::id) + .do_update() + .set(dsl::time_modified.eq(dsl::time_modified)), + ) + .insert_and_get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| match e { + AsyncInsertError::CollectionNotFound => authz_silo.not_found(), + AsyncInsertError::DatabaseError(e) => { + public_error_from_diesel_pool( + e, + ErrorHandler::Conflict( + ResourceType::ProjectImage, + name.as_str(), + ), + ) + } + })?; + Ok(image) + } + pub async fn project_image_create( &self, opctx: &OpContext, authz_project: &authz::Project, - image: Image, + project_image: ProjectImage, ) -> CreateResult { + let image: Image = project_image.into(); opctx.authorize(authz::Action::CreateChild, authz_project).await?; let name = image.name().clone(); - let project_id = image.project_id; + let project_id = image.parent_id; use db::schema::image::dsl; let image: Image = Project::insert_resource( @@ -78,7 +148,10 @@ impl DataStore { AsyncInsertError::DatabaseError(e) => { public_error_from_diesel_pool( e, - ErrorHandler::Conflict(ResourceType::Image, name.as_str()), + ErrorHandler::Conflict( + ResourceType::ProjectImage, + name.as_str(), + ), ) } })?; diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 6ccb629534..374c2eef78 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -165,6 +165,9 @@ impl<'a> LookupPath<'a> { Disk::PrimaryKey(Root { lookup_root: self }, id) } + // TODO: This needs a custom implementation of some sort I guess. Given a UUID we don't know if it's a silo or project image + // which is ultimately fine b/c its all stored in a single table anyway so the UUID is unique. But we need to be able to get + // it out of the shared table and coerced into the right format. /// Select a resource of type Image, identified by its id pub fn image_id(self, id: Uuid) -> Image<'a> { Image::PrimaryKey(Root { lookup_root: self }, id) @@ -417,7 +420,7 @@ impl<'a> Root<'a> { lookup_resource! { name = "Silo", ancestors = [], - children = [ "IdentityProvider", "SamlIdentityProvider", "Project" ], + children = [ "IdentityProvider", "SamlIdentityProvider", "Project", "SiloImage" ], lookup_by_name = true, soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] @@ -442,6 +445,15 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } +lookup_resource! { + name = "SiloImage", + ancestors = [ "Silo" ], + children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} + lookup_resource! { name = "IdentityProvider", ancestors = [ "Silo" ], @@ -487,7 +499,7 @@ lookup_resource! { lookup_resource! { name = "Project", ancestors = [ "Silo" ], - children = [ "Disk", "Instance", "Vpc", "Snapshot", "Image" ], + children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage" ], lookup_by_name = true, soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] @@ -503,7 +515,7 @@ lookup_resource! { } lookup_resource! { - name = "Image", + name = "ProjectImage", ancestors = [ "Silo", "Project" ], children = [], lookup_by_name = true, diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 5c20e01b4b..8e8b937322 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -128,14 +128,22 @@ pub struct GlobalImage { pub size: ByteCount, } +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum SiloOrProjectId { + SiloId(Uuid), + ProjectId(Uuid), +} + /// Client view of project Images #[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] // CC @ahl Is this fine? pub struct Image { #[serde(flatten)] pub identity: IdentityMetadata, - /// The project the image belongs to - pub project_id: Uuid, + #[serde(flatten)] + pub parent_id: SiloOrProjectId, /// URL source of this image, if any pub url: Option, From 3343ead4a2c973df888dddc07fdd7d826e5ecc6a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 10 Apr 2023 14:45:05 -0400 Subject: [PATCH 03/27] Put a pin in current progress --- common/src/sql/dbinit.sql | 27 ++--- nexus/db-model/src/image.rs | 59 ++++----- nexus/db-model/src/project.rs | 2 +- nexus/db-model/src/schema.rs | 5 +- nexus/db-model/src/silo.rs | 6 +- nexus/db-queries/src/authz/api_resources.rs | 9 ++ nexus/db-queries/src/authz/oso_generic.rs | 5 +- nexus/db-queries/src/db/datastore/image.rs | 32 ++--- nexus/db-queries/src/db/datastore/project.rs | 4 +- nexus/db-queries/src/db/lookup.rs | 9 ++ nexus/src/app/image.rs | 121 ++++++++++++++----- nexus/src/external_api/http_entrypoints.rs | 45 +++++-- 12 files changed, 209 insertions(+), 115 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index cc1a23611f..6c6dd0f4b3 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -837,12 +837,6 @@ CREATE INDEX ON omicron.public.disk ( ) WHERE time_deleted IS NULL AND attach_instance_id IS NOT NULL; -/* The kind of the image */ -CREATE TYPE omicron.public.image_kind AS ENUM ( - 'project', - 'silo' -); - CREATE TABLE omicron.public.image ( /* Identity metadata (resource) */ id UUID PRIMARY KEY, @@ -853,10 +847,8 @@ CREATE TABLE omicron.public.image ( /* Indicates that the object has been deleted */ time_deleted TIMESTAMPTZ, - kind omicron.public.image_kind NOT NULL, - - /* Reference to the parent resource whose type is determined by kind */ - parent_id UUID NOT NULL, + silo_id UUID NOT NULL, + project_id UUID, volume_id UUID NOT NULL, @@ -876,7 +868,8 @@ SELECT time_created, time_modified, time_deleted, - parent_id AS project_id, + silo_id, + project_id, volume_id, url, os, @@ -887,7 +880,7 @@ SELECT FROM omicron.public.image WHERE - kind = 'project'; + project_id IS NOT NULL; CREATE VIEW omicron.public.silo_image AS SELECT @@ -897,7 +890,7 @@ SELECT time_created, time_modified, time_deleted, - parent_id AS silo_id, + silo_id, volume_id, url, os, @@ -908,12 +901,12 @@ SELECT FROM omicron.public.image WHERE - kind = 'silo'; + project_id IS NULL; CREATE UNIQUE INDEX on omicron.public.image ( - parent_id, - name, - kind + silo_id, + project_id, + name ) WHERE time_deleted is NULL; diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index f010f706c0..6c65809f2f 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -3,7 +3,6 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{BlockSize, ByteCount, Digest}; -use crate::impl_enum_type; use crate::schema::{image, project_image, silo_image}; use db_macros::Resource; use nexus_types::external_api::views; @@ -12,19 +11,6 @@ use omicron_common::api::external::Error; use serde::{Deserialize, Serialize}; use uuid::Uuid; -impl_enum_type! { - #[derive(SqlType, QueryId, Debug, Clone, Copy)] - #[diesel(postgres_type(name = "image_kind"))] - pub struct ImageKindEnum; - - #[derive(Clone, Copy, Serialize, Deserialize, Debug, AsExpression, FromSqlRow, PartialEq)] - #[diesel(sql_type = ImageKindEnum)] - pub enum ImageKind; - - Silo => b"silo" - Project => b"project" -} - // Shared image definition #[derive( Queryable, @@ -41,8 +27,9 @@ pub struct Image { #[diesel(embed)] pub identity: ImageIdentity, - pub kind: ImageKind, - pub parent_id: Uuid, + pub silo_id: Uuid, + pub project_id: Option, + pub volume_id: Uuid, pub url: Option, pub os: String, @@ -69,6 +56,7 @@ pub struct ProjectImage { #[diesel(embed)] pub identity: ProjectImageIdentity, + pub silo_id: Uuid, pub project_id: Uuid, pub volume_id: Uuid, pub url: Option, @@ -112,8 +100,8 @@ impl TryFrom for ProjectImage { type Error = Error; fn try_from(image: Image) -> Result { - match image.kind { - ImageKind::Project => Ok(Self { + match image.project_id { + Some(project_id) => Ok(Self { identity: ProjectImageIdentity { id: image.id(), name: image.name().clone().into(), @@ -122,7 +110,8 @@ impl TryFrom for ProjectImage { time_modified: image.time_modified(), time_deleted: image.time_deleted(), }, - project_id: image.parent_id, + silo_id: image.silo_id, + project_id, volume_id: image.volume_id, url: image.url, os: image.os, @@ -131,7 +120,7 @@ impl TryFrom for ProjectImage { block_size: image.block_size, size: image.size, }), - _ => Err(Error::internal_error( + None => Err(Error::internal_error( "tried to convert non-project image to project image", )), } @@ -142,8 +131,11 @@ impl TryFrom for SiloImage { type Error = Error; fn try_from(image: Image) -> Result { - match image.kind { - ImageKind::Silo => Ok(Self { + match image.project_id { + Some(_) => Err(Error::internal_error( + "tried to convert non-silo image to silo image", + )), + None => Ok(Self { identity: SiloImageIdentity { id: image.id(), name: image.name().clone().into(), @@ -152,7 +144,7 @@ impl TryFrom for SiloImage { time_modified: image.time_modified(), time_deleted: image.time_deleted(), }, - silo_id: image.parent_id, + silo_id: image.silo_id, volume_id: image.volume_id, url: image.url, os: image.os, @@ -161,9 +153,6 @@ impl TryFrom for SiloImage { block_size: image.block_size, size: image.size, }), - _ => Err(Error::internal_error( - "tried to convert non-silo image to silo image", - )), } } } @@ -179,8 +168,8 @@ impl From for Image { time_modified: image.time_modified(), time_deleted: image.time_deleted(), }, - kind: ImageKind::Project, - parent_id: image.project_id, + silo_id: image.silo_id, + project_id: Some(image.project_id), volume_id: image.volume_id, url: image.url, os: image.os, @@ -203,8 +192,8 @@ impl From for Image { time_modified: image.time_modified(), time_deleted: image.time_deleted(), }, - kind: ImageKind::Silo, - parent_id: image.silo_id, + silo_id: image.silo_id, + project_id: None, volume_id: image.volume_id, url: image.url, os: image.os, @@ -220,13 +209,9 @@ impl From for views::Image { fn from(image: Image) -> Self { Self { identity: image.identity(), - parent_id: match image.kind { - ImageKind::Project => { - views::SiloOrProjectId::ProjectId(image.parent_id) - } - ImageKind::Silo => { - views::SiloOrProjectId::SiloId(image.parent_id) - } + parent_id: match image.project_id { + Some(id) => views::SiloOrProjectId::ProjectId(id), + None => views::SiloOrProjectId::SiloId(image.silo_id), }, url: image.url, os: image.os, diff --git a/nexus/db-model/src/project.rs b/nexus/db-model/src/project.rs index 62ea594fa5..aada0724df 100644 --- a/nexus/db-model/src/project.rs +++ b/nexus/db-model/src/project.rs @@ -64,7 +64,7 @@ impl DatastoreCollectionConfig for Project { type CollectionId = Uuid; type GenerationNumberColumn = project::dsl::rcgen; type CollectionTimeDeletedColumn = project::dsl::time_deleted; - type CollectionIdColumn = image::dsl::parent_id; + type CollectionIdColumn = image::dsl::project_id; } impl DatastoreCollectionConfig for Project { diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index b37819b5b3..5677808d88 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -37,8 +37,8 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, - kind -> crate::ImageKindEnum, - parent_id -> Uuid, + silo_id -> Uuid, + project_id -> Nullable, volume_id -> Uuid, url -> Nullable, os -> Text, @@ -57,6 +57,7 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, + silo_id -> Uuid, project_id -> Uuid, volume_id -> Uuid, url -> Nullable, diff --git a/nexus/db-model/src/silo.rs b/nexus/db-model/src/silo.rs index 0d18a35d74..5f28818efa 100644 --- a/nexus/db-model/src/silo.rs +++ b/nexus/db-model/src/silo.rs @@ -4,8 +4,8 @@ use super::{Generation, Project}; use crate::collection::DatastoreCollectionConfig; -use crate::schema::{image, project, silo, silo_image}; -use crate::{impl_enum_type, Image, SiloImage}; +use crate::schema::{image, project, silo}; +use crate::{impl_enum_type, Image}; use db_macros::Resource; use nexus_types::external_api::shared::SiloIdentityMode; use nexus_types::external_api::views; @@ -159,5 +159,5 @@ impl DatastoreCollectionConfig for Silo { type CollectionId = Uuid; type GenerationNumberColumn = silo::dsl::rcgen; type CollectionTimeDeletedColumn = silo::dsl::time_deleted; - type CollectionIdColumn = image::dsl::parent_id; + type CollectionIdColumn = image::dsl::silo_id; } diff --git a/nexus/db-queries/src/authz/api_resources.rs b/nexus/db-queries/src/authz/api_resources.rs index 34bad7e3b8..e95499aa25 100644 --- a/nexus/db-queries/src/authz/api_resources.rs +++ b/nexus/db-queries/src/authz/api_resources.rs @@ -875,6 +875,15 @@ authz_resource! { polar_snippet = InSilo, } +// This resource is a collection of _all_ images in a silo, including project images. +authz_resource! { + name = "Image", + parent = "Silo", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = InSilo, +} + authz_resource! { name = "IdentityProvider", parent = "Silo", diff --git a/nexus/db-queries/src/authz/oso_generic.rs b/nexus/db-queries/src/authz/oso_generic.rs index 2ed0d67bc8..6f365825cf 100644 --- a/nexus/db-queries/src/authz/oso_generic.rs +++ b/nexus/db-queries/src/authz/oso_generic.rs @@ -121,7 +121,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { Project::init(), Disk::init(), Snapshot::init(), - Image::init(), + ProjectImage::init(), Instance::init(), IpPool::init(), InstanceNetworkInterface::init(), @@ -129,6 +129,9 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { VpcRouter::init(), RouterRoute::init(), VpcSubnet::init(), + // Silo-level resources + Image::init(), + SiloImage::init(), // Fleet-level resources Certificate::init(), ConsoleSession::init(), diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 6da6924eed..0d072f8033 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -1,5 +1,4 @@ use diesel::prelude::*; -use nexus_db_model::ImageKind; use nexus_db_model::Name; use nexus_types::identity::Resource; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -36,24 +35,24 @@ impl DataStore { ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_project).await?; - use db::schema::image::dsl; + use db::schema::project_image::dsl; match pagparams { PaginatedBy::Id(pagparams) => { - paginated(dsl::image, dsl::id, &pagparams) + paginated(dsl::project_image, dsl::id, &pagparams) } PaginatedBy::Name(pagparams) => paginated( - dsl::image, + dsl::project_image, dsl::name, &pagparams.map_name(|n| Name::ref_cast(n)), ), } .filter(dsl::time_deleted.is_null()) - .filter(dsl::kind.eq(ImageKind::Project)) - .filter(dsl::parent_id.eq(authz_project.id())) - .select(Image::as_select()) - .load_async::(self.pool_authorized(opctx).await?) + .filter(dsl::project_id.eq(authz_project.id())) + .select(ProjectImage::as_select()) + .load_async::(self.pool_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + .map(|v| v.into_iter().map(|v| v.into()).collect()) } pub async fn silo_image_list( @@ -64,23 +63,24 @@ impl DataStore { ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_silo).await?; - use db::schema::image::dsl; + use db::schema::silo_image::dsl; match pagparams { PaginatedBy::Id(pagparams) => { - paginated(dsl::image, dsl::id, &pagparams) + paginated(dsl::silo_image, dsl::id, &pagparams) } PaginatedBy::Name(pagparams) => paginated( - dsl::image, + dsl::silo_image, dsl::name, &pagparams.map_name(|n| Name::ref_cast(n)), ), } .filter(dsl::time_deleted.is_null()) - .filter(dsl::parent_id.eq(authz_silo.id())) - .select(Image::as_select()) - .load_async::(self.pool_authorized(opctx).await?) + .filter(dsl::silo_id.eq(authz_silo.id())) + .select(SiloImage::as_select()) + .load_async::(self.pool_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + .map(|v| v.into_iter().map(|v| v.into()).collect()) } pub async fn silo_image_create( @@ -93,7 +93,7 @@ impl DataStore { opctx.authorize(authz::Action::CreateChild, authz_silo).await?; let name = image.name().clone(); - let silo_id = image.parent_id; + let silo_id = image.silo_id; use db::schema::image::dsl; let image: Image = Silo::insert_resource( @@ -126,11 +126,11 @@ impl DataStore { authz_project: &authz::Project, project_image: ProjectImage, ) -> CreateResult { + let project_id = project_image.project_id.clone(); let image: Image = project_image.into(); opctx.authorize(authz::Action::CreateChild, authz_project).await?; let name = image.name().clone(); - let project_id = image.parent_id; use db::schema::image::dsl; let image: Image = Project::insert_resource( diff --git a/nexus/db-queries/src/db/datastore/project.rs b/nexus/db-queries/src/db/datastore/project.rs index 97f26b9ef0..c05429bd35 100644 --- a/nexus/db-queries/src/db/datastore/project.rs +++ b/nexus/db-queries/src/db/datastore/project.rs @@ -167,7 +167,7 @@ impl DataStore { generate_fn_to_ensure_none_in_project!(instance, name, String); generate_fn_to_ensure_none_in_project!(disk, name, String); - generate_fn_to_ensure_none_in_project!(image, name, String); + generate_fn_to_ensure_none_in_project!(project_image, name, String); generate_fn_to_ensure_none_in_project!(snapshot, name, String); generate_fn_to_ensure_none_in_project!(vpc, name, String); @@ -183,7 +183,7 @@ impl DataStore { // Verify that child resources do not exist. self.ensure_no_instances_in_project(opctx, authz_project).await?; self.ensure_no_disks_in_project(opctx, authz_project).await?; - self.ensure_no_images_in_project(opctx, authz_project).await?; + self.ensure_no_project_images_in_project(opctx, authz_project).await?; self.ensure_no_snapshots_in_project(opctx, authz_project).await?; self.ensure_no_vpcs_in_project(opctx, authz_project).await?; diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 6a86dfcdcd..8ff3e9e2bc 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -517,6 +517,15 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } +lookup_resource! { + name = "Image", + ancestors = ["Silo"], + children = [], + lookup_by_name = false, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} + lookup_resource! { name = "ProjectImage", ancestors = [ "Silo", "Project" ], diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index fe9334fc28..b58308b189 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -29,28 +29,53 @@ use std::str::FromStr; use std::sync::Arc; use uuid::Uuid; +pub enum ImageLookup<'a> { + ProjectImage(lookup::ProjectImage<'a>), + SiloImage(lookup::SiloImage<'a>), +} + +pub enum ImageParentLookup<'a> { + Project(lookup::Project<'a>), + Silo(lookup::Silo<'a>), +} + impl super::Nexus { - pub fn image_lookup<'a>( + pub async fn image_lookup<'a>( &'a self, opctx: &'a OpContext, image_selector: &'a params::ImageSelector, - ) -> LookupResult> { + ) -> LookupResult> { match image_selector { params::ImageSelector { image: NameOrId::Id(id), project_selector: None, } => { - let image = LookupPath::new(opctx, &self.db_datastore) - .image_id(*id); - Ok(image) + let (.., db_image) = LookupPath::new(opctx, &self.db_datastore) + .image_id(*id).fetch().await?; + let lookup = match db_image.project_id { + Some(id) => ImageLookup::ProjectImage(LookupPath::new(opctx, &self.db_datastore) + .project_id(id)), + None => { + ImageLookup::SiloImage(LookupPath::new(opctx, &self.db_datastore) + .silo_image_id(db_image.silo_id)) + }, + }; + Ok(lookup) } params::ImageSelector { image: NameOrId::Name(name), project_selector: Some(project_selector), } => { let image = - self.project_lookup(opctx, project_selector)?.image_name(Name::ref_cast(name)); - Ok(image) + self.project_lookup(opctx, project_selector)?.project_image_name(Name::ref_cast(name)); + Ok(ImageLookup::ProjectImage(image)) + } + params::ImageSelector { + image: NameOrId::Name(name), + project_selector: None, + } => { + let image = self.silo_image_name(Name::ref_cast(name)); + Ok(ImageLookup::SiloImage(image)) } params::ImageSelector { image: NameOrId::Id(_), @@ -58,21 +83,28 @@ impl super::Nexus { } => Err(Error::invalid_request( "when providing image as an ID, project should not be specified", )), - _ => Err(Error::invalid_request( - "image should either be UUID or project should be specified", - )), } } - /// Creates a project image + /// Creates an image pub async fn image_create( self: &Arc, opctx: &OpContext, - lookup_project: &lookup::Project<'_>, + lookup_parent: &ImageParentLookup<'_>, params: ¶ms::ImageCreate, ) -> CreateResult { - let (.., authz_project) = - lookup_project.lookup_for(authz::Action::CreateChild).await?; + let (authz_silo, maybe_authz_project) = match lookup_parent { + ImageParentLookup::Project(project) => { + let (authz_silo, authz_project) = + project.lookup_for(authz::Action::CreateChild).await?; + (authz_silo, Some(authz_project)) + } + ImageParentLookup::Silo(silo) => { + let (.., authz_silo) = + silo.lookup_for(authz::Action::CreateChild).await?; + (authz_silo, None) + } + }; let new_image = match ¶ms.source { params::ImageSource::Url { url } => { let db_block_size = db::model::BlockSize::try_from( @@ -179,7 +211,8 @@ impl super::Nexus { image_id, params.identity.clone(), ), - project_id: authz_project.id(), + silo_id: authz_silo.id(), + project_id: maybe_authz_project.map(|p| p.id()), volume_id: volume.id(), url: Some(url.clone()), os: params.os.clone(), @@ -216,7 +249,8 @@ impl super::Nexus { image_id, params.identity.clone(), ), - project_id: authz_project.id(), + silo_id: authz_silo.id(), + project_id: maybe_authz_project.map(|p| p.id()), volume_id: image_volume.id(), url: None, os: params.os.clone(), @@ -269,7 +303,8 @@ impl super::Nexus { global_image_id, params.identity.clone(), ), - project_id: authz_project.id(), + silo_id: authz_silo.id(), + project_id: maybe_authz_project.map(|p| p.id()), volume_id: volume.id(), url: None, os: "alpine".into(), @@ -281,29 +316,61 @@ impl super::Nexus { } }; - self.db_datastore.image_create(opctx, &authz_project, new_image).await + match maybe_authz_project { + Some(authz_project) => { + self.db_datastore + .project_image_create(opctx, &authz_project, new_image.into()) + .await + } + None => { + self.db_datastore + .silo_image_create(opctx, &authz_silo, new_image.into()) + .await + } + } } pub async fn image_list( &self, opctx: &OpContext, - project_lookup: &lookup::Project<'_>, + parent_lookup: &ImageParentLookup<'_>, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { - let (.., authz_project) = - project_lookup.lookup_for(authz::Action::ListChildren).await?; - self.db_datastore.image_list(opctx, &authz_project, pagparams).await + match parent_lookup { + ImageParentLookup::Project(project) => { + let (.., authz_project) = + project.lookup_for(authz::Action::ListChildren).await?; + self.db_datastore + .project_image_list(opctx, &authz_project, pagparams) + .await + } + ImageParentLookup::Silo(silo) => { + let (.., authz_silo) = + silo.lookup_for(authz::Action::ListChildren).await?; + self.db_datastore + .silo_image_list(opctx, &authz_silo, pagparams) + .await + } + } } - // TODO-MVP: Implement pub async fn image_delete( self: &Arc, opctx: &OpContext, - image_lookup: &lookup::Image<'_>, + image_lookup: &ImageLookup<'_>, ) -> DeleteResult { - let (.., authz_image) = - image_lookup.lookup_for(authz::Action::Delete).await?; - let lookup_type = LookupType::ById(authz_image.id()); + let lookup_type = match image_lookup { + ImageLookup::ProjectImage(lookup) => { + let (.., authz_image) = + lookup.lookup_for(authz::Action::Delete).await?; + LookupType::ById(authz_image.id()) + } + ImageLookup::SiloImage(lookup) => { + let (.., authz_image) = + lookup.lookup_for(authz::Action::Delete).await?; + LookupType::ById(authz_image.id()) + } + }; let error = lookup_type.into_not_found(ResourceType::Image); Err(self .unimplemented_todo(opctx, Unimpl::ProtectedLookup(error)) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 3f73e45f93..3e54cf1f56 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2397,7 +2397,7 @@ async fn system_image_delete( }] async fn image_list( rqctx: RequestContext>, - query_params: Query>, + query_params: Query>, ) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { @@ -2407,10 +2407,23 @@ async fn image_list( let pag_params = data_page_params_for(&rqctx, &query)?; let scan_params = ScanByNameOrId::from_query(&query)?; let paginated_by = name_or_id_pagination(&pag_params, scan_params)?; - let project_lookup = - nexus.project_lookup(&opctx, &scan_params.selector)?; + let parent_lookup = match scan_params.selector.project_selector { + Some(selector) => { + let project_lookup = nexus.project_lookup(&opctx, &selector)?; + ImageParentLookup::Project(project_lookup) + } + None => { + let silo = self + .opctx + .authn + .silo_required() + .internal_context("looking up Organization by name"); + let silo_lookup = nexus.silo_lookup(&opctx, &silo.id)?; + ImageParentLookup::Silo(silo_lookup) + } + }; let images = nexus - .image_list(&opctx, &project_lookup, &paginated_by) + .image_list(&opctx, &parent_lookup, &paginated_by) .await? .into_iter() .map(|d| d.into()) @@ -2434,7 +2447,7 @@ async fn image_list( }] async fn image_create( rqctx: RequestContext>, - query_params: Query, + query_params: Query, new_image: TypedBody, ) -> Result, HttpError> { let apictx = rqctx.context(); @@ -2443,9 +2456,23 @@ async fn image_create( let nexus = &apictx.nexus; let query = query_params.into_inner(); let params = &new_image.into_inner(); - let project_lookup = nexus.project_lookup(&opctx, &query)?; + let parent_lookup = match query.project_selector { + Some(project_selector) => { + let project_lookup = nexus.project_lookup(&opctx, &project_selector)?; + ImageParentLookup::Project(project_lookup) + } + None => { + let silo = self + .opctx + .authn + .silo_required() + .internal_context("looking up Organization by name"); + let silo_lookup = nexus.silo_lookup(&opctx, &silo.id)?; + ImageParentLookup::Silo(silo_lookup) + } + }; let image = - nexus.image_create(&opctx, &project_lookup, ¶ms).await?; + nexus.image_create(&opctx, &parent_lookup, ¶ms).await?; Ok(HttpResponseCreated(image.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2475,7 +2502,7 @@ async fn image_view( project_selector: query.project_selector, }; let (.., image) = - nexus.image_lookup(&opctx, &image_selector)?.fetch().await?; + nexus.image_lookup(&opctx, &image_selector).await?.fetch().await?; Ok(HttpResponseOk(image.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2506,7 +2533,7 @@ async fn image_delete( image: path.image, project_selector: query.project_selector, }; - let image_lookup = nexus.image_lookup(&opctx, &image_selector)?; + let image_lookup = nexus.image_lookup(&opctx, &image_selector).await?; nexus.image_delete(&opctx, &image_lookup).await?; Ok(HttpResponseDeleted()) }; From 094ec23f640bfc5085d357ea5d48a77d347ca059 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 11 Apr 2023 11:55:41 -0400 Subject: [PATCH 04/27] Add silo image id, name lookups --- nexus/db-queries/src/db/lookup.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 8ff3e9e2bc..305d0fec5c 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -318,6 +318,33 @@ impl<'a> LookupPath<'a> { PhysicalDisk::PrimaryKey(Root { lookup_root: self }, id) } + pub fn silo_image_id(self, id: Uuid) -> SiloImage<'a> { + SiloImage::PrimaryKey(Root { lookup_root: self }, id) + } + + pub fn silo_image_name<'b, 'c>(self, name: &'b Name) -> SiloImage<'c> + where + 'a: 'c, + 'b: 'c, + { + match self + .opctx + .authn + .silo_required() + .internal_context("looking up Organization by name") + { + Ok(authz_silo) => { + let root = Root { lookup_root: self }; + let silo_key = Silo::PrimaryKey(root, authz_silo.id()); + SiloImage::Name(silo_key, name) + } + Err(error) => { + let root = Root { lookup_root: self }; + SiloImage::Error(root, error) + } + } + } + /// Select a resource of type UpdateAvailableArtifact, identified by its /// `(name, version, kind)` tuple pub fn update_available_artifact_tuple( From 974f31ebf4f143a666bd560d71781a2bb1ad2779 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 11 Apr 2023 13:18:25 -0400 Subject: [PATCH 05/27] Fixup some lookup issues --- nexus/db-queries/src/db/lookup.rs | 20 ++++++++++++++++---- nexus/src/app/image.rs | 23 +++++++++-------------- nexus/src/app/silo.rs | 13 ++++++++++++- 3 files changed, 37 insertions(+), 19 deletions(-) diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 305d0fec5c..bea3297b32 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -165,14 +165,14 @@ impl<'a> LookupPath<'a> { Disk::PrimaryKey(Root { lookup_root: self }, id) } - // TODO: This needs a custom implementation of some sort I guess. Given a UUID we don't know if it's a silo or project image - // which is ultimately fine b/c its all stored in a single table anyway so the UUID is unique. But we need to be able to get - // it out of the shared table and coerced into the right format. - /// Select a resource of type Image, identified by its id pub fn image_id(self, id: Uuid) -> Image<'a> { Image::PrimaryKey(Root { lookup_root: self }, id) } + pub fn project_image_id(self, id: Uuid) -> Image<'a> { + Image::PrimaryKey(Root { lookup_root: self }, id) + } + /// Select a resource of type Snapshot, identified by its id pub fn snapshot_id(self, id: Uuid) -> Snapshot<'a> { Snapshot::PrimaryKey(Root { lookup_root: self }, id) @@ -757,6 +757,18 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } +// Helpers for unifying the interfaces around images + +pub enum ImageLookup<'a> { + ProjectImage(ProjectImage<'a>), + SiloImage(SiloImage<'a>), +} + +pub enum ImageParentLookup<'a> { + Project(Project<'a>), + Silo(Silo<'a>), +} + #[cfg(test)] mod test { use super::Instance; diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index b58308b189..8e02657969 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -8,11 +8,12 @@ use super::Unimpl; use crate::authz; use crate::db; use crate::db::identity::Asset; -use crate::db::lookup; use crate::db::lookup::LookupPath; use crate::db::model::Name; use crate::external_api::params; use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::lookup::ImageLookup; +use nexus_db_queries::db::lookup::ImageParentLookup; use omicron_common::api::external; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::CreateResult; @@ -29,16 +30,6 @@ use std::str::FromStr; use std::sync::Arc; use uuid::Uuid; -pub enum ImageLookup<'a> { - ProjectImage(lookup::ProjectImage<'a>), - SiloImage(lookup::SiloImage<'a>), -} - -pub enum ImageParentLookup<'a> { - Project(lookup::Project<'a>), - Silo(lookup::Silo<'a>), -} - impl super::Nexus { pub async fn image_lookup<'a>( &'a self, @@ -54,7 +45,7 @@ impl super::Nexus { .image_id(*id).fetch().await?; let lookup = match db_image.project_id { Some(id) => ImageLookup::ProjectImage(LookupPath::new(opctx, &self.db_datastore) - .project_id(id)), + .project_image_id(id)), None => { ImageLookup::SiloImage(LookupPath::new(opctx, &self.db_datastore) .silo_image_id(db_image.silo_id)) @@ -74,7 +65,7 @@ impl super::Nexus { image: NameOrId::Name(name), project_selector: None, } => { - let image = self.silo_image_name(Name::ref_cast(name)); + let image = self.current_silo_lookup(opctx)?.silo_image_name(Name::ref_cast(name)); Ok(ImageLookup::SiloImage(image)) } params::ImageSelector { @@ -319,7 +310,11 @@ impl super::Nexus { match maybe_authz_project { Some(authz_project) => { self.db_datastore - .project_image_create(opctx, &authz_project, new_image.into()) + .project_image_create( + opctx, + &authz_project, + new_image.into(), + ) .await } None => { diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index a0e621e52c..fb6ef9d254 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -17,13 +17,13 @@ use anyhow::Context; use nexus_db_model::UserProvisionType; use nexus_db_queries::context::OpContext; use omicron_common::api::external::http_pagination::PaginatedBy; -use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::{CreateResult, LookupType}; use omicron_common::api::external::{DataPageParams, ResourceType}; use omicron_common::api::external::{DeleteResult, NameOrId}; +use omicron_common::api::external::{Error, InternalContext}; use omicron_common::bail_unless; use ref_cast::RefCast; use std::str::FromStr; @@ -31,6 +31,17 @@ use uuid::Uuid; impl super::Nexus { // Silos + pub fn current_silo_lookup( + &self, + opctx: &OpContext, + ) -> LookupResult> { + let silo = opctx + .authn + .silo_required() + .internal_context("looking up current silo")?; + let silo = self.silo_lookup(opctx, &NameOrId::Id(silo.id()))?; + Ok(silo) + } pub fn silo_lookup<'a>( &'a self, opctx: &'a OpContext, From 8c057765519b1c830a8b1313900e40dc54c8f03a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 11 Apr 2023 14:32:30 -0400 Subject: [PATCH 06/27] Fix more import/life cycle/lookup references --- nexus/db-queries/src/db/lookup.rs | 4 +-- nexus/src/app/image.rs | 14 +++++--- nexus/src/app/silo.rs | 8 ++--- nexus/src/external_api/http_entrypoints.rs | 39 +++++++++++----------- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index bea3297b32..3f17a113c0 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -169,8 +169,8 @@ impl<'a> LookupPath<'a> { Image::PrimaryKey(Root { lookup_root: self }, id) } - pub fn project_image_id(self, id: Uuid) -> Image<'a> { - Image::PrimaryKey(Root { lookup_root: self }, id) + pub fn project_image_id(self, id: Uuid) -> ProjectImage<'a> { + ProjectImage::PrimaryKey(Root { lookup_root: self }, id) } /// Select a resource of type Snapshot, identified by its id diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 8e02657969..028211fcff 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -203,7 +203,7 @@ impl super::Nexus { params.identity.clone(), ), silo_id: authz_silo.id(), - project_id: maybe_authz_project.map(|p| p.id()), + project_id: maybe_authz_project.clone().map(|p| p.id()), volume_id: volume.id(), url: Some(url.clone()), os: params.os.clone(), @@ -241,7 +241,7 @@ impl super::Nexus { params.identity.clone(), ), silo_id: authz_silo.id(), - project_id: maybe_authz_project.map(|p| p.id()), + project_id: maybe_authz_project.clone().map(|p| p.id()), volume_id: image_volume.id(), url: None, os: params.os.clone(), @@ -295,7 +295,7 @@ impl super::Nexus { params.identity.clone(), ), silo_id: authz_silo.id(), - project_id: maybe_authz_project.map(|p| p.id()), + project_id: maybe_authz_project.clone().map(|p| p.id()), volume_id: volume.id(), url: None, os: "alpine".into(), @@ -313,13 +313,17 @@ impl super::Nexus { .project_image_create( opctx, &authz_project, - new_image.into(), + new_image.try_into()?, ) .await } None => { self.db_datastore - .silo_image_create(opctx, &authz_silo, new_image.into()) + .silo_image_create( + opctx, + &authz_silo, + new_image.try_into()?, + ) .await } } diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index fb6ef9d254..5cb2a426e4 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -31,10 +31,10 @@ use uuid::Uuid; impl super::Nexus { // Silos - pub fn current_silo_lookup( - &self, - opctx: &OpContext, - ) -> LookupResult> { + pub fn current_silo_lookup<'a>( + &'a self, + opctx: &'a OpContext, + ) -> LookupResult> { let silo = opctx .authn .silo_required() diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 3e54cf1f56..7aef80ccf3 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -39,6 +39,8 @@ use dropshot::{ channel, endpoint, WebsocketChannelResult, WebsocketConnection, }; use ipnetwork::IpNetwork; +use nexus_db_queries::db::lookup::ImageLookup; +use nexus_db_queries::db::lookup::ImageParentLookup; use nexus_types::identity::AssetIdentityMetadata; use omicron_common::api::external::http_pagination::data_page_params_for; use omicron_common::api::external::http_pagination::marker_for_name; @@ -2407,18 +2409,13 @@ async fn image_list( let pag_params = data_page_params_for(&rqctx, &query)?; let scan_params = ScanByNameOrId::from_query(&query)?; let paginated_by = name_or_id_pagination(&pag_params, scan_params)?; - let parent_lookup = match scan_params.selector.project_selector { + let parent_lookup = match &scan_params.selector.project_selector { Some(selector) => { let project_lookup = nexus.project_lookup(&opctx, &selector)?; ImageParentLookup::Project(project_lookup) } None => { - let silo = self - .opctx - .authn - .silo_required() - .internal_context("looking up Organization by name"); - let silo_lookup = nexus.silo_lookup(&opctx, &silo.id)?; + let silo_lookup = nexus.current_silo_lookup(&opctx)?; ImageParentLookup::Silo(silo_lookup) } }; @@ -2456,23 +2453,18 @@ async fn image_create( let nexus = &apictx.nexus; let query = query_params.into_inner(); let params = &new_image.into_inner(); - let parent_lookup = match query.project_selector { + let parent_lookup = match &query.project_selector { Some(project_selector) => { - let project_lookup = nexus.project_lookup(&opctx, &project_selector)?; + let project_lookup = + nexus.project_lookup(&opctx, project_selector)?; ImageParentLookup::Project(project_lookup) } None => { - let silo = self - .opctx - .authn - .silo_required() - .internal_context("looking up Organization by name"); - let silo_lookup = nexus.silo_lookup(&opctx, &silo.id)?; + let silo_lookup = nexus.current_silo_lookup(&opctx)?; ImageParentLookup::Silo(silo_lookup) } }; - let image = - nexus.image_create(&opctx, &parent_lookup, ¶ms).await?; + let image = nexus.image_create(&opctx, &parent_lookup, ¶ms).await?; Ok(HttpResponseCreated(image.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2501,8 +2493,17 @@ async fn image_view( image: path.image, project_selector: query.project_selector, }; - let (.., image) = - nexus.image_lookup(&opctx, &image_selector).await?.fetch().await?; + let image: nexus_db_model::Image = + match nexus.image_lookup(&opctx, &image_selector).await? { + ImageLookup::ProjectImage(image) => { + let (.., db_image) = image.fetch().await?; + db_image.into() + } + ImageLookup::SiloImage(image) => { + let (.., db_image) = image.fetch().await?; + db_image.into() + } + }; Ok(HttpResponseOk(image.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await From 7016701579e411af3b88a8274883f7fd9d6f7969 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 12 Apr 2023 13:14:54 -0400 Subject: [PATCH 07/27] Fixup selectors --- nexus/src/app/image.rs | 8 +-- nexus/src/app/silo.rs | 2 +- nexus/src/external_api/http_entrypoints.rs | 65 +++++++++++++--------- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 50d7e06ed8..5d75aba05e 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -41,7 +41,7 @@ impl super::Nexus { project: None, } => { let (.., db_image) = LookupPath::new(opctx, &self.db_datastore) - .image_id(*id).fetch().await?; + .image_id(id).fetch().await?; let lookup = match db_image.project_id { Some(id) => ImageLookup::ProjectImage(LookupPath::new(opctx, &self.db_datastore) .project_image_id(id)), @@ -57,14 +57,14 @@ impl super::Nexus { project: Some(project), } => { let image = - self.project_lookup(opctx, project_selector)?.project_image_name(Name::ref_cast(name)); + self.project_lookup(opctx, params::ProjectSelector { project })?.project_image_name_owned(name.into()); Ok(ImageLookup::ProjectImage(image)) } params::ImageSelector { image: NameOrId::Name(name), - project_selector: None, + project: None, } => { - let image = self.current_silo_lookup(opctx)?.silo_image_name(Name::ref_cast(name)); + let image = self.current_silo_lookup(opctx)?.silo_image_name_owned(name.into()); Ok(ImageLookup::SiloImage(image)) } params::ImageSelector { diff --git a/nexus/src/app/silo.rs b/nexus/src/app/silo.rs index 9b14ab9480..fbadb5f5bf 100644 --- a/nexus/src/app/silo.rs +++ b/nexus/src/app/silo.rs @@ -39,7 +39,7 @@ impl super::Nexus { .authn .silo_required() .internal_context("looking up current silo")?; - let silo = self.silo_lookup(opctx, &NameOrId::Id(silo.id()))?; + let silo = self.silo_lookup(opctx, NameOrId::Id(silo.id()))?; Ok(silo) } pub fn silo_lookup<'a>( diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 97cd07c96c..3ad2292804 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2399,9 +2399,12 @@ async fn image_list( let pag_params = data_page_params_for(&rqctx, &query)?; let scan_params = ScanByNameOrId::from_query(&query)?; let paginated_by = name_or_id_pagination(&pag_params, scan_params)?; - let parent_lookup = match &scan_params.selector.project_selector { - Some(selector) => { - let project_lookup = nexus.project_lookup(&opctx, &selector)?; + let parent_lookup = match scan_params.selector.project.clone() { + Some(project) => { + let project_lookup = nexus.project_lookup( + &opctx, + params::ProjectSelector { project }, + )?; ImageParentLookup::Project(project_lookup) } None => { @@ -2443,10 +2446,12 @@ async fn image_create( let nexus = &apictx.nexus; let query = query_params.into_inner(); let params = &new_image.into_inner(); - let parent_lookup = match &query.project_selector { - Some(project_selector) => { - let project_lookup = - nexus.project_lookup(&opctx, project_selector)?; + let parent_lookup = match query.project.clone() { + Some(project) => { + let project_lookup = nexus.project_lookup( + &opctx, + params::ProjectSelector { project }, + )?; ImageParentLookup::Project(project_lookup) } None => { @@ -2479,21 +2484,25 @@ async fn image_view( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); - let image_selector = params::ImageSelector { - image: path.image, - project_selector: query.project_selector, + let image: nexus_db_model::Image = match nexus + .image_lookup( + &opctx, + params::ImageSelector { + image: path.image, + project: query.project, + }, + ) + .await? + { + ImageLookup::ProjectImage(image) => { + let (.., db_image) = image.fetch().await?; + db_image.into() + } + ImageLookup::SiloImage(image) => { + let (.., db_image) = image.fetch().await?; + db_image.into() + } }; - let image: nexus_db_model::Image = - match nexus.image_lookup(&opctx, &image_selector).await? { - ImageLookup::ProjectImage(image) => { - let (.., db_image) = image.fetch().await?; - db_image.into() - } - ImageLookup::SiloImage(image) => { - let (.., db_image) = image.fetch().await?; - db_image.into() - } - }; Ok(HttpResponseOk(image.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -2520,11 +2529,15 @@ async fn image_delete( let nexus = &apictx.nexus; let path = path_params.into_inner(); let query = query_params.into_inner(); - let image_selector = params::ImageSelector { - image: path.image, - project_selector: query.project_selector, - }; - let image_lookup = nexus.image_lookup(&opctx, &image_selector).await?; + let image_lookup = nexus + .image_lookup( + &opctx, + params::ImageSelector { + image: path.image, + project: query.project, + }, + ) + .await?; nexus.image_delete(&opctx, &image_lookup).await?; Ok(HttpResponseDeleted()) }; From 053d14e72fe41f8e6f7012272cd8a471a915888c Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 12 Apr 2023 13:49:39 -0400 Subject: [PATCH 08/27] Update image result type for project_id to be optional --- nexus/db-model/src/image.rs | 5 +---- .../db-queries/src/authz/policy_test/resources.rs | 2 +- nexus/types/src/external_api/views.rs | 14 +++----------- openapi/nexus.json | 7 +++---- 4 files changed, 8 insertions(+), 20 deletions(-) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index 6c65809f2f..0d783958bf 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -209,10 +209,7 @@ impl From for views::Image { fn from(image: Image) -> Self { Self { identity: image.identity(), - parent_id: match image.project_id { - Some(id) => views::SiloOrProjectId::ProjectId(id), - None => views::SiloOrProjectId::SiloId(image.silo_id), - }, + project_id: image.project_id, url: image.url, os: image.os, version: image.version, diff --git a/nexus/db-queries/src/authz/policy_test/resources.rs b/nexus/db-queries/src/authz/policy_test/resources.rs index 0f3a2ed709..c9761bcd04 100644 --- a/nexus/db-queries/src/authz/policy_test/resources.rs +++ b/nexus/db-queries/src/authz/policy_test/resources.rs @@ -256,7 +256,7 @@ async fn make_project( )); let image_name = format!("{}-image1", project_name); - builder.new_resource(authz::Image::new( + builder.new_resource(authz::ProjectImage::new( project.clone(), Uuid::new_v4(), LookupType::ByName(image_name), diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 8e8b937322..0c0a00bc58 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -128,22 +128,14 @@ pub struct GlobalImage { pub size: ByteCount, } -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub enum SiloOrProjectId { - SiloId(Uuid), - ProjectId(Uuid), -} - -/// Client view of project Images +/// Client view of images #[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] -#[serde(deny_unknown_fields)] // CC @ahl Is this fine? pub struct Image { #[serde(flatten)] pub identity: IdentityMetadata, - #[serde(flatten)] - pub parent_id: SiloOrProjectId, + /// ID of the parent project if the image is a project image + pub project_id: Option, /// URL source of this image, if any pub url: Option, diff --git a/openapi/nexus.json b/openapi/nexus.json index 07a2ff8262..14d0440f5c 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1192,7 +1192,6 @@ "in": "query", "name": "project", "description": "Name or ID of the project", - "required": true, "schema": { "$ref": "#/components/schemas/NameOrId" } @@ -8752,7 +8751,7 @@ ] }, "Image": { - "description": "Client view of project Images", + "description": "Client view of images", "type": "object", "properties": { "block_size": { @@ -8794,7 +8793,8 @@ "type": "string" }, "project_id": { - "description": "The project the image belongs to", + "nullable": true, + "description": "ID of the parent project if the image is a project image", "type": "string", "format": "uuid" }, @@ -8832,7 +8832,6 @@ "id", "name", "os", - "project_id", "size", "time_created", "time_modified", From 96ab34194191b192e74fda06ae698fe8905c2c3f Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 12 Apr 2023 14:27:29 -0400 Subject: [PATCH 09/27] Add image promote --- nexus/db-queries/src/db/datastore/image.rs | 40 +++++++++++++++++++ nexus/src/app/image.rs | 23 +++++++++++ nexus/src/external_api/http_entrypoints.rs | 33 ++++++++++++++++ nexus/tests/output/nexus_tags.txt | 1 + openapi/nexus.json | 46 ++++++++++++++++++++++ 5 files changed, 143 insertions(+) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 0d072f8033..884b53fd34 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -5,6 +5,7 @@ use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::CreateResult; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::ResourceType; +use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; use crate::authz; @@ -120,6 +121,7 @@ impl DataStore { })?; Ok(image) } + pub async fn project_image_create( &self, opctx: &OpContext, @@ -157,4 +159,42 @@ impl DataStore { })?; Ok(image) } + + pub async fn project_image_promote( + &self, + opctx: &OpContext, + authz_silo: &authz::Silo, + project_image: ProjectImage, + ) -> UpdateResult { + let silo_id = authz_silo.id().clone(); + let image: Image = project_image.into(); + let name = image.name().clone(); + + opctx.authorize(authz::Action::CreateChild, authz_silo).await?; + + use db::schema::image::dsl; + let image: Image = Silo::insert_resource( + silo_id, + diesel::insert_into(dsl::image) + .values(image) + .on_conflict(dsl::id) + .do_update() + .set(dsl::time_modified.eq(dsl::time_modified)), + ) + .insert_and_get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| match e { + AsyncInsertError::CollectionNotFound => authz_silo.not_found(), + AsyncInsertError::DatabaseError(e) => { + public_error_from_diesel_pool( + e, + ErrorHandler::Conflict( + ResourceType::SiloImage, + name.as_str(), + ), + ) + } + })?; + Ok(image) + } } diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 5d75aba05e..10592cb155 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -25,6 +25,7 @@ use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::NameOrId; use omicron_common::api::external::ResourceType; +use omicron_common::api::external::UpdateResult; use std::str::FromStr; use std::sync::Arc; use uuid::Uuid; @@ -375,6 +376,28 @@ impl super::Nexus { .await) } + pub async fn image_promote( + self: &Arc, + opctx: &OpContext, + image_lookup: &ImageLookup<'_>, + ) -> UpdateResult { + match image_lookup { + ImageLookup::ProjectImage(lookup) => { + let (authz_silo, _, _, db_image) = + lookup.fetch_for(authz::Action::Modify).await?; + opctx + .authorize(authz::Action::CreateChild, &authz_silo) + .await?; + self.db_datastore + .project_image_promote(opctx, &authz_silo, db_image) + .await + } + ImageLookup::SiloImage(_) => Err(Error::InvalidRequest { + message: "Cannot promote a silo image".to_string(), + }), + } + } + // Globally-Scoped Images // TODO-v1: Delete post migration diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 3ad2292804..9461e7319b 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -142,6 +142,7 @@ pub fn external_api() -> NexusApiDescription { api.register(image_create)?; api.register(image_view)?; api.register(image_delete)?; + api.register(image_promote)?; api.register(snapshot_list)?; api.register(snapshot_create)?; @@ -2544,6 +2545,38 @@ async fn image_delete( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +/// Promote a project image to be visible to all projects in the silo +#[endpoint { + method = POST, + path = "/v1/images/{image}/promote", + tags = ["images"] +}] +async fn image_promote( + rqctx: RequestContext>, + path_params: Path, + query_params: Query, +) -> 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 image_lookup = nexus + .image_lookup( + &opctx, + params::ImageSelector { + image: path.image, + project: query.project, + }, + ) + .await?; + let image = nexus.image_promote(&opctx, &image_lookup).await?; + Ok(HttpResponseAccepted(image.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// List network interfaces #[endpoint { method = GET, diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 29e736fbc6..fbfc3d5fb7 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -24,6 +24,7 @@ OPERATION ID METHOD URL PATH image_create POST /v1/images image_delete DELETE /v1/images/{image} image_list GET /v1/images +image_promote POST /v1/images/{image}/promote image_view GET /v1/images/{image} API operations found with tag "instances" diff --git a/openapi/nexus.json b/openapi/nexus.json index 14d0440f5c..7f74503820 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1312,6 +1312,52 @@ } } }, + "/v1/images/{image}/promote": { + "post": { + "tags": [ + "images" + ], + "summary": "Promote a project image to be visible to all projects in the silo", + "operationId": "image_promote", + "parameters": [ + { + "in": "path", + "name": "image", + "description": "Name or ID of the image", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "description": "Name or ID of the project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Image" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/instances": { "get": { "tags": [ From acdb77e21d9f271734495e8cd62f60a23454c90a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 12 Apr 2023 16:57:36 -0400 Subject: [PATCH 10/27] Enable listing silo images alongside project images --- nexus/db-queries/src/db/datastore/image.rs | 64 ++++++++++++++++------ nexus/src/app/image.rs | 8 ++- nexus/src/external_api/http_entrypoints.rs | 9 ++- nexus/types/src/external_api/params.rs | 11 ++++ openapi/nexus.json | 9 +++ 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 884b53fd34..d171099289 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -32,28 +32,56 @@ impl DataStore { &self, opctx: &OpContext, authz_project: &authz::Project, + include_silo_images: bool, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, authz_project).await?; - use db::schema::project_image::dsl; - match pagparams { - PaginatedBy::Id(pagparams) => { - paginated(dsl::project_image, dsl::id, &pagparams) + use db::schema::image::dsl; + use db::schema::project_image::dsl as project_dsl; + match include_silo_images { + true => match pagparams { + PaginatedBy::Id(pagparams) => { + paginated(dsl::image, dsl::id, &pagparams) + } + PaginatedBy::Name(pagparams) => paginated( + dsl::image, + dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), } - PaginatedBy::Name(pagparams) => paginated( - dsl::project_image, - dsl::name, - &pagparams.map_name(|n| Name::ref_cast(n)), - ), + .filter(dsl::time_deleted.is_null()) + .filter( + dsl::project_id + .is_null() + .or(dsl::project_id.eq(authz_project.id())), + ) + .select(Image::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel_pool(e, ErrorHandler::Server) + }), + false => match pagparams { + PaginatedBy::Id(pagparams) => paginated( + project_dsl::project_image, + project_dsl::id, + &pagparams, + ), + PaginatedBy::Name(pagparams) => paginated( + project_dsl::project_image, + project_dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), + } + .filter(project_dsl::time_deleted.is_null()) + .filter(project_dsl::project_id.eq(authz_project.id())) + .select(ProjectImage::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + .map(|v| v.into_iter().map(|v| v.into()).collect()), } - .filter(dsl::time_deleted.is_null()) - .filter(dsl::project_id.eq(authz_project.id())) - .select(ProjectImage::as_select()) - .load_async::(self.pool_authorized(opctx).await?) - .await - .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) - .map(|v| v.into_iter().map(|v| v.into()).collect()) } pub async fn silo_image_list( @@ -128,7 +156,7 @@ impl DataStore { authz_project: &authz::Project, project_image: ProjectImage, ) -> CreateResult { - let project_id = project_image.project_id.clone(); + let project_id = project_image.project_id; let image: Image = project_image.into(); opctx.authorize(authz::Action::CreateChild, authz_project).await?; @@ -166,7 +194,7 @@ impl DataStore { authz_silo: &authz::Silo, project_image: ProjectImage, ) -> UpdateResult { - let silo_id = authz_silo.id().clone(); + let silo_id = authz_silo.id(); let image: Image = project_image.into(); let name = image.name().clone(); diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 10592cb155..0a4a96d877 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -333,6 +333,7 @@ impl super::Nexus { &self, opctx: &OpContext, parent_lookup: &ImageParentLookup<'_>, + include_silo_images: Option, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { match parent_lookup { @@ -340,7 +341,12 @@ impl super::Nexus { let (.., authz_project) = project.lookup_for(authz::Action::ListChildren).await?; self.db_datastore - .project_image_list(opctx, &authz_project, pagparams) + .project_image_list( + opctx, + &authz_project, + include_silo_images.unwrap_or(false), + pagparams, + ) .await } ImageParentLookup::Silo(silo) => { diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 9461e7319b..6731b39811 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2390,7 +2390,7 @@ async fn system_image_delete( }] async fn image_list( rqctx: RequestContext>, - query_params: Query>, + query_params: Query>, ) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { @@ -2414,7 +2414,12 @@ async fn image_list( } }; let images = nexus - .image_list(&opctx, &parent_lookup, &paginated_by) + .image_list( + &opctx, + &parent_lookup, + scan_params.selector.include_silo_images, + &paginated_by, + ) .await? .into_iter() .map(|d| d.into()) diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index e36f56fcf2..aa512d094d 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -115,6 +115,17 @@ pub struct SnapshotSelector { pub snapshot: NameOrId, } +/// A specialized selector for image list, it contains an extra feield to indicate +/// if silo scoped images should be included when listing project images. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] +pub struct ImageListSelector { + /// Name or ID of the project + pub project: Option, + /// Flag used to indicate if silo scoped images should be included when + /// listing project images. Only valid when `project` is provided. + pub include_silo_images: Option, +} + #[derive(Deserialize, JsonSchema)] pub struct ImageSelector { /// Name or ID of the project, only required if `image` is provided as a `Name` diff --git a/openapi/nexus.json b/openapi/nexus.json index 7f74503820..b4ee070386 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1124,6 +1124,15 @@ "description": "List images which are global or scoped to the specified project. The images are returned sorted by creation date, with the most recent images appearing first.", "operationId": "image_list", "parameters": [ + { + "in": "query", + "name": "include_silo_images", + "description": "Flag used to indicate if silo scoped images should be included when listing project images. Only valid when `project` is provided.", + "schema": { + "nullable": true, + "type": "boolean" + } + }, { "in": "query", "name": "limit", From 3b1c5761400e431788801c81fa6c36415bace05a Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 14 Apr 2023 13:20:16 -0400 Subject: [PATCH 11/27] Fix incorrect error message matching in test --- nexus/tests/integration_tests/projects.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/projects.rs b/nexus/tests/integration_tests/projects.rs index a07e0d2f40..054c0f63ad 100644 --- a/nexus/tests/integration_tests/projects.rs +++ b/nexus/tests/integration_tests/projects.rs @@ -248,7 +248,7 @@ async fn test_project_deletion_with_image(cptestctx: &ControlPlaneTestContext) { .await; assert_eq!( - "project to be deleted contains an image: alpine-edge", + "project to be deleted contains a project image: alpine-edge", delete_project_expect_fail(&url, &client).await, ); From 68e649a141683ac2b6aa6399c0227cd6f275146f Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Fri, 14 Apr 2023 17:05:56 -0400 Subject: [PATCH 12/27] Attempt to resolve the stub test failure --- nexus/src/app/image.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 0a4a96d877..a6520d110d 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -364,19 +364,17 @@ impl super::Nexus { opctx: &OpContext, image_lookup: &ImageLookup<'_>, ) -> DeleteResult { - let lookup_type = match image_lookup { + match image_lookup { ImageLookup::ProjectImage(lookup) => { - let (.., authz_image) = - lookup.lookup_for(authz::Action::Delete).await?; - LookupType::ById(authz_image.id()) + lookup.lookup_for(authz::Action::Delete).await?; } ImageLookup::SiloImage(lookup) => { - let (.., authz_image) = - lookup.lookup_for(authz::Action::Delete).await?; - LookupType::ById(authz_image.id()) + lookup.lookup_for(authz::Action::Delete).await?; } }; - let error = lookup_type.into_not_found(ResourceType::Image); + let error = Error::InternalError { + internal_message: "Endpoint not implemented".to_string(), + }; Err(self .unimplemented_todo(opctx, Unimpl::ProtectedLookup(error)) .await) From e74f5953304ceffc03428ab931a8c0c8a17ed4db Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 10:36:24 -0400 Subject: [PATCH 13/27] Fix incorrect project image lookup --- nexus/src/app/image.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index a6520d110d..dc0785455f 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -44,7 +44,7 @@ impl super::Nexus { let (.., db_image) = LookupPath::new(opctx, &self.db_datastore) .image_id(id).fetch().await?; let lookup = match db_image.project_id { - Some(id) => ImageLookup::ProjectImage(LookupPath::new(opctx, &self.db_datastore) + Some(_) => ImageLookup::ProjectImage(LookupPath::new(opctx, &self.db_datastore) .project_image_id(id)), None => { ImageLookup::SiloImage(LookupPath::new(opctx, &self.db_datastore) From 239c37a1b067236cf76fb02d7766ed8cd8e64782 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 13:04:48 -0400 Subject: [PATCH 14/27] Fixup authz test for silo images --- .../src/authz/policy_test/resources.rs | 15 +++++ nexus/db-queries/tests/output/authz-roles.out | 62 ++++++++++++++++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/nexus/db-queries/src/authz/policy_test/resources.rs b/nexus/db-queries/src/authz/policy_test/resources.rs index 6c1c4d4536..f60ebd04ae 100644 --- a/nexus/db-queries/src/authz/policy_test/resources.rs +++ b/nexus/db-queries/src/authz/policy_test/resources.rs @@ -188,6 +188,21 @@ async fn make_silo( silo_group_id, LookupType::ByName(format!("{}-group", silo_name)), )); + let silo_image_id = Uuid::new_v4(); + builder.new_resource(authz::SiloImage::new( + silo.clone(), + silo_image_id, + LookupType::ByName(format!("{}-silo-image", silo_name)), + )); + + // Image is a special case in that this resource is technically just a pass-through for + // `SiloImage` and `ProjectImage` resources. + let image_id = Uuid::new_v4(); + builder.new_resource(authz::Image::new( + silo.clone(), + image_id, + LookupType::ByName(format!("{}-image", silo_name)), + )); let nprojects = if first_branch { 2 } else { 1 }; for i in 0..nprojects { diff --git a/nexus/db-queries/tests/output/authz-roles.out b/nexus/db-queries/tests/output/authz-roles.out index bab2a424ac..320e5b8585 100644 --- a/nexus/db-queries/tests/output/authz-roles.out +++ b/nexus/db-queries/tests/output/authz-roles.out @@ -194,6 +194,34 @@ resource: SiloGroup "silo1-group" silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: SiloImage "silo1-silo-image" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + +resource: Image "silo1-image" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Project "silo1-proj1" USER Q R LC RP M MP CC D @@ -292,7 +320,7 @@ resource: Snapshot "silo1-proj1-disk1-snapshot1" silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! -resource: Image "silo1-proj1-image1" +resource: ProjectImage "silo1-proj1-image1" USER Q R LC RP M MP CC D fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ @@ -404,7 +432,7 @@ resource: Snapshot "silo1-proj2-disk1-snapshot1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! -resource: Image "silo1-proj2-image1" +resource: ProjectImage "silo1-proj2-image1" USER Q R LC RP M MP CC D fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ @@ -530,6 +558,34 @@ resource: SiloGroup "silo2-group" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: SiloImage "silo2-silo-image" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + +resource: Image "silo2-image" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Project "silo2-proj1" USER Q R LC RP M MP CC D @@ -628,7 +684,7 @@ resource: Snapshot "silo2-proj1-disk1-snapshot1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! -resource: Image "silo2-proj1-image1" +resource: ProjectImage "silo2-proj1-image1" USER Q R LC RP M MP CC D fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ From 7fd93292fc697664e7b3627b5921e77ee19b5c18 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 13:31:45 -0400 Subject: [PATCH 15/27] Remove insertable from the resources based off of db views --- nexus/db-model/src/image.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index 0d783958bf..997e628c7c 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -42,14 +42,7 @@ pub struct Image { } #[derive( - Queryable, - Insertable, - Selectable, - Clone, - Debug, - Resource, - Serialize, - Deserialize, + Queryable, Selectable, Clone, Debug, Resource, Serialize, Deserialize, )] #[diesel(table_name = project_image)] pub struct ProjectImage { @@ -70,14 +63,7 @@ pub struct ProjectImage { } #[derive( - Queryable, - Insertable, - Selectable, - Clone, - Debug, - Resource, - Serialize, - Deserialize, + Queryable, Selectable, Clone, Debug, Resource, Serialize, Deserialize, )] #[diesel(table_name = silo_image)] pub struct SiloImage { From abfb63491ab5680e557dc02c7f183bf28b7e58da Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 14:37:12 -0400 Subject: [PATCH 16/27] Fixup promotion to have the correct authz (and to actually convert the image to a silo image) --- nexus/db-model/src/image.rs | 7 +++++++ nexus/db-queries/src/db/datastore/image.rs | 5 ++++- nexus/src/app/image.rs | 13 ++++++++++--- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index 997e628c7c..3636e38eae 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -113,6 +113,13 @@ impl TryFrom for ProjectImage { } } +impl From for SiloImage { + fn from(value: ProjectImage) -> Self { + let image: Image = value.into(); + Image { project_id: None, ..image }.try_into().unwrap() + } +} + impl TryFrom for SiloImage { type Error = Error; diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index d171099289..6993ee9f34 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -192,13 +192,16 @@ impl DataStore { &self, opctx: &OpContext, authz_silo: &authz::Silo, + authz_project_image: &authz::ProjectImage, project_image: ProjectImage, ) -> UpdateResult { let silo_id = authz_silo.id(); - let image: Image = project_image.into(); + let silo_image: SiloImage = project_image.into(); + let image: Image = silo_image.into(); let name = image.name().clone(); opctx.authorize(authz::Action::CreateChild, authz_silo).await?; + opctx.authorize(authz::Action::Modify, authz_project_image).await?; use db::schema::image::dsl; let image: Image = Silo::insert_resource( diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index dc0785455f..70bda33e80 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.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/. -//! Images (both project and globally scoped) +//! Images (both project and silo scoped) use super::Unimpl; use crate::authz; @@ -359,6 +359,7 @@ impl super::Nexus { } } + // TODO-MVP: Implement pub async fn image_delete( self: &Arc, opctx: &OpContext, @@ -380,6 +381,7 @@ impl super::Nexus { .await) } + /// Converts a project scoped image into a silo scoped image pub async fn image_promote( self: &Arc, opctx: &OpContext, @@ -387,13 +389,18 @@ impl super::Nexus { ) -> UpdateResult { match image_lookup { ImageLookup::ProjectImage(lookup) => { - let (authz_silo, _, _, db_image) = + let (authz_silo, _, authz_project_image, db_image) = lookup.fetch_for(authz::Action::Modify).await?; opctx .authorize(authz::Action::CreateChild, &authz_silo) .await?; self.db_datastore - .project_image_promote(opctx, &authz_silo, db_image) + .project_image_promote( + opctx, + &authz_silo, + &authz_project_image, + db_image, + ) .await } ImageLookup::SiloImage(_) => Err(Error::InvalidRequest { From 093f09b1592318d71e649c0be5782fc38e199379 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 14:45:43 -0400 Subject: [PATCH 17/27] Ensure project image list w/ silos is scoped to the silo --- nexus/db-queries/src/db/datastore/image.rs | 2 ++ nexus/src/app/image.rs | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 6993ee9f34..5e7b934776 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -31,6 +31,7 @@ impl DataStore { pub async fn project_image_list( &self, opctx: &OpContext, + authz_silo: &authz::Silo, authz_project: &authz::Project, include_silo_images: bool, pagparams: &PaginatedBy<'_>, @@ -51,6 +52,7 @@ impl DataStore { ), } .filter(dsl::time_deleted.is_null()) + .filter(dsl::silo_id.eq(authz_silo.id())) .filter( dsl::project_id .is_null() diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 70bda33e80..a1aa1b71f7 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -338,11 +338,12 @@ impl super::Nexus { ) -> ListResultVec { match parent_lookup { ImageParentLookup::Project(project) => { - let (.., authz_project) = + let (authz_silo, authz_project) = project.lookup_for(authz::Action::ListChildren).await?; self.db_datastore .project_image_list( opctx, + &authz_silo, &authz_project, include_silo_images.unwrap_or(false), pagparams, From c2a605e1acbf3aedac10679179c5e7934d2cbd57 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 16:18:30 -0400 Subject: [PATCH 18/27] Update image create test; default include_silo_images to false --- nexus/src/app/image.rs | 4 +- nexus/tests/integration_tests/images.rs | 58 ++++++++++++++++++++----- nexus/types/src/external_api/params.rs | 3 +- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index a1aa1b71f7..022f25c684 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -333,7 +333,7 @@ impl super::Nexus { &self, opctx: &OpContext, parent_lookup: &ImageParentLookup<'_>, - include_silo_images: Option, + include_silo_images: bool, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { match parent_lookup { @@ -345,7 +345,7 @@ impl super::Nexus { opctx, &authz_silo, &authz_project, - include_silo_images.unwrap_or(false), + include_silo_images, pagparams, ) .await diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 351ba1df12..ae948c1965 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -26,10 +26,14 @@ const PROJECT_NAME: &str = "myproj"; const PROJECT_NAME_2: &str = "myproj2"; -fn get_images_url(project_name: &str) -> String { +fn get_project_images_url(project_name: &str) -> String { format!("/v1/images?project={}", project_name) } +fn get_project_images_with_silo_images_url(project_name: &str) -> String { + format!("/v1/images?project={}&include_silo_images=true", project_name) +} + fn get_image_create(source: params::ImageSource) -> params::ImageCreate { params::ImageCreate { identity: IdentityMetadataCreateParams { @@ -62,7 +66,9 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { ), ); - let images_url = get_images_url(PROJECT_NAME); + let silo_images_url = format!("/v1/images"); + let project_images_url = get_project_images_url(PROJECT_NAME); + let images_url = get_project_images_with_silo_images_url(PROJECT_NAME); // Before project exists, image list 404s NexusRequest::expect_failure( @@ -110,7 +116,8 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { // create another project, which is empty until we promote the image to global create_project(client, PROJECT_NAME_2).await; - let project_2_images_url = get_images_url(PROJECT_NAME_2); + let project_2_images_url = + get_project_images_with_silo_images_url(PROJECT_NAME_2); let images = NexusRequest::object_get(client, &project_2_images_url) .authn_as(AuthnMode::PrivilegedUser) @@ -119,8 +126,37 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { .items; assert_eq!(images.len(), 0); - // TODO: once we reimplement promote, promote image to global and check that - // it shows up in the other project + // promote the image to the silo + let promote_url = format!("/v1/images/{}/promote", images[0].identity.id); + NexusRequest::objects_post(client, &promote_url, &()) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let silo_images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(silo_images.len(), 1); + assert_eq!(images[0].identity.name, "alpine-edge"); + + // Ensure original project images is empty + let images = NexusRequest::object_get(client, &project_images_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + assert_eq!(images.len(), 0); + + // Ensure project 2 images with silos lists the promoted image + let images = NexusRequest::object_get(client, &project_2_images_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + assert_eq!(images.len(), 0); } #[nexus_test] @@ -142,7 +178,7 @@ async fn test_image_create_url_404(cptestctx: &ControlPlaneTestContext) { url: server.url("/image.raw").to_string(), }); - let images_url = get_images_url(PROJECT_NAME); + let images_url = get_project_images_url(PROJECT_NAME); let error = NexusRequest::new( RequestBuilder::new(client, Method::POST, &images_url) @@ -173,7 +209,7 @@ async fn test_image_create_bad_url(cptestctx: &ControlPlaneTestContext) { url: "not_a_url".to_string(), }); - let images_url = get_images_url(PROJECT_NAME); + let images_url = get_project_images_url(PROJECT_NAME); let error = NexusRequest::new( RequestBuilder::new(client, Method::POST, &images_url) @@ -215,7 +251,7 @@ async fn test_image_create_bad_content_length( url: server.url("/image.raw").to_string(), }); - let images_url = get_images_url(PROJECT_NAME); + let images_url = get_project_images_url(PROJECT_NAME); let error = NexusRequest::new( RequestBuilder::new(client, Method::POST, &images_url) @@ -256,7 +292,7 @@ async fn test_image_create_bad_image_size(cptestctx: &ControlPlaneTestContext) { url: server.url("/image.raw").to_string(), }); - let images_url = get_images_url(PROJECT_NAME); + let images_url = get_project_images_url(PROJECT_NAME); let error = NexusRequest::new( RequestBuilder::new(client, Method::POST, &images_url) @@ -300,7 +336,7 @@ async fn test_make_disk_from_image(cptestctx: &ControlPlaneTestContext) { url: server.url("/alpine/edge.raw").to_string(), }); - let images_url = get_images_url(PROJECT_NAME); + let images_url = get_project_images_url(PROJECT_NAME); let alpine_image = NexusRequest::objects_post(client, &images_url, &image_create_params) @@ -351,7 +387,7 @@ async fn test_make_disk_from_image_too_small( url: server.url("/alpine/edge.raw").to_string(), }); - let images_url = get_images_url(PROJECT_NAME); + let images_url = get_project_images_url(PROJECT_NAME); let alpine_image = NexusRequest::objects_post(client, &images_url, &image_create_params) diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index aa512d094d..935a663864 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -123,7 +123,8 @@ pub struct ImageListSelector { pub project: Option, /// Flag used to indicate if silo scoped images should be included when /// listing project images. Only valid when `project` is provided. - pub include_silo_images: Option, + #[serde(default)] + pub include_silo_images: bool, } #[derive(Deserialize, JsonSchema)] From fd48b2103bd49cca96d8f72bbf292e7c8df5da4d Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 17:23:40 -0400 Subject: [PATCH 19/27] Update nexus/types/src/external_api/params.rs Co-authored-by: David Crespo --- nexus/types/src/external_api/params.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 935a663864..82e1e25bc7 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -115,7 +115,7 @@ pub struct SnapshotSelector { pub snapshot: NameOrId, } -/// A specialized selector for image list, it contains an extra feield to indicate +/// A specialized selector for image list, it contains an extra field to indicate /// if silo scoped images should be included when listing project images. #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema, PartialEq)] pub struct ImageListSelector { From d48460b99b20b4eb3066ef18ddc28a5f42dbefeb Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 17 Apr 2023 18:08:43 -0400 Subject: [PATCH 20/27] Fix parsing issues w/ include_silo_images flag --- nexus/src/external_api/http_entrypoints.rs | 2 +- nexus/tests/integration_tests/images.rs | 16 ++++++---- nexus/types/src/external_api/params.rs | 35 +++++++++++++++++++++- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 6731b39811..f4366de5a2 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -2417,7 +2417,7 @@ async fn image_list( .image_list( &opctx, &parent_lookup, - scan_params.selector.include_silo_images, + scan_params.selector.include_silo_images.unwrap_or(false), &paginated_by, ) .await? diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index ae948c1965..9337d4aa88 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -127,11 +127,17 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { assert_eq!(images.len(), 0); // promote the image to the silo - let promote_url = format!("/v1/images/{}/promote", images[0].identity.id); - NexusRequest::objects_post(client, &promote_url, &()) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap::() - .await; + let promote_url = format!( + "/v1/images/{}/promote?project={}", + "alpine-edge", PROJECT_NAME + ); + NexusRequest::new( + RequestBuilder::new(client, http::Method::POST, &promote_url) + .expect_status(Some(http::StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::() + .await; let silo_images = NexusRequest::object_get(client, &silo_images_url) .authn_as(AuthnMode::PrivilegedUser) diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 82e1e25bc7..dfd6ec3c05 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -124,7 +124,40 @@ pub struct ImageListSelector { /// Flag used to indicate if silo scoped images should be included when /// listing project images. Only valid when `project` is provided. #[serde(default)] - pub include_silo_images: bool, + #[serde(deserialize_with = "deserialize_optional_bool_from_string")] + pub include_silo_images: Option, +} + +// Unfortunately `include_silo_images` can't used the default `Deserialize` +// derive given the selector that uses it is embedded via `serde(flatten)` which +// causes it to attempt to deserialize all flattened values a string. Similar workarounds +// have been implemented here: https://github.com/oxidecomputer/omicron/blob/efb03b501d7febe961cc8793b4d72e8542d28eab/gateway/src/http_entrypoints.rs#L443 +fn deserialize_optional_bool_from_string<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + use serde::de::Unexpected; + + #[derive(Debug, Deserialize)] + #[serde(untagged)] + enum StringOrOptionalBool { + String(String), + OptionalBool(Option), + } + + match StringOrOptionalBool::deserialize(deserializer)? { + StringOrOptionalBool::String(s) => match s.as_str() { + "true" => Ok(Some(true)), + "false" => Ok(None), + "" => Ok(None), + _ => { + Err(de::Error::invalid_type(Unexpected::Str(&s), &"a boolean")) + } + }, + StringOrOptionalBool::OptionalBool(b) => Ok(b), + } } #[derive(Deserialize, JsonSchema)] From da9d1e75cec346a5b5d5ff16f4b12c86624e59d8 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 12:37:38 -0400 Subject: [PATCH 21/27] Change promote query to an update --- nexus/db-model/src/image.rs | 22 +++++++++----- nexus/db-queries/src/db/datastore/image.rs | 35 ++++++++-------------- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index 3636e38eae..b76f420f02 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -4,6 +4,7 @@ use super::{BlockSize, ByteCount, Digest}; use crate::schema::{image, project_image, silo_image}; +use chrono::{DateTime, Utc}; use db_macros::Resource; use nexus_types::external_api::views; use nexus_types::identity::Resource; @@ -113,13 +114,6 @@ impl TryFrom for ProjectImage { } } -impl From for SiloImage { - fn from(value: ProjectImage) -> Self { - let image: Image = value.into(); - Image { project_id: None, ..image }.try_into().unwrap() - } -} - impl TryFrom for SiloImage { type Error = Error; @@ -212,3 +206,17 @@ impl From for views::Image { } } } + +// Changeset used to +#[derive(AsChangeset)] +#[diesel(table_name = image)] +pub struct ImagePromotionUpdate { + pub project_id: Option, + pub time_modified: DateTime, +} + +impl From for ImagePromotionUpdate { + fn from(_image: ProjectImage) -> Self { + Self { project_id: None, time_modified: Utc::now() } + } +} diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 5e7b934776..382b5152ed 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -1,4 +1,5 @@ use diesel::prelude::*; +use nexus_db_model::ImagePromotionUpdate; use nexus_db_model::Name; use nexus_types::identity::Resource; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -197,37 +198,25 @@ impl DataStore { authz_project_image: &authz::ProjectImage, project_image: ProjectImage, ) -> UpdateResult { - let silo_id = authz_silo.id(); - let silo_image: SiloImage = project_image.into(); - let image: Image = silo_image.into(); - let name = image.name().clone(); + let image_update: ImagePromotionUpdate = project_image.into(); opctx.authorize(authz::Action::CreateChild, authz_silo).await?; opctx.authorize(authz::Action::Modify, authz_project_image).await?; use db::schema::image::dsl; - let image: Image = Silo::insert_resource( - silo_id, - diesel::insert_into(dsl::image) - .values(image) - .on_conflict(dsl::id) - .do_update() - .set(dsl::time_modified.eq(dsl::time_modified)), - ) - .insert_and_get_result_async(self.pool_authorized(opctx).await?) - .await - .map_err(|e| match e { - AsyncInsertError::CollectionNotFound => authz_silo.not_found(), - AsyncInsertError::DatabaseError(e) => { + let image: Image = diesel::update(dsl::image) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(authz_project_image.id())) + .set(image_update) + .returning(Image::as_returning()) + .get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| { public_error_from_diesel_pool( e, - ErrorHandler::Conflict( - ResourceType::SiloImage, - name.as_str(), - ), + ErrorHandler::NotFoundByResource(authz_project_image), ) - } - })?; + })?; Ok(image) } } From ae6779329cc5df8a95e255377b59b9ad8cc4c52f Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 14:14:21 -0400 Subject: [PATCH 22/27] Add silo image creation test --- nexus/tests/integration_tests/images.rs | 49 +++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 9337d4aa88..ecfd6f0f43 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -165,6 +165,55 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { assert_eq!(images.len(), 0); } +#[nexus_test] +async fn test_silo_image_create(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + DiskTest::new(&cptestctx).await; + + let server = ServerBuilder::new().run().unwrap(); + server.expect( + Expectation::matching(request::method_path("HEAD", "/image.raw")) + .times(1..) + .respond_with( + status_code(200).append_header( + "Content-Length", + format!("{}", 4096 * 1000), + ), + ), + ); + + let silo_images_url = format!("/v1/images"); + + // Expect no images in the silo + let images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(images.len(), 0); + + // Create an image in the project + let image_create_params = get_image_create(params::ImageSource::Url { + url: server.url("/image.raw").to_string(), + }); + + // Create image + NexusRequest::objects_post(client, &silo_images_url, &image_create_params) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::() + .await; + + let images = NexusRequest::object_get(client, &silo_images_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap::>() + .await + .items; + + assert_eq!(images.len(), 1); + assert_eq!(images[0].identity.name, "alpine-edge"); +} + #[nexus_test] async fn test_image_create_url_404(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; From 5d9dd3f3046e19c353cf3f2ecd526824f233f5d3 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 15:26:14 -0400 Subject: [PATCH 23/27] Fix image promotion process Co-authored-by: David Crespo --- nexus/db-model/src/image.rs | 14 -------------- nexus/db-queries/src/db/datastore/image.rs | 11 ++++++----- nexus/src/app/image.rs | 5 ++--- nexus/tests/integration_tests/images.rs | 4 ++-- 4 files changed, 10 insertions(+), 24 deletions(-) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index b76f420f02..e75b84c903 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -206,17 +206,3 @@ impl From for views::Image { } } } - -// Changeset used to -#[derive(AsChangeset)] -#[diesel(table_name = image)] -pub struct ImagePromotionUpdate { - pub project_id: Option, - pub time_modified: DateTime, -} - -impl From for ImagePromotionUpdate { - fn from(_image: ProjectImage) -> Self { - Self { project_id: None, time_modified: Utc::now() } - } -} diff --git a/nexus/db-queries/src/db/datastore/image.rs b/nexus/db-queries/src/db/datastore/image.rs index 382b5152ed..6ad705c5e5 100644 --- a/nexus/db-queries/src/db/datastore/image.rs +++ b/nexus/db-queries/src/db/datastore/image.rs @@ -1,5 +1,5 @@ +use chrono::Utc; use diesel::prelude::*; -use nexus_db_model::ImagePromotionUpdate; use nexus_db_model::Name; use nexus_types::identity::Resource; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -25,6 +25,7 @@ use crate::db::model::SiloImage; use crate::db::pagination::paginated; use async_bb8_diesel::AsyncRunQueryDsl; +use uuid::Uuid; use super::DataStore; @@ -196,10 +197,7 @@ impl DataStore { opctx: &OpContext, authz_silo: &authz::Silo, authz_project_image: &authz::ProjectImage, - project_image: ProjectImage, ) -> UpdateResult { - let image_update: ImagePromotionUpdate = project_image.into(); - opctx.authorize(authz::Action::CreateChild, authz_silo).await?; opctx.authorize(authz::Action::Modify, authz_project_image).await?; @@ -207,7 +205,10 @@ impl DataStore { let image: Image = diesel::update(dsl::image) .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(authz_project_image.id())) - .set(image_update) + .set(( + dsl::project_id.eq(None::), + dsl::time_modified.eq(Utc::now()), + )) .returning(Image::as_returning()) .get_result_async(self.pool_authorized(opctx).await?) .await diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 022f25c684..5ad484af81 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -390,8 +390,8 @@ impl super::Nexus { ) -> UpdateResult { match image_lookup { ImageLookup::ProjectImage(lookup) => { - let (authz_silo, _, authz_project_image, db_image) = - lookup.fetch_for(authz::Action::Modify).await?; + let (authz_silo, _, authz_project_image) = + lookup.lookup_for(authz::Action::Modify).await?; opctx .authorize(authz::Action::CreateChild, &authz_silo) .await?; @@ -400,7 +400,6 @@ impl super::Nexus { opctx, &authz_silo, &authz_project_image, - db_image, ) .await } diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index ecfd6f0f43..5181ded5b1 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -146,7 +146,7 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { .items; assert_eq!(silo_images.len(), 1); - assert_eq!(images[0].identity.name, "alpine-edge"); + assert_eq!(silo_images[0].identity.name, "alpine-edge"); // Ensure original project images is empty let images = NexusRequest::object_get(client, &project_images_url) @@ -162,7 +162,7 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { .execute_and_parse_unwrap::>() .await .items; - assert_eq!(images.len(), 0); + assert_eq!(images.len(), 1); } #[nexus_test] From 28c87cbfce19cad213200d4ca1ddd1b228ed62a4 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 15:55:51 -0400 Subject: [PATCH 24/27] Remove unused import --- nexus/db-model/src/image.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index e75b84c903..997e628c7c 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -4,7 +4,6 @@ use super::{BlockSize, ByteCount, Digest}; use crate::schema::{image, project_image, silo_image}; -use chrono::{DateTime, Utc}; use db_macros::Resource; use nexus_types::external_api::views; use nexus_types::identity::Resource; From c96d48424cc9fa3f55b6013a02c9e545266f3071 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 16:04:19 -0400 Subject: [PATCH 25/27] Add module docs for image models co-authored-by: David Crespo --- nexus/db-model/src/image.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index 997e628c7c..91a9469d30 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -2,6 +2,12 @@ // 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/. +//! Image, ProjectImage, and SiloImage are almost identical. Image is the model +//! for the underlying table and ProjectImage and SiloImage are backed by views +//! of that table. They have all the same fields except ProjectImage has both a +//! silo_id and a project_id, while SiloImage only has a silo_id. Image has a +//! silo_id and an optional project_id to cover both possibilities. + use super::{BlockSize, ByteCount, Digest}; use crate::schema::{image, project_image, silo_image}; use db_macros::Resource; From c288e85862c4ff5f2cbb190eabce299713988ccb Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 17:02:11 -0400 Subject: [PATCH 26/27] Fix clippy issues --- nexus/tests/integration_tests/images.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index 5181ded5b1..3a013255a8 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -66,7 +66,7 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { ), ); - let silo_images_url = format!("/v1/images"); + let silo_images_url = "/v1/images"; let project_images_url = get_project_images_url(PROJECT_NAME); let images_url = get_project_images_with_silo_images_url(PROJECT_NAME); @@ -182,7 +182,7 @@ async fn test_silo_image_create(cptestctx: &ControlPlaneTestContext) { ), ); - let silo_images_url = format!("/v1/images"); + let silo_images_url = "/v1/images"; // Expect no images in the silo let images = NexusRequest::object_get(client, &silo_images_url) From 2f9ef0cebf8ff21e8cb7fbda64d2fdd6d4bc9034 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 18 Apr 2023 19:59:16 -0400 Subject: [PATCH 27/27] Add promote endpoint to authz coverage --- nexus/tests/integration_tests/endpoints.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 0924678fe2..489176d45a 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -346,6 +346,8 @@ lazy_static! { format!("/v1/images?project={}", *DEMO_PROJECT_NAME); pub static ref DEMO_PROJECT_IMAGE_URL: String = format!("/v1/images/{}?project={}", *DEMO_IMAGE_NAME, *DEMO_PROJECT_NAME); + pub static ref DEMO_PROJECT_PROMOTE_IMAGE_URL: String = + format!("/v1/images/{}/promote?project={}", *DEMO_IMAGE_NAME, *DEMO_PROJECT_NAME); pub static ref DEMO_IMAGE_CREATE: params::ImageCreate = params::ImageCreate { identity: IdentityMetadataCreateParams { @@ -1219,6 +1221,15 @@ lazy_static! { ], }, + VerifyEndpoint { + url: &DEMO_PROJECT_PROMOTE_IMAGE_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post(serde_json::value::Value::Null), + ], + }, + /* Snapshots */ VerifyEndpoint {