kamu add --replace: use correct alias

kamu-data · Dec 25, 2024 · dde1bb8 · dde1bb8
1 parent 34f8f2d
commit dde1bb8
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 22 deletions.
diff --git 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
@@ -31,29 +31,40 @@ pub fn get_command(
     };
 
     let command: Box<dyn Command> = 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::<kamu_accounts::PredefinedAccountsConfig>()?;
+
+            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::<kamu_accounts::PredefinedAccountsConfig>()?;
 
             let (dataset_registry, remote_repo_reg, remote_alias_reg, current_account) =
                 if in_workspace {
+                    let predefined_accounts =
+                        cli_catalog.get_one::<kamu_accounts::PredefinedAccountsConfig>()?;
+
                     (
                         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
@@ -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<OutputConfig>,
     tenancy_config: TenancyConfig,
     confirm_delete_service: Arc<ConfirmDeleteService>,
+    current_account: accounts::CurrentAccountIndication,
 }
 
 impl AddCommand {
@@ -49,6 +50,7 @@ impl AddCommand {
         output_config: Arc<OutputConfig>,
         tenancy_config: TenancyConfig,
         confirm_delete_service: Arc<ConfirmDeleteService>,
+        current_account: accounts::CurrentAccountIndication,
     ) -> Self
     where
         I: IntoIterator<Item = S>,
@@ -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?;
                 }