[nexus] Safer project deletion (#2024)

## Previously

Project deletion was very problematic - even though projects contain
many resources, they could be deleted while their children objects still
exist. This would effectively "orphan" those resources, making them
inaccessible.

For example: Create an instance in a project, delete the project. Now
the instance exists, but cannot be deleted.

## This PR

Adds `rcgen` to projects, and ensures that they can no longer be deleted
while containing child resources.

- Implements `DatastoreCollectionConfig<ChildResource>` for `Project`,
for many child resources
- Uses `Project::insert_resource` to bump `rcgen` while creating child
resources
- Checks for child resources existing during project deletion
- Adds tests
 
Fixes #1482

Author: smklein

Cargo.lock

@@ -537,6 +537,9 @@ CREATE TABLE omicron.public.project (
     /* Indicates that the object has been deleted */
     time_deleted TIMESTAMPTZ,
 
+    /* child resource generation number, per RFD 192 */
+    rcgen INT NOT NULL,
+
     /* Which organization this project belongs to */
     organization_id UUID NOT NULL /* foreign key into "Organization" table */
 );

common/src/sql/dbinit.sql

@@ -2,7 +2,9 @@ use crate::helpers::generate_name;
 use anyhow::{Context as _, Result};
 use omicron_sled_agent::rack_setup::config::SetupServiceConfig;
 use oxide_client::types::{Name, OrganizationCreate, ProjectCreate};
-use oxide_client::{Client, ClientOrganizationsExt, ClientProjectsExt};
+use oxide_client::{
+    Client, ClientOrganizationsExt, ClientProjectsExt, ClientVpcsExt,
+};
 use reqwest::header::{HeaderMap, HeaderValue, AUTHORIZATION};
 use reqwest::Url;
 use std::net::SocketAddr;
@@ -49,6 +51,21 @@ impl Context {
     }
 
     pub async fn cleanup(self) -> Result<()> {
+        self.client
+            .vpc_subnet_delete()
+            .organization_name(self.org_name.clone())
+            .project_name(self.project_name.clone())
+            .vpc_name("default")
+            .subnet_name("default")
+            .send()
+            .await?;
+        self.client
+            .vpc_delete()
+            .organization_name(self.org_name.clone())
+            .project_name(self.project_name.clone())
+            .vpc_name("default")
+            .send()
+            .await?;
         self.client
             .project_delete()
             .organization_name(self.org_name.clone())

end-to-end-tests/src/helpers/ctx.rs

@@ -61,13 +61,14 @@ async fn instance_launch() -> Result<()> {
         .id;
 
     eprintln!("create disk");
+    let disk_name = generate_name("disk")?;
     let disk_name = ctx
         .client
         .disk_create()
         .organization_name(ctx.org_name.clone())
         .project_name(ctx.project_name.clone())
         .body(DiskCreate {
-            name: generate_name("disk")?,
+            name: disk_name.clone(),
             description: String::new(),
             disk_source: DiskSource::GlobalImage { image_id },
             size: ByteCount(2048 * 1024 * 1024),
@@ -89,7 +90,9 @@ async fn instance_launch() -> Result<()> {
             hostname: "localshark".into(), // 🦈
             memory: ByteCount(1024 * 1024 * 1024),
             ncpus: InstanceCpuCount(2),
-            disks: vec![InstanceDiskAttachment::Attach { name: disk_name }],
+            disks: vec![InstanceDiskAttachment::Attach {
+                name: disk_name.clone(),
+            }],
             network_interfaces: InstanceNetworkInterfaceAttachment::Default,
             external_ips: vec![ExternalIpCreate::Ephemeral { pool_name: None }],
             user_data: String::new(),
@@ -245,6 +248,23 @@ async fn instance_launch() -> Result<()> {
     )
     .await?;
 
+    eprintln!("deleting disk");
+    wait_for_condition(
+        || async {
+            ctx.client
+                .disk_delete()
+                .organization_name(ctx.org_name.clone())
+                .project_name(ctx.project_name.clone())
+                .disk_name(disk_name.clone())
+                .send()
+                .await
+                .map_err(|_| CondCheckError::<oxide_client::Error>::NotYet)
+        },
+        &Duration::from_secs(1),
+        &Duration::from_secs(60),
+    )
+    .await?;
+
     ctx.cleanup().await
 }
 

end-to-end-tests/src/instance_launch.rs

@@ -45,6 +45,7 @@ oso = "0.26"
 oximeter-client = { path = "../oximeter-client" }
 oximeter-db = { path = "../oximeter/db/" }
 parse-display = "0.7.0"
+paste = "1.0"
 # See omicron-rpaths for more about the "pq-sys" dependency.
 pq-sys = "*"
 propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "b596e72b6b93bc412d675358f782ae7d53f8bf7a", features = [ "generated" ] }

nexus/Cargo.toml

@@ -2,8 +2,13 @@
 // 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::Name;
-use crate::schema::project;
+use super::{
+    Disk, ExternalIp, Generation, Image, Instance, IpPool, Name, Snapshot, Vpc,
+};
+use crate::collection::DatastoreCollectionConfig;
+use crate::schema::{
+    disk, external_ip, image, instance, ip_pool, project, snapshot, vpc,
+};
 use chrono::{DateTime, Utc};
 use db_macros::Resource;
 use nexus_types::external_api::params;
@@ -18,6 +23,8 @@ pub struct Project {
     #[diesel(embed)]
     identity: ProjectIdentity,
 
+    /// child resource generation number, per RFD 192
+    pub rcgen: Generation,
     pub organization_id: Uuid,
 }
 
@@ -26,6 +33,7 @@ impl Project {
     pub fn new(organization_id: Uuid, params: params::ProjectCreate) -> Self {
         Self {
             identity: ProjectIdentity::new(Uuid::new_v4(), params.identity),
+            rcgen: Generation::new(),
             organization_id,
         }
     }
@@ -40,6 +48,61 @@ impl From<Project> for views::Project {
     }
 }
 
+impl DatastoreCollectionConfig<Instance> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = instance::dsl::project_id;
+}
+
+impl DatastoreCollectionConfig<Disk> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = disk::dsl::project_id;
+}
+
+impl DatastoreCollectionConfig<Image> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = image::dsl::project_id;
+}
+
+impl DatastoreCollectionConfig<Snapshot> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = snapshot::dsl::project_id;
+}
+
+impl DatastoreCollectionConfig<Vpc> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = vpc::dsl::project_id;
+}
+
+// NOTE: "IpPoolRange" also contains a reference to "project_id", but
+// ranges should only exist within IP Pools.
+impl DatastoreCollectionConfig<IpPool> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = ip_pool::dsl::project_id;
+}
+
+// TODO(https://github.com/oxidecomputer/omicron/issues/1482): Not yet utilized,
+// but needed for project deletion safety.
+// TODO(https://github.com/oxidecomputer/omicron/issues/1334): Cannot be
+// utilized until floating IPs are implemented.
+impl DatastoreCollectionConfig<ExternalIp> for Project {
+    type CollectionId = Uuid;
+    type GenerationNumberColumn = project::dsl::rcgen;
+    type CollectionTimeDeletedColumn = project::dsl::time_deleted;
+    type CollectionIdColumn = external_ip::dsl::project_id;
+}
+
 /// Describes a set of updates for the [`Project`] model.
 #[derive(AsChangeset)]
 #[diesel(table_name = project)]

nexus/db-model/src/project.rs

@@ -330,6 +330,7 @@ table! {
         time_created -> Timestamptz,
         time_modified -> Timestamptz,
         time_deleted -> Nullable<Timestamptz>,
+        rcgen -> Int8,
         organization_id -> Uuid,
     }
 }

nexus/db-model/src/schema.rs

@@ -40,6 +40,10 @@ impl super::Nexus {
             .project_name(project_name)
             .lookup_for(authz::Action::CreateChild)
             .await?;
+
+        // TODO(https://github.com/oxidecomputer/omicron/issues/1482): When
+        // we implement this, remember to insert the image within a Project
+        // using "insert_resource".
         Err(self.unimplemented_todo(opctx, Unimpl::Public).await)
     }
 

nexus/src/app/image.rs

@@ -197,12 +197,15 @@ impl super::Nexus {
         organization_name: &Name,
         project_name: &Name,
     ) -> DeleteResult {
-        let (.., authz_project) = LookupPath::new(opctx, &self.db_datastore)
-            .organization_name(organization_name)
-            .project_name(project_name)
-            .lookup_for(authz::Action::Delete)
-            .await?;
-        self.db_datastore.project_delete(opctx, &authz_project).await
+        let (.., authz_project, db_project) =
+            LookupPath::new(opctx, &self.db_datastore)
+                .organization_name(organization_name)
+                .project_name(project_name)
+                .fetch_for(authz::Action::Delete)
+                .await?;
+        self.db_datastore
+            .project_delete(opctx, &authz_project, &db_project)
+            .await
     }
 
     // Role assignments

nexus/src/app/project.rs

@@ -200,9 +200,15 @@ async fn sdc_create_disk_record(
         ActionError::action_failed(Error::invalid_request(&e.to_string()))
     })?;
 
+    let (.., authz_project) = LookupPath::new(&opctx, &osagactx.datastore())
+        .project_id(params.project_id)
+        .lookup_for(authz::Action::CreateChild)
+        .await
+        .map_err(ActionError::action_failed)?;
+
     let disk_created = osagactx
         .datastore()
-        .project_create_disk(disk)
+        .project_create_disk(&opctx, &authz_project, disk)
         .await
         .map_err(ActionError::action_failed)?;