add unit test for execute_migration, set keyword arg filename when ca…

…lling installation.load

src/databricks/labs/ucx/azure/access.py

@@ -109,6 +109,6 @@ def load(self) -> list[StoragePermissionMapping]:
         Load StoragePermissionMapping info from azure_storage_account_info.csv
         :return:
         """
-        storage_account_infos = self._installation.load(list[StoragePermissionMapping], self._filename)
+        storage_account_infos = self._installation.load(list[StoragePermissionMapping], filename=self._filename)
 
         return storage_account_infos

src/databricks/labs/ucx/migration/azure_credentials.py

@@ -8,17 +8,14 @@
 from databricks.sdk import WorkspaceClient
 from databricks.sdk.errors import (
     InternalError,
-    PermissionDenied,
-    ResourceDoesNotExist,
-    Unauthenticated,
+    ResourceDoesNotExist
 )
 from databricks.sdk.service.catalog import (
     AzureServicePrincipal,
     Privilege,
     StorageCredentialInfo,
     ValidateStorageCredentialResponse,
     ValidationResult,
-    ValidationResultOperation,
 )
 
 from databricks.labs.ucx.assessment.azure import (
@@ -44,8 +41,6 @@ class StorageCredentialValidationResult:
     azure_service_principal: AzureServicePrincipal
     created_by: str
     read_only: bool
-    message: str
-    operation: ValidationResultOperation
     results: list[ValidationResult]
 
     @classmethod
@@ -57,7 +52,7 @@ def from_storage_credential_validation(
             azure_service_principal=storage_credential.azure_service_principal,
             created_by=storage_credential.created_by,
             read_only=storage_credential.read_only,
-            results=validation.results,
+            results=validation.results
         )
 
 
@@ -219,7 +214,9 @@ def _create_storage_credential(self, sp_migration: ServicePrincipalMigrationInfo
         )
         comment = f"Created by UCX during migration to UC using Azure Service Principal: {sp_migration.service_principal.principal}"
         read_only = False
-        if sp_migration.service_principal.privilege == Privilege.READ_FILES:
+        p = Privilege.READ_FILES
+        sp = sp_migration.service_principal.privilege
+        if sp_migration.service_principal.privilege == Privilege.READ_FILES.value:
             read_only = True
         # create the storage credential
         storage_credential = self._ws.storage_credentials.create(

tests/unit/migration/test_azure_credentials.py

@@ -13,6 +13,8 @@
     AzureManagedIdentity,
     AzureServicePrincipal,
     StorageCredentialInfo,
+    ValidateStorageCredentialResponse,
+    ValidationResult
 )
 from databricks.sdk.service.workspace import GetSecretResponse
 
@@ -26,21 +28,22 @@
 from tests.unit.test_cli import ws
 
 
-def test_for_cli_not_azure():
+def test_for_cli_not_azure(caplog):
     w = create_autospec(WorkspaceClient)
-    w.config.is_azure.return_value = False
+    w.config.is_azure = False
     assert AzureServicePrincipalMigration.for_cli(w, MagicMock()) is None
+    assert "Workspace is not on azure, please run this command on azure databricks workspaces." in caplog.text
 
 
 def test_for_cli_not_prompts():
     w = create_autospec(WorkspaceClient)
-    w.config.is_azure.return_value = True
+    w.config.is_azure = True
     prompts = MockPrompts({"Have you reviewed the azure_storage_account_info.csv *": "No"})
     assert AzureServicePrincipalMigration.for_cli(w, prompts) is None
 
 
 def test_for_cli(ws):
-    ws.config.is_azure.return_value = True
+    ws.config.is_azure = True
     prompts = MockPrompts({"Have you reviewed the azure_storage_account_info.csv *": "Yes"})
 
     assert isinstance(AzureServicePrincipalMigration.for_cli(ws, prompts), AzureServicePrincipalMigration)
@@ -140,7 +143,7 @@ def test_print_action_plan(capsys):
 
 
 def test_generate_migration_list(capsys, mocker, ws):
-    ws.config.is_azure.return_value = True
+    ws.config.is_azure = True
     ws.secrets.get_secret.return_value = GetSecretResponse(value="aGVsbG8gd29ybGQ=")
     ws.storage_credentials.list.return_value = [
         StorageCredentialInfo(
@@ -164,3 +167,75 @@ def test_generate_migration_list(capsys, mocker, ws):
 
     assert "app_secret2" in capsys.readouterr().out
 
+
+def test_execute_migration_no_confirmation(mocker, ws):
+    ws.config.is_azure = True
+    prompts = MockPrompts({"Have you reviewed the azure_storage_account_info.csv *": "Yes",
+                           "Above Azure Service Principals will be migrated to UC storage credentials*": "No"})
+
+    mocker.patch("databricks.labs.ucx.migration.azure_credentials.AzureServicePrincipalMigration._generate_migration_list")
+
+    with patch("databricks.labs.ucx.migration.azure_credentials.AzureServicePrincipalMigration._create_storage_credential") as c:
+        sp_migration = AzureServicePrincipalMigration.for_cli(ws, prompts)
+        sp_migration.execute_migration(prompts)
+        c.assert_not_called()
+
+
+def side_effect_create_storage_credential(name, azure_service_principal, comment, read_only):
+    return StorageCredentialInfo(name=name, azure_service_principal=azure_service_principal, comment=comment, read_only=read_only)
+
+def side_effect_validate_storage_credential(storage_credential_name, url):
+    if "read" in storage_credential_name:
+        response = {
+            "is_dir": True,
+            "results": [
+                {
+                    "message": "",
+                    "operation":["DELETE", "LIST", "READ", "WRITE"],
+                    "result": ["SKIP", "PASS", "PASS", "SKIP"]
+                }
+            ]
+        }
+        return ValidateStorageCredentialResponse.from_dict(response)
+    else:
+        response = {
+            "is_dir": True,
+            "results": [
+                {
+                    "message": "",
+                    "operation":["DELETE", "LIST", "READ", "WRITE"],
+                    "result": ["PASS", "PASS", "PASS", "PASS"]
+                }
+            ]
+        }
+        return ValidateStorageCredentialResponse.from_dict(response)
+
+def test_execute_migration(capsys, mocker, ws):
+    ws.config.is_azure = True
+    ws.secrets.get_secret.return_value = GetSecretResponse(value="aGVsbG8gd29ybGQ=")
+    ws.storage_credentials.list.return_value = [
+        StorageCredentialInfo(
+            azure_service_principal=AzureServicePrincipal(
+                application_id="app_secret1",
+                directory_id="directory_id_1",
+                client_secret="hello world",
+            )
+        )
+    ]
+    ws.storage_credentials.create.side_effect = side_effect_create_storage_credential
+    ws.storage_credentials.validate.side_effect = side_effect_validate_storage_credential
+
+    prompts = MockPrompts({"Have you reviewed the azure_storage_account_info.csv *": "Yes",
+                           "Above Azure Service Principals will be migrated to UC storage credentials*": "Yes"})
+
+    mocker.patch("databricks.labs.ucx.assessment.azure.AzureResourcePermissions.load", return_value = [StoragePermissionMapping(prefix="prefix1",client_id="app_secret1",principal="principal_1",privilege="WRITE_FILES",directory_id="directory_id_1"),
+                                                                                                       StoragePermissionMapping(prefix="prefix2",client_id="app_secret2",principal="principal_read",privilege="READ_FILES",directory_id="directory_id_1"),
+                                                                                                       StoragePermissionMapping(prefix="prefix3",client_id="app_secret3",principal="principal_write",privilege="WRITE_FILES",directory_id="directory_id_2")])
+    mocker.patch("databricks.labs.ucx.assessment.azure.AzureServicePrincipalCrawler.snapshot", return_value=[AzureServicePrincipalInfo("app_secret1", "test_scope", "test_key", "tenant_id_1", "storage1"),
+                                                                                                             AzureServicePrincipalInfo("app_secret2", "test_scope", "test_key", "tenant_id_1", "storage1"),
+                                                                                                             AzureServicePrincipalInfo("app_secret3", "test_scope", "test_key", "tenant_id_2", "storage1")])
+
+    sp_migration = AzureServicePrincipalMigration.for_cli(ws, prompts)
+    sp_migration.execute_migration(prompts)
+
+    assert "Completed migration" in capsys.readouterr().out