replace TUF artifact primary key with a UUID (#7340)

`(name, version, kind)` is a natural key for the `tuf_artifact` table:
given a kind and name for an artifact, we should only ever have one copy
of any particular version.

However, this natural key does not distribute well, which [makes it a
poor choice for a primary key in CockroachDB][1]. It also makes
pagination more difficult.

Instead of altering the schema we drop the tables and recreate them, as
any potential contents would be useless to us; until #7129 lands we have
no means to store the artifacts from a repository.

[1]: https://www.cockroachlabs.com/blog/how-to-choose-a-primary-key/

Author: iliana

nexus/auth/src/authz/api_resources.rs

@@ -38,7 +38,6 @@ use authz_macros::authz_resource;
 use futures::future::BoxFuture;
 use futures::FutureExt;
 use nexus_db_fixed_data::FLEET_ID;
-use nexus_db_model::{ArtifactId, SemverVersion};
 use nexus_types::external_api::shared::{FleetRole, ProjectRole, SiloRole};
 use omicron_common::api::external::{Error, LookupType, ResourceType};
 use once_cell::sync::Lazy;
@@ -1014,8 +1013,7 @@ authz_resource! {
 authz_resource! {
     name = "TufArtifact",
     parent = "Fleet",
-    primary_key = (String, SemverVersion, String),
-    input_key = ArtifactId,
+    primary_key = { uuid_kind = TufArtifactKind },
     roles_allowed = false,
     polar_snippet = FleetChild,
 }

nexus/db-model/src/schema.rs

@@ -1336,7 +1336,8 @@ table! {
 }
 
 table! {
-    tuf_artifact (name, version, kind) {
+    tuf_artifact (id) {
+        id -> Uuid,
         name -> Text,
         version -> Text,
         kind -> Text,
@@ -1347,11 +1348,9 @@ table! {
 }
 
 table! {
-    tuf_repo_artifact (tuf_repo_id, tuf_artifact_name, tuf_artifact_version, tuf_artifact_kind) {
+    tuf_repo_artifact (tuf_repo_id, tuf_artifact_id) {
         tuf_repo_id -> Uuid,
-        tuf_artifact_name -> Text,
-        tuf_artifact_version -> Text,
-        tuf_artifact_kind -> Text,
+        tuf_artifact_id -> Uuid,
     }
 }
 
@@ -1361,8 +1360,7 @@ allow_tables_to_appear_in_same_query!(
     tuf_artifact
 );
 joinable!(tuf_repo_artifact -> tuf_repo (tuf_repo_id));
-// Can't specify joinable for a composite primary key (tuf_repo_artifact ->
-// tuf_artifact).
+joinable!(tuf_repo_artifact -> tuf_artifact (tuf_artifact_id));
 
 table! {
     support_bundle {

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(118, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(119, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(119, "tuf-artifact-key-uuid"),
         KnownVersion::new(118, "support-bundles"),
         KnownVersion::new(117, "add-completing-and-new-region-volume"),
         KnownVersion::new(116, "bp-physical-disk-disposition"),

nexus/db-model/src/tuf_repo.rs

@@ -13,11 +13,9 @@ use chrono::{DateTime, Utc};
 use diesel::{deserialize::FromSql, serialize::ToSql, sql_types::Text};
 use omicron_common::{
     api::external,
-    update::{
-        ArtifactHash as ExternalArtifactHash, ArtifactId as ExternalArtifactId,
-        ArtifactKind,
-    },
+    update::{ArtifactHash as ExternalArtifactHash, ArtifactId, ArtifactKind},
 };
+use omicron_uuid_kinds::TufArtifactKind;
 use omicron_uuid_kinds::TufRepoKind;
 use omicron_uuid_kinds::TypedUuid;
 use serde::{Deserialize, Serialize};
@@ -148,8 +146,10 @@ impl TufRepo {
 #[derive(Queryable, Insertable, Clone, Debug, Selectable, AsChangeset)]
 #[diesel(table_name = tuf_artifact)]
 pub struct TufArtifact {
-    #[diesel(embed)]
-    pub id: ArtifactId,
+    pub id: DbTypedUuid<TufArtifactKind>,
+    pub name: String,
+    pub version: SemverVersion,
+    pub kind: String,
     pub time_created: DateTime<Utc>,
     pub sha256: ArtifactHash,
     artifact_size: i64,
@@ -158,12 +158,15 @@ pub struct TufArtifact {
 impl TufArtifact {
     /// Creates a new `TufArtifact` ready for insertion.
     pub fn new(
-        id: ArtifactId,
+        artifact_id: ArtifactId,
         sha256: ArtifactHash,
         artifact_size: u64,
     ) -> Self {
         Self {
-            id,
+            id: TypedUuid::new_v4().into(),
+            name: artifact_id.name,
+            version: artifact_id.version.into(),
+            kind: artifact_id.kind.as_str().to_owned(),
             time_created: Utc::now(),
             sha256,
             artifact_size: artifact_size as i64,
@@ -177,21 +180,31 @@ impl TufArtifact {
     /// as part of the process, which `From` doesn't necessarily communicate
     /// and can be surprising.
     pub fn from_external(artifact: external::TufArtifactMeta) -> Self {
-        Self::new(artifact.id.into(), artifact.hash.into(), artifact.size)
+        Self::new(artifact.id, artifact.hash.into(), artifact.size)
     }
 
     /// Converts self into [`external::TufArtifactMeta`].
     pub fn into_external(self) -> external::TufArtifactMeta {
         external::TufArtifactMeta {
-            id: self.id.into(),
+            id: ArtifactId {
+                name: self.name,
+                version: self.version.into(),
+                kind: ArtifactKind::new(self.kind),
+            },
             hash: self.sha256.into(),
             size: self.artifact_size as u64,
         }
     }
 
     /// Returns the artifact's ID.
-    pub fn id(&self) -> (String, SemverVersion, String) {
-        (self.id.name.clone(), self.id.version.clone(), self.id.kind.clone())
+    pub fn id(&self) -> TypedUuid<TufArtifactKind> {
+        self.id.into()
+    }
+
+    /// Returns the artifact's name, version, and kind, which is unique across
+    /// all artifacts.
+    pub fn nvk(&self) -> (&str, &SemverVersion, &str) {
+        (&self.name, &self.version, &self.kind)
     }
 
     /// Returns the artifact length in bytes.
@@ -200,70 +213,12 @@ impl TufArtifact {
     }
 }
 
-/// The ID (primary key) of a [`TufArtifact`].
-///
-/// This is the internal variant of a [`ExternalArtifactId`].
-#[derive(
-    Queryable,
-    Insertable,
-    Clone,
-    Debug,
-    Selectable,
-    PartialEq,
-    Eq,
-    Hash,
-    Deserialize,
-    Serialize,
-)]
-#[diesel(table_name = tuf_artifact)]
-pub struct ArtifactId {
-    pub name: String,
-    pub version: SemverVersion,
-    pub kind: String,
-}
-
-impl From<ExternalArtifactId> for ArtifactId {
-    fn from(id: ExternalArtifactId) -> Self {
-        Self {
-            name: id.name,
-            version: id.version.into(),
-            kind: id.kind.as_str().to_owned(),
-        }
-    }
-}
-
-impl From<ArtifactId> for ExternalArtifactId {
-    fn from(id: ArtifactId) -> Self {
-        Self {
-            name: id.name,
-            version: id.version.into(),
-            kind: ArtifactKind::new(id.kind),
-        }
-    }
-}
-
-impl fmt::Display for ArtifactId {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // This is the same as ExternalArtifactId's Display impl.
-        write!(f, "{} v{} ({})", self.name, self.version, self.kind)
-    }
-}
-
-/// Required by the authz_resource macro.
-impl From<ArtifactId> for (String, SemverVersion, String) {
-    fn from(id: ArtifactId) -> Self {
-        (id.name, id.version, id.kind)
-    }
-}
-
 /// A many-to-many relationship between [`TufRepo`] and [`TufArtifact`].
 #[derive(Queryable, Insertable, Clone, Debug, Selectable)]
 #[diesel(table_name = tuf_repo_artifact)]
 pub struct TufRepoArtifact {
     pub tuf_repo_id: Uuid,
-    pub tuf_artifact_name: String,
-    pub tuf_artifact_version: SemverVersion,
-    pub tuf_artifact_kind: String,
+    pub tuf_artifact_id: Uuid,
 }
 
 /// A wrapper around omicron-common's [`ArtifactHash`](ExternalArtifactHash),

nexus/db-queries/src/db/datastore/update.rs

@@ -50,15 +50,8 @@ async fn artifacts_for_repo(
     use db::schema::tuf_artifact::dsl as tuf_artifact_dsl;
     use db::schema::tuf_repo_artifact::dsl as tuf_repo_artifact_dsl;
 
-    let join_on_dsl = tuf_artifact_dsl::name
-        .eq(tuf_repo_artifact_dsl::tuf_artifact_name)
-        .and(
-            tuf_artifact_dsl::version
-                .eq(tuf_repo_artifact_dsl::tuf_artifact_version),
-        )
-        .and(
-            tuf_artifact_dsl::kind.eq(tuf_repo_artifact_dsl::tuf_artifact_kind),
-        );
+    let join_on_dsl =
+        tuf_artifact_dsl::id.eq(tuf_repo_artifact_dsl::tuf_artifact_id);
     // Don't bother paginating because each repo should only have a few (under
     // 20) artifacts.
     tuf_repo_artifact_dsl::tuf_repo_artifact
@@ -215,9 +208,9 @@ async fn insert_impl(
         for artifact in desc.artifacts.clone() {
             filter_dsl = filter_dsl.or_filter(
                 dsl::name
-                    .eq(artifact.id.name)
-                    .and(dsl::version.eq(artifact.id.version))
-                    .and(dsl::kind.eq(artifact.id.kind)),
+                    .eq(artifact.name)
+                    .and(dsl::version.eq(artifact.version))
+                    .and(dsl::kind.eq(artifact.kind)),
             );
         }
 
@@ -233,7 +226,7 @@ async fn insert_impl(
 
         let results_by_id = results
             .iter()
-            .map(|artifact| (&artifact.id, artifact))
+            .map(|artifact| (artifact.nvk(), artifact))
             .collect::<HashMap<_, _>>();
 
         // uploaded_and_existing contains non-matching artifacts in pairs of
@@ -244,7 +237,7 @@ async fn insert_impl(
 
         for uploaded_artifact in desc.artifacts.clone() {
             let Some(&existing_artifact) =
-                results_by_id.get(&uploaded_artifact.id)
+                results_by_id.get(&uploaded_artifact.nvk())
             else {
                 // This is a new artifact.
                 new_artifacts.push(uploaded_artifact.clone());
@@ -301,9 +294,7 @@ async fn insert_impl(
             );
             values.push((
                 dsl::tuf_repo_id.eq(desc.repo.id),
-                dsl::tuf_artifact_name.eq(artifact.id.name),
-                dsl::tuf_artifact_version.eq(artifact.id.version),
-                dsl::tuf_artifact_kind.eq(artifact.id.kind),
+                dsl::tuf_artifact_id.eq(artifact.id),
             ));
         }
 

nexus/db-queries/src/db/lookup.rs

@@ -22,6 +22,7 @@ use omicron_common::api::external::Error;
 use omicron_common::api::external::InternalContext;
 use omicron_common::api::external::{LookupResult, LookupType, ResourceType};
 use omicron_uuid_kinds::PhysicalDiskUuid;
+use omicron_uuid_kinds::TufArtifactKind;
 use omicron_uuid_kinds::TufRepoKind;
 use omicron_uuid_kinds::TypedUuid;
 use uuid::Uuid;
@@ -446,18 +447,11 @@ impl<'a> LookupPath<'a> {
 
     /// Select a resource of type UpdateArtifact, identified by its
     /// `(name, version, kind)` tuple
-    pub fn tuf_artifact_tuple(
+    pub fn tuf_artifact_id(
         self,
-        name: impl Into<String>,
-        version: db::model::SemverVersion,
-        kind: impl Into<String>,
+        id: TypedUuid<TufArtifactKind>,
     ) -> TufArtifact<'a> {
-        TufArtifact::PrimaryKey(
-            Root { lookup_root: self },
-            name.into(),
-            version,
-            kind.into(),
-        )
+        TufArtifact::PrimaryKey(Root { lookup_root: self }, id)
     }
 
     /// Select a resource of type UserBuiltin, identified by its `name`
@@ -895,11 +889,7 @@ lookup_resource! {
     children = [],
     lookup_by_name = false,
     soft_deletes = false,
-    primary_key_columns = [
-        { column_name = "name", rust_type = String },
-        { column_name = "version", rust_type = db::model::SemverVersion },
-        { column_name = "kind", rust_type = String },
-    ]
+    primary_key_columns = [ { column_name = "id", uuid_kind = TufArtifactKind } ]
 }
 
 lookup_resource! {

nexus/db-queries/src/policy_test/resources.rs

@@ -6,9 +6,7 @@
 
 use super::resource_builder::ResourceBuilder;
 use super::resource_builder::ResourceSet;
-use crate::db::model::ArtifactId;
 use nexus_auth::authz;
-use nexus_db_model::SemverVersion;
 use omicron_common::api::external::LookupType;
 use omicron_uuid_kinds::GenericUuid;
 use omicron_uuid_kinds::PhysicalDiskUuid;
@@ -139,16 +137,12 @@ pub async fn make_resources(
         LookupType::ById(tuf_repo_id.into_untyped_uuid()),
     ));
 
-    let artifact_id = ArtifactId {
-        name: "a".to_owned(),
-        version: SemverVersion("1.0.0".parse().unwrap()),
-        kind: "b".to_owned(),
-    };
-    let artifact_id_desc = artifact_id.to_string();
+    let tuf_artifact_id =
+        "6827813e-bfaa-4205-9b9f-9f7901e4aab1".parse().unwrap();
     builder.new_resource(authz::TufArtifact::new(
         authz::FLEET,
-        artifact_id,
-        LookupType::ByCompositeId(artifact_id_desc),
+        tuf_artifact_id,
+        LookupType::ById(tuf_artifact_id.into_untyped_uuid()),
     ));
 
     let address_lot_id =

nexus/db-queries/tests/output/authz-roles.out

@@ -1090,7 +1090,7 @@ resource: TufRepo id "3c52d72f-cbf7-4951-a62f-a4154e74da87"
   silo1-proj1-viewer               ✘  ✘  ✘  ✘  ✘  ✘  ✘  ✘
   unauthenticated                  !  !  !  !  !  !  !  !
 
-resource: TufArtifact id "a v1.0.0 (b)"
+resource: TufArtifact id "6827813e-bfaa-4205-9b9f-9f7901e4aab1"
 
   USER                             Q  R LC RP  M MP CC  D
   fleet-admin                      ✘  ✔  ✔  ✔  ✔  ✔  ✔  ✔