Skip to content

Conversation

Tanchwa
Copy link

@Tanchwa Tanchwa commented Apr 1, 2025

Changes

  • added skelleton for methods needed for new resource struct to satisfy
  • added confuration method for both workspace and account clients (need assistance in how to grab that provider config attribute to check logic)
  • added resource model struct for golang data structure
  • started Create method call

Tests

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

Copy link

github-actions bot commented Apr 1, 2025

Please ensure that the NEXT_CHANGELOG.md file is updated with any relevant changes.
If this is not necessary for your PR, please include the following in your PR description:
NO_CHANGELOG=true
and rerun the job.

}

var provider
//TODO CHECK IF ACCOUNTID ATTRIBUTE IS SET IN PROVIDER CONFIG STRUCT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permission is a workspace-level resource, so we should not worry about account-level

@Tanchwa
Copy link
Author

Tanchwa commented Apr 7, 2025

I'm getting a weird error with both the PUT and PATCH methods on the access control request API call,

databricks_permission.test: Creating...
2025-04-07T10:22:13.339-0400 [INFO]  Starting apply for databricks_permission.test
2025-04-07T10:22:13.339-0400 [DEBUG] databricks_permission.test: applying the planned Create change
2025-04-07T10:22:15.918-0400 [DEBUG] provider.terraform-provider-databricks: PUT /api/2.0/permissions/clusters/0407-133323-2hof39fk
> {
>   "access_control_list": [
>     {
>       "group_name": "\"Test Group\"",
>       "permission_level": "\"CAN_MANAGE\"",
>       "service_principal_name": "\u003cunknown\u003e",
>       "user_name": "\u003cunknown\u003e"
>     }
>   ]
> }
< HTTP/2.0 400 Bad Request
< {
<   "error_code": "INVALID_PARAMETER_VALUE",
<   "message": "Permission type not defined"
< }: tf_mux_provider=tf5to6server.v5tov6Server tf_provider_addr=registry.terraform.io/databricks/databricks tf_rpc=ConfigureProvider @module=databricks tf_req_id=36ef38c0-5f98-9305-1b2e-def698be5d43 @caller=/Users/asutliff/Documents/Repos/Terraform-Providers/terraform-provider-databricks/logger/logger.go:38 timestamp=2025-04-07T10:22:15.918-0400
2025-04-07T10:22:15.918-0400 [DEBUG] provider.terraform-provider-databricks: non-retriable error: Permission type not defined: tf_mux_provider=tf5to6server.v5tov6Server tf_provider_addr=registry.terraform.io/databricks/databricks tf_req_id=36ef38c0-5f98-9305-1b2e-def698be5d43 tf_rpc=ConfigureProvider @caller=/Users/asutliff/Documents/Repos/Terraform-Providers/terraform-provider-databricks/logger/logger.go:38 @module=databricks timestamp=2025-04-07T10:22:15.918-0400
2025-04-07T10:22:15.919-0400 [ERROR] provider.terraform-provider-databricks: Response contains error diagnostic: diagnostic_summary="Unable to Create Permission" tf_proto_version=6.8 tf_resource_type=databricks_permission @module=sdk.proto diagnostic_detail="current object_id: 0407-133323-2hof39fk current object type clusters" diagnostic_severity=ERROR tf_req_id=0ae814ee-2245-592e-7240-ba1e19d3165f tf_rpc=ApplyResourceChange @caller=/Users/asutliff/Documents/Repos/Terraform-Providers/terraform-provider-databricks/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/diag/diagnostics.go:58 tf_provider_addr=registry.terraform.io/databricks/databricks timestamp=2025-04-07T10:22:15.919-0400
2025-04-07T10:22:15.927-0400 [DEBUG] State storage *statemgr.Filesystem declined to persist a state snapshot
2025-04-07T10:22:15.927-0400 [ERROR] vertex "databricks_permission.test" error: Unable to Create Permission
2025-04-07T10:22:15.927-0400 [ERROR] vertex "databricks_permission.test" error: Provider returned invalid result object after apply
2025-04-07T10:22:15.927-0400 [ERROR] vertex "databricks_permission.test" error: Provider returned invalid result object after apply
╷
│ Error: Provider returned invalid result object after apply
│ 
│ After the apply operation, the provider still indicated an unknown value for databricks_permission.test.access_control[...].service_principal_id. All values must
│ be known after apply, so this is always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known
│ object values in the state.
╵
╷
│ Error: Provider returned invalid result object after apply
│ 
│ After the apply operation, the provider still indicated an unknown value for databricks_permission.test.access_control[...].user_name. All values must be known
│ after apply, so this is always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known object
│ values in the state.
╵
╷
│ Error: Unable to Create Permission
│ 
│   with databricks_permission.test,
│   on main.tf line 15, in resource "databricks_permission" "test":
│   15: resource "databricks_permission" "test" {
│ 
│ current object_id: 0407-133323-2hof39fk current object type clusters

I have the error out to make sure that they're the current strings I'm passing in for fields RequestObjectId and RequestObjectType respectively. I'm not sure if this needs to be a special value for the JSON request to format properly, or it just needs to be a normal string, but it looks like it's populating the API path properly

@nkvuong

@Tanchwa
Copy link
Author

Tanchwa commented Apr 7, 2025

I'm getting a weird error with both the PUT and PATCH methods on the access control request API call,

databricks_permission.test: Creating...
2025-04-07T10:22:13.339-0400 [INFO]  Starting apply for databricks_permission.test
2025-04-07T10:22:13.339-0400 [DEBUG] databricks_permission.test: applying the planned Create change
2025-04-07T10:22:15.918-0400 [DEBUG] provider.terraform-provider-databricks: PUT /api/2.0/permissions/clusters/0407-133323-2hof39fk
> {
>   "access_control_list": [
>     {
>       "group_name": "\"Test Group\"",
>       "permission_level": "\"CAN_MANAGE\"",
>       "service_principal_name": "\u003cunknown\u003e",
>       "user_name": "\u003cunknown\u003e"
>     }
>   ]
> }
< HTTP/2.0 400 Bad Request
< {
<   "error_code": "INVALID_PARAMETER_VALUE",
<   "message": "Permission type not defined"
< }: tf_mux_provider=tf5to6server.v5tov6Server tf_provider_addr=registry.terraform.io/databricks/databricks tf_rpc=ConfigureProvider @module=databricks tf_req_id=36ef38c0-5f98-9305-1b2e-def698be5d43 @caller=/Users/asutliff/Documents/Repos/Terraform-Providers/terraform-provider-databricks/logger/logger.go:38 timestamp=2025-04-07T10:22:15.918-0400
2025-04-07T10:22:15.918-0400 [DEBUG] provider.terraform-provider-databricks: non-retriable error: Permission type not defined: tf_mux_provider=tf5to6server.v5tov6Server tf_provider_addr=registry.terraform.io/databricks/databricks tf_req_id=36ef38c0-5f98-9305-1b2e-def698be5d43 tf_rpc=ConfigureProvider @caller=/Users/asutliff/Documents/Repos/Terraform-Providers/terraform-provider-databricks/logger/logger.go:38 @module=databricks timestamp=2025-04-07T10:22:15.918-0400
2025-04-07T10:22:15.919-0400 [ERROR] provider.terraform-provider-databricks: Response contains error diagnostic: diagnostic_summary="Unable to Create Permission" tf_proto_version=6.8 tf_resource_type=databricks_permission @module=sdk.proto diagnostic_detail="current object_id: 0407-133323-2hof39fk current object type clusters" diagnostic_severity=ERROR tf_req_id=0ae814ee-2245-592e-7240-ba1e19d3165f tf_rpc=ApplyResourceChange @caller=/Users/asutliff/Documents/Repos/Terraform-Providers/terraform-provider-databricks/vendor/github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/diag/diagnostics.go:58 tf_provider_addr=registry.terraform.io/databricks/databricks timestamp=2025-04-07T10:22:15.919-0400
2025-04-07T10:22:15.927-0400 [DEBUG] State storage *statemgr.Filesystem declined to persist a state snapshot
2025-04-07T10:22:15.927-0400 [ERROR] vertex "databricks_permission.test" error: Unable to Create Permission
2025-04-07T10:22:15.927-0400 [ERROR] vertex "databricks_permission.test" error: Provider returned invalid result object after apply
2025-04-07T10:22:15.927-0400 [ERROR] vertex "databricks_permission.test" error: Provider returned invalid result object after apply
╷
│ Error: Provider returned invalid result object after apply
│ 
│ After the apply operation, the provider still indicated an unknown value for databricks_permission.test.access_control[...].service_principal_id. All values must
│ be known after apply, so this is always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known
│ object values in the state.
╵
╷
│ Error: Provider returned invalid result object after apply
│ 
│ After the apply operation, the provider still indicated an unknown value for databricks_permission.test.access_control[...].user_name. All values must be known
│ after apply, so this is always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known object
│ values in the state.
╵
╷
│ Error: Unable to Create Permission
│ 
│   with databricks_permission.test,
│   on main.tf line 15, in resource "databricks_permission" "test":
│   15: resource "databricks_permission" "test" {
│ 
│ current object_id: 0407-133323-2hof39fk current object type clusters

I have the error out to make sure that they're the current strings I'm passing in for fields RequestObjectId and RequestObjectType respectively. I'm not sure if this needs to be a special value for the JSON request to format properly, or it just needs to be a normal string, but it looks like it's populating the API path properly

@nkvuong

I got it, this one just didn't like the extra strings that were getting passed in...

@Tanchwa
Copy link
Author

Tanchwa commented Apr 15, 2025

@nkvuong how do we want to deal with existing objects when we do a state refresh? Do we want to pull all of those into state for the particular object? or just the ones that are made by terraform? The second one presents a bit of a challenge because it means we'd have to do some weird comparisons and decisions for what is considered "changed" for individual permissions.

@nkvuong
Copy link
Contributor

nkvuong commented Apr 15, 2025

@Tanchwa we should make this similar to databricks_grant, the unique key will be resource_id + identity, so permissions added out of bound for the identity will be overwritten

basically

resource "databricks_permission" "cluster_data_eng_permission" {
  cluster_id = databricks_cluster.shared_autoscaling.id
  group_name       = databricks_group.data_eng.display_name
  permission_level = "CAN_ATTACH_TO"
}

will not touch permissions of other groups/users, but will always overwrite permissions for data_eng group

so in the read method, we should call GET, and filter for just the permissions of the right identity

@mgyucht what do you think, as you did some refactoring for databricks_permissions recently

@Tanchwa
Copy link
Author

Tanchwa commented Apr 16, 2025

@nkvuong
Is this what you're talking about in grant?
catalog/resource_grant.go

func toSecurableId(d *schema.ResourceData) string {
	principal := d.Get("principal").(string)
	return fmt.Sprintf("%s/%s", permissions.Mappings.Id(d), principal)
}

and are you talking about the key for the mapping in Go or in the state file?

@Tanchwa
Copy link
Author

Tanchwa commented Apr 16, 2025

Also, I'm having trouble figuring out how to have all the different field options for each type of permission. Would it just be an optional attribute for each one in the schema?

@nkvuong
Copy link
Contributor

nkvuong commented Apr 18, 2025

@Tanchwa
for the databricks_grant implementation, we call d.SetId(toSecurableId(d)) at the end of Create, and for Read, we call filterPermissionsForPrincipal. The same approach here would work.

In order to add all the different field options, most of the helper functions are in https://github.com/databricks/terraform-provider-databricks/blob/main/permissions/permission_definitions.go. The tf schema is defined as

s := common.StructToSchema(entity.PermissionsEntity{}, func(s map[string]*schema.Schema) map[string]*schema.Schema {

@nkvuong nkvuong force-pushed the tanchwa/databricks-permission branch from 788f0c4 to 8c77103 Compare April 18, 2025 12:17
Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4609
  • Commit SHA: 8c77103e4f0e3fa83d702497250567f32e936547

Checks will be approved automatically on success.

@nkvuong nkvuong temporarily deployed to test-trigger-is April 18, 2025 12:17 — with GitHub Actions Inactive
@Tanchwa
Copy link
Author

Tanchwa commented May 1, 2025

@nkvuong sorry I'm just getting back into this... we're in the middle of migrating our Azure DevOps agents to Openshift at work and I didn't have the time to give this attention...

so schema.ResourceData is not available in schema in the new plugin framework. I can call individual structs on the schema that I set which... probably means I need to figure out your guys' method of creating the schema first...

Ngl I found it easier to figure out how to build the schema myself from Terraform's developer documentation than it was to use your guys' struct to schema function.

because I'm trying to use the new plugin framework, I used the tfsdk tags to create the resource model structs

type permissionResourceModel struct {
	ObjectID          types.String                       `tfsdk:"object_id"`
	ObjectType        types.String                       `tfsdk:"object_type"`
	AccessControlList permissionAccessControlModel       `tfsdk:"access_control"`
	LastUpdated       types.String                       `tfsdk:"last_updated"`
}

// accessControlListModel is the same as iam.AccessControlRequest
// was originally just called this way in entity.go
type permissionAccessControlModel struct {
	ServicePrincipalId types.String `tfsdk:"service_principal_id"`
	GroupName            types.String `tfsdk:"group_name"`
	UserName             types.String `tfsdk:"user_name"`
	PermissionLevel      types.String `tfsdk:"permission_level"`
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants