Fix persistence issues

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

@@ -49,17 +49,40 @@ 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
+    ///  - if the vault name is not specified then the default vault is used
+    ///  - if no name is specified:
+    ///     - if there is no default identity, create an identity with a random name
+    ///     - if there is a default identity with the correct vault, return it
+    ///     - otherwise create a new identity
+    ///  - if a name is given create a new identity with the proper vault (unless it has already been created)
     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
+        let vault_name = self.get_named_vault_or_default(vault_name).await?.name();
+        match name {
+            Some(name) => {
+                self.create_identity_with_name_and_vault(name, &vault_name)
+                    .await
+            }
+            None => {
+                let default_identity = self
+                    .identities_repository()
+                    .await?
+                    .get_default_named_identity()
+                    .await?;
+                if let Some(default_identity) = default_identity {
+                    if default_identity.vault_name == vault_name {
+                        return Ok(default_identity);
+                    }
+                };
+                self.create_identity_with_name_and_vault(&random_name(), &vault_name)
+                    .await
+            }
+        }
     }
 
     /// Create an identity with specific key id.
@@ -332,7 +355,7 @@ impl CliState {
                     node_names.join(", ")
                 ),
             )
-            .into())
+                .into())
         }
     }
 }
@@ -509,6 +532,112 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn test_create_identity_with_a_name_and_no_vault() -> Result<()> {
+        let cli = CliState::test().await?;
+
+        // create a named identity using the default vault
+        let identity = cli
+            .create_identity_with_optional_name_and_optional_vault(&Some("name".to_string()), &None)
+            .await?;
+
+        // the created identity becomes the default one
+        let expected = cli.get_default_named_identity().await?;
+        assert_eq!(identity, expected);
+
+        // the identity can be retrieved by name
+        let expected = cli.get_named_identity("name").await?;
+        assert_eq!(identity, expected);
+
+        // the identity will not be recreated a second time
+        let identity = cli
+            .create_identity_with_optional_name_and_optional_vault(&Some("name".to_string()), &None)
+            .await?;
+        assert_eq!(identity, expected);
+        let identities = cli.get_named_identities().await?;
+        assert_eq!(identities.len(), 1);
+
+        // the created 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_create_identity_with_no_name_and_a_vault() -> Result<()> {
+        let cli = CliState::test().await?;
+
+        // create a named identity using a given vault
+        let vault = cli.create_named_vault("vault").await?;
+
+        let identity = cli
+            .create_identity_with_optional_name_and_optional_vault(&None, &Some(vault.name()))
+            .await?;
+
+        // the created identity becomes the default one
+        let expected = cli.get_default_named_identity().await?;
+        assert_eq!(identity, expected);
+
+        // the identity can be retrieved by name
+        let expected = cli.get_named_identity(&identity.name()).await?;
+        assert_eq!(identity, expected);
+
+        // the identity will not be recreated a second time
+        let identity = cli
+            .create_identity_with_optional_name_and_optional_vault(
+                &Some(identity.name()),
+                &Some(vault.name()),
+            )
+            .await?;
+        assert_eq!(identity, expected);
+        let identities = cli.get_named_identities().await?;
+        assert_eq!(identities.len(), 1);
+
+        // the created identity uses the specified vault
+        assert_eq!(identity.vault_name, vault.name());
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_create_identity_with_no_name_and_a_non_default_vault() -> Result<()> {
+        let cli = CliState::test().await?;
+
+        // create two vaults
+        let _ = cli.create_named_vault("vault1").await?;
+        let vault2 = cli.create_named_vault("vault2").await?;
+
+        // create an identity with vault2 which is not the default vault
+        let identity = cli
+            .create_identity_with_optional_name_and_optional_vault(&None, &Some(vault2.name()))
+            .await?;
+
+        // the created identity becomes the default one
+        let expected = cli.get_default_named_identity().await?;
+        assert_eq!(identity, expected);
+
+        // the identity can be retrieved by name
+        let expected = cli.get_named_identity(&identity.name()).await?;
+        assert_eq!(identity, expected);
+
+        // the identity will not be recreated a second time
+        let identity = cli
+            .create_identity_with_optional_name_and_optional_vault(
+                &Some(identity.name()),
+                &Some(vault2.name()),
+            )
+            .await?;
+        assert_eq!(identity, expected);
+        let identities = cli.get_named_identities().await?;
+        assert_eq!(identities.len(), 1);
+
+        // the created identity uses the specified vault
+        assert_eq!(identity.vault_name, vault2.name());
+
+        Ok(())
+    }
+
     #[tokio::test]
     async fn test_get_default_identity() -> Result<()> {
         let cli = CliState::test().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/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)