Fix saga undo types; Copy over all keys associated to the instance

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

@@ -171,6 +171,25 @@ impl DataStore {
         Ok(())
     }
 
+    pub async fn instance_ssh_keys_list(
+        &self,
+        opctx: &OpContext,
+        instance_id: Uuid,
+    ) -> ListResultVec<SshKey> {
+        use db::schema::instance_ssh_key::dsl;
+        dsl::instance_ssh_key
+            .filter(dsl::instance_id.eq(instance_id))
+            .inner_join(
+                db::schema::ssh_key::table
+                    .on(dsl::ssh_key_id.eq(db::schema::ssh_key::dsl::id)),
+            )
+            .filter(db::schema::ssh_key::dsl::time_deleted.is_null())
+            .select(SshKey::as_select())
+            .get_results_async(&*self.pool_connection_authorized(opctx).await?)
+            .await
+            .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
+    }
+
     pub async fn instance_ssh_keys_delete(
         &self,
         opctx: &OpContext,

nexus/src/app/instance.rs

@@ -59,8 +59,6 @@ use std::sync::Arc;
 use tokio::io::{AsyncRead, AsyncWrite};
 use uuid::Uuid;
 
-const MAX_KEYS_PER_INSTANCE: u32 = 8;
-
 type SledAgentClientError =
     sled_agent_client::Error<sled_agent_client::types::Error>;
 
@@ -1001,7 +999,6 @@ impl super::Nexus {
         initial_vmm: &db::model::Vmm,
     ) -> Result<(), Error> {
         opctx.authorize(authz::Action::Modify, authz_instance).await?;
-        let params = sagactx.saga_params::<Params>()?;
 
         // Gather disk information and turn that into DiskRequests
         let disks = self
@@ -1132,35 +1129,14 @@ impl super::Nexus {
             vec![]
         };
 
-        // Gather the SSH public keys of the actor make the request so
-        // that they may be injected into the new image via cloud-init.
-        // TODO-security: this should be replaced with a lookup based on
-        // on `SiloUser` role assignments once those are in place.
-        let actor = opctx.authn.actor_required().internal_context(
-            "loading current user's ssh keys for new Instance",
-        )?;
-        let (.., authz_user) = LookupPath::new(opctx, &self.db_datastore)
-            .silo_user_id(actor.actor_id())
-            .lookup_for(authz::Action::ListChildren)
-            .await?;
-
         let ssh_keys = self
             .db_datastore
-            .ssh_keys_batch_fetch(opctx, &authz_user, params.ssh_keys)
+            .instance_ssh_keys_list(opctx, authz_instance.id())
             .await?
             .into_iter();
 
-        // Returning the intersection of a users ssh_keys and the
-        // keys that were included in the instance create API request
-        let ssh_keys: Vec<String> = ssh_keys
-            .filter(|ssh_key| {
-                let id_str = ssh_key.id().to_string();
-                let name_str = ssh_key.name().to_string();
-                checked_keys.contains(&id_str)
-                    || checked_keys.contains(&name_str)
-            })
-            .map(|ssh_key| ssh_key.public_key)
-            .collect();
+        let ssh_keys: Vec<String> =
+            ssh_keys.map(|ssh_key| ssh_key.public_key).collect();
 
         // Ask the sled agent to begin the state change.  Then update the
         // database to reflect the new intermediate state.  If this update is

nexus/src/app/sagas/instance_create.rs

@@ -17,10 +17,10 @@ use nexus_db_queries::db::queries::network_interface::InsertError as InsertNicEr
 use nexus_db_queries::{authn, authz, db};
 use nexus_defaults::DEFAULT_PRIMARY_NIC_NAME;
 use nexus_types::external_api::params::InstanceDiskAttachment;
-use omicron_common::api::external::Error;
 use omicron_common::api::external::IdentityMetadataCreateParams;
 use omicron_common::api::external::InstanceState;
 use omicron_common::api::external::Name;
+use omicron_common::api::external::{Error, InternalContext};
 use omicron_common::api::internal::shared::SwitchLocation;
 use serde::Deserialize;
 use serde::Serialize;
@@ -319,20 +319,48 @@ async fn sic_associate_ssh_keys(
     );
     let instance_id = sagactx.lookup::<Uuid>("instance_id")?;
 
+    // Gather the SSH public keys of the actor make the request so
+    // that they may be injected into the new image via cloud-init.
+    // TODO-security: this should be replaced with a lookup based on
+    // on `SiloUser` role assignments once those are in place.
+    let actor = opctx
+        .authn
+        .actor_required()
+        .internal_context("loading current user's ssh keys for new Instance")
+        .map_err(ActionError::action_failed)?;
+
+    let (.., authz_user) = LookupPath::new(&opctx, &datastore)
+        .silo_user_id(actor.actor_id())
+        .lookup_for(authz::Action::ListChildren)
+        .await
+        .map_err(ActionError::action_failed)?;
+
     datastore
         .ssh_keys_batch_assign(
             &opctx,
-            authz_user,
+            &authz_user,
             instance_id,
-            &saga_params.create_params.ssh_keys,
+            &saga_params.create_params.ssh_keys.map(|k| {
+                // Before the instance_create saga is kicked off all entries
+                // in `ssh_keys` are validated and converted to `Uuids`.
+                k.iter()
+                    .filter_map(|n| match n {
+                        omicron_common::api::external::NameOrId::Id(id) => {
+                            Some(*id)
+                        }
+                        _ => None,
+                    })
+                    .collect()
+            }),
         )
-        .await?;
+        .await
+        .map_err(ActionError::action_failed)?;
     Ok(())
 }
 
 async fn sic_associate_ssh_keys_undo(
     sagactx: NexusActionContext,
-) -> Result<(), ActionError> {
+) -> Result<(), anyhow::Error> {
     let osagactx = sagactx.user_data();
     let datastore = osagactx.datastore();
     let saga_params = sagactx.saga_params::<Params>()?;
@@ -342,7 +370,10 @@ async fn sic_associate_ssh_keys_undo(
         &saga_params.serialized_authn,
     );
     let instance_id = sagactx.lookup::<Uuid>("instance_id")?;
-    datastore.instance_ssh_keys_delete(&opctx, instance_id).await?;
+    datastore
+        .instance_ssh_keys_delete(&opctx, instance_id)
+        .await
+        .map_err(ActionError::action_failed)?;
     Ok(())
 }