Skip to content

Commit

Permalink
Exporter: modify grants for some UC objects to allow handling of othe…
Browse files Browse the repository at this point in the history
…r owners (#3357)

* Exporter: modify grants for some UC objects to allow handling of other owners

If UC objects, such as, metastore, catalogs, schemas, volumes, etc. have an owner other
than current user, then it's impossible to create child objects during apply.  To mitigate
this, the list of grants is expanded/changed to allow creation of child objects by the
identity who does the export (it's done only on workspace level, and the same identity
must be used when doing import).

* Add possibility to pass extra data together with resource

Primary use - handling of ownership in the grants

* Improve handling of other owners

* check if the current user is already owner of an object, or no owner is specified
* set minimum permissions to make it working - like, `CREATE_SCHEMA` on catalogs, etc.
  • Loading branch information
alexott authored Mar 13, 2024
1 parent 13622e3 commit 7352c6e
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 44 deletions.
2 changes: 1 addition & 1 deletion docs/guides/experimental-exporter.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Services are just logical groups of resources used for filtering and organizatio
* `uc-catalogs` - **listing** [databricks_catalog](../resources/catalog.md) and [databricks_catalog_workspace_binding](../resources/catalog_workspace_binding.md)
* `uc-connections` - **listing** [databricks_connection](../resources/connection.md). *Please note that because API doesn't return sensitive fields, such as, passwords, tokens, ..., the generated `options` block could be incomplete!*
* `uc-external-locations` - **listing** exports [databricks_external_location](../resources/external_location.md) resource.
* `uc-grants` - [databricks_grants](../resources/grants.md)
* `uc-grants` - [databricks_grants](../resources/grants.md). *Please note that during export the list of grants is expanded to include the identity that does the export! This is done to allow to create objects in case when catalogs/schemas have different owners than current identity.*.
* `uc-metastores` - **listing** [databricks_metastore](../resources/metastore.md) and [databricks_metastore_assignment](../resource/metastore_assignment.md) (only on account-level). *Please note that when using workspace-level configuration, only metastores from the workspace's region are listed!*
* `uc-models` - [databricks_registered_model](../resources/registered_model.md)
* `uc-schemas` - [databricks_schema](../resources/schema.md)
Expand Down
3 changes: 3 additions & 0 deletions exporter/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type importContext struct {
lastActiveMs int64
generateDeclaration bool
meAdmin bool
meUserName string
prefix string
accountLevel bool
shImports map[string]bool
Expand Down Expand Up @@ -339,6 +340,7 @@ func (ic *importContext) Run() error {
ic.accountLevel = ic.Client.Config.IsAccountClient()
if ic.accountLevel {
ic.meAdmin = true
// TODO: check if we can get the current user from the account client
ic.accountClient, err = ic.Client.AccountClient()
if err != nil {
return err
Expand All @@ -355,6 +357,7 @@ func (ic *importContext) Run() error {
for _, g := range me.Groups {
if g.Display == "admins" {
ic.meAdmin = true
ic.meUserName = me.UserName
break
}
}
Expand Down
111 changes: 68 additions & 43 deletions exporter/importables.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"github.com/databricks/databricks-sdk-go/service/settings"
"github.com/databricks/databricks-sdk-go/service/sharing"
"github.com/databricks/databricks-sdk-go/service/sql"
tfuc "github.com/databricks/terraform-provider-databricks/catalog"
tfcatalog "github.com/databricks/terraform-provider-databricks/catalog"
"github.com/databricks/terraform-provider-databricks/clusters"
"github.com/databricks/terraform-provider-databricks/common"
"github.com/databricks/terraform-provider-databricks/jobs"
Expand Down Expand Up @@ -68,6 +68,17 @@ var (
"DBC": ".dbc",
"R_MARKDOWN": ".Rmd",
}
grantsPrivilegesToAdd = map[string][]string{
// TODO: think how to handle these if the TF user isn't owner of the metastore...
// "metastore": {`CREATE_CATALOG`, `CREATE_CONNECTION`, `CREATE_EXTERNAL_LOCATION`, `CREATE_PROVIDER`,
// `CREATE_RECIPIENT`, `CREATE_SCHEMA`, `CREATE_STORAGE_CREDENTIAL`, `SET_SHARE_PERMISSION`},
"catalog": {`CREATE_SCHEMA`},
"schema": {`CREATE_FUNCTION`, `CREATE_TABLE`, `CREATE_MODEL`, `CREATE_VOLUME`},
"volume": {`WRITE_VOLUME`},
"external_location": {`CREATE_EXTERNAL_TABLE`, `CREATE_EXTERNAL_VOLUME`, `CREATE_MANAGED_STORAGE`},
"storage_credential": {`CREATE_EXTERNAL_LOCATION`, `CREATE_EXTERNAL_TABLE`},
"foreign_connection": {`CREATE_FOREIGN_CATALOG`},
}
)

func generateMountBody(ic *importContext, body *hclwrite.Body, r *resource) error {
Expand Down Expand Up @@ -2351,15 +2362,12 @@ var resourcesMap map[string]importable = map[string]importable{
return nil
},
Import: func(ic *importContext, r *resource) error {
var cat tfuc.CatalogInfo
var cat tfcatalog.CatalogInfo
s := ic.Resources["databricks_catalog"].Schema
common.DataToStructPointer(r.Data, s, &cat)

// Emit: UC Connection, List schemas, Catalog grants, ...
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "catalog/" + cat.Name,
})
ic.emitUCGrantsWithOwner("catalog/"+cat.Name, r)
// TODO: emit owner? Should we do this? Because it's a account-level identity... Create a separate function for that...
if cat.ConnectionName != "" {
ic.Emit(&resource{
Expand Down Expand Up @@ -2436,10 +2444,7 @@ var resourcesMap map[string]importable = map[string]importable{
schemaFullName := r.ID
catalogName := r.Data.Get("catalog_name").(string)
schemaName := r.Data.Get("name").(string)
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "schema/" + schemaFullName,
})
ic.emitUCGrantsWithOwner("schema/"+schemaFullName, r)
ic.Emit(&resource{
Resource: "databricks_catalog",
ID: catalogName,
Expand Down Expand Up @@ -2510,10 +2515,8 @@ var resourcesMap map[string]importable = map[string]importable{
Service: "uc-volumes",
Import: func(ic *importContext, r *resource) error {
volumeFullName := r.ID
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "volume/" + volumeFullName,
})
ic.emitUCGrantsWithOwner("volume/"+volumeFullName, r)

catalogName := r.Data.Get("catalog_name").(string)
ic.Emit(&resource{
Resource: "databricks_schema",
Expand Down Expand Up @@ -2546,10 +2549,7 @@ var resourcesMap map[string]importable = map[string]importable{
Service: "uc-tables",
Import: func(ic *importContext, r *resource) error {
tableFullName := r.ID
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "table/" + tableFullName,
})
ic.emitUCGrantsWithOwner("table/"+tableFullName, r)
catalogName := r.Data.Get("catalog_name").(string)
ic.Emit(&resource{
Resource: "databricks_schema",
Expand Down Expand Up @@ -2583,6 +2583,49 @@ var resourcesMap map[string]importable = map[string]importable{
Service: "uc-grants",
// TODO: Should we try to make name unique?
// TODO: do we need to emit principals? Maybe only on account level? See comment for the owner...
Import: func(ic *importContext, r *resource) error {
if ic.meUserName == "" {
return nil
}
// https://docs.databricks.com/en/data-governance/unity-catalog/manage-privileges/privileges.html#privilege-types-by-securable-object-in-unity-catalog
var newPrivileges []string
for k, v := range grantsPrivilegesToAdd {
if r.Data.Get(k).(string) != "" {
newPrivileges = append(newPrivileges, v...)
break
}
}
if len(newPrivileges) == 0 {
return nil
}

owner, found := r.GetExtraData("owner")
if !found || owner == "" || owner == ic.meUserName {
// We don't need to change permissions if owner isn't set, or it's the same user
return nil
}

var pList tfcatalog.PermissionsList
s := ic.Resources["databricks_grants"].Schema
common.DataToStructPointer(r.Data, s, &pList)
foundExisting := false
for i, v := range pList.Assignments {
if v.Principal == ic.meUserName {
pList.Assignments[i].Privileges = append(pList.Assignments[i].Privileges, newPrivileges...)
slices.Sort(pList.Assignments[i].Privileges)
pList.Assignments[i].Privileges = slices.Compact(pList.Assignments[i].Privileges)
foundExisting = true
break
}
}
if !foundExisting {
pList.Assignments = append(pList.Assignments, tfcatalog.PrivilegeAssignment{
Principal: ic.meUserName,
Privileges: newPrivileges,
})
}
return common.StructToData(pList, s, r.Data)
},
Ignore: func(ic *importContext, r *resource) bool {
return r.Data.Get("grant.#").(int) == 0
},
Expand All @@ -2608,10 +2651,7 @@ var resourcesMap map[string]importable = map[string]importable{
AccountLevel: true,
Service: "uc-storage-credentials",
Import: func(ic *importContext, r *resource) error {
ic.Emit(&resource{
Resource: "databricks_grants",
ID: fmt.Sprintf("storage_credential/%s", r.ID),
})
ic.emitUCGrantsWithOwner("storage_credential/"+r.ID, r)
return nil
},
List: func(ic *importContext) error {
Expand Down Expand Up @@ -2650,10 +2690,7 @@ var resourcesMap map[string]importable = map[string]importable{
WorkspaceLevel: true,
Service: "uc-external-locations",
Import: func(ic *importContext, r *resource) error {
ic.Emit(&resource{
Resource: "databricks_grants",
ID: fmt.Sprintf("external_location/%s", r.ID),
})
ic.emitUCGrantsWithOwner("external_location/"+r.ID, r)
ic.Emit(&resource{
Resource: "databricks_storage_credential",
ID: r.Data.Get("credential_name").(string),
Expand Down Expand Up @@ -2712,10 +2749,7 @@ var resourcesMap map[string]importable = map[string]importable{
Import: func(ic *importContext, r *resource) error {
// TODO: do we need to emit the owner See comment for the owner...
connectionName := r.Data.Get("name").(string)
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "foreign_connection/" + connectionName,
})
ic.emitUCGrantsWithOwner("foreign_connection/"+connectionName, r)
return nil
},
ShouldOmitField: shouldOmitForUnityCatalog,
Expand All @@ -2738,14 +2772,11 @@ var resourcesMap map[string]importable = map[string]importable{
},
Import: func(ic *importContext, r *resource) error {
// TODO: do we need to emit the owner See comment for the owner...
var share tfuc.ShareInfo
var share tfcatalog.ShareInfo
s := ic.Resources["databricks_share"].Schema
common.DataToStructPointer(r.Data, s, &share)
// TODO: how to link recipients to share?
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "share/" + r.ID,
})
ic.emitUCGrantsWithOwner("share/"+r.ID, r)
for _, obj := range share.Objects {
switch obj.DataObjectType {
case "TABLE":
Expand Down Expand Up @@ -2819,10 +2850,7 @@ var resourcesMap map[string]importable = map[string]importable{
// },
Import: func(ic *importContext, r *resource) error {
modelFullName := r.ID
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "model/" + modelFullName,
})
ic.emitUCGrantsWithOwner("model/"+modelFullName, r)
catalogName := r.Data.Get("catalog_name").(string)
ic.Emit(&resource{
Resource: "databricks_schema",
Expand Down Expand Up @@ -2884,10 +2912,7 @@ var resourcesMap map[string]importable = map[string]importable{
return nil
},
Import: func(ic *importContext, r *resource) error {
ic.Emit(&resource{
Resource: "databricks_grants",
ID: "metastore/" + r.ID,
})
ic.emitUCGrantsWithOwner("metastore/"+r.ID, r)
// TODO: emit owner? See comment in catalog resource
if ic.accountLevel { // emit metastore assignments
assignments, err := ic.accountClient.MetastoreAssignments.ListByMetastoreId(ic.Context, r.ID)
Expand Down
66 changes: 66 additions & 0 deletions exporter/importables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"log"
"os"
"sort"
"sync"
"testing"

Expand Down Expand Up @@ -2165,3 +2166,68 @@ func TestImportUcVolumeFile(t *testing.T) {
assert.Equal(t, "main/default/wheels/some.whl_f27badf8", resourcesMap["databricks_file"].Name(nil, d))
})
}

func sortStringsCopy(s []string) []string {
c := make([]string, len(s))
copy(c, s)
sort.Strings(c)
return c
}

func TestImportGrants(t *testing.T) {
ic := importContextForTest()

s := ic.Resources["databricks_grants"].Schema
d := tfcatalog.ResourceGrants().ToResource().TestResourceData()
id := "metastore/1234"
d.SetId(id)
d.MarkNewResource()
r := &resource{ID: id, Data: d}
err := resourcesMap["databricks_grants"].Import(ic, r)
assert.NoError(t, err)

// Test ignore function
assert.True(t, resourcesMap["databricks_grants"].Ignore(ic, r))

var pList tfcatalog.PermissionsList
common.DataToStructPointer(r.Data, s, &pList)
assert.Empty(t, pList.Assignments)

// Test with a filled user name and no owner
ic.meUserName = "user@domain.com"
d.Set("catalog", "1234")
err = resourcesMap["databricks_grants"].Import(ic, r)
assert.NoError(t, err)
common.DataToStructPointer(r.Data, s, &pList)
require.Equal(t, 0, len(pList.Assignments))

// Test with a filled user name and permissions
r.AddExtraData("owner", "otheruser@domain.com")
err = resourcesMap["databricks_grants"].Import(ic, r)
assert.NoError(t, err)
common.DataToStructPointer(r.Data, s, &pList)
require.Equal(t, 1, len(pList.Assignments))
assert.Equal(t, ic.meUserName, pList.Assignments[0].Principal)
assert.Equal(t, sortStringsCopy(grantsPrivilegesToAdd["catalog"]), sortStringsCopy(pList.Assignments[0].Privileges))

// Test with a filled user name and permissions
d.Set("metastore", "")
d.Set("catalog", "test")
pList.Assignments = []tfcatalog.PrivilegeAssignment{
{Principal: ic.meUserName, Privileges: []string{"USE_CATALOG", "USE_SCHEMA"}},
}
common.StructToData(pList, s, d)
err = resourcesMap["databricks_grants"].Import(ic, r)
assert.NoError(t, err)
common.DataToStructPointer(r.Data, s, &pList)
require.Equal(t, 1, len(pList.Assignments))
assert.Equal(t, ic.meUserName, pList.Assignments[0].Principal)
assert.Equal(t, sortStringsCopy(append([]string{"USE_CATALOG", "USE_SCHEMA"}, grantsPrivilegesToAdd["catalog"]...)),
sortStringsCopy(pList.Assignments[0].Privileges))

// Test with a filled user name and unsupported objects
d.Set("catalog", "")
d.Set("model", "test")
err = resourcesMap["databricks_grants"].Import(ic, r)
assert.NoError(t, err)
}
17 changes: 17 additions & 0 deletions exporter/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,23 @@ type resource struct {
Incremental bool
// Actual Terraform data
Data *schema.ResourceData
// Arbitrary data to be used by importable
ExtraData map[string]any
}

func (r *resource) AddExtraData(key string, value any) {
if r.ExtraData == nil {
r.ExtraData = map[string]any{}
}
r.ExtraData[key] = value
}

func (r *resource) GetExtraData(key string) (any, bool) {
if r.ExtraData == nil {
return nil, false
}
v, ok := r.ExtraData[key]
return v, ok
}

func (r *resource) MatchPair() (string, string) {
Expand Down
11 changes: 11 additions & 0 deletions exporter/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,14 @@ func TestResourceApproaximationGet(t *testing.T) {
require.True(t, found)
assert.Equal(t, "42", v.(string))
}

func TestExtraDataGet(t *testing.T) {
r := &resource{}
_, found := r.GetExtraData("test")
assert.False(t, found)

r.AddExtraData("test", "42")
v, found := r.GetExtraData("test")
require.True(t, found)
assert.Equal(t, "42", v.(string))
}
13 changes: 13 additions & 0 deletions exporter/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1337,3 +1337,16 @@ func generateIgnoreObjectWithoutName(resourceType string) func(ic *importContext
return res
}
}

func (ic *importContext) emitUCGrantsWithOwner(id string, parentResource *resource) {
gr := &resource{
Resource: "databricks_grants",
ID: id,
}
if parentResource.Data != nil {
if owner, ok := parentResource.Data.GetOk("owner"); ok {
gr.AddExtraData("owner", owner)
}
}
ic.Emit(gr)
}

0 comments on commit 7352c6e

Please sign in to comment.