Skip to content

Commit 996dd25

Browse files
alexottalexott
authored
[Exporter] Improve handling of owner for UC resources (#4669)
## Changes <!-- Summary of your changes that are easy to understand --> Changes include emitting of `owner` for UC resources and handling dependencies to corresponding users/SPS/groups. ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] using Go SDK - [ ] using TF Plugin Framework
1 parent 557785b commit 996dd25

File tree

4 files changed

+129
-20
lines changed

4 files changed

+129
-20
lines changed

‎NEXT_CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
* Correctly handle account-level identities when generating the code ([#4650](https://github.com/databricks/terraform-provider-databricks/pull/4650))
2323
* Add export of dashboard tasks in `datarbicks_job` ([#4665](https://github.com/databricks/terraform-provider-databricks/pull/4665))
2424
* Add export of PowerBI tasks in `datarbicks_job` ([#4668](https://github.com/databricks/terraform-provider-databricks/pull/4668))
25-
* Add `Ignore` implementation for `databricks_grants` to fix issue with wrongly generated dependencies ([#4661](https://github.com/databricks/terraform-provider-databricks/pull/4650))
25+
* Add `Ignore` implementation for `databricks_grants` to fix issue with wrongly generated dependencies ([#4650](https://github.com/databricks/terraform-provider-databricks/pull/4650))
26+
* Improve handling of `owner` for UC resources ([#4669](https://github.com/databricks/terraform-provider-databricks/pull/4669))
2627

2728
### Internal Changes

‎exporter/impl_uc.go‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"io"
66
"log"
7+
"regexp"
78
"strings"
89

910
"github.com/databricks/databricks-sdk-go/service/catalog"
@@ -35,6 +36,8 @@ func listUcCatalogs(ic *importContext) error {
3536
}, v.Name, v.UpdatedAt, fmt.Sprintf("catalog '%s'", v.Name))
3637
}
3738
default:
39+
// TODO: remove this skipping - we need to emit grants for all catalogs, but
40+
// we need to convert these catalog types to data sources
3841
log.Printf("[INFO] Skipping catalog %s of type %s", v.Name, v.CatalogType)
3942
}
4043
}
@@ -46,13 +49,13 @@ func importUcCatalog(ic *importContext, r *resource) error {
4649
s := ic.Resources["databricks_catalog"].Schema
4750
common.DataToStructPointer(r.Data, s, &cat)
4851

52+
// TODO: convert `main` catalog into the data source as it's automatically created?
4953
// Emit: UC Connection, List schemas, Catalog grants, ...
5054
owner, catalogGrantsResource := ic.emitUCGrantsWithOwner("catalog/"+cat.Name, r)
5155
dependsOn := []*resource{}
5256
if owner != "" && owner != ic.meUserName {
5357
dependsOn = append(dependsOn, catalogGrantsResource)
5458
}
55-
// TODO: emit owner? Should we do this? Because it's a account-level identity... Create a separate function for that...
5659
if cat.ConnectionName != "" {
5760
ic.Emit(&resource{
5861
Resource: "databricks_connection",
@@ -99,10 +102,6 @@ func importUcSchema(ic *importContext, r *resource) error {
99102
Resource: "databricks_catalog",
100103
ID: catalogName,
101104
})
102-
// r.AddDependsOn(&resource{Resource: "databricks_grants", ID: "catalog/" + catalogName})
103-
104-
// TODO: somehow add depends on catalog's grant...
105-
// TODO: emit owner? See comment in catalog resource
106105
if ic.isServiceInListing("uc-models") {
107106
it := ic.workspaceClient.RegisteredModels.List(ic.Context,
108107
catalog.ListRegisteredModelsRequest{
@@ -200,8 +199,6 @@ func importUcVolume(ic *importContext, r *resource) error {
200199
Resource: "databricks_schema",
201200
ID: schemaFullName,
202201
})
203-
// r.AddDependsOn(&resource{Resource: "databricks_grants", ID: "schema/" + schemaFullName})
204-
// TODO: emit owner? See comment in catalog resource
205202
return nil
206203
}
207204

@@ -334,12 +331,39 @@ func (ic *importContext) emitUCGrantsWithOwner(id string, parentResource *resour
334331
if ok {
335332
gr.AddExtraData("owner", ownerRaw)
336333
owner = ownerRaw.(string)
334+
emitUserSpOrGroup(ic, owner)
337335
}
338336
}
339337
ic.Emit(gr)
340338
return owner, gr
341339
}
342340

341+
var (
342+
emailDomainRegex = regexp.MustCompile(`^.*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+\.?$`)
343+
)
344+
345+
func emitUserSpOrGroup(ic *importContext, userOrSOrGroupPName string) {
346+
if common.StringIsUUID(userOrSOrGroupPName) {
347+
ic.Emit(&resource{
348+
Resource: "databricks_service_principal",
349+
Attribute: "application_id",
350+
Value: userOrSOrGroupPName,
351+
})
352+
} else if emailDomainRegex.MatchString(userOrSOrGroupPName) {
353+
ic.Emit(&resource{
354+
Resource: "databricks_user",
355+
Attribute: "user_name",
356+
Value: userOrSOrGroupPName,
357+
})
358+
} else {
359+
ic.Emit(&resource{
360+
Resource: "databricks_group",
361+
Attribute: "display_name",
362+
Value: userOrSOrGroupPName,
363+
})
364+
}
365+
}
366+
343367
func shouldOmitForUnityCatalog(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool {
344368
if pathString == "owner" {
345369
return d.Get(pathString).(string) == ""
@@ -539,7 +563,6 @@ func listUcMetastores(ic *importContext) error {
539563

540564
func importUcMetastores(ic *importContext, r *resource) error {
541565
ic.emitUCGrantsWithOwner("metastore/"+r.ID, r)
542-
// TODO: emit owner? See comment in catalog resource
543566
if ic.accountLevel {
544567
// emit metastore assignments
545568
assignments, err := ic.accountClient.MetastoreAssignments.ListByMetastoreId(ic.Context, r.ID)
@@ -656,7 +679,5 @@ func importSqlTable(ic *importContext, r *resource) error {
656679
Resource: "databricks_schema",
657680
ID: schemaFullName,
658681
})
659-
// r.AddDependsOn(&resource{Resource: "databricks_grants", ID: "schema/" + schemaFullName})
660-
// TODO: emit owner? See comment in catalog resource
661682
return nil
662683
}

‎exporter/impl_uc_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package exporter
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestEmitUserSpOrGroup(t *testing.T) {
10+
ic := importContextForTest()
11+
ic.enableServices("users,groups")
12+
emitUserSpOrGroup(ic, "[email protected]")
13+
assert.Equal(t, 1, len(ic.testEmits))
14+
assert.Contains(t, ic.testEmits, "databricks_user[<unknown>] (user_name: [email protected])")
15+
16+
emitUserSpOrGroup(ic, "users")
17+
assert.Equal(t, 2, len(ic.testEmits))
18+
assert.Contains(t, ic.testEmits, "databricks_group[<unknown>] (display_name: users)")
19+
20+
emitUserSpOrGroup(ic, "abcd1234-ab12-cd34-ef56-abcdef123456")
21+
assert.Equal(t, 3, len(ic.testEmits))
22+
assert.Contains(t, ic.testEmits, "databricks_service_principal[<unknown>] (application_id: abcd1234-ab12-cd34-ef56-abcdef123456)")
23+
24+
emitUserSpOrGroup(ic, "users @ test.com")
25+
assert.Equal(t, 4, len(ic.testEmits))
26+
assert.Contains(t, ic.testEmits, "databricks_group[<unknown>] (display_name: users @ test.com)")
27+
28+
}

‎exporter/importables.go‎

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,13 +1796,20 @@ var resourcesMap map[string]importable = map[string]importable{
17961796
}
17971797
return shouldOmitForUnityCatalog(ic, pathString, as, d)
17981798
},
1799-
Ignore: generateIgnoreObjectWithEmptyAttributeValue("databricks_catalog", "name"),
1799+
Ignore: func(ic *importContext, r *resource) bool {
1800+
res := (r.Data != nil && (r.Data.Get("name").(string) == "" || r.Data.Get("name").(string) == "system"))
1801+
if res {
1802+
ic.addIgnoredResource(fmt.Sprintf("databricks_catalog. id=%s", r.ID))
1803+
}
1804+
return res
1805+
},
18001806
Depends: []reference{
18011807
{Path: "connection_name", Resource: "databricks_connection", Match: "name"},
18021808
{Path: "storage_root", Resource: "databricks_external_location", Match: "url", MatchType: MatchLongestPrefix},
1809+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1810+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1811+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
18031812
},
1804-
// TODO: convert `main` catalog into the data source as it's automatically created?
1805-
// This will require addition of the databricks_catalog data source
18061813
},
18071814
"databricks_schema": {
18081815
WorkspaceLevel: true,
@@ -1813,6 +1820,9 @@ var resourcesMap map[string]importable = map[string]importable{
18131820
Depends: []reference{
18141821
{Path: "catalog_name", Resource: "databricks_catalog"},
18151822
{Path: "storage_root", Resource: "databricks_external_location", Match: "url", MatchType: MatchLongestPrefix},
1823+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1824+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1825+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
18161826
},
18171827
},
18181828
"databricks_volume": {
@@ -1833,6 +1843,9 @@ var resourcesMap map[string]importable = map[string]importable{
18331843
SkipDirectLookup: true},
18341844
{Path: "storage_location", Resource: "databricks_external_location",
18351845
Match: "url", MatchType: MatchLongestPrefix},
1846+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1847+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1848+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
18361849
},
18371850
},
18381851
"databricks_sql_table": {
@@ -1857,6 +1870,9 @@ var resourcesMap map[string]importable = map[string]importable{
18571870
SkipDirectLookup: true},
18581871
{Path: "storage_location", Resource: "databricks_external_location",
18591872
Match: "url", MatchType: MatchLongestPrefix},
1873+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1874+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1875+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
18601876
},
18611877
},
18621878
"databricks_grants": {
@@ -1893,6 +1909,9 @@ var resourcesMap map[string]importable = map[string]importable{
18931909
ShouldOmitField: shouldOmitWithIsolationMode,
18941910
Depends: []reference{
18951911
{Path: "azure_service_principal.client_secret", Variable: true},
1912+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1913+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1914+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
18961915
},
18971916
},
18981917
"databricks_credential": {
@@ -1903,6 +1922,9 @@ var resourcesMap map[string]importable = map[string]importable{
19031922
ShouldOmitField: shouldOmitWithIsolationMode,
19041923
Depends: []reference{
19051924
{Path: "azure_service_principal.client_secret", Variable: true},
1925+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1926+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1927+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
19061928
},
19071929
},
19081930
"databricks_external_location": {
@@ -1925,6 +1947,9 @@ var resourcesMap map[string]importable = map[string]importable{
19251947
},
19261948
Depends: []reference{
19271949
{Path: "credential_name", Resource: "databricks_storage_credential", Match: "name"},
1950+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1951+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1952+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
19281953
},
19291954
},
19301955
"databricks_connection": {
@@ -1941,12 +1966,25 @@ var resourcesMap map[string]importable = map[string]importable{
19411966
List: listUcConnections,
19421967
// TODO: think what to do with the sensitive fields in the `options`?
19431968
Import: func(ic *importContext, r *resource) error {
1944-
// TODO: do we need to emit the owner See comment for the owner...
19451969
connectionName := r.Data.Get("name").(string)
19461970
ic.emitUCGrantsWithOwner("foreign_connection/"+connectionName, r)
19471971
return nil
19481972
},
1973+
Ignore: func(ic *importContext, r *resource) bool {
1974+
res := (r.Data.Get("connection_type").(string) == "ONLINE_CATALOG" &&
1975+
strings.HasPrefix(r.Data.Get("name").(string), "internal-") &&
1976+
r.Data.Get("owner").(string) == "System user")
1977+
if res {
1978+
ic.addIgnoredResource(fmt.Sprintf("databricks_connection. id=%s", r.ID))
1979+
}
1980+
return res
1981+
},
19491982
ShouldOmitField: shouldOmitForUnityCatalog,
1983+
Depends: []reference{
1984+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
1985+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
1986+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
1987+
},
19501988
},
19511989
"databricks_share": {
19521990
WorkspaceLevel: true,
@@ -1966,7 +2004,6 @@ var resourcesMap map[string]importable = map[string]importable{
19662004
return nil
19672005
},
19682006
Import: func(ic *importContext, r *resource) error {
1969-
// TODO: do we need to emit the owner See comment for the owner...
19702007
var share tf_sharing.ShareInfo
19712008
s := ic.Resources["databricks_share"].Schema
19722009
common.DataToStructPointer(r.Data, s, &share)
@@ -2003,6 +2040,9 @@ var resourcesMap map[string]importable = map[string]importable{
20032040
{Path: "object.name", Resource: "databricks_registered_model", IsValidApproximation: isMatchignShareObject("MODEL")},
20042041
{Path: "object.name", Resource: "databricks_schema", IsValidApproximation: isMatchignShareObject("SCHEMA")},
20052042
{Path: "object.name", Resource: "databricks_sql_table", IsValidApproximation: isMatchignShareObject("TABLE")},
2043+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
2044+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
2045+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
20062046
},
20072047
},
20082048
"databricks_recipient": {
@@ -2022,7 +2062,18 @@ var resourcesMap map[string]importable = map[string]importable{
20222062
}
20232063
return nil
20242064
},
2025-
// TODO: do we need to emit the owner See comment for the owner...
2065+
Import: func(ic *importContext, r *resource) error {
2066+
owner := r.Data.Get("owner").(string)
2067+
if owner != "" {
2068+
emitUserSpOrGroup(ic, owner)
2069+
}
2070+
return nil
2071+
},
2072+
Depends: []reference{
2073+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
2074+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
2075+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
2076+
},
20262077
// TODO: emit variable for sharing_code ...
20272078
// TODO: add depends for sharing_code?
20282079
},
@@ -2052,8 +2103,6 @@ var resourcesMap map[string]importable = map[string]importable{
20522103
Resource: "databricks_schema",
20532104
ID: schemaFullName,
20542105
})
2055-
// r.AddDependsOn(&resource{Resource: "databricks_grants", ID: "schema/" + schemaFullName})
2056-
// TODO: emit owner? See comment in catalog resource
20572106
return nil
20582107
},
20592108
ShouldOmitField: func(ic *importContext, pathString string, as *schema.Schema, d *schema.ResourceData) bool {
@@ -2073,6 +2122,9 @@ var resourcesMap map[string]importable = map[string]importable{
20732122
IsValidApproximation: createIsMatchingCatalogAndSchema("catalog_name", "schema_name"),
20742123
SkipDirectLookup: true},
20752124
{Path: "storage_root", Resource: "databricks_external_location", Match: "url", MatchType: MatchLongestPrefix},
2125+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
2126+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
2127+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
20762128
},
20772129
},
20782130
"databricks_metastore": {
@@ -2088,6 +2140,11 @@ var resourcesMap map[string]importable = map[string]importable{
20882140
}
20892141
return shouldOmitForUnityCatalog(ic, pathString, as, d)
20902142
},
2143+
Depends: []reference{
2144+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
2145+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
2146+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
2147+
},
20912148
},
20922149
"databricks_metastore_assignment": {
20932150
AccountLevel: true,
@@ -2343,7 +2400,6 @@ var resourcesMap map[string]importable = map[string]importable{
23432400
Resource: "databricks_sql_table",
23442401
ID: r.Data.Get("spec.0.source_table_full_name").(string),
23452402
})
2346-
// TODO: emit owner? See comment in catalog resource
23472403
return nil
23482404
},
23492405
Ignore: generateIgnoreObjectWithEmptyAttributeValue("databricks_online_table", "name"),
@@ -2354,6 +2410,9 @@ var resourcesMap map[string]importable = map[string]importable{
23542410
IsValidApproximation: createIsMatchingCatalogAndSchema("catalog_name", "schema_name"),
23552411
SkipDirectLookup: true},
23562412
{Path: "spec.source_table_full_name", Resource: "databricks_sql_table"},
2413+
{Path: "owner", Resource: "databricks_service_principal", Match: "application_id"},
2414+
{Path: "owner", Resource: "databricks_group", Match: "display_name"},
2415+
{Path: "owner", Resource: "databricks_user", Match: "user_name", MatchType: MatchCaseInsensitive},
23572416
},
23582417
},
23592418
"databricks_vector_search_endpoint": {

0 commit comments

Comments
 (0)