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

Improved Coffee Verification and CoffeeNurse Functionality #207

Merged
merged 3 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions coffee_cmd/src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ pub struct CoffeeArgs {
pub network: Option<String>,
#[clap(short, long, value_parser, name = "data-dir")]
pub data_dir: Option<String>,
#[clap(short, long, action = clap::ArgAction::SetTrue)]
pub skip_verify: bool,
vincenzopalazzo marked this conversation as resolved.
Show resolved Hide resolved
}

/// Coffee subcommand of the command line daemon.
Expand Down Expand Up @@ -59,7 +61,11 @@ pub enum CoffeeCommand {
Search { plugin: String },
/// clean up remote repositories storage information
#[clap(arg_required_else_help = false)]
Nurse {},
Nurse {
/// verify that coffee configuration is sane (without taking any action)
#[arg(short, long, action = clap::ArgAction::SetTrue)]
verify: bool,
vincenzopalazzo marked this conversation as resolved.
Show resolved Hide resolved
},
}

#[derive(Debug, Subcommand)]
Expand Down Expand Up @@ -96,7 +102,7 @@ impl From<&CoffeeCommand> for coffee_core::CoffeeOperation {
CoffeeCommand::Remove { plugin } => Self::Remove(plugin.to_owned()),
CoffeeCommand::Show { plugin } => Self::Show(plugin.to_owned()),
CoffeeCommand::Search { plugin } => Self::Search(plugin.to_owned()),
CoffeeCommand::Nurse {} => Self::Nurse,
CoffeeCommand::Nurse { verify } => Self::Nurse(*verify),
}
}
}
Expand Down Expand Up @@ -127,4 +133,8 @@ impl coffee_core::CoffeeArgs for CoffeeArgs {
fn network(&self) -> Option<String> {
self.network.clone()
}

fn skip_verify(&self) -> bool {
self.skip_verify
}
}
4 changes: 1 addition & 3 deletions coffee_cmd/src/coffee_term/command_show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub fn show_nurse_result(
Ok(nurse) => {
// special case: if the nurse is sane
// we print a message and return
if nurse.status[0] == NurseStatus::Sane {
if nurse.is_sane() {
vincenzopalazzo marked this conversation as resolved.
Show resolved Hide resolved
term::success!("Coffee configuration is not corrupt! No need to run coffee nurse");
return Ok(());
}
Expand All @@ -98,14 +98,12 @@ pub fn show_nurse_result(

for status in &nurse.status {
let action_str = match status {
NurseStatus::Sane => "".to_string(),
NurseStatus::RepositoryLocallyRestored(_) => "Restored using Git".to_string(),
NurseStatus::RepositoryLocallyRemoved(_) => {
"Removed from local storage".to_string()
}
};
let repos_str = match status {
NurseStatus::Sane => "".to_string(),
NurseStatus::RepositoryLocallyRestored(repos)
| NurseStatus::RepositoryLocallyRemoved(repos) => repos.join(", "),
};
Expand Down
15 changes: 12 additions & 3 deletions coffee_cmd/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,18 @@ async fn main() -> Result<(), CoffeeError> {
}
Err(err) => Err(err),
},
CoffeeCommand::Nurse {} => {
let nurse_result = coffee.nurse().await;
coffee_term::show_nurse_result(nurse_result)
CoffeeCommand::Nurse { verify } => {
if verify {
let result = coffee.nurse_verify().await?;
term::info!("{}", result);
if !result.is_sane() {
term::info!("Coffee local directory is damaged, please run `coffee nurse` to try to fix it");
}
Ok(())
} else {
let nurse_result = coffee.nurse().await;
coffee_term::show_nurse_result(nurse_result)
}
}
};

Expand Down
38 changes: 31 additions & 7 deletions coffee_core/src/coffee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,19 @@ impl CoffeeManager {
if let Err(err) = self.coffee_cln_config.parse() {
log::error!("{}", err.cause);
}
if !self.config.skip_verify {
// Check for the chain of responsibility
let status = self.recovery_strategies.scan(self).await?;
log::debug!("Chain of responsibility status: {:?}", status);
// if any defect is found, we print a warning message (we don't take action)
if !status.defects.is_empty() {
return Err(
error!("Coffee found some defects in the configuration. Please run `coffee nurse` to fix them.
If you are want to skip the verification, please add the `--skip-verify ` flag to the command.")
);
};
}

self.load_cln_conf().await?;
log::debug!("cln conf {:?}", self.coffee_cln_config);
log::debug!("finish plugin manager inventory");
Expand Down Expand Up @@ -453,6 +466,10 @@ impl PluginManager for CoffeeManager {
Err(err)
}

async fn nurse_verify(&self) -> Result<ChainOfResponsibilityStatus, CoffeeError> {
self.recovery_strategies.scan(self).await
}

async fn nurse(&mut self) -> Result<CoffeeNurse, CoffeeError> {
let status = self.recovery_strategies.scan(self).await?;
let mut nurse_actions: Vec<NurseStatus> = vec![];
Expand All @@ -465,14 +482,11 @@ impl PluginManager for CoffeeManager {
}
}
}

// If there was no actions taken by nurse, we return a sane status.
if nurse_actions.is_empty() {
nurse_actions.push(NurseStatus::Sane);
}
Ok(CoffeeNurse {
let mut nurse = CoffeeNurse {
status: nurse_actions,
})
};
nurse.organize();
Ok(nurse)
}

async fn patch_repository_locally_absent(
Expand Down Expand Up @@ -503,6 +517,16 @@ impl PluginManager for CoffeeManager {
}
Err(err) => {
log::debug!("error while recovering repository {repo_name}: {err}");
// We make sure that the repository folder is removed
// from local storage.
// Maybe when trying to recover the repository,
// we have created the folder but we were not able
// to clone the repository.
let repo_path = repo.url().path_string;
// This shouldn't return an error if the repository
// is not present locally.
let _ = fs::remove_dir_all(repo_path).await;

log::info!("removing repository {}", repo_name.clone());
self.repos.remove(repo_name);
log::debug!("remote removed: {}", repo_name);
Expand Down
20 changes: 20 additions & 0 deletions coffee_core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use log::info;
use serde::{Deserialize, Serialize};
use std::env;

use crate::CoffeeOperation;
tareknaser marked this conversation as resolved.
Show resolved Hide resolved
use coffee_lib::utils::check_dir_or_make_if_missing;
use coffee_lib::{errors::CoffeeError, plugin::Plugin};

Expand All @@ -28,6 +29,10 @@ pub struct CoffeeConf {
/// all plugins that are installed
/// with the plugin manager.
pub plugins: Vec<Plugin>,
/// A flag that indicates if the
/// user wants to skip the verification
/// of nurse.
pub skip_verify: bool,
}

impl CoffeeConf {
Expand All @@ -51,6 +56,7 @@ impl CoffeeConf {
plugins: vec![],
cln_config_path: None,
cln_root: None,
skip_verify: false,
};

// check the command line arguments and bind them
Expand Down Expand Up @@ -82,6 +88,20 @@ impl CoffeeConf {
self.config_path = config.to_owned();
}

// If the command is nurse we skip the verification
// because nurse is the command that needs
// to solve the configuration problems.
if !conf.skip_verify() {
match conf.command() {
CoffeeOperation::Nurse(_) => {
self.skip_verify = true;
}
_ => {
self.skip_verify = false;
}
}
}

// FIXME: be able to put the directory also in another place!
// for now it is fixed in the Home/.coffee but another good place
// will be, the .lightning dir
Expand Down
4 changes: 3 additions & 1 deletion coffee_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub enum CoffeeOperation {
Show(String),
/// Search(plugin name)
Search(String),
Nurse,
Nurse(bool),
}

#[derive(Clone, Debug)]
Expand All @@ -40,4 +40,6 @@ pub trait CoffeeArgs: Send + Sync {
fn network(&self) -> Option<String>;
/// return the data dir
fn data_dir(&self) -> Option<String>;
/// return the skip verify flag
fn skip_verify(&self) -> bool;
}
7 changes: 7 additions & 0 deletions coffee_httpd/src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,11 @@ impl coffee_core::CoffeeArgs for HttpdArgs {
fn network(&self) -> Option<String> {
self.network.clone()
}

// We don't need to verify the nurse in the httpd
// daemon.
// there is no endpoint for `nurse` in the httpd
fn skip_verify(&self) -> bool {
true
}
}
3 changes: 3 additions & 0 deletions coffee_lib/src/plugin_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ pub trait PluginManager {
/// clean up storage information about the remote repositories of the plugin manager.
async fn nurse(&mut self) -> Result<CoffeeNurse, CoffeeError>;

/// verify that coffee configuration is sane without taking any action.
async fn nurse_verify(&self) -> Result<ChainOfResponsibilityStatus, CoffeeError>;

/// patch coffee configuration in the case that a repository is present in the coffee
/// configuration but is absent from the local storage.
async fn patch_repository_locally_absent(
Expand Down
69 changes: 63 additions & 6 deletions coffee_lib/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ pub mod response {

/// This struct is used to represent a defect
/// that can be patched by the nurse.
#[derive(Clone, Debug, Serialize, Deserialize)]
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub enum Defect {
// A patch operation when a git repository is present in the coffee configuration
// but is absent from the local storage.
Expand All @@ -167,12 +167,38 @@ pub mod response {
pub defects: Vec<Defect>,
}

impl ChainOfResponsibilityStatus {
pub fn is_sane(&self) -> bool {
self.defects.is_empty()
}
}

impl fmt::Display for ChainOfResponsibilityStatus {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.defects.is_empty() {
write!(f, "Coffee is sane")
} else {
writeln!(f, "Coffee has the following defects:")?;
for (i, defect) in self.defects.iter().enumerate() {
match defect {
Defect::RepositoryLocallyAbsent(repos) => {
write!(f, "{}. Repository missing locally: ", i + 1)?;
for repo in repos {
write!(f, " {}", repo)?;
}
}
}
}
Ok(())
}
}
}

/// This struct is used to represent the status of nurse,
/// either sane or not.
/// If not sane, return the action that nurse has taken.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub enum NurseStatus {
Sane,
RepositoryLocallyRestored(Vec<String>),
RepositoryLocallyRemoved(Vec<String>),
}
Expand All @@ -182,13 +208,44 @@ pub mod response {
pub status: Vec<NurseStatus>,
}

impl CoffeeNurse {
pub fn is_sane(&self) -> bool {
self.status.is_empty()
}

pub fn organize(&mut self) {
// For every action taken by the nurse, we want to
// have 1 entry with the list of repositories affected.
let mut new_status: Vec<NurseStatus> = vec![];
let mut repositories_locally_removed: Vec<String> = vec![];
let mut repositories_locally_restored: Vec<String> = vec![];
for repo in self.status.iter() {
match repo {
NurseStatus::RepositoryLocallyRemoved(repos) => {
repositories_locally_removed.append(&mut repos.clone())
}
NurseStatus::RepositoryLocallyRestored(repos) => {
repositories_locally_restored.append(&mut repos.clone())
}
}
}
if !repositories_locally_removed.is_empty() {
new_status.push(NurseStatus::RepositoryLocallyRemoved(
repositories_locally_removed,
));
}
if !repositories_locally_restored.is_empty() {
new_status.push(NurseStatus::RepositoryLocallyRestored(
repositories_locally_restored,
));
}
self.status = new_status;
}
}

impl fmt::Display for NurseStatus {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
NurseStatus::Sane => write!(
f,
"coffee configuration is not corrupt! No need to run coffee nurse"
),
NurseStatus::RepositoryLocallyRestored(val) => {
write!(f, "Repositories restored locally: {}", val.join(" "))
}
Expand Down
5 changes: 5 additions & 0 deletions coffee_plugin/src/plugin/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ impl CoffeeArgs for PluginArgs {
fn network(&self) -> Option<String> {
Some(self.network.clone())
}

// we don't need to verify the nurse
fn skip_verify(&self) -> bool {
true
}
}

impl From<CLNConf> for PluginArgs {
Expand Down
4 changes: 4 additions & 0 deletions coffee_testing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ impl coffee_core::CoffeeArgs for CoffeeTestingArgs {
fn network(&self) -> Option<String> {
Some(self.network.clone())
}

fn skip_verify(&self) -> bool {
true
}
}

/// Coffee testing manager
Expand Down
6 changes: 6 additions & 0 deletions docs/docs-book/src/using-coffee.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ the correct network.
- `--data-dir`: by default set to `/home/alice/.coffee`, you may want to set
this option if you are looking to specify a different directory for the
Coffee home.
- `--skip-verify`: Use this option to bypass `coffee`'s validation process, which checks for conflicts between its configuration and the local storage.

### Add a Plugin Repository

Expand Down Expand Up @@ -152,6 +153,11 @@ coffee search <plugin_name>
```bash
coffee nurse
```
Additionally, if you wish to perform a verification of coffee without making any changes, you can use the `--verify` flag:

```bash
coffee nurse --verify
```
_________
## Running coffee as a server

Expand Down
Loading
Loading