Skip to content

Commit

Permalink
[Fix] Tolerate databricks_permissions resources for SQL warehouses …
Browse files Browse the repository at this point in the history
…with `/warehouses/...` IDs (#4158)

## Changes
#4143 reported a regression to the `databricks_permissions` resource
caused by
#3956.
Normally, the ID for this resource when configured for a SQL warehouse
is `/sql/warehouses/<ID>`. However, it seems like at some point in the
past, some users may have had an ID of `/warehouses/<ID>`. It's possible
that importing this resource worked like this: when calling the
permissions REST API, whether using object type `sql/warehouses` or
`warehouses`, the API returns permissions for the same resources:

```
15:13:01 DEBUG GET /api/2.0/permissions/sql/warehouses/<ID>
< HTTP/2.0 200 OK
< {
<   "access_control_list": [
<     {
<       "all_permissions": [
<         {
<           "inherited": false,
<           "permission_level": "IS_OWNER"
<         }
<       ],
<       "display_name": "<ME>",
<       "user_name": "<ME>"
<     },
<     {
<       "all_permissions": [
<         {
<           "inherited": true,
<           "inherited_from_object": [
<             "/sql/warehouses/"
<           ],
<           "permission_level": "CAN_MANAGE"
<         }
<       ],
<       "group_name": "admins"
<     }
<   ],
<   "object_id": "/sql/warehouses/<ID>",
<   "object_type": "warehouses"
< } pid=53287 sdk=true
...
15:12:56 DEBUG GET /api/2.0/permissions/warehouses/<ID>
< HTTP/2.0 200 OK
< {
<   "access_control_list": [
<     {
<       "all_permissions": [
<         {
<           "inherited": false,
<           "permission_level": "IS_OWNER"
<         }
<       ],
<       "display_name": "<ME>",
<       "user_name": "<ME>"
<     },
<     {
<       "all_permissions": [
<         {
<           "inherited": true,
<           "inherited_from_object": [
<             "/sql/warehouses/"
<           ],
<           "permission_level": "CAN_MANAGE"
<         }
<       ],
<       "group_name": "admins"
<     }
<   ],
<   "object_id": "/sql/warehouses/<ID>",
<   "object_type": "warehouses"
< } pid=53248 sdk=true
```

This PR modifies the SQL warehouse configuration for
`databricks_permissions` to be chosen for instances with an ID of the
form `/warehouses/...`.

## Tests
The additional integration test ensures that a resource can be imported
with the `/warehouses/<ID>` format.
  • Loading branch information
mgyucht authored Oct 28, 2024
1 parent 8b2a735 commit e504790
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
26 changes: 22 additions & 4 deletions internal/acceptance/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,16 @@ func TestAccPermissions_SqlWarehouses(t *testing.T) {
resource "databricks_sql_endpoint" "this" {
name = "{var.STICKY_RANDOM}"
cluster_size = "2X-Small"
tags {
custom_tags {
key = "Owner"
value = "eng-dev-ecosystem-team_at_databricks.com"
}
}
}`
ctx := context.Background()
w := databricks.Must(databricks.NewWorkspaceClient())
var warehouseId string
WorkspaceLevel(t, Step{
Template: sqlWarehouseTemplate + makePermissionsTestStage("sql_endpoint_id", "databricks_sql_endpoint.this.id", groupPermissions("CAN_USE")),
}, Step{
Expand All @@ -638,15 +647,24 @@ func TestAccPermissions_SqlWarehouses(t *testing.T) {
}, Step{
Template: sqlWarehouseTemplate,
Check: func(s *terraform.State) error {
w := databricks.Must(databricks.NewWorkspaceClient())
id := s.RootModule().Resources["databricks_sql_endpoint.this"].Primary.ID
warehouse, err := w.Warehouses.GetById(context.Background(), id)
warehouseId = s.RootModule().Resources["databricks_sql_endpoint.this"].Primary.ID
warehouse, err := w.Warehouses.GetById(ctx, warehouseId)
assert.NoError(t, err)
permissions, err := w.Permissions.GetByRequestObjectTypeAndRequestObjectId(context.Background(), "warehouses", id)
permissions, err := w.Permissions.GetByRequestObjectTypeAndRequestObjectId(context.Background(), "warehouses", warehouseId)
assert.NoError(t, err)
assertContainsPermission(t, permissions, currentPrincipalType(t), warehouse.CreatorName, iam.PermissionLevelIsOwner)
return nil
},
}, Step{
// To test import, a new permission must be added to the warehouse, as it is not possible to import databricks_permissions
// for a warehouse that has the default permissions (i.e. current user has IS_OWNER and admins have CAN_MANAGE).
Template: sqlWarehouseTemplate + makePermissionsTestStage("sql_endpoint_id", "databricks_sql_endpoint.this.id", groupPermissions("CAN_USE")),
}, Step{
Template: sqlWarehouseTemplate + makePermissionsTestStage("sql_endpoint_id", "databricks_sql_endpoint.this.id", groupPermissions("CAN_USE")),
// Verify that we can use "/warehouses/<ID>" instead of "/sql/warehouses/<ID>"
ResourceName: "databricks_permissions.this",
ImportState: true,
ImportStateIdFunc: func(s *terraform.State) (string, error) { return "/warehouses/" + warehouseId, nil },
})
}

Expand Down
5 changes: 5 additions & 0 deletions permissions/permission_definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,11 @@ func allResourcePermissions() []resourcePermissions {
field: "sql_endpoint_id",
objectType: "warehouses",
requestObjectType: "sql/warehouses",
// ISSUE-4143: some older warehouse permissions have an ID that starts with "/warehouses" instead of "/sql/warehouses"
// Because no idRetriever is defined, any warehouse permissions resources will be created with the "/sql/warehouses" prefix.
idMatcher: func(id string) bool {
return strings.HasPrefix(id, "/sql/warehouses/") || strings.HasPrefix(id, "/warehouses/")
},
allowedPermissionLevels: map[string]permissionLevelOptions{
"CAN_USE": {isManagementPermission: false},
"CAN_MANAGE": {isManagementPermission: true},
Expand Down

0 comments on commit e504790

Please sign in to comment.