From cea0e37263267d804b8f49480837f10e84eedc22 Mon Sep 17 00:00:00 2001 From: Yong Date: Sun, 12 Apr 2026 01:40:01 -0500 Subject: [PATCH 1/2] Fix setup command default issue --- .../cli/command/catalog_roles.py | 80 +++++---- .../apache_polaris/cli/command/catalogs.py | 157 ++++++++++------- .../python/apache_polaris/cli/command/find.py | 18 +- .../apache_polaris/cli/command/namespaces.py | 74 +++++--- .../apache_polaris/cli/command/policies.py | 93 ++++++---- .../cli/command/principal_roles.py | 65 ++++--- .../apache_polaris/cli/command/principals.py | 77 ++++++--- .../apache_polaris/cli/command/privileges.py | 51 +++--- .../apache_polaris/cli/command/profiles.py | 26 ++- .../apache_polaris/cli/command/setup.py | 160 +++++++----------- .../apache_polaris/cli/command/tables.py | 59 ++++--- .../python/test/test_catalog_roles_command.py | 21 ++- client/python/test/test_catalogs_command.py | 19 +++ client/python/test/test_namespaces_command.py | 28 ++- client/python/test/test_principals_command.py | 15 ++ client/python/test/test_privileges_command.py | 42 +++++ client/python/test/test_setup_command.py | 16 ++ 17 files changed, 624 insertions(+), 377 deletions(-) diff --git a/client/python/apache_polaris/cli/command/catalog_roles.py b/client/python/apache_polaris/cli/command/catalog_roles.py index de1b40bfc3..6d6ad617b4 100644 --- a/client/python/apache_polaris/cli/command/catalog_roles.py +++ b/client/python/apache_polaris/cli/command/catalog_roles.py @@ -16,8 +16,8 @@ # specific language governing permissions and limitations # under the License. # -from dataclasses import dataclass -from typing import Dict, Optional, List +from dataclasses import dataclass, field +from typing import Dict, Optional, List, cast from pydantic import StrictStr @@ -48,18 +48,39 @@ class CatalogRolesCommand(Command): """ catalog_roles_subcommand: str - catalog_name: str - catalog_role_name: str - principal_role_name: str - properties: Optional[Dict[str, StrictStr]] - set_properties: Dict[str, StrictStr] - remove_properties: List[str] + catalog_name: Optional[str] = None + catalog_role_name: Optional[str] = None + principal_role_name: Optional[str] = None + properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + set_properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + remove_properties: Optional[List[str]] = None + + def __post_init__(self) -> None: + if self.properties is None: + self.properties = {} + if self.set_properties is None: + self.set_properties = {} def validate(self) -> None: if not self.catalog_name: raise Exception( f"Missing required argument: {Argument.to_flag_name(Arguments.CATALOG)}" ) + + if self.catalog_roles_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.GET, + Subcommands.UPDATE, + Subcommands.GRANT, + Subcommands.REVOKE, + Subcommands.SUMMARIZE, + }: + if not self.catalog_role_name: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.CATALOG_ROLE)}" + ) + if self.catalog_roles_subcommand in {Subcommands.GRANT, Subcommands.REVOKE}: if not self.principal_role_name: raise Exception( @@ -67,34 +88,31 @@ def validate(self) -> None: ) def execute(self, api: PolarisDefaultApi) -> None: + catalog_name = cast(str, self.catalog_name) + catalog_role_name = cast(str, self.catalog_role_name) + if self.catalog_roles_subcommand == Subcommands.CREATE: request = CreateCatalogRoleRequest( catalog_role=CatalogRole( - name=self.catalog_role_name, properties=self.properties + name=catalog_role_name, properties=self.properties ) ) - api.create_catalog_role(self.catalog_name, request) + api.create_catalog_role(catalog_name, request) elif self.catalog_roles_subcommand == Subcommands.DELETE: - api.delete_catalog_role(self.catalog_name, self.catalog_role_name) + api.delete_catalog_role(catalog_name, catalog_role_name) elif self.catalog_roles_subcommand == Subcommands.GET: - print( - api.get_catalog_role( - self.catalog_name, self.catalog_role_name - ).to_json() - ) + print(api.get_catalog_role(catalog_name, catalog_role_name).to_json()) elif self.catalog_roles_subcommand == Subcommands.LIST: if self.principal_role_name: for catalog_role in api.list_catalog_roles_for_principal_role( - self.principal_role_name, self.catalog_name + cast(str, self.principal_role_name), catalog_name ).roles: print(catalog_role.to_json()) else: - for catalog_role in api.list_catalog_roles(self.catalog_name).roles: + for catalog_role in api.list_catalog_roles(catalog_name).roles: print(catalog_role.to_json()) elif self.catalog_roles_subcommand == Subcommands.UPDATE: - catalog_role = api.get_catalog_role( - self.catalog_name, self.catalog_role_name - ) + catalog_role = api.get_catalog_role(catalog_name, catalog_role_name) new_properties = catalog_role.properties or {} # Add or update all entries specified in set_properties @@ -110,18 +128,18 @@ def execute(self, api: PolarisDefaultApi) -> None: current_entity_version=catalog_role.entity_version, properties=new_properties, ) - api.update_catalog_role(self.catalog_name, self.catalog_role_name, request) + api.update_catalog_role(catalog_name, catalog_role_name, request) elif self.catalog_roles_subcommand == Subcommands.GRANT: request = GrantCatalogRoleRequest( - catalog_role=CatalogRole(name=self.catalog_role_name), + catalog_role=CatalogRole(name=catalog_role_name), properties=self.properties, ) api.assign_catalog_role_to_principal_role( - self.principal_role_name, self.catalog_name, request + cast(str, self.principal_role_name), catalog_name, request ) elif self.catalog_roles_subcommand == Subcommands.REVOKE: api.revoke_catalog_role_from_principal_role( - self.principal_role_name, self.catalog_name, self.catalog_role_name + cast(str, self.principal_role_name), catalog_name, catalog_role_name ) elif self.catalog_roles_subcommand == Subcommands.SUMMARIZE: self._generate_summary(api) @@ -131,16 +149,16 @@ def execute(self, api: PolarisDefaultApi) -> None: ) def _generate_summary(self, api: PolarisDefaultApi) -> None: + catalog_name = cast(str, self.catalog_name) + catalog_role_name = cast(str, self.catalog_role_name) grants = ( - api.list_grants_for_catalog_role( - self.catalog_name, self.catalog_role_name - ).grants + api.list_grants_for_catalog_role(catalog_name, catalog_role_name).grants or [] ) - print(f"Catalog Role: {self.catalog_role_name} [Catalog: {self.catalog_name}]") + print(f"Catalog Role: {catalog_role_name} [Catalog: {catalog_name}]") print("-" * 80) # Metadata - role = api.get_catalog_role(self.catalog_name, self.catalog_role_name) + role = api.get_catalog_role(catalog_name, catalog_role_name) print("Metadata") print(f" {'Created:':<30} {format_timestamp(role.create_timestamp)}") print(f" {'Modified:':<30} {format_timestamp(role.last_update_timestamp)}") @@ -149,7 +167,7 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: # Assignments principal_roles = ( api.list_assignee_principal_roles_for_catalog_role( - self.catalog_name, self.catalog_role_name + catalog_name, catalog_role_name ).roles or [] ) diff --git a/client/python/apache_polaris/cli/command/catalogs.py b/client/python/apache_polaris/cli/command/catalogs.py index a1963ca9da..be4b4a58fa 100644 --- a/client/python/apache_polaris/cli/command/catalogs.py +++ b/client/python/apache_polaris/cli/command/catalogs.py @@ -30,9 +30,9 @@ ) from apache_polaris.cli.options.option_tree import Argument -from dataclasses import dataclass +from dataclasses import dataclass, field from pydantic import StrictStr, SecretStr -from typing import Dict, List, Optional, Union, Tuple, Callable +from typing import Dict, List, Optional, Union, Tuple, Callable, cast from apache_polaris.sdk.management import ( PolarisDefaultApi, @@ -73,51 +73,77 @@ class CatalogsCommand(Command): """ catalogs_subcommand: str - catalog_type: str - default_base_location: str - storage_type: str - allowed_locations: List[str] - role_arn: str - external_id: str - user_arn: str - region: str - tenant_id: str - multi_tenant_app_name: str - hierarchical: bool - consent_url: str - service_account: str - catalog_name: str - properties: Dict[str, StrictStr] - set_properties: Dict[str, StrictStr] - remove_properties: List[str] - hadoop_warehouse: str - hive_warehouse: str - iceberg_remote_catalog_name: str - endpoint: str - endpoint_internal: str - sts_endpoint: str - sts_unavailable: bool - kms_unavailable: bool - path_style_access: bool - current_kms_key: str - allowed_kms_keys: List[str] - catalog_connection_type: str - catalog_authentication_type: str - catalog_service_identity_type: str - catalog_service_identity_iam_arn: str - catalog_uri: str - catalog_token_uri: str - catalog_client_id: str - catalog_client_secret: str - catalog_client_scopes: List[str] - catalog_bearer_token: str - catalog_role_arn: str - catalog_role_session_name: str - catalog_external_id: str - catalog_signing_region: str - catalog_signing_name: str + catalog_type: Optional[str] = None + default_base_location: Optional[str] = None + storage_type: Optional[str] = None + allowed_locations: Optional[List[str]] = None + role_arn: Optional[str] = None + external_id: Optional[str] = None + user_arn: Optional[str] = None + region: Optional[str] = None + tenant_id: Optional[str] = None + multi_tenant_app_name: Optional[str] = None + hierarchical: Optional[bool] = None + consent_url: Optional[str] = None + service_account: Optional[str] = None + catalog_name: Optional[str] = None + properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + set_properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + remove_properties: Optional[List[str]] = None + hadoop_warehouse: Optional[str] = None + hive_warehouse: Optional[str] = None + iceberg_remote_catalog_name: Optional[str] = None + endpoint: Optional[str] = None + endpoint_internal: Optional[str] = None + sts_endpoint: Optional[str] = None + sts_unavailable: Optional[bool] = None + kms_unavailable: Optional[bool] = None + path_style_access: Optional[bool] = None + current_kms_key: Optional[str] = None + allowed_kms_keys: Optional[List[str]] = None + catalog_connection_type: Optional[str] = None + catalog_authentication_type: Optional[str] = None + catalog_service_identity_type: Optional[str] = None + catalog_service_identity_iam_arn: Optional[str] = None + catalog_uri: Optional[str] = None + catalog_token_uri: Optional[str] = None + catalog_client_id: Optional[str] = None + catalog_client_secret: Optional[str] = None + catalog_client_scopes: Optional[List[str]] = None + catalog_bearer_token: Optional[str] = None + catalog_role_arn: Optional[str] = None + catalog_role_session_name: Optional[str] = None + catalog_external_id: Optional[str] = None + catalog_signing_region: Optional[str] = None + catalog_signing_name: Optional[str] = None + + def __post_init__(self) -> None: + if self.properties is None: + self.properties = {} + if self.set_properties is None: + self.set_properties = {} + if self.hierarchical is None: + self.hierarchical = False + if self.sts_unavailable is None: + self.sts_unavailable = False + if self.kms_unavailable is None: + self.kms_unavailable = False + if self.path_style_access is None: + self.path_style_access = False def validate(self) -> None: + if self.catalogs_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.GET, + Subcommands.UPDATE, + Subcommands.SUMMARIZE, + }: + if not self.catalog_name: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.CATALOG)}" + ) + if self.catalogs_subcommand == Subcommands.CREATE: if self.catalog_type != CatalogType.EXTERNAL.value: if not self.storage_type: @@ -136,6 +162,7 @@ def validate(self) -> None: not self.catalog_token_uri or not self.catalog_client_id or not self.catalog_client_secret + or not self.catalog_client_scopes or len(self.catalog_client_scopes) == 0 ): raise Exception( @@ -380,14 +407,17 @@ def _build_connection_config_info( return config def execute(self, api: PolarisDefaultApi) -> None: + catalog_type = cast(str, self.catalog_type) + catalog_name = cast(str, self.catalog_name) + if self.catalogs_subcommand == Subcommands.CREATE: storage_config = self._build_storage_config_info() connection_config = self._build_connection_config_info() if self.catalog_type == CatalogType.EXTERNAL.value: request = CreateCatalogRequest( catalog=ExternalCatalog( - type=self.catalog_type.upper(), - name=self.catalog_name, + type=catalog_type.upper(), + name=catalog_name, storage_config_info=storage_config, properties=CatalogProperties( default_base_location=self.default_base_location, @@ -399,8 +429,8 @@ def execute(self, api: PolarisDefaultApi) -> None: else: request = CreateCatalogRequest( catalog=PolarisCatalog( - type=self.catalog_type.upper(), - name=self.catalog_name, + type=catalog_type.upper(), + name=catalog_name, storage_config_info=storage_config, properties=CatalogProperties( default_base_location=self.default_base_location, @@ -411,14 +441,14 @@ def execute(self, api: PolarisDefaultApi) -> None: ) api.create_catalog(request) elif self.catalogs_subcommand == Subcommands.DELETE: - api.delete_catalog(self.catalog_name) + api.delete_catalog(catalog_name) elif self.catalogs_subcommand == Subcommands.GET: - print(api.get_catalog(self.catalog_name).to_json()) + print(api.get_catalog(catalog_name).to_json()) elif self.catalogs_subcommand == Subcommands.LIST: for catalog in api.list_catalogs().catalogs: print(catalog.to_json()) elif self.catalogs_subcommand == Subcommands.UPDATE: - catalog = api.get_catalog(self.catalog_name) + catalog = api.get_catalog(catalog_name) if ( self.default_base_location @@ -490,24 +520,25 @@ def execute(self, api: PolarisDefaultApi) -> None: properties=catalog.properties.to_dict(), ) - api.update_catalog(self.catalog_name, request) + api.update_catalog(catalog_name, request) elif self.catalogs_subcommand == Subcommands.SUMMARIZE: self._generate_summary(api) else: raise Exception(f"{self.catalogs_subcommand} is not supported in the CLI") def _generate_summary(self, api: PolarisDefaultApi) -> None: - print(f"Catalog: {self.catalog_name}") + catalog_name = cast(str, self.catalog_name) + print(f"Catalog: {catalog_name}") print("-" * 80) # Metadata - catalog = api.get_catalog(self.catalog_name) + catalog = api.get_catalog(catalog_name) print("Metadata") print(f" {'Type:':<30} {catalog.type}") storage_info = catalog.storage_config_info print(f" {'Storage Type:':<30} {storage_info.storage_type}") print(f" {'Base Location:':<30} {catalog.properties.default_base_location}") print( - f" {'Allowed Locations:':<30} {', '.join(storage_info.allowed_locations or [])}" + f" {'Allowed Locations:':<30} {', '.join(storage_info.allowed_locations)}" ) print(f" {'Created:':<30} {format_timestamp(catalog.create_timestamp)}") print(f" {'Modified:':<30} {format_timestamp(catalog.last_update_timestamp)}") @@ -522,11 +553,11 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: print(f" {'Views:':<30} {total_views}") # Access Control - catalog_roles = api.list_catalog_roles(self.catalog_name).roles or [] + catalog_roles = api.list_catalog_roles(catalog_name).roles or [] principal_roles_names = set() for catalog_role in catalog_roles: pr_resp = api.list_assignee_principal_roles_for_catalog_role( - self.catalog_name, catalog_role.name + catalog_name, catalog_role.name ) roles = pr_resp.roles or [] for pr in roles: @@ -537,7 +568,7 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: # Policies policy_api = PolicyAPI(catalog_api.api_client) - policies_resp = policy_api.get_applicable_policies(prefix=self.catalog_name) + policies_resp = policy_api.get_applicable_policies(prefix=catalog_name) print("\nApplicable Policies") applicable_policies = policies_resp.applicable_policies or [] if applicable_policies: @@ -550,9 +581,11 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: def _get_sub_entities_count( self, catalog_api: IcebergCatalogAPI ) -> Tuple[int, int, int]: + catalog_name = cast(str, self.catalog_name) + def count_entities(func: Callable, ns_str: str) -> int: try: - resp = func(prefix=self.catalog_name, namespace=ns_str) + resp = func(prefix=catalog_name, namespace=ns_str) return len(resp.identifiers or []) except Exception as e: print( @@ -561,9 +594,7 @@ def count_entities(func: Callable, ns_str: str) -> int: return 0 def recursive(parent: Optional[str] = None) -> Tuple[int, int, int]: - ns_resp = catalog_api.list_namespaces( - prefix=self.catalog_name, parent=parent - ) + ns_resp = catalog_api.list_namespaces(prefix=catalog_name, parent=parent) namespaces = ns_resp.namespaces or [] total_ns, total_tables, total_views = len(namespaces), 0, 0 for ns in namespaces: diff --git a/client/python/apache_polaris/cli/command/find.py b/client/python/apache_polaris/cli/command/find.py index 62b17a1c73..080f1a5a50 100644 --- a/client/python/apache_polaris/cli/command/find.py +++ b/client/python/apache_polaris/cli/command/find.py @@ -18,7 +18,7 @@ # from collections import Counter from dataclasses import dataclass, field -from typing import Optional, List, Tuple +from typing import Optional, List, Tuple, cast from apache_polaris.cli.command import Command from apache_polaris.cli.command.utils import ( @@ -52,12 +52,13 @@ class FindCommand(Command): _type_counts: Counter = field(default_factory=Counter, repr=False) def validate(self) -> None: - if not self.identifier.strip(): + if not self.identifier or not self.identifier.strip(): raise Exception("The search identifier cannot be empty.") def execute(self, api: PolarisDefaultApi) -> None: - print(f"Searching for '{self.identifier}'...") - parts = self.identifier.split(".") + identifier = cast(str, self.identifier) + print(f"Searching for '{identifier}'...") + parts = identifier.split(".") target_ns = parts[:-1] target_leaf = parts[-1] @@ -106,7 +107,7 @@ def execute(self, api: PolarisDefaultApi) -> None: ) print(f"\nFound {total_results} matches ({types_str}).") else: - print(f"No identifiers found matching '{self.identifier}'.") + print(f"No identifiers found matching '{identifier}'.") def _resolve_search_scope( self, @@ -114,12 +115,13 @@ def _resolve_search_scope( target_ns: List[str], ) -> Tuple[Optional[List[str]], List[str]]: if self.catalog_name: + catalog_name = cast(str, self.catalog_name) try: - api.get_catalog(self.catalog_name) - return [self.catalog_name], target_ns + api.get_catalog(catalog_name) + return [catalog_name], target_ns except Exception as e: if getattr(e, "status", None) == 404: - print(f"Catalog '{self.catalog_name}' not found.") + print(f"Catalog '{catalog_name}' not found.") return None, [] handle_api_exception("Catalog Access", e) return [], target_ns diff --git a/client/python/apache_polaris/cli/command/namespaces.py b/client/python/apache_polaris/cli/command/namespaces.py index b85e4c6507..354b90da69 100644 --- a/client/python/apache_polaris/cli/command/namespaces.py +++ b/client/python/apache_polaris/cli/command/namespaces.py @@ -17,9 +17,9 @@ # under the License. # import json -from dataclasses import dataclass +from dataclasses import dataclass, field from pydantic import StrictStr -from typing import Dict, Optional, List +from typing import Dict, Optional, List, cast from apache_polaris.cli.command import Command from apache_polaris.cli.command.utils import get_catalog_api_client @@ -44,48 +44,69 @@ class NamespacesCommand(Command): """ namespaces_subcommand: str - catalog: str - namespace: List[StrictStr] - parent: List[StrictStr] - location: str - properties: Optional[Dict[str, StrictStr]] + catalog: Optional[str] = None + namespace: Optional[List[StrictStr]] = None + parent: Optional[List[StrictStr]] = None + location: Optional[str] = None + properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + + def __post_init__(self) -> None: + if self.properties is None: + self.properties = {} def validate(self) -> None: if not self.catalog: raise Exception( f"Missing required argument: {Argument.to_flag_name(Arguments.CATALOG)}" ) + if self.namespaces_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.GET, + Subcommands.SUMMARIZE, + }: + if not self.namespace: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.NAMESPACE)}" + ) def execute(self, api: PolarisDefaultApi) -> None: catalog_api_client = get_catalog_api_client(api) catalog_api = IcebergCatalogAPI(catalog_api_client) + + catalog_name = cast(str, self.catalog) + namespace = cast(List[str], self.namespace) + if self.namespaces_subcommand == Subcommands.CREATE: req_properties = self.properties or {} if self.location: req_properties = {**req_properties, Arguments.LOCATION: self.location} request = CreateNamespaceRequest( - namespace=self.namespace, properties=req_properties + namespace=namespace, + properties=req_properties, ) catalog_api.create_namespace( - prefix=self.catalog, create_namespace_request=request + prefix=catalog_name, create_namespace_request=request ) elif self.namespaces_subcommand == Subcommands.LIST: - if self.parent is not None: + if self.parent: result = catalog_api.list_namespaces( - prefix=self.catalog, parent=UNIT_SEPARATOR.join(self.parent) + prefix=catalog_name, parent=UNIT_SEPARATOR.join(self.parent) ) else: - result = catalog_api.list_namespaces(prefix=self.catalog) - for namespace in result.namespaces: - print(json.dumps({"namespace": ".".join(namespace)})) + result = catalog_api.list_namespaces(prefix=catalog_name) + for namespace_item in result.namespaces: + print(json.dumps({"namespace": ".".join(namespace_item)})) elif self.namespaces_subcommand == Subcommands.DELETE: catalog_api.drop_namespace( - prefix=self.catalog, namespace=UNIT_SEPARATOR.join(self.namespace) + prefix=catalog_name, + namespace=UNIT_SEPARATOR.join(namespace), ) elif self.namespaces_subcommand == Subcommands.GET: print( catalog_api.load_namespace_metadata( - prefix=self.catalog, namespace=UNIT_SEPARATOR.join(self.namespace) + prefix=catalog_name, + namespace=UNIT_SEPARATOR.join(namespace), ).to_json() ) elif self.namespaces_subcommand == Subcommands.SUMMARIZE: @@ -94,26 +115,29 @@ def execute(self, api: PolarisDefaultApi) -> None: raise Exception(f"{self.namespaces_subcommand} is not supported in the CLI") def _generate_summary(self, catalog_api: IcebergCatalogAPI) -> None: - ns_str = UNIT_SEPARATOR.join(self.namespace) - print(f"Namespace: {'.'.join(self.namespace)}") + catalog_name = cast(str, self.catalog) + namespace = cast(List[str], self.namespace) + ns_str = UNIT_SEPARATOR.join(namespace) + + print(f"Namespace: {'.'.join(namespace)}") print("-" * 80) # Metadata print("Metadata") - print(f" {'Level:':<30} {len(self.namespace)}") - if len(self.namespace) > 1: - print(f" {'Parent:':<30} {'.'.join(self.namespace[:-1])}") + print(f" {'Level:':<30} {len(namespace)}") + if len(namespace) > 1: + print(f" {'Parent:':<30} {'.'.join(namespace[:-1])}") sub_ns = ( - catalog_api.list_namespaces(prefix=self.catalog, parent=ns_str).namespaces + catalog_api.list_namespaces(prefix=catalog_name, parent=ns_str).namespaces or [] ) print(f" {'Sub-namespaces:':<30} {len(sub_ns)}") tables = ( - catalog_api.list_tables(prefix=self.catalog, namespace=ns_str).identifiers + catalog_api.list_tables(prefix=catalog_name, namespace=ns_str).identifiers or [] ) print(f" {'Tables:':<30} {len(tables)}") views = ( - catalog_api.list_views(prefix=self.catalog, namespace=ns_str).identifiers + catalog_api.list_views(prefix=catalog_name, namespace=ns_str).identifiers or [] ) print(f" {'Views:':<30} {len(views)}") @@ -121,7 +145,7 @@ def _generate_summary(self, catalog_api: IcebergCatalogAPI) -> None: # Effective policies policy_api = PolicyAPI(catalog_api.api_client) policies_resp = policy_api.get_applicable_policies( - prefix=self.catalog, namespace=ns_str + prefix=catalog_name, namespace=ns_str ) print("\nEffective Policies") applicable_policies = policies_resp.applicable_policies or [] diff --git a/client/python/apache_polaris/cli/command/policies.py b/client/python/apache_polaris/cli/command/policies.py index 783f605d16..601fcdd34e 100644 --- a/client/python/apache_polaris/cli/command/policies.py +++ b/client/python/apache_polaris/cli/command/policies.py @@ -19,8 +19,8 @@ import json -from dataclasses import dataclass -from typing import Optional, Dict, List +from dataclasses import dataclass, field +from typing import Optional, Dict, List, cast from apache_polaris.cli.command import Command from apache_polaris.cli.command.utils import get_catalog_api_client @@ -51,24 +51,42 @@ class PoliciesCommand(Command): """ policies_subcommand: str - catalog_name: str - namespace: str - policy_name: str - policy_file: str - policy_type: Optional[str] - policy_description: Optional[str] - target_name: Optional[str] - parameters: Optional[Dict[str, str]] - detach_all: Optional[bool] - applicable: Optional[bool] - attachment_type: Optional[str] - attachment_path: Optional[str] + catalog_name: Optional[str] = None + namespace: Optional[str] = None + policy_name: Optional[str] = None + policy_file: Optional[str] = None + policy_type: Optional[str] = None + policy_description: Optional[str] = None + target_name: Optional[str] = None + parameters: Optional[Dict[str, str]] = field(default_factory=dict) + detach_all: Optional[bool] = None + applicable: Optional[bool] = None + attachment_type: Optional[str] = None + attachment_path: Optional[str] = None + + def __post_init__(self) -> None: + if self.parameters is None: + self.parameters = {} def validate(self) -> None: if not self.catalog_name: raise Exception( f"Missing required argument: {Argument.to_flag_name(Arguments.CATALOG)}" ) + + if self.policies_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.GET, + Subcommands.UPDATE, + Subcommands.ATTACH, + Subcommands.DETACH, + }: + if not self.policy_name: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.POLICY)}" + ) + if self.policies_subcommand in [Subcommands.CREATE, Subcommands.UPDATE]: if not self.policy_file: raise Exception( @@ -104,17 +122,21 @@ def execute(self, api: PolarisDefaultApi) -> None: catalog_api_client = get_catalog_api_client(api) policy_api = PolicyAPI(catalog_api_client) + catalog_name = cast(str, self.catalog_name) + policy_name = cast(str, self.policy_name) + namespace_str = ( self.namespace.replace(".", UNIT_SEPARATOR) if self.namespace else "" ) if self.policies_subcommand == Subcommands.CREATE: - with open(self.policy_file, "r") as f: + policy_file = cast(str, self.policy_file) + with open(policy_file, "r") as f: policy = json.load(f) policy_api.create_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, create_policy_request=CreatePolicyRequest( - name=self.policy_name, + name=policy_name, type=self.policy_type, description=self.policy_description, content=json.dumps(policy), @@ -122,17 +144,17 @@ def execute(self, api: PolarisDefaultApi) -> None: ) elif self.policies_subcommand == Subcommands.DELETE: policy_api.drop_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, - policy_name=self.policy_name, + policy_name=policy_name, detach_all=self.detach_all, ) elif self.policies_subcommand == Subcommands.GET: print( policy_api.load_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, - policy_name=self.policy_name, + policy_name=policy_name, ).to_json() ) elif self.policies_subcommand == Subcommands.LIST: @@ -142,7 +164,7 @@ def execute(self, api: PolarisDefaultApi) -> None: if self.target_name: # Table-like level policies applicable_policies_list = policy_api.get_applicable_policies( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, target_name=self.target_name, policy_type=self.policy_type, @@ -150,45 +172,46 @@ def execute(self, api: PolarisDefaultApi) -> None: elif self.namespace: # Namespace level policies applicable_policies_list = policy_api.get_applicable_policies( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, policy_type=self.policy_type, ).applicable_policies else: # Catalog level policies applicable_policies_list = policy_api.get_applicable_policies( - prefix=self.catalog_name, policy_type=self.policy_type + prefix=catalog_name, policy_type=self.policy_type ).applicable_policies for policy in applicable_policies_list: print(policy.to_json()) else: # List all policy identifiers in the namespace policies_response = policy_api.list_policies( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, policy_type=self.policy_type, ).to_json() print(policies_response) elif self.policies_subcommand == Subcommands.UPDATE: - with open(self.policy_file, "r") as f: + policy_file = cast(str, self.policy_file) + with open(policy_file, "r") as f: policy_document = json.load(f) # Fetch the current policy to get its version loaded_policy_response = policy_api.load_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, - policy_name=self.policy_name, + policy_name=policy_name, ) if loaded_policy_response and loaded_policy_response.policy: current_policy_version = loaded_policy_response.policy.version else: raise Exception( - f"Could not retrieve current policy version for {self.policy_name}" + f"Could not retrieve current policy version for {policy_name}" ) policy_api.update_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, - policy_name=self.policy_name, + policy_name=policy_name, update_policy_request=UpdatePolicyRequest( description=self.policy_description, content=json.dumps(policy_document), @@ -202,9 +225,9 @@ def execute(self, api: PolarisDefaultApi) -> None: if self.policies_subcommand == Subcommands.ATTACH: policy_api.attach_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, - policy_name=self.policy_name, + policy_name=policy_name, attach_policy_request=AttachPolicyRequest( target=PolicyAttachmentTarget( type=self.attachment_type, path=attachment_path_list @@ -216,9 +239,9 @@ def execute(self, api: PolarisDefaultApi) -> None: ) else: policy_api.detach_policy( - prefix=self.catalog_name, + prefix=catalog_name, namespace=namespace_str, - policy_name=self.policy_name, + policy_name=policy_name, detach_policy_request=DetachPolicyRequest( target=PolicyAttachmentTarget( type=self.attachment_type, path=attachment_path_list diff --git a/client/python/apache_polaris/cli/command/principal_roles.py b/client/python/apache_polaris/cli/command/principal_roles.py index bedcc54b15..1a205e0e7c 100644 --- a/client/python/apache_polaris/cli/command/principal_roles.py +++ b/client/python/apache_polaris/cli/command/principal_roles.py @@ -16,9 +16,9 @@ # specific language governing permissions and limitations # under the License. # -from dataclasses import dataclass +from dataclasses import dataclass, field from pydantic import StrictStr -from typing import Dict, Optional, List +from typing import Dict, Optional, List, cast from apache_polaris.cli.command import Command from apache_polaris.cli.constants import Subcommands, Arguments @@ -47,14 +47,34 @@ class PrincipalRolesCommand(Command): principal_roles_subcommand: str principal_role_name: str - principal_name: str - catalog_name: str - catalog_role_name: str - properties: Optional[Dict[str, StrictStr]] - set_properties: Dict[str, StrictStr] - remove_properties: List[str] + principal_name: Optional[str] = None + catalog_name: Optional[str] = None + catalog_role_name: Optional[str] = None + properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + set_properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + remove_properties: Optional[List[str]] = None + + def __post_init__(self) -> None: + if self.properties is None: + self.properties = {} + if self.set_properties is None: + self.set_properties = {} def validate(self) -> None: + if self.principal_roles_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.GET, + Subcommands.UPDATE, + Subcommands.GRANT, + Subcommands.REVOKE, + Subcommands.SUMMARIZE, + }: + if not self.principal_role_name: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.PRINCIPAL_ROLE)}" + ) + if self.principal_roles_subcommand == Subcommands.LIST: if self.principal_name and self.catalog_role_name: raise Exception( @@ -69,17 +89,19 @@ def validate(self) -> None: ) def execute(self, api: PolarisDefaultApi) -> None: + principal_role_name = cast(str, self.principal_role_name) + if self.principal_roles_subcommand == Subcommands.CREATE: request = CreatePrincipalRoleRequest( principal_role=PrincipalRole( - name=self.principal_role_name, properties=self.properties + name=principal_role_name, properties=self.properties ) ) api.create_principal_role(request) elif self.principal_roles_subcommand == Subcommands.DELETE: - api.delete_principal_role(self.principal_role_name) + api.delete_principal_role(principal_role_name) elif self.principal_roles_subcommand == Subcommands.GET: - print(api.get_principal_role(self.principal_role_name).to_json()) + print(api.get_principal_role(principal_role_name).to_json()) elif self.principal_roles_subcommand == Subcommands.LIST: if self.catalog_role_name: for principal_role in api.list_principal_roles( @@ -95,7 +117,7 @@ def execute(self, api: PolarisDefaultApi) -> None: for principal_role in api.list_principal_roles().roles: print(principal_role.to_json()) elif self.principal_roles_subcommand == Subcommands.UPDATE: - principal_role = api.get_principal_role(self.principal_role_name) + principal_role = api.get_principal_role(principal_role_name) new_properties = principal_role.properties or {} # Add or update all entries specified in set_properties @@ -111,14 +133,16 @@ def execute(self, api: PolarisDefaultApi) -> None: current_entity_version=principal_role.entity_version, properties=new_properties, ) - api.update_principal_role(self.principal_role_name, request) + api.update_principal_role(principal_role_name, request) elif self.principal_roles_subcommand == Subcommands.GRANT: request = GrantPrincipalRoleRequest( - principal_role=PrincipalRole(name=self.principal_role_name), + principal_role=PrincipalRole(name=principal_role_name), ) - api.assign_principal_role(self.principal_name, request) + api.assign_principal_role(cast(str, self.principal_name), request) elif self.principal_roles_subcommand == Subcommands.REVOKE: - api.revoke_principal_role(self.principal_name, self.principal_role_name) + api.revoke_principal_role( + cast(str, self.principal_name), principal_role_name + ) elif self.principal_roles_subcommand == Subcommands.SUMMARIZE: self._generate_summary(api) else: @@ -127,10 +151,11 @@ def execute(self, api: PolarisDefaultApi) -> None: ) def _generate_summary(self, api: PolarisDefaultApi) -> None: - print(f"Principal Role: {self.principal_role_name}") + principal_role_name = cast(str, self.principal_role_name) + print(f"Principal Role: {principal_role_name}") print("-" * 80) # Metadata - role = api.get_principal_role(self.principal_role_name) + role = api.get_principal_role(principal_role_name) print("Metadata") print(f" {'Created:':<30} {format_timestamp(role.create_timestamp)}") print(f" {'Modified:':<30} {format_timestamp(role.last_update_timestamp)}") @@ -139,7 +164,7 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: # Assigned Principals principals = ( api.list_assignee_principals_for_principal_role( - self.principal_role_name + principal_role_name ).principals or [] ) @@ -156,7 +181,7 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: for catalog in catalogs: catalog_roles = ( api.list_catalog_roles_for_principal_role( - self.principal_role_name, catalog.name + principal_role_name, catalog.name ).roles or [] ) diff --git a/client/python/apache_polaris/cli/command/principals.py b/client/python/apache_polaris/cli/command/principals.py index f21e8b77cf..f866eb0fc7 100644 --- a/client/python/apache_polaris/cli/command/principals.py +++ b/client/python/apache_polaris/cli/command/principals.py @@ -17,13 +17,14 @@ # under the License. # import json -from dataclasses import dataclass -from typing import Dict, Optional, List, Iterator, Any +from dataclasses import dataclass, field +from typing import Dict, Optional, List, Iterator, Any, cast from pydantic import StrictStr from apache_polaris.cli.command import Command -from apache_polaris.cli.constants import Subcommands +from apache_polaris.cli.constants import Subcommands, Arguments +from apache_polaris.cli.options.option_tree import Argument from apache_polaris.sdk.management import ( PolarisDefaultApi, CreatePrincipalRequest, @@ -50,23 +51,29 @@ class PrincipalsCommand(Command): """ principals_subcommand: str - type: str - principal_name: str - client_id: str - principal_role: str - properties: Optional[Dict[str, StrictStr]] - set_properties: Dict[str, StrictStr] - remove_properties: List[str] + type: Optional[str] = None + principal_name: Optional[str] = None + client_id: Optional[str] = None + principal_role: Optional[str] = None + properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + set_properties: Optional[Dict[str, StrictStr]] = field(default_factory=dict) + remove_properties: Optional[List[str]] = None new_client_id: Optional[str] = None new_client_secret: Optional[str] = None + def __post_init__(self) -> None: + if self.properties is None: + self.properties = {} + if self.set_properties is None: + self.set_properties = {} + def _get_catalogs(self, api: PolarisDefaultApi) -> Iterator[str]: for catalog in api.list_catalogs().catalogs: yield catalog.to_dict()["name"] def _get_principal_roles(self, api: PolarisDefaultApi) -> Iterator[str]: for principal_role in api.list_principal_roles_assigned( - self.principal_name + cast(str, self.principal_name) ).roles: yield principal_role.to_dict()["name"] @@ -98,38 +105,51 @@ def build_credential_json( ) def validate(self) -> None: - pass + if self.principals_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.GET, + Subcommands.UPDATE, + Subcommands.ROTATE_CREDENTIALS, + Subcommands.ACCESS, + Subcommands.RESET, + Subcommands.SUMMARIZE, + }: + if not self.principal_name: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.PRINCIPAL)}" + ) def execute(self, api: PolarisDefaultApi) -> None: + principal_name = cast(str, self.principal_name) + if self.principals_subcommand == Subcommands.CREATE: request = CreatePrincipalRequest( principal=Principal( - type=self.type.upper(), - name=self.principal_name, + type=self.type.upper() if self.type else None, + name=principal_name, client_id=self.client_id, properties=self.properties, ) ) print(self.build_credential_json(api.create_principal(request))) elif self.principals_subcommand == Subcommands.DELETE: - api.delete_principal(self.principal_name) + api.delete_principal(principal_name) elif self.principals_subcommand == Subcommands.GET: - print(api.get_principal(self.principal_name).to_json()) + print(api.get_principal(principal_name).to_json()) elif self.principals_subcommand == Subcommands.LIST: if self.principal_role: for principal in api.list_assignee_principals_for_principal_role( - self.principal_role + cast(str, self.principal_role) ).principals: print(principal.to_json()) else: for principal in api.list_principals().principals: print(principal.to_json()) elif self.principals_subcommand == Subcommands.ROTATE_CREDENTIALS: - print( - self.build_credential_json(api.rotate_credentials(self.principal_name)) - ) + print(self.build_credential_json(api.rotate_credentials(principal_name))) elif self.principals_subcommand == Subcommands.UPDATE: - principal = api.get_principal(self.principal_name) + principal = api.get_principal(principal_name) new_properties = principal.properties or {} # Add or update all entries specified in set_properties @@ -145,13 +165,13 @@ def execute(self, api: PolarisDefaultApi) -> None: current_entity_version=principal.entity_version, properties=new_properties, ) - api.update_principal(self.principal_name, request) + api.update_principal(principal_name, request) elif self.principals_subcommand == Subcommands.ACCESS: - principal = api.get_principal(self.principal_name).to_dict()["name"] + principal_obj_name = api.get_principal(principal_name).to_dict()["name"] principal_roles = self._get_principal_roles(api) # Initialize the result structure - result = {"principal": principal, "principal_roles": []} + result = {"principal": principal_obj_name, "principal_roles": []} # Construct the result structure for each principal role for principal_role in principal_roles: @@ -185,13 +205,13 @@ def execute(self, api: PolarisDefaultApi) -> None: ) print( self.build_credential_json( - api.reset_credentials(self.principal_name, request) + api.reset_credentials(principal_name, request) ) ) else: print( self.build_credential_json( - api.reset_credentials(self.principal_name, None) + api.reset_credentials(principal_name, None) ) ) elif self.principals_subcommand == Subcommands.SUMMARIZE: @@ -200,10 +220,11 @@ def execute(self, api: PolarisDefaultApi) -> None: raise Exception(f"{self.principals_subcommand} is not supported in the CLI") def _generate_summary(self, api: PolarisDefaultApi) -> None: - print(f"Principal: {self.principal_name}") + principal_name = cast(str, self.principal_name) + print(f"Principal: {principal_name}") print("-" * 80) # Metadata - principal = api.get_principal(self.principal_name) + principal = api.get_principal(principal_name) print("Metadata") print(f" {'Client ID:':<30} {principal.client_id}") print(f" {'Created:':<30} {format_timestamp(principal.create_timestamp)}") diff --git a/client/python/apache_polaris/cli/command/privileges.py b/client/python/apache_polaris/cli/command/privileges.py index 8853a780da..565ef97901 100644 --- a/client/python/apache_polaris/cli/command/privileges.py +++ b/client/python/apache_polaris/cli/command/privileges.py @@ -17,7 +17,7 @@ # under the License. # from dataclasses import dataclass -from typing import List +from typing import List, Optional, cast from pydantic import StrictStr @@ -53,14 +53,14 @@ class PrivilegesCommand(Command): """ privileges_subcommand: str - action: str - catalog_name: str - catalog_role_name: str - namespace: List[StrictStr] - view: str - table: str - privilege: str - cascade: bool + action: Optional[str] = None + catalog_name: Optional[str] = None + catalog_role_name: Optional[str] = None + namespace: Optional[List[StrictStr]] = None + view: Optional[str] = None + table: Optional[str] = None + privilege: Optional[str] = None + cascade: bool = False def validate(self) -> None: if not self.catalog_name: @@ -72,6 +72,12 @@ def validate(self) -> None: f"Missing required argument: {Argument.to_flag_name(Arguments.CATALOG_ROLE)}" ) + if self.privileges_subcommand != Subcommands.LIST: + if not self.privilege: + raise Exception( + f"Missing required argument: {Argument.to_flag_name(Arguments.PRIVILEGE)}" + ) + if not self.privileges_subcommand: raise Exception("A subcommand must be provided") if ( @@ -100,36 +106,41 @@ def validate(self) -> None: raise Exception(f"Invalid view privilege: {self.privilege}") def execute(self, api: PolarisDefaultApi) -> None: + catalog_name = cast(str, self.catalog_name) + role_name = cast(str, self.catalog_role_name) + namespace = cast(List[str], self.namespace) + if self.privileges_subcommand == Subcommands.LIST: for grant in api.list_grants_for_catalog_role( - self.catalog_name, self.catalog_role_name + catalog_name, role_name ).grants: print(grant.to_json()) else: + privilege_name = cast(str, self.privilege) grant = None if self.privileges_subcommand == Subcommands.CATALOG: grant = CatalogGrant( - type=Subcommands.CATALOG, privilege=CatalogPrivilege(self.privilege) + type=Subcommands.CATALOG, privilege=CatalogPrivilege(privilege_name) ) elif self.privileges_subcommand == Subcommands.NAMESPACE: grant = NamespaceGrant( type=Subcommands.NAMESPACE, - namespace=self.namespace, - privilege=NamespacePrivilege(self.privilege), + namespace=namespace, + privilege=NamespacePrivilege(privilege_name), ) elif self.privileges_subcommand == Subcommands.TABLE: grant = TableGrant( type=Subcommands.TABLE, - namespace=self.namespace, + namespace=namespace, table_name=self.table, - privilege=TablePrivilege(self.privilege), + privilege=TablePrivilege(privilege_name), ) elif self.privileges_subcommand == Subcommands.VIEW: grant = ViewGrant( type=Subcommands.VIEW, - namespace=self.namespace, + namespace=namespace, view_name=self.view, - privilege=ViewPrivilege(self.privilege), + privilege=ViewPrivilege(privilege_name), ) if not grant: @@ -138,13 +149,11 @@ def execute(self, api: PolarisDefaultApi) -> None: ) elif self.action == Actions.GRANT: request = AddGrantRequest(grant=grant) - api.add_grant_to_catalog_role( - self.catalog_name, self.catalog_role_name, request - ) + api.add_grant_to_catalog_role(catalog_name, role_name, request) elif self.action == Actions.REVOKE: request = RevokeGrantRequest(grant=grant) api.revoke_grant_from_catalog_role( - self.catalog_name, self.catalog_role_name, self.cascade, request + catalog_name, role_name, self.cascade, request ) else: raise Exception(f"{self.action} is not supported in the CLI") diff --git a/client/python/apache_polaris/cli/command/profiles.py b/client/python/apache_polaris/cli/command/profiles.py index 739f5c2729..b64c7c9ce2 100644 --- a/client/python/apache_polaris/cli/command/profiles.py +++ b/client/python/apache_polaris/cli/command/profiles.py @@ -22,7 +22,7 @@ import os import sys from dataclasses import dataclass -from typing import Dict, Optional, List, Any +from typing import Dict, Optional, List, Any, cast from apache_polaris.cli.command import Command from apache_polaris.cli.constants import ( @@ -53,7 +53,7 @@ class ProfilesCommand(Command): """ profiles_subcommand: str - profile_name: str + profile_name: Optional[str] = None def _load_profiles(self) -> Dict[str, Dict[str, Any]]: if not os.path.exists(CONFIG_FILE): @@ -145,24 +145,32 @@ def _update_profile(self, name: str) -> None: sys.exit(1) def validate(self) -> None: - pass + if self.profiles_subcommand in { + Subcommands.CREATE, + Subcommands.DELETE, + Subcommands.UPDATE, + Subcommands.GET, + }: + if not self.profile_name: + raise Exception("Missing required argument: profile_name") def execute(self, api: Optional[PolarisDefaultApi] = None) -> None: if self.profiles_subcommand == Subcommands.CREATE: - self._create_profile(self.profile_name) + self._create_profile(cast(str, self.profile_name)) print(f"Polaris profile {self.profile_name} created successfully.") elif self.profiles_subcommand == Subcommands.DELETE: - self._delete_profile(self.profile_name) + self._delete_profile(cast(str, self.profile_name)) print(f"Polaris profile {self.profile_name} deleted successfully.") elif self.profiles_subcommand == Subcommands.UPDATE: - self._update_profile(self.profile_name) + self._update_profile(cast(str, self.profile_name)) print(f"Polaris profile {self.profile_name} updated successfully.") elif self.profiles_subcommand == Subcommands.GET: - profile = self._get_profile(self.profile_name) + profile_name = cast(str, self.profile_name) + profile = self._get_profile(profile_name) if profile: - print(f"Polaris profile {self.profile_name}: {profile}") + print(f"Polaris profile {profile_name}: {profile}") else: - print(f"Polaris profile {self.profile_name} not found.") + print(f"Polaris profile {profile_name} not found.") elif self.profiles_subcommand == Subcommands.LIST: profiles = self._list_profiles() print("Polaris profiles:") diff --git a/client/python/apache_polaris/cli/command/setup.py b/client/python/apache_polaris/cli/command/setup.py index 620645da19..e63feb81ae 100644 --- a/client/python/apache_polaris/cli/command/setup.py +++ b/client/python/apache_polaris/cli/command/setup.py @@ -21,7 +21,7 @@ import yaml import json from dataclasses import dataclass -from typing import Dict, Optional, List, Any, Set, cast +from typing import Dict, Optional, List, Any, Set from apache_polaris.cli.command import Command from apache_polaris.cli.constants import ( @@ -631,10 +631,6 @@ def _create_principals( "type", PrincipalType.SERVICE.value ).lower(), properties=principal_data.get("properties", {}), - client_id="", - principal_role="", - set_properties={}, - remove_properties=[], ) principal_cmd.validate() principal_cmd.execute(api) @@ -677,11 +673,6 @@ def _create_principals( principal_roles_subcommand=Subcommands.GRANT, principal_name=principal_name, principal_role_name=role_name, - catalog_name="", - catalog_role_name="", - properties={}, - set_properties={}, - remove_properties=[], ) role_cmd.validate() role_cmd.execute(api) @@ -723,12 +714,6 @@ def _create_principal_roles( cmd = PrincipalRolesCommand( principal_roles_subcommand=Subcommands.CREATE, principal_role_name=role_name, - principal_name="", - catalog_name="", - catalog_role_name="", - properties={}, - set_properties={}, - remove_properties=[], ) cmd.validate() cmd.execute(api) @@ -905,73 +890,62 @@ def _create_catalogs( catalogs_subcommand=command_args.get( "catalogs_subcommand", Subcommands.CREATE ), - catalog_name=command_args.get("catalog_name", ""), - catalog_type=command_args.get("catalog_type", "INTERNAL"), - default_base_location=command_args.get( - "default_base_location", "" - ), - storage_type=command_args.get("storage_type", ""), - allowed_locations=command_args.get("allowed_locations") or [], - properties=command_args.get("properties") or {}, - set_properties=command_args.get("set_properties") or {}, - remove_properties=command_args.get("remove_properties") or [], - role_arn=command_args.get("role_arn", ""), - external_id=command_args.get("external_id", ""), - user_arn=command_args.get("user_arn", ""), - region=command_args.get("region", ""), - tenant_id=command_args.get("tenant_id", ""), - multi_tenant_app_name=command_args.get( - "multi_tenant_app_name", "" - ), - consent_url=command_args.get("consent_url", ""), - service_account=command_args.get("service_account", ""), - hierarchical=command_args.get("hierarchical", False), - endpoint=command_args.get("endpoint", ""), - endpoint_internal=command_args.get("endpoint_internal", ""), - sts_endpoint=command_args.get("sts_endpoint", ""), - sts_unavailable=command_args.get("sts_unavailable", False), - kms_unavailable=command_args.get("kms_unavailable", False), - path_style_access=command_args.get("path_style_access", False), - current_kms_key=command_args.get("current_kms_key", ""), - allowed_kms_keys=command_args.get("allowed_kms_keys") or [], + catalog_name=command_args.get("catalog_name"), + catalog_type=command_args.get("catalog_type"), + default_base_location=command_args.get("default_base_location"), + storage_type=command_args.get("storage_type"), + allowed_locations=command_args.get("allowed_locations"), + properties=command_args.get("properties"), + set_properties=command_args.get("set_properties"), + remove_properties=command_args.get("remove_properties"), + role_arn=command_args.get("role_arn"), + external_id=command_args.get("external_id"), + user_arn=command_args.get("user_arn"), + region=command_args.get("region"), + tenant_id=command_args.get("tenant_id"), + multi_tenant_app_name=command_args.get("multi_tenant_app_name"), + consent_url=command_args.get("consent_url"), + service_account=command_args.get("service_account"), + hierarchical=command_args.get("hierarchical"), + endpoint=command_args.get("endpoint"), + endpoint_internal=command_args.get("endpoint_internal"), + sts_endpoint=command_args.get("sts_endpoint"), + sts_unavailable=command_args.get("sts_unavailable"), + kms_unavailable=command_args.get("kms_unavailable"), + path_style_access=command_args.get("path_style_access"), + current_kms_key=command_args.get("current_kms_key"), + allowed_kms_keys=command_args.get("allowed_kms_keys"), catalog_connection_type=command_args.get( - "catalog_connection_type", "" + "catalog_connection_type" ), - catalog_uri=command_args.get("catalog_uri", ""), - hadoop_warehouse=command_args.get("hadoop_warehouse", ""), - hive_warehouse=command_args.get("hive_warehouse", ""), + catalog_uri=command_args.get("catalog_uri"), + hadoop_warehouse=command_args.get("hadoop_warehouse"), + hive_warehouse=command_args.get("hive_warehouse"), iceberg_remote_catalog_name=command_args.get( - "iceberg_remote_catalog_name", "" + "iceberg_remote_catalog_name" ), catalog_authentication_type=command_args.get( - "catalog_authentication_type", "" - ), - catalog_token_uri=command_args.get("catalog_token_uri", ""), - catalog_client_id=command_args.get("catalog_client_id", ""), - catalog_client_secret=command_args.get( - "catalog_client_secret", "" - ), - catalog_client_scopes=command_args.get("catalog_client_scopes") - or [], - catalog_bearer_token=command_args.get( - "catalog_bearer_token", "" + "catalog_authentication_type" ), - catalog_role_arn=command_args.get("catalog_role_arn", ""), + catalog_token_uri=command_args.get("catalog_token_uri"), + catalog_client_id=command_args.get("catalog_client_id"), + catalog_client_secret=command_args.get("catalog_client_secret"), + catalog_client_scopes=command_args.get("catalog_client_scopes"), + catalog_bearer_token=command_args.get("catalog_bearer_token"), + catalog_role_arn=command_args.get("catalog_role_arn"), catalog_role_session_name=command_args.get( - "catalog_role_session_name", "" + "catalog_role_session_name" ), - catalog_external_id=command_args.get("catalog_external_id", ""), + catalog_external_id=command_args.get("catalog_external_id"), catalog_signing_region=command_args.get( - "catalog_signing_region", "" - ), - catalog_signing_name=command_args.get( - "catalog_signing_name", "" + "catalog_signing_region" ), + catalog_signing_name=command_args.get("catalog_signing_name"), catalog_service_identity_type=command_args.get( - "catalog_service_identity_type", "" + "catalog_service_identity_type" ), catalog_service_identity_iam_arn=command_args.get( - "catalog_service_identity_iam_arn", "" + "catalog_service_identity_iam_arn" ), ) cmd.validate() @@ -1016,10 +990,7 @@ def _create_catalog_roles( catalog_roles_subcommand=Subcommands.CREATE, catalog_role_name=role_name, catalog_name=catalog_name, - properties=role_data.get("properties", {}), - principal_role_name="", - set_properties={}, - remove_properties=[], + properties=role_data.get("properties"), ) cmd.validate() cmd.execute(api) @@ -1061,9 +1032,6 @@ def _create_catalog_roles( catalog_role_name=role_name, principal_role_name=principal_role_name, catalog_name=catalog_name, - properties=None, - set_properties={}, - remove_properties=[], ) cmd.validate() cmd.execute(api) @@ -1121,16 +1089,7 @@ def _grant_privilege( try: log_message = f"privilege '{privilege}' on {level} '{namespace or catalog_name}' to role '{catalog_role_name}'" logger.info(f"Granting {log_message}") - privilege_args = { - "action": Actions.GRANT, - "cascade": False, - "privilege": privilege, - "catalog_role_name": catalog_role_name, - "catalog_name": catalog_name, - } - if level == "namespace": - if namespace: - privilege_args["namespace"] = namespace.split(".") + subcommand_map = { "catalog": Subcommands.CATALOG, "namespace": Subcommands.NAMESPACE, @@ -1139,16 +1098,17 @@ def _grant_privilege( if not privileges_subcommand: logger.warning(f"Unsupported privilege level '{level}'") return + cmd = PrivilegesCommand( privileges_subcommand=privileges_subcommand, - action=str(privilege_args.get("action")), - cascade=bool(privilege_args.get("cascade")), - privilege=str(privilege_args.get("privilege")), - catalog_role_name=str(privilege_args.get("catalog_role_name")), - catalog_name=str(privilege_args.get("catalog_name")), - namespace=cast(List[str], privilege_args.get("namespace") or []), - view="", - table="", + action=Actions.GRANT, + cascade=False, + privilege=privilege, + catalog_role_name=catalog_role_name, + catalog_name=catalog_name, + namespace=namespace.split(".") + if level == "namespace" and namespace + else None, ) cmd.validate() cmd.execute(api) @@ -1236,9 +1196,8 @@ def _create_namespaces( namespaces_subcommand=Subcommands.CREATE, catalog=catalog_name, namespace=ns_name.split("."), - parent=[], - location=ns_data.get("location") or "", - properties=ns_data.get("properties", {}), + location=ns_data.get("location"), + properties=ns_data.get("properties"), ) cmd.validate() cmd.execute(api) @@ -1385,11 +1344,6 @@ def _create_policies_and_attachments( attachment_type=attachment_type, attachment_path=attachment_path, parameters=attachment.get("parameters"), - policy_file="", - policy_type="", - policy_description="", - target_name="", - detach_all=False, applicable=False, ) cmd.validate() diff --git a/client/python/apache_polaris/cli/command/tables.py b/client/python/apache_polaris/cli/command/tables.py index a9609a0333..9547c73ab6 100644 --- a/client/python/apache_polaris/cli/command/tables.py +++ b/client/python/apache_polaris/cli/command/tables.py @@ -16,8 +16,8 @@ # specific language governing permissions and limitations # under the License. # -from dataclasses import dataclass -from typing import List +from dataclasses import dataclass, field +from typing import List, Optional, cast from apache_polaris.cli.command import Command from apache_polaris.cli.command.utils import get_catalog_api_client @@ -47,9 +47,9 @@ class TableCommand(Command): """ table_subcommand: str - catalog_name: str - namespace: List[str] - table_name: str + catalog_name: Optional[str] = None + namespace: Optional[List[str]] = field(default_factory=list) + table_name: Optional[str] = None def validate(self) -> None: if not self.catalog_name: @@ -65,59 +65,66 @@ def validate(self) -> None: or self.table_subcommand == Subcommands.SUMMARIZE or self.table_subcommand == Subcommands.DELETE ): - if not self.table_name.strip(): + if not self.table_name or not self.table_name.strip(): raise Exception("The table name cannot be empty.") def execute(self, api: PolarisDefaultApi) -> None: catalog_api = IcebergCatalogAPI(get_catalog_api_client(api)) - ns_str = UNIT_SEPARATOR.join(self.namespace) + catalog_name = cast(str, self.catalog_name) + namespace_list = cast(List[str], self.namespace) + table_name = cast(str, self.table_name) + ns_str = UNIT_SEPARATOR.join(namespace_list) + if self.table_subcommand == Subcommands.LIST: try: - result = catalog_api.list_tables( - prefix=self.catalog_name, namespace=ns_str - ) + result = catalog_api.list_tables(prefix=catalog_name, namespace=ns_str) for table_identifier in result.identifiers: print(table_identifier.to_json()) except Exception as e: - handle_api_exception(f"Table Listing ({'.'.join(self.namespace)})", e) + handle_api_exception( + f"Table Listing ({'.'.join(namespace_list)})", + e, + ) elif self.table_subcommand == Subcommands.GET: try: print( catalog_api.load_table( - prefix=self.catalog_name, + prefix=catalog_name, namespace=ns_str, - table=self.table_name, + table=table_name, ).to_json() ) except Exception as e: - handle_api_exception(f"Table Load ({self.table_name})", e) + handle_api_exception(f"Table Load ({table_name})", e) elif self.table_subcommand == Subcommands.DELETE: - print( - f"De-registering table {'.'.join(self.namespace)}.{self.table_name}..." - ) + namespace_dot = ".".join(namespace_list) + print(f"De-registering table {namespace_dot}.{table_name}...") try: catalog_api.drop_table( - prefix=self.catalog_name, + prefix=catalog_name, namespace=ns_str, - table=self.table_name, + table=table_name, purge_requested=False, ) - print( - f"De-registering table {'.'.join(self.namespace)}.{self.table_name} completed" - ) + print(f"De-registering table {namespace_dot}.{table_name} completed") except Exception as e: - handle_api_exception(f"Table De-registration ({self.table_name})", e) + handle_api_exception(f"Table De-registration ({table_name})", e) elif self.table_subcommand == Subcommands.SUMMARIZE: self._generate_summary(api, catalog_api, ns_str) def _generate_summary( self, api: PolarisDefaultApi, catalog_api: IcebergCatalogAPI, ns_str: str ) -> None: - print(f"Table: {'.'.join(self.namespace)}.{self.table_name}") + catalog_name = cast(str, self.catalog_name) + namespace_list = cast(List[str], self.namespace) + table_name = cast(str, self.table_name) + + namespace_dot = ".".join(namespace_list) + print(f"Table: {namespace_dot}.{table_name}") print("-" * 80) try: resp = catalog_api.load_table( - prefix=self.catalog_name, namespace=ns_str, table=self.table_name + prefix=catalog_name, namespace=ns_str, table=table_name ) # Metadata metadata = resp.metadata @@ -244,7 +251,7 @@ def _generate_summary( try: policy_api = PolicyAPI(catalog_api.api_client) resp = policy_api.get_applicable_policies( - prefix=self.catalog_name, namespace=ns_str, target_name=self.table_name + prefix=catalog_name, namespace=ns_str, target_name=table_name ) applicable_policies = resp.applicable_policies or [] if applicable_policies: diff --git a/client/python/test/test_catalog_roles_command.py b/client/python/test/test_catalog_roles_command.py index 651a5c731b..5c24e84ff8 100644 --- a/client/python/test/test_catalog_roles_command.py +++ b/client/python/test/test_catalog_roles_command.py @@ -24,11 +24,22 @@ class TestCatalogRolessCommand(CLITestBase): def test_catalog_role_commands_validation(self) -> None: mock_client = self.build_mock_client() - # Missing catalog - self.check_exception( - lambda: self.mock_execute(mock_client, ["catalog-roles", "get", "foo"]), - "--catalog", - ) + # Missing --catalog flag + for sub in ["create", "delete", "get", "update", "summarize"]: + with self.subTest(subcommand=sub): + self.check_exception( + lambda: self.mock_execute( + mock_client, ["catalog-roles", sub, "role_name"] + ), + "--catalog", + ) + # Missing positional role name + for sub in ["create", "delete", "get", "update", "summarize"]: + with self.subTest(subcommand=sub, error="missing positional"): + with self.assertRaises(SystemExit): + self.mock_execute( + mock_client, ["catalog-roles", sub, "--catalog", "my_cat"] + ) def test_catalog_role_create(self) -> None: mock_client = self.build_mock_client() diff --git a/client/python/test/test_catalogs_command.py b/client/python/test/test_catalogs_command.py index d365ea9127..d477355db0 100644 --- a/client/python/test/test_catalogs_command.py +++ b/client/python/test/test_catalogs_command.py @@ -19,6 +19,8 @@ from unittest.mock import patch, MagicMock from cli_test_utils import CLITestBase +from apache_polaris.cli.command.catalogs import CatalogsCommand +from apache_polaris.cli.constants import Subcommands from apache_polaris.sdk.management import ( PolarisCatalog, CatalogProperties, @@ -44,6 +46,11 @@ def test_catalog_commands_validation(self) -> None: ), "--default-base-location", ) + # Missing catalog name + for sub in ["delete", "get", "update", "summarize"]: + with self.subTest(subcommand=sub): + with self.assertRaises(SystemExit): + self.mock_execute(mock_client, ["catalogs", sub]) # Invalid property format self.check_exception( lambda: self.mock_execute( @@ -137,6 +144,18 @@ def test_catalog_commands_validation(self) -> None: "Missing required argument for authentication type 'BEARER'", ) + def test_catalog_input_normalization(self) -> None: + cmd = CatalogsCommand( + catalogs_subcommand=Subcommands.CREATE, + properties=None, + hierarchical=None, + sts_unavailable=None, + ) + # __post_init__ should have normalized these + self.assertEqual(cmd.properties, {}) + self.assertEqual(cmd.hierarchical, False) + self.assertEqual(cmd.sts_unavailable, False) + def test_catalog_create_s3_options(self) -> None: mock_client = self.build_mock_client() self.mock_execute( diff --git a/client/python/test/test_namespaces_command.py b/client/python/test/test_namespaces_command.py index 0951af7fc2..a9c86f31dd 100644 --- a/client/python/test/test_namespaces_command.py +++ b/client/python/test/test_namespaces_command.py @@ -19,17 +19,39 @@ from unittest.mock import patch, MagicMock from cli_test_utils import CLITestBase -from apache_polaris.cli.constants import UNIT_SEPARATOR +from apache_polaris.cli.command.namespaces import NamespacesCommand +from apache_polaris.cli.constants import UNIT_SEPARATOR, Subcommands class TestNamespacesCommand(CLITestBase): def test_namespace_validation(self) -> None: mock_client = self.build_mock_client() - # Missing catalog + # Missing --catalog flag self.check_exception( lambda: self.mock_execute(mock_client, ["namespaces", "list"]), - "Missing required argument", + "Missing required argument: --catalog", ) + # Missing positional namespace + for sub in ["create", "delete", "get", "summarize"]: + with self.subTest(subcommand=sub): + with self.assertRaises(SystemExit): + self.mock_execute( + mock_client, ["namespaces", sub, "--catalog", "my_catalog"] + ) + + def test_namespace_input_normalization(self) -> None: + cmd = NamespacesCommand( + namespaces_subcommand=Subcommands.LIST, + catalog="my_cat", + namespace=None, + parent=None, + properties=None, + ) + # __post_init__ should not normalized these + self.assertIsNone(cmd.namespace) + self.assertIsNone(cmd.parent) + # __post_init__ should normalized these + self.assertEqual(cmd.properties, {}) @patch("apache_polaris.cli.command.namespaces.IcebergCatalogAPI") def test_namespace_create(self, mock_iceberg_api_class: MagicMock) -> None: diff --git a/client/python/test/test_principals_command.py b/client/python/test/test_principals_command.py index 5a1cc72597..15639bd811 100644 --- a/client/python/test/test_principals_command.py +++ b/client/python/test/test_principals_command.py @@ -19,6 +19,8 @@ from cli_test_utils import CLITestBase +from apache_polaris.cli.command.principals import PrincipalsCommand +from apache_polaris.cli.constants import Subcommands from apache_polaris.sdk.management import ( Principal, PrincipalWithCredentials, @@ -28,6 +30,19 @@ class TestPrincipalsCommand(CLITestBase): + def test_principal_input_normalization(self) -> None: + cmd = PrincipalsCommand( + principals_subcommand=Subcommands.CREATE, + properties=None, + set_properties=None, + principal_role=None, + ) + # __post_init__ should not normalized these + self.assertIsNone(cmd.principal_role) + # __post_init__ should normalized these + self.assertEqual(cmd.properties, {}) + self.assertEqual(cmd.set_properties, {}) + def test_principal_create(self) -> None: mock_client = self.build_mock_client() mock_client.create_principal.return_value = PrincipalWithCredentials( diff --git a/client/python/test/test_privileges_command.py b/client/python/test/test_privileges_command.py index 9495829539..efb9269b59 100644 --- a/client/python/test/test_privileges_command.py +++ b/client/python/test/test_privileges_command.py @@ -23,6 +23,48 @@ class TestPrivilegesCommand(CLITestBase): def test_privilege_commands_validation(self) -> None: mock_client = self.build_mock_client() + # # Missing --catalog flag + for sub in ["catalog", "namespace", "table", "view", "list"]: + args = ( + ["privileges", sub, "grant", "PRIV"] + if sub != "list" + else ["privileges", "list"] + ) + with self.subTest(subcommand=sub, error="missing catalog"): + self.check_exception( + lambda: self.mock_execute( + mock_client, args + ["--catalog-role", "r"] + ), + "--catalog", + ) + # Missing --catalog-role flag + for sub in ["catalog", "namespace", "table", "view", "list"]: + args = ( + ["privileges", sub, "grant", "PRIV"] + if sub != "list" + else ["privileges", "list"] + ) + with self.subTest(subcommand=sub, error="missing role"): + self.check_exception( + lambda: self.mock_execute(mock_client, args + ["--catalog", "c"]), + "--catalog-role", + ) + # Missing positional privilege + for sub in ["catalog", "namespace", "table", "view"]: + with self.subTest(subcommand=sub, error="missing positional"): + with self.assertRaises(SystemExit): + self.mock_execute( + mock_client, + [ + "privileges", + sub, + "grant", + "--catalog", + "c", + "--catalog-role", + "r", + ], + ) # Invalid privilege self.check_exception( lambda: self.mock_execute( diff --git a/client/python/test/test_setup_command.py b/client/python/test/test_setup_command.py index 41bcc5c641..7a849c7745 100644 --- a/client/python/test/test_setup_command.py +++ b/client/python/test/test_setup_command.py @@ -50,6 +50,22 @@ def test_setup_apply_dry_run( self.mock_execute(mock_client, ["setup", "apply", "config.yaml", "--dry-run"]) mock_client.list_principals.assert_called() + @patch("apache_polaris.cli.command.setup.os.path.isfile") + def test_setup_apply_s3_optional_fields(self, mock_isfile: MagicMock) -> None: + mock_client = self.build_mock_client() + mock_isfile.return_value = True + # Sample S3 catalog without role_arn + setup_yaml = "catalogs:\n - name: s3-catalog\n storage_type: s3\n default_base_location: s3://bucket/path" + with patch( + "apache_polaris.cli.command.setup.open", mock_open(read_data=setup_yaml) + ): + self.mock_execute(mock_client, ["setup", "apply", "config.yaml"]) + mock_client.create_catalog.assert_called_once() + call_args = mock_client.create_catalog.call_args[0][0] + # role_arn should be None, NOT an empty string + self.assertIsNone(call_args.catalog.storage_config_info.role_arn) + self.assertEqual(call_args.catalog.name, "s3-catalog") + @patch( "apache_polaris.cli.command.setup.open", new_callable=mock_open, read_data="{}" ) From 4e431b305695dcae25a786ac268ab4d93b6a9ef7 Mon Sep 17 00:00:00 2001 From: Yong Date: Sun, 12 Apr 2026 19:51:23 -0500 Subject: [PATCH 2/2] Fix accidently removal of empty default for allowed_locations --- client/python/apache_polaris/cli/command/catalogs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/python/apache_polaris/cli/command/catalogs.py b/client/python/apache_polaris/cli/command/catalogs.py index be4b4a58fa..9a26915659 100644 --- a/client/python/apache_polaris/cli/command/catalogs.py +++ b/client/python/apache_polaris/cli/command/catalogs.py @@ -538,7 +538,7 @@ def _generate_summary(self, api: PolarisDefaultApi) -> None: print(f" {'Storage Type:':<30} {storage_info.storage_type}") print(f" {'Base Location:':<30} {catalog.properties.default_base_location}") print( - f" {'Allowed Locations:':<30} {', '.join(storage_info.allowed_locations)}" + f" {'Allowed Locations:':<30} {', '.join(storage_info.allowed_locations or [])}" ) print(f" {'Created:':<30} {format_timestamp(catalog.create_timestamp)}") print(f" {'Modified:':<30} {format_timestamp(catalog.last_update_timestamp)}")