Added support for Unity Catalog databricks_catalog_workspace_binding resource

[ehab-eb]

Changes

Closes #2312

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

[ehab-eb]

@nkvuong It's far from done but I though I'd open a draft PR so you can see the direction I'm going in.

The main challenge so far is that the terraform schema doesn't match the API schema so I'm trying to figure out the best way to work around that.

[nkvuong]

Leave a couple of comments on how to proceed

[nfx]

1. use BindResource
2. add acceptance test
3. add documentation

[ehab-eb]

Thank you both for the feedback! I've addressed most of it but I'm running into issues here with the pair id.

The workspace id is defined as int64 and the Unpack function is expecting string. Since the unpacking infers the types from the schema I'm not entirely sure how to convert the type without affecting the terraform schema data types. Any advice?

[nfx]

@ehab-eb use BindResource and convert between int64/string within the methods.

[ehab-eb]

@nfx @nkvuong I've updated the code to use BindResource and the tests are now passing! Feels like the tests aren't covering all bases though, so I'm happy to discuss what other cases to implement.

[nkvuong]

acceptance test is failing with cannot read catalog workspace binding: Catalog has no binding to this workspace. This is because the catalog isolation mode is not correctly set. That fix is in this PR #2357

[ehab-eb]

acceptance test is failing with cannot read catalog workspace binding: Catalog has no binding to this workspace. This is because the catalog isolation mode is not correctly set. That fix is in this PR #2357

Once your PR is merged should the isolation level be implicitly updated in this resource or should it rely on the catalog resource setting the correct isolation mode?

[nkvuong]

should the isolation level be implicitly updated in this resource

this may cause drifts in the catalog resource, which is not ideal

should it rely on the catalog resource setting the correct isolation mode?

this is better, but then require the catalog to be created in Terraform, but I would prefer this approach. maybe worth checking that the catalog has the correct isolation mode during creation?

[ehab-eb]

this is better, but then require the catalog to be created in Terraform,

Or just that the user has toggled any of the workspaces attached to this catalog using the UI.

maybe worth checking that the catalog has the correct isolation mode during creation?

I was thinking the same thing but I'm wondering if it makes more sense to just let the API fail naturally?

Either way I'll add a note in the docs about the isolation mode.

[nkvuong]

so we have a slight problem - creating catalog with isolation_mode = "ISOLATED" will result in GET API failing with the message cannot read catalog: Catalog 'dev{var.random}' is not accessible in current workspace and cannot read catalog: Catalog 'test{var.random}' is not accessible in current workspace

I'm going to check with our engineering team on how they expect it to work

[ehab-eb]

That's kind of strange since I'm not specifying an isolation mode for the prod catalog. My understanding is that it should default to OPEN and be accessible from all workspaces.

[nkvuong]

@ehab-eb sorry, copied the wrong error message, as I was trying to test locally as well.

I checked in the UI, and the API actually returns a 403 as well and the UI just hides the error. However, workspace binding APIs and patching the catalog still works, which means we need to handle this 403 somehow

[ehab-eb]

@nkvuong so you're saying that when a catalog is created in isolated mode and then we try to attach it to the current workspace the bindings API returns a 403 AND succeeds in making the binding?

[nkvuong]

@ehab-eb this is the behaviour I am seeing:

• POST a catalog in isolated mode - 200
• GET a catalog in isolated mode - 403 (unless workspace is bound) <--- this is the problem for Terraform, as it needs to refresh the state by calling GET
• GET catalog workspace bindings - 200
• PATCH catalog isolation mode - 200

[ehab-eb]

@nkvuong Ah okay that makes sense.

The options I see for this are either co-locating the the bindings and isolation mode calls by combining the resources or attaching the current workspace by default when a catalog is set to ISOLATED.

What do you think?

[nkvuong]

@ehab-eb we won't have an ideal solution either way - I prefer the 2nd option (and we add it to the doc). A 3rd option is to handle the 403 response in the ReadContext for catalog, but that would cause issues with drift detection.

[ehab-eb]

Yep I agree. Option 2 makes the most sense. Will make the changes

[ehab-eb]

@nkvuong I'm scratching my head a little bit trying to find a way to get the current workspace id. I can find the host url but not sure how to connect that to an actual id. Any tips on where to look?

I considered implicitly calling mws_workspaces data source but that's an account level auth and I imagine it needs a different set of permissions than creating a catalog.

[nkvuong]

within go sdk, there is this method that should return the workspace_id - https://github.com/databricks/databricks-sdk-go/blob/main/service/catalog/api.go#LL783C25-L783C32

[ehab-eb]

@nkvuong Thanks! That did it :)

[ehab-eb]

@ehab-eb re-running acceptance tests, but there is an issue with catalog.NewMetastores(c.DatabricksClient).Current(ctx) where the struct has the wrong type, leading to cannot unmarshal number into Go struct field MetastoreAssignment.workspace_id of type string

I'll ask the engineering team to fix this

@nkvuong I've made a change to how I identify the current metastore. I believe it's more or less the same call underneath but I'm not sure what the root cause was for the error you got so it might be worth running the acceptance tests again?

[nkvuong]

@ehab-eb acceptance tests are still failing with

 Error: cannot create catalog: json: cannot unmarshal number into Go struct field MetastoreAssignment.workspace_id of type string
    
      with databricks_catalog.dev,
      on terraform_plugin_test.tf line 1, in resource "databricks_catalog" "dev":
       1: resource "databricks_catalog" "dev" {
    
    
    Error: cannot create catalog: json: cannot unmarshal number into Go struct field MetastoreAssignment.workspace_id of type string
    
      with databricks_catalog.test,
      on terraform_plugin_test.tf line 5, in resource "databricks_catalog" "test":
       5: resource "databricks_catalog" "test" {

[nfx]

try to use stronger typed APIs

Suggested change

	if d.Get("isolation_mode") != "ISOLATED" {
	if ci.IsolationMode != "ISOLATED" {

[thaiphv]

Thanks for implementing this resource. @nfx when do you plan to release this?

[Hong-Kit]

@nfx Thanks this is awesome work! Would like to know about the release schedule too 🙏