Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Support databricks_grant, databricks_permission etc. resources #1976

Open
karlschriek opened this issue Feb 2, 2023 · 16 comments
Open
Labels
feature New feature or request needs-api changes to Databricks Platform APIs are required

Comments

@karlschriek
Copy link

karlschriek commented Feb 2, 2023

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 karlschriek added the feature New feature or request label Feb 2, 2023
@nkvuong nkvuong added the needs-api changes to Databricks Platform APIs are required label Feb 14, 2023
@nkvuong
Copy link
Contributor

nkvuong commented Feb 14, 2023

@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

@karlschriek
Copy link
Author

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

@nkvuong
Copy link
Contributor

nkvuong commented Feb 15, 2023

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

@dsfrederic
Copy link
Contributor

dsfrederic commented Jun 13, 2023

@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?

@orolega
Copy link

orolega commented Jul 14, 2023

+1 to this feature.

@oschistad
Copy link

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.

@Ahmet-Djedovic
Copy link

+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.

@pedroGitt
Copy link

+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..

@martin-walsh
Copy link
Contributor

#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.

@alexott
Copy link
Contributor

alexott commented Jan 2, 2024

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

@martin-walsh
Copy link
Contributor

@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 ?

@alexott
Copy link
Contributor

alexott commented Jan 2, 2024

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

@kvedes
Copy link

kvedes commented Apr 4, 2024

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.

@nkvuong
Copy link
Contributor

nkvuong commented Apr 4, 2024

@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.

@kvedes
Copy link

kvedes commented Apr 5, 2024

@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
Copy link

kvedes commented Jul 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request needs-api changes to Databricks Platform APIs are required
Projects
None yet
Development

No branches or pull requests

10 participants