Skip to content

Commit

Permalink
Relaxed cluster check for databricks_sql_permissions (databricks#3683)
Browse files Browse the repository at this point in the history 
* relax cluster check

* fix

* fix
  • Loading branch information
@nkvuong
nkvuong authored Jun 20, 2024
1 parent eaa2772 commit 66a881f
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 33 deletions.
18 changes: 8 additions & 10 deletions access/resource_sql_permissions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ func (ta *SqlPermissions) initCluster(ctx context.Context, d *schema.ResourceDat
if err != nil {
return
}
if v, ok := clusterInfo.SparkConf["spark.databricks.acl.dfAclsEnabled"]; !ok || v != "true" {
err = fmt.Errorf("cluster_id: not a High-Concurrency cluster: %s (%s)",
if v, ok := clusterInfo.SparkConf["spark.databricks.acl.dfAclsEnabled"]; (!ok || v != "true") && clusterInfo.DataSecurityMode != "USER_ISOLATION" && clusterInfo.DataSecurityMode != "LEGACY_TABLE_ACL" {
err = fmt.Errorf("cluster_id: pecified cluster does not support setting Table ACL: %s (%s)",
clusterInfo.ClusterName, clusterInfo.ClusterID)
return
}
Expand All @@ -282,11 +282,10 @@ func (ta *SqlPermissions) getOrCreateCluster(clustersAPI clusters.ClustersAPI) (
SparkVersion: sparkVersion,
NodeTypeID: nodeType,
AutoterminationMinutes: 10,
DataSecurityMode: "LEGACY_TABLE_ACL",
SparkConf: map[string]string{
"spark.databricks.acl.dfAclsEnabled": "true",
"spark.databricks.repl.allowedLanguages": "python,sql",
"spark.databricks.cluster.profile": "singleNode",
"spark.master": "local[*]",
"spark.databricks.cluster.profile": "singleNode",
"spark.master": "local[*]",
},
CustomTags: map[string]string{
"ResourceClass": "SingleNode",
Expand All @@ -305,8 +304,7 @@ func tableAclForUpdate(ctx context.Context, d *schema.ResourceData,
return
}

func tableAclForLoad(ctx context.Context, d *schema.ResourceData,
s map[string]*schema.Schema, c *common.DatabricksClient) (ta SqlPermissions, err error) {
func tableAclForLoad(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) (ta SqlPermissions, err error) {
ta, err = loadTableACL(d.Id())
if err != nil {
return
Expand Down Expand Up @@ -345,7 +343,7 @@ func ResourceSqlPermissions() common.Resource {
return nil
},
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
ta, err := tableAclForLoad(ctx, d, s, c)
ta, err := tableAclForLoad(ctx, d, c)
if err != nil {
return err
}
Expand All @@ -370,7 +368,7 @@ func ResourceSqlPermissions() common.Resource {
return ta.enforce()
},
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
ta, err := tableAclForLoad(ctx, d, s, c)
ta, err := tableAclForLoad(ctx, d, c)
if err != nil {
return err
}
Expand Down
109 changes: 101 additions & 8 deletions access/resource_sql_permissions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,10 @@ var createHighConcurrencyCluster = []qa.HTTPFixture{
"ResourceClass": "SingleNode",
},
SparkConf: map[string]string{
"spark.databricks.acl.dfAclsEnabled": "true",
"spark.databricks.repl.allowedLanguages": "python,sql",
"spark.databricks.cluster.profile": "singleNode",
"spark.master": "local[*]",
"spark.databricks.cluster.profile": "singleNode",
"spark.master": "local[*]",
},
DataSecurityMode: "LEGACY_TABLE_ACL",
},
Response: clusters.ClusterID{
ClusterID: "bcd",
Expand All @@ -242,13 +241,87 @@ var createHighConcurrencyCluster = []qa.HTTPFixture{
ReuseRequest: true,
Resource: "/api/2.0/clusters/get?cluster_id=bcd",
Response: clusters.ClusterInfo{
ClusterID: "bcd",
State: "RUNNING",
ClusterID: "bcd",
State: "RUNNING",
DataSecurityMode: "LEGACY_TABLE_ACL",
SparkConf: map[string]string{
"spark.databricks.cluster.profile": "singleNode",
},
},
},
}

var createSharedCluster = []qa.HTTPFixture{
{
Method: "GET",
ReuseRequest: true,
Resource: "/api/2.0/clusters/list",
Response: map[string]any{},
},
{
Method: "GET",
ReuseRequest: true,
Resource: "/api/2.0/clusters/spark-versions",
Response: clusters.SparkVersionsList{
SparkVersions: []clusters.SparkVersion{
{
Version: "7.1.x-cpu-ml-scala2.12",
Description: "7.1 ML (includes Apache Spark 3.0.0, Scala 2.12)",
},
},
},
},
{
Method: "GET",
ReuseRequest: true,
Resource: "/api/2.0/clusters/list-node-types",
Response: compute.ListNodeTypesResponse{
NodeTypes: []compute.NodeType{
{
NodeTypeId: "Standard_F4s",
InstanceTypeId: "Standard_F4s",
MemoryMb: 8192,
NumCores: 4,
NodeInstanceType: &compute.NodeInstanceType{
LocalDisks: 1,
InstanceTypeId: "Standard_F4s",
LocalDiskSizeGb: 16,
},
},
},
},
},
{
Method: "POST",
ReuseRequest: true,
Resource: "/api/2.0/clusters/create",
ExpectedRequest: clusters.Cluster{
AutoterminationMinutes: 10,
ClusterName: "terraform-table-acl",
NodeTypeID: "Standard_F4s",
SparkVersion: "7.3.x-scala2.12",
CustomTags: map[string]string{
"ResourceClass": "SingleNode",
},
DataSecurityMode: "LEGACY_TABLE_ACL",
SparkConf: map[string]string{
"spark.databricks.acl.dfAclsEnabled": "true",
"spark.databricks.cluster.profile": "singleNode",
"spark.databricks.cluster.profile": "singleNode",
"spark.master": "local[*]",
},
},
Response: clusters.ClusterID{
ClusterID: "bcd",
},
},
{
Method: "GET",
ReuseRequest: true,
Resource: "/api/2.0/clusters/get?cluster_id=bcd",
Response: clusters.ClusterInfo{
ClusterID: "bcd",
State: "RUNNING",
DataSecurityMode: "USER_ISOLATION",
},
},
}

Expand All @@ -272,6 +345,26 @@ func TestResourceSqlPermissions_Read(t *testing.T) {
}.ApplyNoError(t)
}

func TestResourceSqlPermissions_ReadSharedCluster(t *testing.T) {
qa.ResourceFixture{
CommandMock: mockData{
"SHOW GRANT ON TABLE `default`.`foo`": {
{"users", "SELECT", "database", "foo"},
{"users", "SELECT", "table", "`default`.`foo`"},
{"[email protected]", "OWN", "table", "`default`.`foo`"},
{"users", "READ", "table", "`default`.`foo`"},
{"users", "SELECT", "database", "default"},
{"interns", "DENIED_SELECT", "table", "`default`.`foo`"},
},
}.toCommandMock(),
Fixtures: createSharedCluster,
Resource: ResourceSqlPermissions(),
Read: true,
New: true,
ID: "table/default.foo",
}.ApplyNoError(t)
}

func TestResourceSqlPermissions_Read_Error(t *testing.T) {
qa.ResourceFixture{
Resource: ResourceSqlPermissions(),
Expand Down
27 changes: 12 additions & 15 deletions docs/resources/sql_permissions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,6 @@ resource "databricks_cluster" "cluster_with_table_access_control" {
}
```

It could be combined with creation of High-Concurrency and Single-Node clusters - in this case it should have corresponding `custom_tags` and `spark.databricks.cluster.profile` in Spark configuration as described in [documentation for `databricks_cluster` resource](cluster.md).

The created cluster could be referred to by providing its ID as `cluster_id` property.


```hcl
resource "databricks_sql_permissions" "foo_table" {
cluster_id = databricks_cluster.cluster_name.id
#...
}
```

It is required to define all permissions for a securable in a single resource, otherwise Terraform cannot guarantee config drift prevention.

## Example Usage
Expand Down Expand Up @@ -60,11 +48,20 @@ resource "databricks_sql_permissions" "foo_table" {

## Argument Reference

* `cluster_id` - (Optional) Id of an existing [databricks_cluster](cluster.md), where the appropriate `GRANT`/`REVOKE` commands are executed. This cluster must have the appropriate data security mode (`USER_ISOLATION` or `LEGACY_TABLE_ACL` specified). If no `cluster_id` is specified, a single-node TACL cluster named `terraform-table-acl` is automatically created.

```hcl
resource "databricks_sql_permissions" "foo_table" {
cluster_id = databricks_cluster.cluster_name.id
#...
}
```

The following arguments are available to specify the data object you need to enforce access controls on. You must specify only one of those arguments (except for `table` and `view`), otherwise resource creation will fail.

* `database` - Name of the database. Has default value of `default`.
* `table` - Name of the table. Can be combined with `database`.
* `view` - Name of the view. Can be combined with `database`.
* `table` - Name of the table. Can be combined with `database`.
* `view` - Name of the view. Can be combined with `database`.
* `catalog` - (Boolean) If this access control for the entire catalog. Defaults to `false`.
* `any_file` - (Boolean) If this access control for reading/writing any file. Defaults to `false`.
* `anonymous_function` - (Boolean) If this access control for using anonymous function. Defaults to `false`.
Expand Down Expand Up @@ -100,7 +97,7 @@ The resource can be imported using a synthetic identifier. Examples of valid syn
* `anonymous function/` - anonymous function. `/` suffix is mandatory.

```bash
$ terraform import databricks_sql_permissions.foo /<object-type>/<object-name>
terraform import databricks_sql_permissions.foo /<object-type>/<object-name>
```

## Related Resources
Expand Down

0 comments on commit 66a881f

Please sign in to comment.