Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed updating owners for UC resources #3189

Merged
merged 17 commits into from
Feb 2, 2024
4 changes: 4 additions & 0 deletions catalog/resource_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ func ResourceCatalog() common.Resource {
}
}

if !d.HasChangeExcept("owner") {
return nil
}

updateCatalogRequest.Owner = ""
ci, err := w.Catalogs.Update(ctx, updateCatalogRequest)

Expand Down
4 changes: 4 additions & 0 deletions catalog/resource_external_location.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ func ResourceExternalLocation() common.Resource {
}
}

if !d.HasChangeExcept("owner") {
return nil
}

updateExternalLocationRequest.Owner = ""
_, err = w.ExternalLocations.Update(ctx, updateExternalLocationRequest)
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions catalog/resource_metastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ func ResourceMetastore() common.Resource {
return err
}
}

if !d.HasChangeExcept("owner") {
return nil
}

update.Owner = ""
_, err := acc.Metastores.Update(ctx, catalog.AccountsUpdateMetastore{
MetastoreId: d.Id(),
Expand Down Expand Up @@ -171,6 +176,11 @@ func ResourceMetastore() common.Resource {
return err
}
}

if !d.HasChangeExcept("owner") {
return nil
}

update.Owner = ""
_, err := w.Metastores.Update(ctx, update)
if err != nil {
Expand Down
93 changes: 44 additions & 49 deletions catalog/resource_metastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,42 @@ func TestUpdateMetastore_NoChanges(t *testing.T) {
StorageRoot: "s3://b/abc",
Name: "abc",
}, nil)
},
Resource: ResourceMetastore(),
ID: "abc",
Update: true,
RequiresNew: true,
InstanceState: map[string]string{
"name": "abc",
"storage_root": "s3:/a",
"owner": "admin",
"delta_sharing_scope": "INTERNAL_AND_EXTERNAL",
"delta_sharing_recipient_token_lifetime_in_seconds": "1002",
},
HCL: `
name = "abc"
storage_root = "s3:/a"
owner = "admin"
delta_sharing_scope = "INTERNAL_AND_EXTERNAL"
delta_sharing_recipient_token_lifetime_in_seconds = 1002
`,
}.ApplyNoError(t)
}

func TestUpdateMetastore_OnlyOwnerChanges(t *testing.T) {
qa.ResourceFixture{
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
e := w.GetMockMetastoresAPI().EXPECT()
e.Update(mock.Anything, catalog.UpdateMetastore{
Id: "abc",
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"},
Id: "abc",
Owner: "updatedOwner",
}).Return(&catalog.MetastoreInfo{
Name: "a",
Name: "abc",
Owner: "updatedOwner",
}, nil)
e.GetById(mock.Anything, "abc").Return(&catalog.MetastoreInfo{
Name: "abc",
Owner: "updatedOwner",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
}, nil)
Expand All @@ -198,14 +226,14 @@ func TestUpdateMetastore_NoChanges(t *testing.T) {
HCL: `
name = "abc"
storage_root = "s3:/a"
owner = "admin"
owner = "updatedOwner"
delta_sharing_scope = "INTERNAL_AND_EXTERNAL"
delta_sharing_recipient_token_lifetime_in_seconds = 1002
`,
}.ApplyNoError(t)
}

func TestUpdateMetastore_OwnerChanges(t *testing.T) {
func TestUpdateMetastore_OwnerAndOtherChanges(t *testing.T) {
qa.ResourceFixture{
MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) {
e := w.GetMockMetastoresAPI().EXPECT()
Expand All @@ -220,19 +248,18 @@ func TestUpdateMetastore_OwnerChanges(t *testing.T) {
Id: "abc",
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
DeltaSharingRecipientTokenLifetimeInSeconds: 1004,
ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"},
}).Return(&catalog.MetastoreInfo{
Name: "a",
Owner: "updatedOwner",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
DeltaSharingRecipientTokenLifetimeInSeconds: 1004,
}, nil)
e.GetById(mock.Anything, "abc").Return(&catalog.MetastoreInfo{
Name: "abc",
Owner: "updatedOwner",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
DeltaSharingRecipientTokenLifetimeInSeconds: 1004,
}, nil)
},
Resource: ResourceMetastore(),
Expand All @@ -251,7 +278,7 @@ func TestUpdateMetastore_OwnerChanges(t *testing.T) {
storage_root = "s3:/a"
owner = "updatedOwner"
delta_sharing_scope = "INTERNAL_AND_EXTERNAL"
delta_sharing_recipient_token_lifetime_in_seconds = 1002
delta_sharing_recipient_token_lifetime_in_seconds = 1004
`,
}.ApplyNoError(t)
}
Expand All @@ -271,7 +298,7 @@ func TestUpdateMetastore_Rollback(t *testing.T) {
Id: "abc",
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
DeltaSharingRecipientTokenLifetimeInSeconds: 1004,
ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"},
}).Return(nil, errors.New("Something unexpected happened"))
e.Update(mock.Anything, catalog.UpdateMetastore{
Expand All @@ -298,7 +325,7 @@ func TestUpdateMetastore_Rollback(t *testing.T) {
storage_root = "s3:/a"
owner = "updatedOwner"
delta_sharing_scope = "INTERNAL_AND_EXTERNAL"
delta_sharing_recipient_token_lifetime_in_seconds = 1002
delta_sharing_recipient_token_lifetime_in_seconds = 1004
`,
}.Apply(t)
qa.AssertErrorStartsWith(t, err, "Something unexpected happened")
Expand Down Expand Up @@ -507,22 +534,6 @@ func TestUpdateAccountMetastore_NoChanges(t *testing.T) {
qa.ResourceFixture{
MockAccountClientFunc: func(a *mocks.MockAccountClient) {
e := a.GetMockAccountMetastoresAPI().EXPECT()
e.Update(mock.Anything, catalog.AccountsUpdateMetastore{
MetastoreId: "abc",
MetastoreInfo: &catalog.UpdateMetastore{
Id: "abc",
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"},
},
}).Return(&catalog.AccountsMetastoreInfo{
MetastoreInfo: &catalog.MetastoreInfo{
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
},
}, nil)
e.GetByMetastoreId(mock.Anything, "abc").Return(&catalog.AccountsMetastoreInfo{
MetastoreInfo: &catalog.MetastoreInfo{
StorageRoot: "s3://b/abc",
Expand Down Expand Up @@ -569,22 +580,6 @@ func TestUpdateAccountMetastore_OwnerChanges(t *testing.T) {
Owner: "updatedOwner",
},
}, nil)
e.Update(mock.Anything, catalog.AccountsUpdateMetastore{
MetastoreId: "abc",
MetastoreInfo: &catalog.UpdateMetastore{
Id: "abc",
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"},
},
}).Return(&catalog.AccountsMetastoreInfo{
MetastoreInfo: &catalog.MetastoreInfo{
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
},
}, nil)
e.GetByMetastoreId(mock.Anything, "abc").Return(&catalog.AccountsMetastoreInfo{
MetastoreInfo: &catalog.MetastoreInfo{
StorageRoot: "s3://b/abc",
Expand Down Expand Up @@ -637,7 +632,7 @@ func TestUpdateAccountMetastore_Rollback(t *testing.T) {
Id: "abc",
Name: "abc",
DeltaSharingScope: "INTERNAL_AND_EXTERNAL",
DeltaSharingRecipientTokenLifetimeInSeconds: 1002,
DeltaSharingRecipientTokenLifetimeInSeconds: 1004,
ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"},
},
}).Return(nil, errors.New("Something unexpected happened"))
Expand Down Expand Up @@ -671,7 +666,7 @@ func TestUpdateAccountMetastore_Rollback(t *testing.T) {
storage_root = "s3:/a"
owner = "updatedOwner"
delta_sharing_scope = "INTERNAL_AND_EXTERNAL"
delta_sharing_recipient_token_lifetime_in_seconds = 1002
delta_sharing_recipient_token_lifetime_in_seconds = 1004
`,
}.Apply(t)
qa.AssertErrorStartsWith(t, err, "Something unexpected happened")
Expand Down
4 changes: 4 additions & 0 deletions catalog/resource_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ func ResourceSchema() common.Resource {
}
}

if !d.HasChangeExcept("owner") {
return nil
}

updateSchemaRequest.Owner = ""
schema, err := w.Schemas.Update(ctx, updateSchemaRequest)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions catalog/resource_share.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,10 @@ func ResourceShare() common.Resource {
}
}

if !d.HasChangeExcept("owner") {
return nil
}

err = NewSharesAPI(ctx, c).update(d.Id(), ShareUpdates{
Updates: changes,
})
Expand Down
10 changes: 10 additions & 0 deletions catalog/resource_storage_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ func ResourceStorageCredential() common.Resource {
return err
}
}

if !d.HasChangeExcept("owner") {
return nil
}

update.Owner = ""
_, err := acc.StorageCredentials.Update(ctx, catalog.AccountsUpdateStorageCredential{
CredentialInfo: &update,
Expand Down Expand Up @@ -191,6 +196,11 @@ func ResourceStorageCredential() common.Resource {
return err
}
}

if !d.HasChangeExcept("owner") {
return nil
}

update.Owner = ""
_, err = w.StorageCredentials.Update(ctx, update)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions catalog/resource_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ func ResourceVolume() common.Resource {
}
}

if !d.HasChangeExcept("owner") {
return nil
}

updateVolumeRequestContent.Owner = ""
v, err := w.Volumes.Update(ctx, updateVolumeRequestContent)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,10 @@ Except for metastore, metastore assignment and storage credential objects, Unity

If you are configuring a new Databricks account for the first time, please create at least one workspace with an identity (user or service principal) that you intend to use for Unity Catalog rollout. You can then configure the provider using that identity and workspace to provision the required Unity Catalog resources.

## Special considerations for Unity Catalog Resources

When performing a single Terraform apply to update both the owner and other fields for Unity Catalog resources, the process first updates the owner, followed by the other fields using the new owner's permissions. If your principal is not the owner (specifically, the newly updated owner), you will not have the authority to modify those fields. In cases where you wish to change the owner to another individual and also update other fields, we recommend initially updating the fields using your principal, which should have owner permissions, and then updating the owner in a separate step.

## Miscellaneous configuration parameters

!> **Warning** Combination of `debug_headers` and `debug_truncate_bytes` results in dumping of sensitive information to logs. Use it for troubleshooting purposes only.
Expand Down
10 changes: 10 additions & 0 deletions internal/acceptance/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ func TestUcAccCatalogUpdate(t *testing.T) {
}
owner = "account users"
}`,
}, step{
Template: `
resource "databricks_catalog" "sandbox" {
name = "sandbox{var.STICKY_RANDOM}"
comment = "this catalog is managed by terraform"
properties = {
purpose = "testing"
}
owner = "{env.TEST_DATA_ENG_GROUP}"
}`,
}, step{
Template: `
resource "databricks_catalog" "sandbox" {
Expand Down
4 changes: 4 additions & 0 deletions internal/acceptance/external_location_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ func TestUcAccExternalLocationUpdate(t *testing.T) {
Template: storageCredentialTemplateWithOwner("Managed by TF -- Updated Comment", "account users") +
externalLocationTemplateWithOwner("Managed by TF -- Updated Comment", "account users") +
grantsTemplateForExternalLocation,
}, step{
Template: storageCredentialTemplateWithOwner("Managed by TF -- Updated Comment", "{env.TEST_DATA_ENG_GROUP}") +
externalLocationTemplateWithOwner("Managed by TF -- Updated Comment", "{env.TEST_DATA_ENG_GROUP}") +
grantsTemplateForExternalLocation,
}, step{
Template: storageCredentialTemplateWithOwner("Managed by TF -- Updated Comment 2", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}") +
externalLocationTemplateWithOwner("Managed by TF -- Updated Comment 2", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}") +
Expand Down
7 changes: 7 additions & 0 deletions internal/acceptance/metastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ func runMetastoreTestWithOwnerUpdates(t *testing.T, extraAttributes map[string]a
owner = "account users"
%s
}`, template),
}, step{
Template: fmt.Sprintf(`resource "databricks_metastore" "this" {
name = "{var.STICKY_RANDOM}"
force_destroy = true
owner = "{env.TEST_DATA_ENG_GROUP}"
%s
}`, template),
}, step{
Template: fmt.Sprintf(`resource "databricks_metastore" "this" {
name = "{var.STICKY_RANDOM}-updated"
Expand Down
2 changes: 2 additions & 0 deletions internal/acceptance/recipient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ func TestUcAccUpdateRecipientDb2Open(t *testing.T) {
Template: recipientTemplateWithOwner("made by terraform", "account users"),
}, step{
Template: recipientTemplateWithOwner("made by terraform -- updated comment", "account users"),
}, step{
Template: recipientTemplateWithOwner("made by terraform -- updated comment", "{env.TEST_DATA_ENG_GROUP}"),
}, step{
Template: recipientTemplateWithOwner("made by terraform -- updated comment 2", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}"),
})
Expand Down
2 changes: 2 additions & 0 deletions internal/acceptance/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ func TestUcAccSchemaUpdate(t *testing.T) {
Template: catalogTemplate + schemaTemplateWithOwner("this database is managed by terraform", "account users"),
}, step{
Template: catalogTemplate + schemaTemplateWithOwner("this database is managed by terraform -- updated comment", "account users"),
}, step{
Template: catalogTemplate + schemaTemplateWithOwner("this database is managed by terraform -- updated comment", "{env.TEST_DATA_ENG_GROUP}"),
}, step{
Template: catalogTemplate + schemaTemplateWithOwner("this database is managed by terraform -- updated comment 2", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}"),
})
Expand Down
2 changes: 2 additions & 0 deletions internal/acceptance/share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ func TestUcAccUpdateShare(t *testing.T) {
Template: preTestTemplate + preTestTemplateUpdate + shareTemplateWithOwner("c", "account users"),
}, step{
Template: preTestTemplate + preTestTemplateUpdate + shareTemplateWithOwner("e", "account users"),
}, step{
Template: preTestTemplate + preTestTemplateUpdate + shareTemplateWithOwner("e", "{env.TEST_DATA_ENG_GROUP}"),
}, step{
Template: preTestTemplate + preTestTemplateUpdate + shareTemplateWithOwner("f", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}"),
})
Expand Down
Loading
Loading