diff --git a/catalog/permissions/permissions.go b/catalog/permissions/permissions.go index efd1f962b2..c0c21b45e9 100644 --- a/catalog/permissions/permissions.go +++ b/catalog/permissions/permissions.go @@ -92,6 +92,7 @@ func (sm SecurableMapping) KeyValue(d attributeGetter) (string, string) { } return field, v } + log.Printf("[WARN] Unexpected resource or permissions. Please proceed at your own risk.") return "unknown", "unknown" } func (sm SecurableMapping) Id(d *schema.ResourceData) string { diff --git a/catalog/resource_grants.go b/catalog/resource_grants.go index 8834e609c9..a33fac2535 100644 --- a/catalog/resource_grants.go +++ b/catalog/resource_grants.go @@ -89,171 +89,6 @@ func replaceAllPermissions(a permissions.UnityCatalogPermissionsAPI, securable s }) } -type securableMapping map[string]map[string]bool - -// reuse ResourceDiff and ResourceData -type attributeGetter interface { - Get(key string) any -} - -func (sm securableMapping) kv(d attributeGetter) (string, string) { - for field := range sm { - v := d.Get(field).(string) - if v == "" { - continue - } - return field, v - } - return "unknown", "unknown" -} - -func (sm securableMapping) id(d *schema.ResourceData) string { - securable, name := sm.kv(d) - return fmt.Sprintf("%s/%s", securable, name) -} - -func (sm securableMapping) validate(d attributeGetter, pl PermissionsList) error { - securable, _ := sm.kv(d) - allowed, ok := sm[securable] - if !ok { - return fmt.Errorf(`%s is not fully supported yet`, securable) - } - for _, v := range pl.Assignments { - for _, priv := range v.Privileges { - if !allowed[strings.ToUpper(priv)] { - // check if user uses spaces instead of underscores - if allowed[strings.ReplaceAll(priv, " ", "_")] { - return fmt.Errorf(`%s is not allowed on %s. Did you mean %s?`, priv, securable, strings.ReplaceAll(priv, " ", "_")) - } - return fmt.Errorf(`%s is not allowed on %s`, priv, securable) - } - } - } - return nil -} - -var mapping = securableMapping{ - // add other securable mappings once needed - "table": { - "MODIFY": true, - "SELECT": true, - - // v1.0 - "ALL_PRIVILEGES": true, - "APPLY_TAG": true, - "BROWSE": true, - }, - "catalog": { - "CREATE": true, - "USAGE": true, - - // v1.0 - "ALL_PRIVILEGES": true, - "APPLY_TAG": true, - "USE_CATALOG": true, - "USE_SCHEMA": true, - "CREATE_SCHEMA": true, - "CREATE_TABLE": true, - "CREATE_FUNCTION": true, - "CREATE_MATERIALIZED_VIEW": true, - "CREATE_MODEL": true, - "CREATE_VOLUME": true, - "READ_VOLUME": true, - "WRITE_VOLUME": true, - "EXECUTE": true, - "MODIFY": true, - "SELECT": true, - "REFRESH": true, - "BROWSE": true, - }, - "schema": { - "CREATE": true, - "USAGE": true, - - // v1.0 - "ALL_PRIVILEGES": true, - "APPLY_TAG": true, - "USE_SCHEMA": true, - "CREATE_TABLE": true, - "CREATE_FUNCTION": true, - "CREATE_MATERIALIZED_VIEW": true, - "CREATE_MODEL": true, - "CREATE_VOLUME": true, - "READ_VOLUME": true, - "WRITE_VOLUME": true, - "EXECUTE": true, - "MODIFY": true, - "SELECT": true, - "REFRESH": true, - "BROWSE": true, - }, - "storage_credential": { - "CREATE_TABLE": true, - "READ_FILES": true, - "WRITE_FILES": true, - "CREATE_EXTERNAL_LOCATION": true, - - // v1.0 - "ALL_PRIVILEGES": true, - "CREATE_EXTERNAL_TABLE": true, - }, - "external_location": { - "CREATE_TABLE": true, - "READ_FILES": true, - "WRITE_FILES": true, - - // v1.0 - "ALL_PRIVILEGES": true, - "CREATE_EXTERNAL_TABLE": true, - "CREATE_MANAGED_STORAGE": true, - "CREATE_EXTERNAL_VOLUME": true, - "BROWSE": true, - }, - "metastore": { - // v1.0 - "CREATE_CATALOG": true, - "CREATE_CLEAN_ROOM": true, - "CREATE_CONNECTION": true, - "CREATE_EXTERNAL_LOCATION": true, - "CREATE_STORAGE_CREDENTIAL": true, - "CREATE_SHARE": true, - "CREATE_RECIPIENT": true, - "CREATE_PROVIDER": true, - "MANAGE_ALLOWLIST": true, - "USE_CONNECTION": true, - "USE_PROVIDER": true, - "USE_SHARE": true, - "USE_RECIPIENT": true, - "USE_MARKETPLACE_ASSETS": true, - "SET_SHARE_PERMISSION": true, - }, - "function": { - "ALL_PRIVILEGES": true, - "EXECUTE": true, - }, - "model": { - "ALL_PRIVILEGES": true, - "APPLY_TAG": true, - "EXECUTE": true, - }, - "share": { - "SELECT": true, - }, - "volume": { - "ALL_PRIVILEGES": true, - "READ_VOLUME": true, - "WRITE_VOLUME": true, - }, - // avoid reserved field - "foreign_connection": { - "ALL_PRIVILEGES": true, - "CREATE_FOREIGN_CATALOG": true, - "CREATE_FOREIGN_SCHEMA": true, - "CREATE_FOREIGN_TABLE": true, - "USE_CONNECTION": true, - }, -} - func (pl PermissionsList) toSdkPermissionsList() (out catalog.PermissionsList) { for _, v := range pl.Assignments { privileges := []catalog.Privilege{} @@ -294,7 +129,7 @@ func ResourceGrants() common.Resource { s := common.StructToSchema(PermissionsList{}, func(s map[string]*schema.Schema) map[string]*schema.Schema { alof := []string{} - for field := range mapping { + for field := range permissions.Mappings { s[field] = &schema.Schema{ Type: schema.TypeString, ForceNew: true, @@ -302,22 +137,13 @@ func ResourceGrants() common.Resource { } alof = append(alof, field) } - for field := range mapping { + for field := range permissions.Mappings { s[field].AtLeastOneOf = alof } return s }) return common.Resource{ Schema: s, - CustomizeDiff: func(ctx context.Context, d *schema.ResourceDiff) error { - if d.Id() == "" { - // unfortunately we cannot do validation before dependent resources exist with tfsdkv2 - return nil - } - var grants PermissionsList - common.DiffToStructPointer(d, s, &grants) - return mapping.validate(d, grants) - }, Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { w, err := c.WorkspaceClient() if err != nil { @@ -329,13 +155,13 @@ func ResourceGrants() common.Resource { } var grants PermissionsList common.DataToStructPointer(d, s, &grants) - securable, name := mapping.kv(d) + securable, name := permissions.Mappings.KeyValue(d) unityCatalogPermissionsAPI := permissions.NewUnityCatalogPermissionsAPI(ctx, c) err = replaceAllPermissions(unityCatalogPermissionsAPI, securable, name, grants.toSdkPermissionsList()) if err != nil { return err } - d.SetId(mapping.id(d)) + d.SetId(permissions.Mappings.Id(d)) return nil }, Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { diff --git a/catalog/resource_grants_test.go b/catalog/resource_grants_test.go index 01660863b0..81fa93f88f 100644 --- a/catalog/resource_grants_test.go +++ b/catalog/resource_grants_test.go @@ -358,31 +358,6 @@ func TestGrantReadMalformedId(t *testing.T) { }.ExpectError(t, "ID must be two elements split by `/`: foo.bar") } -type data map[string]string - -func (a data) Get(k string) any { - return a[k] -} - -func TestMappingUnsupported(t *testing.T) { - d := data{"nothing": "here"} - err := mapping.validate(d, PermissionsList{}) - assert.EqualError(t, err, "unknown is not fully supported yet") -} - -func TestInvalidPrivilege(t *testing.T) { - d := data{"table": "me"} - err := mapping.validate(d, PermissionsList{ - Assignments: []PrivilegeAssignment{ - { - Principal: "me", - Privileges: []string{"EVERYTHING"}, - }, - }, - }) - assert.EqualError(t, err, "EVERYTHING is not allowed on table") -} - func TestPermissionsList_Diff_ExternallyAddedPrincipal(t *testing.T) { diff := diffPermissions( catalog.PermissionsList{ // config @@ -600,30 +575,6 @@ func TestShareGrantUpdate(t *testing.T) { }.ApplyNoError(t) } -func TestPrivilegeWithSpace(t *testing.T) { - d := data{"table": "me"} - err := mapping.validate(d, PermissionsList{ - Assignments: []PrivilegeAssignment{ - { - Principal: "me", - Privileges: []string{"ALL PRIVILEGES"}, - }, - }, - }) - assert.EqualError(t, err, "ALL PRIVILEGES is not allowed on table. Did you mean ALL_PRIVILEGES?") - - d = data{"external_location": "me"} - err = mapping.validate(d, PermissionsList{ - Assignments: []PrivilegeAssignment{ - { - Principal: "me", - Privileges: []string{"CREATE TABLE"}, - }, - }, - }) - assert.EqualError(t, err, "CREATE TABLE is not allowed on external_location. Did you mean CREATE_TABLE?") -} - func TestConnectionGrantCreate(t *testing.T) { qa.ResourceFixture{ Fixtures: []qa.HTTPFixture{ diff --git a/internal/acceptance/grant_test.go b/internal/acceptance/grant_test.go index 08d328db38..19898e477d 100644 --- a/internal/acceptance/grant_test.go +++ b/internal/acceptance/grant_test.go @@ -1,6 +1,8 @@ package acceptance import ( + "fmt" + "regexp" "strings" "testing" ) @@ -99,8 +101,36 @@ resource "databricks_grant" "some" { func TestUcAccGrant(t *testing.T) { unityWorkspaceLevel(t, step{ Template: strings.ReplaceAll(grantTemplate, "%s", "{env.TEST_DATA_ENG_GROUP}"), - }, - step{ - Template: strings.ReplaceAll(grantTemplate, "%s", "{env.TEST_DATA_SCI_GROUP}"), - }) + }, step{ + Template: strings.ReplaceAll(grantTemplate, "%s", "{env.TEST_DATA_SCI_GROUP}"), + }) +} + +func grantTemplateForNamePermissionChange(suffix string, permission string) string { + return fmt.Sprintf(` + resource "databricks_storage_credential" "external" { + name = "cred-{var.STICKY_RANDOM}%s" + aws_iam_role { + role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}" + } + comment = "Managed by TF" + } + + resource "databricks_grant" "cred" { + storage_credential = databricks_storage_credential.external.id + principal = "{env.TEST_DATA_ENG_GROUP}" + privileges = ["%s"] + } + `, suffix, permission) +} + +func TestUcAccGrantForIdChange(t *testing.T) { + unityWorkspaceLevel(t, step{ + Template: grantTemplateForNamePermissionChange("-old", "ALL_PRIVILEGES"), + }, step{ + Template: grantTemplateForNamePermissionChange("-new", "ALL_PRIVILEGES"), + }, step{ + Template: grantTemplateForNamePermissionChange("-fail", "abc"), + ExpectError: regexp.MustCompile(`cannot create grant: Privilege abc is not applicable to this entity`), + }) } diff --git a/internal/acceptance/grants_test.go b/internal/acceptance/grants_test.go index 303e325f2d..23beb2fe3b 100644 --- a/internal/acceptance/grants_test.go +++ b/internal/acceptance/grants_test.go @@ -1,6 +1,8 @@ package acceptance import ( + "fmt" + "regexp" "strings" "testing" ) @@ -105,8 +107,38 @@ resource "databricks_grants" "some" { func TestUcAccGrants(t *testing.T) { unityWorkspaceLevel(t, step{ Template: strings.ReplaceAll(grantsTemplate, "%s", "{env.TEST_DATA_ENG_GROUP}"), - }, - step{ - Template: strings.ReplaceAll(grantsTemplate, "%s", "{env.TEST_DATA_SCI_GROUP}"), - }) + }, step{ + Template: strings.ReplaceAll(grantsTemplate, "%s", "{env.TEST_DATA_SCI_GROUP}"), + }) +} + +func grantsTemplateForNamePermissionChange(suffix string, permission string) string { + return fmt.Sprintf(` + resource "databricks_storage_credential" "external" { + name = "cred-{var.STICKY_RANDOM}%s" + aws_iam_role { + role_arn = "{env.TEST_METASTORE_DATA_ACCESS_ARN}" + } + comment = "Managed by TF" + } + + resource "databricks_grants" "cred" { + storage_credential = databricks_storage_credential.external.id + grant { + principal = "{env.TEST_DATA_ENG_GROUP}" + privileges = ["%s"] + } + } + `, suffix, permission) +} + +func TestUcAccGrantsForIdChange(t *testing.T) { + unityWorkspaceLevel(t, step{ + Template: grantsTemplateForNamePermissionChange("-old", "ALL_PRIVILEGES"), + }, step{ + Template: grantsTemplateForNamePermissionChange("-new", "ALL_PRIVILEGES"), + }, step{ + Template: grantsTemplateForNamePermissionChange("-fail", "abc"), + ExpectError: regexp.MustCompile(`Error: cannot create grants: Privilege abc is not applicable to this entity`), + }) }