Fix persistence issues

implementations/rust/ockam/ockam_api/src/cli_state/identities.rs

@@ -49,18 +49,6 @@ impl CliState {
         self.create_identity_with_name_and_vault(name, &vault.name())
             .await
     }
-    /// Create an identity associated with an optional name and an optional vault name
-    /// If the vault name is not specified then the default vault is used
-    pub async fn create_identity_with_optional_name_and_optional_vault(
-        &self,
-        name: &Option<String>,
-        vault_name: &Option<String>,
-    ) -> Result<NamedIdentity> {
-        let name = name.clone().unwrap_or_else(random_name);
-        let vault = self.get_named_vault_or_default(vault_name).await?.name();
-        self.create_identity_with_name_and_vault(&name, &vault)
-            .await
-    }
 
     /// Create an identity with specific key id.
     /// This method is used when the vault is a KMS vault and we just need to store a key id
@@ -332,7 +320,7 @@ impl CliState {
                     node_names.join(", ")
                 ),
             )
-            .into())
+                .into())
         }
     }
 }
@@ -491,24 +479,6 @@ mod tests {
         Ok(())
     }
 
-    #[tokio::test]
-    async fn test_create_default_identity() -> Result<()> {
-        let cli = CliState::test().await?;
-
-        // create a default identity using the default vault
-        let identity = cli
-            .create_identity_with_optional_name_and_optional_vault(&None, &None)
-            .await?;
-        let expected = cli.get_default_named_identity().await?;
-        assert_eq!(identity, expected);
-
-        // check that the identity uses the default vault
-        let default_vault = cli.get_default_named_vault().await?;
-        assert_eq!(identity.vault_name, default_vault.name());
-
-        Ok(())
-    }
-
     #[tokio::test]
     async fn test_get_default_identity() -> Result<()> {
         let cli = CliState::test().await?;
@@ -523,6 +493,10 @@ mod tests {
         let result = cli.get_named_identity(&identity.name()).await;
         assert!(result.is_ok());
 
+        // check that the identity uses the default vault
+        let default_vault = cli.get_default_named_vault().await?;
+        assert_eq!(identity.vault_name, default_vault.name());
+
         Ok(())
     }
 

implementations/rust/ockam/ockam_api/src/nodes/service/in_memory_node.rs

@@ -76,28 +76,31 @@ impl InMemoryNode {
         project_name: Option<String>,
         trust_context: Option<NamedTrustContext>,
     ) -> miette::Result<Self> {
-        Self::start_node(ctx, cli_state, None, None, project_name, trust_context).await
+        let default_identity_name = cli_state.get_default_named_identity().await?.name();
+        Self::start_node(
+            ctx,
+            cli_state,
+            &default_identity_name,
+            project_name,
+            trust_context,
+        )
+        .await
     }
 
     /// Start an in memory node
     pub async fn start_node(
         ctx: &Context,
         cli_state: &CliState,
-        identity_name: Option<String>,
-        vault_name: Option<String>,
+        identity_name: &str,
         project_name: Option<String>,
         trust_context: Option<NamedTrustContext>,
     ) -> miette::Result<InMemoryNode> {
         let defaults = NodeManagerDefaults::default();
 
-        // if no identity is specified, create one
-        let identity = cli_state
-            .create_identity_with_optional_name_and_optional_vault(&identity_name, &vault_name)
-            .await?;
         let node = cli_state
             .create_node_with_optional_values(
                 &defaults.node_name,
-                &Some(identity.name()),
+                &Some(identity_name.to_string()),
                 &project_name,
             )
             .await?;

implementations/rust/ockam/ockam_app_lib/src/enroll/enroll_user.rs

@@ -190,7 +190,7 @@ impl AppState {
                 });
                 let ctx = &self.context();
                 let project = node_manager
-                    .create_project(ctx, &space.id, PROJECT_NAME, vec![])
+                    .create_project(ctx, &space.name, PROJECT_NAME, vec![])
                     .await?;
                 node_manager
                     .wait_until_project_is_ready(ctx, project)

implementations/rust/ockam/ockam_command/src/authority/create.rs

@@ -110,6 +110,13 @@ async fn spawn_background_node(
     opts: &CommandGlobalOpts,
     cmd: &CreateCommand,
 ) -> miette::Result<()> {
+    // Create the authority identity if it has not been created before
+    // If no name is specified on the command line, use "authority"
+    let identity_name = cmd.identity.clone().unwrap_or("authority".to_string());
+    if opts.state.get_named_identity(&identity_name).await.is_err() {
+        opts.state.create_identity_with_name(&identity_name).await?;
+    };
+
     opts.state
         .create_node_with_optional_values(&cmd.node_name, &cmd.identity, &None)
         .await?;
@@ -257,19 +264,13 @@ async fn start_authority_node(
 ) -> miette::Result<()> {
     let (opts, cmd) = args;
 
-    // Retrieve the authority identity if it has been created before
-    // otherwise create a new one
-    let identity_name = match &cmd.identity {
-        Some(identity_name) => opts.state.get_named_identity(identity_name).await?.name(),
-        None => match opts.state.get_default_named_identity().await {
-            Ok(identity) => identity.name(),
-            Err(_) => opts
-                .state
-                .create_identity_with_name("authority")
-                .await?
-                .name(),
-        },
+    // Create the authority identity if it has not been created before
+    // If no name is specified on the command line, use "authority"
+    let identity_name = cmd.identity.clone().unwrap_or("authority".to_string());
+    if opts.state.get_named_identity(&identity_name).await.is_err() {
+        opts.state.create_identity_with_name(&identity_name).await?;
     };
+
     let node = opts
         .state
         .create_node_with_optional_values(&cmd.node_name, &Some(identity_name), &None)

implementations/rust/ockam/ockam_command/src/enroll/command.rs

@@ -307,7 +307,7 @@ async fn get_user_project(
             let project_name = "default".to_string();
             let get_project = async {
                 let project = node
-                    .create_project(ctx, &space.id, &project_name, vec![])
+                    .create_project(ctx, &space.name, &project_name, vec![])
                     .await?;
                 *is_finished.lock().await = true;
                 Ok(project)

implementations/rust/ockam/ockam_command/src/message/send.rs

@@ -109,8 +109,7 @@ async fn rpc(ctx: Context, (opts, cmd): (CommandGlobalOpts, SendCommand)) -> mie
             let node_manager = InMemoryNode::start_node(
                 ctx,
                 &opts.state,
-                Some(identity_name.clone()),
-                None,
+                &identity_name,
                 cmd.trust_context_opts.project_name,
                 named_trust_context,
             )

implementations/rust/ockam/ockam_command/src/node/models/show.rs

@@ -23,6 +23,7 @@ pub struct ShowNodeResponse {
     pub is_default: bool,
     pub name: String,
     pub is_up: bool,
+    pub node_pid: Option<u32>,
     pub route: RouteToNode,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub identity: Option<String>,
@@ -46,6 +47,7 @@ impl ShowNodeResponse {
         name: &str,
         is_up: bool,
         node_port: Option<u16>,
+        node_pid: Option<u32>,
     ) -> ShowNodeResponse {
         let mut m = MultiAddr::default();
         let short = m.push_back(Node::new(name)).ok().map(|_| m);
@@ -64,6 +66,7 @@ impl ShowNodeResponse {
             is_default,
             name: name.to_owned(),
             is_up,
+            node_pid,
             route: RouteToNode { short, verbose },
             identity: None,
             transports: Default::default(),
@@ -94,6 +97,10 @@ impl Display for ShowNodeResponse {
             }
         )?;
 
+        if let Some(node_pid) = self.node_pid {
+            writeln!(buffer, "  PID: {}", node_pid)?;
+        }
+
         writeln!(buffer, "  Route To Node:")?;
         if let Some(short) = &self.route.short {
             writeln!(buffer, "    Short: {short}")?;

implementations/rust/ockam/ockam_command/src/node/show.rs

@@ -148,13 +148,15 @@ pub async fn print_query_status(
             &node_name,
             is_authority_node,
             node_info.tcp_listener_port(),
+            node_info.pid(),
         )
     } else {
         let mut show_node = ShowNodeResponse::new(
             node_info.is_default(),
             &node_name,
             true,
             node_info.tcp_listener_port(),
+            node_info.pid(),
         );
         // Get list of services for the node
         let services: ServiceList = node.ask(ctx, api::list_services()).await?;

implementations/rust/ockam/ockam_command/tests/bats/authority.bats

@@ -24,6 +24,22 @@ teardown() {
   assert_output --partial "\"is_up\": true"
 }
 
+@test "authority - an authority identity is created by default for the authority node" {
+  port="$(random_port)"
+
+  trusted="{}"
+  run_success "$OCKAM" authority create --tcp-listener-address="127.0.0.1:$port" --project-identifier 1 --trusted-identities "$trusted"
+  run_success "$OCKAM" identity show authority
+}
+
+@test "authority - an authority identity is created by default for the authority node - with a given name" {
+  port="$(random_port)"
+
+  trusted="{}"
+  run_success "$OCKAM" authority create --tcp-listener-address="127.0.0.1:$port" --project-identifier 1 --trusted-identities "$trusted" --identity ockam
+  run_success "$OCKAM" identity show ockam
+}
+
 @test "authority - standalone authority, enrollers, members" {
   port="$(random_port)"
 

implementations/rust/ockam/ockam_command/tests/bats/nodes.bats

@@ -15,7 +15,19 @@ teardown() {
 # ===== UTILS
 
 force_kill_node() {
-  run killall ockam
+  max_retries=5
+    i=0
+    while [[ $i -lt $max_retries ]]; do
+      pid="$($OCKAM node show $1 --output json | jq .node_pid)"
+      run kill -9 $pid
+      # Killing a node created without `-f` leaves the
+      # process in a defunct state when running within Docker.
+      if ! ps -p $pid || ps -p $pid | grep defunct; then
+        return
+      fi
+      sleep 0.2
+      ((i = i + 1))
+    done
 }
 
 # ===== TESTS

implementations/rust/ockam/ockam_identity/src/error.rs

@@ -1,17 +1,19 @@
+use ockam_core::compat::string::String;
 use ockam_core::{
     errcode::{Kind, Origin},
     Error,
 };
 
 /// Identity crate error
+#[repr(u8)]
 #[derive(Clone, Debug)]
 pub enum IdentityError {
     /// Invalid key type
     InvalidKeyType = 1,
     /// Invalid Key Data
     InvalidKeyData,
     /// Invalid Identifier format
-    InvalidIdentifier,
+    InvalidIdentifier(String),
     /// Identity Change History is empty
     EmptyIdentity,
     /// Identity Verification Failed
@@ -65,6 +67,7 @@ pub enum IdentityError {
 }
 
 impl ockam_core::compat::error::Error for IdentityError {}
+
 impl core::fmt::Display for IdentityError {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         core::fmt::Debug::fmt(self, f)

implementations/rust/ockam/ockam_identity/src/models/utils/identifiers.rs

@@ -1,13 +1,14 @@
+use core::fmt::{Display, Formatter};
+use core::str::FromStr;
+
+use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+
 use ockam_core::compat::{string::String, vec::Vec};
 use ockam_core::env::FromString;
 use ockam_core::{Error, Result};
 
-use crate::{Identifier, IdentityError};
-
 use crate::models::{ChangeHash, CHANGE_HASH_LEN, IDENTIFIER_LEN};
-use core::fmt::{Display, Formatter};
-use core::str::FromStr;
-use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+use crate::{Identifier, IdentityError};
 
 impl Serialize for Identifier {
     fn serialize<S>(&self, serializer: S) -> core::result::Result<S::Ok, S::Error>
@@ -60,10 +61,10 @@ impl TryFrom<&str> for Identifier {
             if let Ok(data) = hex::decode(value) {
                 data.try_into()
             } else {
-                Err(IdentityError::InvalidIdentifier.into())
+                Err(IdentityError::InvalidIdentifier(value.into()).into())
             }
         } else {
-            Err(IdentityError::InvalidIdentifier.into())
+            Err(IdentityError::InvalidIdentifier(value.into()).into())
         }
     }
 }
@@ -75,7 +76,7 @@ impl TryFrom<&[u8]> for Identifier {
         if let Ok(value) = <[u8; IDENTIFIER_LEN]>::try_from(value) {
             Ok(Self(value))
         } else {
-            Err(IdentityError::InvalidIdentifier.into())
+            Err(IdentityError::InvalidIdentifier(hex::encode(value)).into())
         }
     }
 }
@@ -142,7 +143,7 @@ impl TryFrom<&str> for ChangeHash {
         if let Ok(data) = hex::decode(value) {
             data.try_into()
         } else {
-            Err(IdentityError::InvalidIdentifier.into())
+            Err(IdentityError::InvalidIdentifier(value.into()).into())
         }
     }
 }
@@ -154,7 +155,7 @@ impl TryFrom<&[u8]> for ChangeHash {
         if let Ok(value) = <[u8; CHANGE_HASH_LEN]>::try_from(value) {
             Ok(Self(value))
         } else {
-            Err(IdentityError::InvalidIdentifier.into())
+            Err(IdentityError::InvalidIdentifier(hex::encode(value)).into())
         }
     }
 }
@@ -197,9 +198,10 @@ impl AsRef<[u8]> for ChangeHash {
 
 #[cfg(test)]
 mod test {
-    use super::*;
     use quickcheck::{quickcheck, Arbitrary, Gen};
 
+    use super::*;
+
     impl Arbitrary for Identifier {
         fn arbitrary(g: &mut Gen) -> Self {
             let mut data = [0u8; IDENTIFIER_LEN];