From dde1bb889e85566e643f0bcb9c48886907906c66 Mon Sep 17 00:00:00 2001 From: Dima Pristupa Date: Tue, 24 Dec 2024 20:43:12 +0200 Subject: [PATCH] kamu add --replace: use correct alias --- CHANGELOG.md | 3 ++ src/app/cli/src/cli_commands.rs | 45 +++++++++++++++---------- src/app/cli/src/commands/add_command.rs | 13 ++++--- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a0a1999a9..4be553e0d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,9 @@ Recommendation: for ease of reading, use the following order: - GQL, `DatasetMetadata.currentUpstreamDependencies`: indication if datasets not found/not accessed - GQL, `DatasetMetadata.currentDownstreamDependencies`: exclude datasets that cannot be accessed - E2E: added the ability to create an account using CLI +### Fixed +- `kamu add --replace`: fixing a potential bug with the ability to delete + another user's dataset with the same name ## [0.213.1] - 2024-12-18 ### Fixed diff --git a/src/app/cli/src/cli_commands.rs b/src/app/cli/src/cli_commands.rs index 674e3b40ba..3f8187deff 100644 --- a/src/app/cli/src/cli_commands.rs +++ b/src/app/cli/src/cli_commands.rs @@ -31,29 +31,40 @@ pub fn get_command( }; let command: Box = match args.command { - cli::Command::Add(c) => Box::new(AddCommand::new( - cli_catalog.get_one()?, - cli_catalog.get_one()?, - cli_catalog.get_one()?, - cli_catalog.get_one()?, - c.manifest, - c.name, - c.recursive, - c.replace, - c.stdin, - c.visibility.into(), - cli_catalog.get_one()?, - tenancy_config, - cli_catalog.get_one()?, - )), + cli::Command::Add(c) => { + let predefined_accounts = + cli_catalog.get_one::()?; + + Box::new(AddCommand::new( + cli_catalog.get_one()?, + cli_catalog.get_one()?, + cli_catalog.get_one()?, + cli_catalog.get_one()?, + c.manifest, + c.name, + c.recursive, + c.replace, + c.stdin, + c.visibility.into(), + cli_catalog.get_one()?, + tenancy_config, + cli_catalog.get_one()?, + accounts::AccountService::current_account_indication( + args.account, + tenancy_config, + &predefined_accounts, + ), + )) + } cli::Command::Complete(c) => { let in_workspace = workspace_svc.is_in_workspace() && !workspace_svc.is_upgrade_needed()?; - let predefined_accounts = - cli_catalog.get_one::()?; let (dataset_registry, remote_repo_reg, remote_alias_reg, current_account) = if in_workspace { + let predefined_accounts = + cli_catalog.get_one::()?; + ( Some(cli_catalog.get_one()?), Some(cli_catalog.get_one()?), diff --git a/src/app/cli/src/commands/add_command.rs b/src/app/cli/src/commands/add_command.rs index d4fa4eb206..4d86bc0714 100644 --- a/src/app/cli/src/commands/add_command.rs +++ b/src/app/cli/src/commands/add_command.rs @@ -14,7 +14,7 @@ use kamu::domain::*; use opendatafabric::*; use super::{BatchError, CLIError, Command}; -use crate::{ConfirmDeleteService, OutputConfig}; +use crate::{accounts, ConfirmDeleteService, OutputConfig}; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -32,6 +32,7 @@ pub struct AddCommand { output_config: Arc, tenancy_config: TenancyConfig, confirm_delete_service: Arc, + current_account: accounts::CurrentAccountIndication, } impl AddCommand { @@ -49,6 +50,7 @@ impl AddCommand { output_config: Arc, tenancy_config: TenancyConfig, confirm_delete_service: Arc, + current_account: accounts::CurrentAccountIndication, ) -> Self where I: IntoIterator, @@ -68,6 +70,7 @@ impl AddCommand { output_config, tenancy_config, confirm_delete_service, + current_account, } } @@ -277,10 +280,12 @@ impl Command for AddCommand { // Delete existing datasets if we are replacing if self.replace { let mut already_exist = Vec::new(); - for s in &snapshots { + for snapshot in &snapshots { + let mut dataset_alias = snapshot.name.clone(); + if let Some(hdl) = self .dataset_registry - .try_resolve_dataset_handle_by_ref(&s.name.as_local_ref()) + .try_resolve_dataset_handle_by_ref(&dataset_alias.into_local_ref()) .await? { already_exist.push(hdl); @@ -292,8 +297,6 @@ impl Command for AddCommand { .confirm_delete(&already_exist) .await?; - // TODO: Private Datasets: delete permissions should be checked in multi-tenant - // scenario for hdl in already_exist { self.delete_dataset.execute_via_handle(&hdl).await?; }