terraform-provider-databricks icon indicating copy to clipboard operation
terraform-provider-databricks copied to clipboard

[FEATURE] Support `databricks_grant`, `databricks_permission` etc. resources

Open karlschriek opened this issue 2 years ago • 22 comments

Use-cases

The databricks provider currently offers resources such as databricks_grants and databricks_permissions (and probably several others that work in the same manner, but those are the two I am working with right now). These resources require you to assign all grants/permissions for a specific entity (such as a metastore, catalog, etc.) within the same resource.

For example (from the official docs):

resource "databricks_grants" "sandbox" {
  metastore = databricks_metastore.this.id
  grant {
    principal  = "Data Engineers"
    privileges = ["CREATE_CATALOG", "CREATE_EXTERNAL_LOCATION"]
  }
  grant {
    principal  = "Data Sharer"
    privileges = ["CREATE_RECIPIENT", "CREATE_SHARE"]
  }
}

The above is the only place where I can add grants to the databricks_metastore.this.id resource. Any additional combinations of entity, principal and privilege can only be added here. This makes composition in complicated terraform projects extremely difficult. We generally follow the principle of modular composition where each individual module is self-contained and additional, self-contained modules can be added without needing to change existing ones. With a resource such as databricks_grants this is not possible. We would prefer to set a singular resources such as databricks_grant (and databricks_permission etc.) for each specific combination of entity/principal/privileges.

This would be in line with how role assignments are for example dealt with by the Azure Resource Manager.

resource "azurerm_role_assignment" "example2" {
  name                 = "role-assignement-1" //this field is optional. if left out a GUID will be generated
  scope                = var.my_resource_id
  principal_id         = var_principal_id
  role_definition_id   = var_role_definition_1_resource_id
}

resource "azurerm_role_assignment" "example2" {
  name                 = "role-assignement-2"
  scope                = var.my_resource_id
  principal_id         = var_principal_id
  role_definition_id   = var_role_definition_2_resource_id
}

This is possible because the "name" field is what uniquely defines the role assignment resource.

Proposal

I would suggest allow for a databricks_grant (and similar for other resources) as follows:

resource "databricks_grant" "data_sharers" {
  name       = "data-sharer-1" // make this field optional; generate GUID if not set. This is the resource ID
  metastore  = databricks_metastore.this.id
  principal  = "Data Sharer"
  privileges =  ["CREATE_RECIPIENT", "CREATE_SHARE"]
}
resource "databricks_grant" "data_engineers" {
  name       = "data-engineers-1" // make this field optional; generate GUID if not set. This is the resource ID
  metastore  = databricks_metastore.this.id
  principal  = "Data Engineers"
  privileges = ["CREATE_CATALOG", "CREATE_EXTERNAL_LOCATION"]
}

karlschriek avatar Feb 02 '23 18:02 karlschriek

@karlschriek we will need changes to the existing permissions/grants API for this - at the moment the API does not have the concept of role assignments and therefore drift configuration in Terraform will be very flaky

nkvuong avatar Feb 14 '23 10:02 nkvuong

That's a shame, should I log a request somewhere for the API to be expanded?

karlschriek avatar Feb 14 '23 11:02 karlschriek

@karlschriek if you mention it to your account team, I have also flagged it with the product team to put it on their backlog

nkvuong avatar Feb 15 '23 09:02 nkvuong

@karlschriek @nkvuong any update on this topic?

This should have priority imho.

Side question: how would is it possible to add a single grant in the UI but not via API?

dsfrederic avatar Jun 13 '23 10:06 dsfrederic

+1 to this feature.

orolega avatar Jul 14 '23 01:07 orolega

My client, a large governmental agency, is also butting against this limitation and is sorely in need of a more flexible way to manage permissions in a higly distributed environment.

As proposed in the feature request, you would need to implement a unique identifier per role assignment / ACL instance, so that incremental changes can be applied and removed without any risk of modifying or deleting the wrong ACL. Going this route would enable Terraform to track the assignment instance as a stateful resource rather than the entire access control list.

oschistad avatar Sep 13 '23 07:09 oschistad

+1 We face limitation without the Databricks grant feature. Catalog resources require you to assign all grants/permissions for a specific catalog within the same resource. This makes Terraform codebases that use the same catalog for their solution overwrite each other's grant in each Terraform apply.

Ahmet-Djedovic avatar Dec 07 '23 13:12 Ahmet-Djedovic

+1 We are in multi project environment, and it is jut not possible to establish a reliable configuration on the unity catalog metastore. As explained, the buggy resource "databricks_grants" API is removing all the existing granting configuration. This defect should be in high priority.

The only workaround for now, is that all the parties involved in granting privileges in the metastore/catalog.. use the Update permissions databricks API which has a nice add and remove API approach.

Any update about this topic? It lasts for February..

pedroGitt avatar Dec 19 '23 09:12 pedroGitt

https://github.com/databricks/terraform-provider-databricks/pull/3024 addresses databricks_grant feature request. The backing API is quite nice as it allows PATCH updates for changes only.

databricks_permission is not so easy as the backing API is a full replacement PUT update.

martin-walsh avatar Jan 02 '24 00:01 martin-walsh

@martin-walsh there is PATCH support for permissions as well: https://docs.databricks.com/api/workspace/tokenmanagement/updatepermissions

alexott avatar Jan 02 '24 07:01 alexott

@martin-walsh there is PATCH support for permissions as well: https://docs.databricks.com/api/workspace/tokenmanagement/updatepermissions

Oh nice, and that won't overwrite all the existing permissions for the token ?

martin-walsh avatar Jan 02 '24 10:01 martin-walsh

yes, it's in general API design: PATCH is doing partial update, and PUT is overwriting everything

alexott avatar Jan 02 '24 11:01 alexott

What is the status on this? - I see that databricks_grant has been added, but my tests show that it has the same behaviour as databricks_grants and thus overwrites existing grants.

kvedes avatar Apr 04 '24 13:04 kvedes

@kvedes databricks_grant relaxes the restriction of databricks_grants, but it is still authoritative for a given principal, so you need to specify all the permissions for a principal in a single resource.

nkvuong avatar Apr 04 '24 13:04 nkvuong

@nkvuong Thank you, I thought I had tested that but it seems to work. I would suggest that the docs are updated to reflect the difference between databricks_grant and databricks_grants in relation to when they are authoritative.

kvedes avatar Apr 05 '24 06:04 kvedes

Is databricks_permission on the roadmap? - Would be really nice to have this. It is extremely difficult to make modular code when using databricks_permissions.

kvedes avatar Jul 04 '24 07:07 kvedes