Add resource databricks_grant for managing singular principal

[martin-walsh]

Changes

Add resource "databricks_grant" for managing grants for a single principal on a single securable. This allows securing of resources in distributed terraform repos e.g. granting an api user SELECT on a table without wiping out centrally managed privileges such as human user access.

Opening up PR to discuss whether the approach is acceptable to you and will add documentation, and clean up if it is.

Fixes #2704

Tests

• make test run locally
• relevant change in docs/ folder
• covered with integration tests in internal/acceptance
• relevant acceptance tests are passing
• using Go SDK

[martin-walsh]

@Jonathan-Choi FYI

[hectorcast-db]

There is an issue with the current implementation that we are fixing here
#3026

This will need to be adjusted here after we merged.

[martin-walsh]

Added documentation for databricks_grant also.

[nkvuong]

the content of this page would be mostly duplicated from docs/resources/grants.md, so might make sense to just collapse them in a single page, highlighting the differences
wdyt @alexott

[codecov-commenter]

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (5269041) 84.59% compared to head (85332a5) 84.60%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3024    +/-   ##
========================================
  Coverage   84.59%   84.60%            
========================================
  Files         161      162     +1     
  Lines       14214    14332   +118     
========================================
+ Hits        12025    12126   +101     
- Misses       1507     1516     +9     
- Partials      682      690     +8     

Files	Coverage Δ
provider/provider.go	92.55% <100.00%> (+0.03%)	⬆️
catalog/resource_grant.go	87.17% <87.17%> (ø)

... and 1 file with indirect coverage changes

[mgyucht]

This seems great, thanks for addressing my points. Just one question about diffPermissionsForPrincipal, which I think was kind of written to support multiple principals but only ever acts on one; can we clean this up?

[mgyucht]

Ping here

[martin-walsh]

This seems great, thanks for addressing my points. Just one question about diffPermissionsForPrincipal, which I think was kind of written to support multiple principals but only ever acts on one; can we clean this up?

It returns an array as that's what the UpdatePermissions request takes downstream. It is also a near carbon copy of the impl from resource_grants which to me is a positive as it makes addressing potential bugs easier. I would suggest leaving it as is for those reasons. I have a subsequent PR to update that resource to use go sdk and may refactor further there.

[mgyucht]

Gotcha, that makes a lot of sense.

[mgyucht]

Got clarification on the main point. Thanks!

[mgyucht]

This failed one of our nightly tests. @nkvuong could you take a look at https://github.com/databricks/eng-dev-ecosystem/actions/runs/7399425920/job/20130815184 and see if you can debug this?

[nkvuong]

@mgyucht basically TestUcAccGrant and TestUcAccGrants are overwriting each other permissions at metastore-level

resource "databricks_grant" "metastore" {
	metastore = "{env.TEST_METASTORE_ID}"
	principal  = "%s"
	privileges = ["CREATE_STORAGE_CREDENTIAL"]
}

resource "databricks_grants" "metastore" {
	metastore = "{env.TEST_METASTORE_ID}"
	grant {
		principal  = "%s"
		privileges = ["CREATE_STORAGE_CREDENTIAL"]
	}
}

so either we need to make these 2 tests run sequentially, or remove the metastore bit for databricks_grant

[mgyucht]

Ah, that makes perfect sense. Can we comment out the databricks_grant test for metastore for now to merge? we need to fix this for metastore assignment for workspaces as well, which also races between two test cases. I can fix both of these at the same time.

[martin-walsh]

Ah, that makes perfect sense. Can we comment out the databricks_grant test for metastore for now to merge? we need to fix this for metastore assignment for workspaces as well, which also races between two test cases. I can fix both of these at the same time.

@mgyucht done.

[nkvuong]

@mgyucht integration tests now passed (failed tests are not related to this PR)

[mgyucht]

All nightlies passed. Merging now!

@martin-walsh thank you so much for your work on this and your patience through this process! We and our other customers really appreciate contributions like these. This is one of the biggest pain points with permission management for the provider, so you've solved a major problem with this.

[kvedes]

Using provider version 1.39, and it seems that databricks_grant overwrites current permissions:

resource "databricks_grant" "unity_catalog" {
  catalog = "dummy"
  principal  = "group_a"
  privileges = ["CREATE_SCHEMA"]
}

resource "databricks_grant" "unity_catalog2" {
  catalog = "dummy"
  principal  = "group_a"
  privileges = ["USE_CATALOG"]
}

This leaves me only with the USE_CATALOG permission.