Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix persistence issues #6991

Merged
merged 9 commits into from
Nov 29, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -332,7 +320,7 @@ impl CliState {
node_names.join(", ")
),
)
.into())
.into())
}
}
}
Expand Down Expand Up @@ -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?;
Expand All @@ -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(())
}

Expand Down
28 changes: 26 additions & 2 deletions implementations/rust/ockam/ockam_api/src/cli_state/projects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ impl CliState {
Some(project.name()),
Some(project.id()),
None,
Some(project.authority_identity().await?),
Some(project.authority_access_route()?),
project.authority_identity().await.ok(),
project.authority_access_route().ok(),
)
.await?;
Ok(())
Expand Down Expand Up @@ -152,3 +152,27 @@ impl CliState {
Ok(projects)
}
}

#[cfg(test)]
mod tests {
use ockam_core::env::FromString;

use super::*;

#[tokio::test]
async fn test_import_project() -> Result<()> {
let cli = CliState::test().await?;

// a project can be created without specifying its authority
cli.import_project(
"project_id",
"project_name",
&None,
&MultiAddr::from_string("/project/default").unwrap(),
&None,
&None,
)
.await?;
Ok(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 13 additions & 12 deletions implementations/rust/ockam/ockam_command/src/authority/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -346,7 +346,9 @@ async fn get_user_project(
}
};

check_project_readiness(opts, ctx, node, project.clone()).await?;
let project = check_project_readiness(opts, ctx, node, project.clone()).await?;
// store the updated project
opts.state.store_project(project.clone()).await?;

opts.terminal.write_line(&fmt_ok!(
"Marked this project as your default project, on this machine.\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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);
Expand All @@ -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(),
Expand Down Expand Up @@ -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}")?;
Expand Down
2 changes: 2 additions & 0 deletions implementations/rust/ockam/ockam_command/src/node/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ async fn run_impl(
let controller = node.create_controller().await?;
check_for_completion(&opts, ctx, &controller, &operation_id).await?;
let project = check_project_readiness(&opts, ctx, &node, project).await?;
// update the project state when it's ready
opts.state.store_project(project.clone()).await?;
opts.println(&project)?;
Ok(())
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ teardown_home_dir() {
cp -r "$OCKAM_HOME/." "$HOME/.bats-tests"
fi
run $OCKAM node delete --all --force --yes
run $OCKAM reset -y
done
}

Expand Down
14 changes: 13 additions & 1 deletion implementations/rust/ockam/ockam_command/tests/bats/nodes.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion implementations/rust/ockam/ockam_identity/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
use ockam_core::compat::string::String;
use ockam_core::{
errcode::{Kind, Origin},
Error,
};

/// Identity crate error
#[repr(u8)]
SanjoDeundiak marked this conversation as resolved.
Show resolved Hide resolved
#[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
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading