Skip to content

Commit b3365e7

Browse files
authored
Fix missing security_policy when updating SQL warehouse config (#2409)
1. Fix `InvalidParameterValue: Endpoint security policy is required and must be one of NONE, DATA_ACCESS_CONTROL, PASSTHROUGH.` error in #2405 2. Print new SQL warehouse data access config in the log when `databricks.sdk.errors.platform.InvalidParameterValue: enable_serverless_compute is required.` error in #2405 occurs. With that users can manually copy paste the config in UI to finish setting up the uber principal,
1 parent e4bebae commit b3365e7

File tree

4 files changed

+109
-7
lines changed

4 files changed

+109
-7
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from databricks.sdk.errors import NotFound, ResourceDoesNotExist
1313
from databricks.sdk.service.catalog import Privilege
1414
from databricks.sdk.service.compute import Policy
15+
from databricks.sdk.service.sql import SetWorkspaceWarehouseConfigRequestSecurityPolicy
1516

1617
from databricks.labs.ucx.assessment.aws import (
1718
AWSInstanceProfile,
@@ -266,10 +267,16 @@ def _update_sql_dac_with_instance_profile(self, iam_instance_profile: AWSInstanc
266267
f"workspace warehouse config. Do you want UCX to to update it with the uber instance profile?"
267268
):
268269
return
270+
security_policy = (
271+
SetWorkspaceWarehouseConfigRequestSecurityPolicy(warehouse_config.security_policy.value)
272+
if warehouse_config.security_policy
273+
else SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE
274+
)
269275
self._ws.warehouses.set_workspace_warehouse_config(
270276
data_access_config=warehouse_config.data_access_config,
271277
sql_configuration_parameters=warehouse_config.sql_configuration_parameters,
272278
instance_profile_arn=iam_instance_profile.instance_profile_arn,
279+
security_policy=security_policy,
273280
)
274281

275282
def get_instance_profile(self, instance_profile_name: str) -> AWSInstanceProfile | None:

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
from databricks.labs.blueprint.parallel import ManyError, Threads
1111
from databricks.labs.blueprint.tui import Prompts
1212
from databricks.sdk import WorkspaceClient
13-
from databricks.sdk.errors import NotFound, ResourceAlreadyExists
13+
from databricks.sdk.errors import InvalidParameterValue, NotFound, ResourceAlreadyExists
1414
from databricks.sdk.service.catalog import Privilege
15-
from databricks.sdk.service.sql import EndpointConfPair
15+
from databricks.sdk.service.sql import EndpointConfPair, SetWorkspaceWarehouseConfigRequestSecurityPolicy
1616

1717
from databricks.labs.ucx.azure.resources import (
1818
AccessConnector,
@@ -240,10 +240,25 @@ def _update_sql_dac_with_spn(
240240
),
241241
]
242242
)
243-
self._ws.warehouses.set_workspace_warehouse_config(
244-
data_access_config=sql_dac,
245-
sql_configuration_parameters=warehouse_config.sql_configuration_parameters,
243+
security_policy = (
244+
SetWorkspaceWarehouseConfigRequestSecurityPolicy(warehouse_config.security_policy.value)
245+
if warehouse_config.security_policy
246+
else SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE
246247
)
248+
try:
249+
self._ws.warehouses.set_workspace_warehouse_config(
250+
data_access_config=sql_dac,
251+
sql_configuration_parameters=warehouse_config.sql_configuration_parameters,
252+
security_policy=security_policy,
253+
)
254+
# TODO: Remove following try except once https://github.com/databricks/databricks-sdk-py/issues/305 is fixed
255+
except InvalidParameterValue as error:
256+
sql_dac_log_msg = "\n".join(f"{config_pair.key} {config_pair.value}" for config_pair in sql_dac)
257+
logger.error(
258+
f'Adding uber principal to SQL warehouse Data Access Properties is failed using Python SDK with error "{error}". '
259+
f'Please try applying the following configs manually in the worksapce admin UI:\n{sql_dac_log_msg}'
260+
)
261+
raise error
247262

248263
def create_uber_principal(self, prompts: Prompts):
249264
config = self._installation.load(WorkspaceConfig)

tests/unit/aws/test_access.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
from databricks.sdk.service import iam
1212
from databricks.sdk.service.catalog import AwsIamRoleResponse, ExternalLocationInfo, StorageCredentialInfo
1313
from databricks.sdk.service.compute import InstanceProfile, Policy
14-
from databricks.sdk.service.sql import EndpointConfPair, GetWorkspaceWarehouseConfigResponse
14+
from databricks.sdk.service.sql import (
15+
EndpointConfPair,
16+
GetWorkspaceWarehouseConfigResponse,
17+
SetWorkspaceWarehouseConfigRequestSecurityPolicy,
18+
GetWorkspaceWarehouseConfigResponseSecurityPolicy,
19+
)
1520

1621
from databricks.labs.ucx.assessment.aws import AWSPolicyAction, AWSResources, AWSRole, AWSRoleAction
1722
from databricks.labs.ucx.aws.access import AWSResourcePermissions
@@ -252,6 +257,7 @@ def test_create_uber_principal_existing_role(mock_ws, mock_installation, backend
252257
data_access_config=None,
253258
instance_profile_arn='arn:aws:iam::12345:instance-profile/role1',
254259
sql_configuration_parameters=None,
260+
security_policy=SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE,
255261
)
256262

257263

@@ -287,9 +293,43 @@ def test_create_uber_principal_no_existing_role(mock_ws, mock_installation, back
287293
data_access_config=[EndpointConfPair("jdbc", "jdbc:sqlserver://localhost:1433;databaseName=master")],
288294
instance_profile_arn='arn:aws:iam::12345:instance-profile/role1',
289295
sql_configuration_parameters=None,
296+
security_policy=SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE,
290297
)
291298

292299

300+
@pytest.mark.parametrize(
301+
"get_security_policy, set_security_policy",
302+
[
303+
(
304+
GetWorkspaceWarehouseConfigResponseSecurityPolicy.DATA_ACCESS_CONTROL,
305+
SetWorkspaceWarehouseConfigRequestSecurityPolicy.DATA_ACCESS_CONTROL,
306+
),
307+
(GetWorkspaceWarehouseConfigResponseSecurityPolicy.NONE, SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE),
308+
],
309+
)
310+
def test_create_uber_principal_set_warehouse_config_security_policy(
311+
mock_ws, mock_installation, backend, get_security_policy, set_security_policy
312+
):
313+
mock_ws.cluster_policies.get.return_value = Policy(definition=json.dumps({"foo": "bar"}))
314+
mock_ws.warehouses.get_workspace_warehouse_config.return_value = GetWorkspaceWarehouseConfigResponse(
315+
security_policy=get_security_policy
316+
)
317+
318+
instance_profile_arn = "arn:aws:iam::12345:instance-profile/role1"
319+
aws = create_autospec(AWSResources)
320+
aws.get_instance_profile_arn.return_value = instance_profile_arn
321+
322+
aws_resource_permissions = AWSResourcePermissions(
323+
mock_installation,
324+
mock_ws,
325+
aws,
326+
ExternalLocations(mock_ws, backend, "ucx"),
327+
)
328+
aws_resource_permissions.create_uber_principal(MockPrompts({".*": "yes"}))
329+
330+
assert mock_ws.warehouses.set_workspace_warehouse_config.call_args.kwargs["security_policy"] == set_security_policy
331+
332+
293333
def test_create_uber_principal_no_storage(mock_ws, mock_installation, locations):
294334
cluster_policy = Policy(
295335
policy_id="foo", name="Unity Catalog Migration (ucx) ([email protected])", definition=json.dumps({"foo": "bar"})

tests/unit/azure/test_access.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@
99
from databricks.sdk import WorkspaceClient
1010
from databricks.sdk.errors import NotFound
1111
from databricks.sdk.service.compute import Policy
12-
from databricks.sdk.service.sql import GetWorkspaceWarehouseConfigResponse, EndpointConfPair
12+
from databricks.sdk.service.sql import (
13+
GetWorkspaceWarehouseConfigResponse,
14+
EndpointConfPair,
15+
GetWorkspaceWarehouseConfigResponseSecurityPolicy,
16+
SetWorkspaceWarehouseConfigRequestSecurityPolicy,
17+
)
1318
from databricks.sdk.service.workspace import GetSecretResponse
1419

1520
from databricks.labs.ucx.azure.access import AzureResourcePermissions
@@ -24,6 +29,7 @@
2429
from databricks.labs.ucx.hive_metastore import ExternalLocations
2530

2631
from . import azure_api_client
32+
from .. import DEFAULT_CONFIG
2733

2834

2935
def test_save_spn_permissions_no_external_table(caplog):
@@ -744,9 +750,43 @@ def test_create_global_spn():
744750
),
745751
],
746752
sql_configuration_parameters=None,
753+
security_policy=SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE,
747754
)
748755

749756

757+
@pytest.mark.parametrize(
758+
"get_security_policy, set_security_policy",
759+
[
760+
(
761+
GetWorkspaceWarehouseConfigResponseSecurityPolicy.DATA_ACCESS_CONTROL,
762+
SetWorkspaceWarehouseConfigRequestSecurityPolicy.DATA_ACCESS_CONTROL,
763+
),
764+
(GetWorkspaceWarehouseConfigResponseSecurityPolicy.NONE, SetWorkspaceWarehouseConfigRequestSecurityPolicy.NONE),
765+
],
766+
)
767+
def test_create_global_spn_set_warehouse_config_security_policy(get_security_policy, set_security_policy):
768+
w = create_autospec(WorkspaceClient)
769+
w.cluster_policies.get.return_value = Policy(definition=json.dumps({"foo": "bar"}))
770+
w.warehouses.get_workspace_warehouse_config.return_value = GetWorkspaceWarehouseConfigResponse(
771+
security_policy=get_security_policy
772+
)
773+
rows = {
774+
"SELECT \\* FROM hive_metastore.ucx.external_locations": [
775+
["abfss://[email protected]/folder1", "1"]
776+
]
777+
}
778+
location = ExternalLocations(w, MockBackend(rows=rows), "ucx")
779+
installation = MockInstallation(DEFAULT_CONFIG.copy())
780+
api_client = azure_api_client()
781+
azure_resources = AzureResources(api_client, api_client, include_subscriptions="002")
782+
azure_resource_permission = AzureResourcePermissions(installation, w, azure_resources, location)
783+
azure_resource_permission.create_uber_principal(
784+
MockPrompts({"Enter a name for the uber service principal to be created*": "UCXServicePrincipal"})
785+
)
786+
787+
assert w.warehouses.set_workspace_warehouse_config.call_args.kwargs["security_policy"] == set_security_policy
788+
789+
750790
def test_create_access_connectors_for_storage_accounts_logs_no_storage_accounts(caplog):
751791
"""A warning should be logged when no storage account is present."""
752792
w = create_autospec(WorkspaceClient)

0 commit comments

Comments
 (0)