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

Added baseline for getting Azure Resource Role Assignments #764

Merged
merged 45 commits into from
Jan 17, 2024
Merged

Conversation

nfx
Copy link
Collaborator

@nfx nfx commented Jan 8, 2024

This pull request adds a new command save-azure-storage-accounts to the UCX CLI tool. This command identifies all storage accounts used by tables in a HMS metastore, identifies the corresponding service principals and their permissions on each storage account, and saves the data in the CSV file on workspace. The new method AzureResourcePermissions.save_spn_permissions performs this functionality.

Additionally, the pull request adds the AzureResources class, which is used to fetch information about Azure resources such as subscriptions, storage accounts, and containers. The AzureRoleAssignment and Principal classes are also added to represent role assignments and their associated principals.

Changes related to tests include:

  • Test cases are added for the new save-azure-storage-accounts command, including tests for valid and invalid subscription IDs, as well as tests for cases where there are no external tables, no Azure storage accounts, or no valid Azure storage accounts.
  • The test case for test_azure_spn_info_without_secret is updated to use the create_autospec function to create a mock WorkspaceClient object instead of creating a mock object manually.
  • The test_move function is updated to use the patch decorator to patch the TableMove.move_tables method instead of using a mock object.
  • The test_save_azure_storage_accounts_no_ucx test case is added to test the behavior when UCX is not installed.
  • The test_save_azure_storage_accounts_not_azure test case is added to test the behavior when the workspace is not on Azure.
  • The test_save_azure_storage_accounts_no_azure_cli test case is added to test the behavior when the Azure CLI authentication method is not used.
  • The test_save_azure_storage_accounts_no_subscription_id test case is added to test the behavior when the subscription ID is not provided.

@nfx nfx requested review from a team and prajin-29 January 8, 2024 20:44
@nfx nfx temporarily deployed to account-admin January 8, 2024 20:44 — with GitHub Actions Inactive
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 30 lines in your changes are missing coverage. Please review.

Comparison is base (369893a) 84.15% compared to head (d8d3059) 84.39%.

Files Patch % Lines
src/databricks/labs/ucx/assessment/azure.py 86.78% 18 Missing and 12 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #764      +/-   ##
==========================================
+ Coverage   84.15%   84.39%   +0.23%     
==========================================
  Files          39       39              
  Lines        4601     4850     +249     
  Branches      859      907      +48     
==========================================
+ Hits         3872     4093     +221     
- Misses        528      545      +17     
- Partials      201      212      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator Author

@nfx nfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests

@nfx nfx temporarily deployed to account-admin January 16, 2024 15:55 — with GitHub Actions Inactive
@nfx nfx temporarily deployed to account-admin January 17, 2024 01:02 — with GitHub Actions Inactive
@nfx nfx merged commit 306e8aa into main Jan 17, 2024
7 checks passed
@nfx nfx deleted the spn/azure-perms branch January 17, 2024 16:19
FastLee pushed a commit that referenced this pull request Jan 19, 2024
This pull request adds a new command `save-azure-storage-accounts` to
the UCX CLI tool. This command identifies all storage accounts used by
tables in a HMS metastore, identifies the corresponding service
principals and their permissions on each storage account, and saves the
data in the CSV file on workspace. The new method
`AzureResourcePermissions.save_spn_permissions` performs this
functionality.

Additionally, the pull request adds the `AzureResources` class, which is
used to fetch information about Azure resources such as subscriptions,
storage accounts, and containers. The `AzureRoleAssignment` and
`Principal` classes are also added to represent role assignments and
their associated principals.

Changes related to tests include:

* Test cases are added for the new `save-azure-storage-accounts`
command, including tests for valid and invalid subscription IDs, as well
as tests for cases where there are no external tables, no Azure storage
accounts, or no valid Azure storage accounts.
* The test case for `test_azure_spn_info_without_secret` is updated to
use the `create_autospec` function to create a mock `WorkspaceClient`
object instead of creating a mock object manually.
* The `test_move` function is updated to use the `patch` decorator to
patch the `TableMove.move_tables` method instead of using a mock object.
* The `test_save_azure_storage_accounts_no_ucx` test case is added to
test the behavior when UCX is not installed.
* The `test_save_azure_storage_accounts_not_azure` test case is added to
test the behavior when the workspace is not on Azure.
* The `test_save_azure_storage_accounts_no_azure_cli` test case is added
to test the behavior when the Azure CLI authentication method is not
used.
* The `test_save_azure_storage_accounts_no_subscription_id` test case is
added to test the behavior when the subscription ID is not provided.

---------

Co-authored-by: Hari Selvarajan <hari.selvarajan@databricks.com>
nfx added a commit that referenced this pull request Jan 19, 2024
* Added `databricks labs ucx validate-groups-membership` command to validate groups to see if they have same membership across acount and workspace level ([#772](#772)).
* Added baseline for getting Azure Resource Role Assignments ([#764](#764)).
* Added issue and pull request templates ([#791](#791)).
* Added linked issues to PR template ([#793](#793)).
* Added optional `debug_truncate_bytes` parameter to the config and extend the default log truncation limit ([#782](#782)).
* Added support for crawling grants and applying Hive Metastore UDF ACLs ([#812](#812)).
* Changed Python requirement from 3.10.6 to 3.10 ([#805](#805)).
* Extend error handling of delta issues in crawlers and hive metastore ([#795](#795)).
* Fixed `databricks labs ucx repair-run` command to execute correctly ([#801](#801)).
* Fixed handling of `DELTASHARING` table format ([#802](#802)).
* Fixed listing of workflows via CLI ([#811](#811)).
* Fixed logger import path for DEBUG notebook ([#792](#792)).
* Fixed move table command to delete table/view regardless if permissions are present, skipping corrupted tables when crawling table size and making existing tests more stable ([#777](#777)).
* Fixed the issue of `databricks labs ucx installations` and `databricks labs ucx manual-workspace-info` ([#814](#814)).
* Increase the unit test coverage for cli.py ([#800](#800)).
* Mount Point crawler lists /Volume with four variations which is confusing ([#779](#779)).
* Updated README.md to remove mention of deprecated install.sh ([#781](#781)).
* Updated `bug` issue template ([#797](#797)).
* Fixed writing log readme in multiprocess safe way ([#794](#794)).
@nfx nfx mentioned this pull request Jan 19, 2024
nfx added a commit that referenced this pull request Jan 19, 2024
* Added `databricks labs ucx validate-groups-membership` command to
validate groups to see if they have same membership across acount and
workspace level
([#772](#772)).
* Added baseline for getting Azure Resource Role Assignments
([#764](#764)).
* Added issue and pull request templates
([#791](#791)).
* Added linked issues to PR template
([#793](#793)).
* Added optional `debug_truncate_bytes` parameter to the config and
extend the default log truncation limit
([#782](#782)).
* Added support for crawling grants and applying Hive Metastore UDF ACLs
([#812](#812)).
* Changed Python requirement from 3.10.6 to 3.10
([#805](#805)).
* Extend error handling of delta issues in crawlers and hive metastore
([#795](#795)).
* Fixed `databricks labs ucx repair-run` command to execute correctly
([#801](#801)).
* Fixed handling of `DELTASHARING` table format
([#802](#802)).
* Fixed listing of workflows via CLI
([#811](#811)).
* Fixed logger import path for DEBUG notebook
([#792](#792)).
* Fixed move table command to delete table/view regardless if
permissions are present, skipping corrupted tables when crawling table
size and making existing tests more stable
([#777](#777)).
* Fixed the issue of `databricks labs ucx installations` and `databricks
labs ucx manual-workspace-info`
([#814](#814)).
* Increase the unit test coverage for cli.py
([#800](#800)).
* Mount Point crawler lists /Volume with four variations which is
confusing ([#779](#779)).
* Updated README.md to remove mention of deprecated install.sh
([#781](#781)).
* Updated `bug` issue template
([#797](#797)).
* Fixed writing log readme in multiprocess safe way
([#794](#794)).
dmoore247 pushed a commit that referenced this pull request Mar 23, 2024
* Added `databricks labs ucx validate-groups-membership` command to
validate groups to see if they have same membership across acount and
workspace level
([#772](#772)).
* Added baseline for getting Azure Resource Role Assignments
([#764](#764)).
* Added issue and pull request templates
([#791](#791)).
* Added linked issues to PR template
([#793](#793)).
* Added optional `debug_truncate_bytes` parameter to the config and
extend the default log truncation limit
([#782](#782)).
* Added support for crawling grants and applying Hive Metastore UDF ACLs
([#812](#812)).
* Changed Python requirement from 3.10.6 to 3.10
([#805](#805)).
* Extend error handling of delta issues in crawlers and hive metastore
([#795](#795)).
* Fixed `databricks labs ucx repair-run` command to execute correctly
([#801](#801)).
* Fixed handling of `DELTASHARING` table format
([#802](#802)).
* Fixed listing of workflows via CLI
([#811](#811)).
* Fixed logger import path for DEBUG notebook
([#792](#792)).
* Fixed move table command to delete table/view regardless if
permissions are present, skipping corrupted tables when crawling table
size and making existing tests more stable
([#777](#777)).
* Fixed the issue of `databricks labs ucx installations` and `databricks
labs ucx manual-workspace-info`
([#814](#814)).
* Increase the unit test coverage for cli.py
([#800](#800)).
* Mount Point crawler lists /Volume with four variations which is
confusing ([#779](#779)).
* Updated README.md to remove mention of deprecated install.sh
([#781](#781)).
* Updated `bug` issue template
([#797](#797)).
* Fixed writing log readme in multiprocess safe way
([#794](#794)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants