From 7932c786daf27aeffd7eefd7b90e3123c315d9fe Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Wed, 31 Jan 2024 17:11:57 +0100 Subject: [PATCH 01/14] Fix updating owners --- catalog/resource_catalog.go | 4 ++++ catalog/resource_external_location.go | 4 ++++ catalog/resource_metastore.go | 10 ++++++++++ catalog/resource_schema.go | 4 ++++ catalog/resource_share.go | 4 ++++ catalog/resource_storage_credential.go | 5 +++++ catalog/resource_volume.go | 4 ++++ sharing/resource_recipient.go | 4 ++++ 8 files changed, 39 insertions(+) diff --git a/catalog/resource_catalog.go b/catalog/resource_catalog.go index b7affc2db2..d61f10bf6d 100644 --- a/catalog/resource_catalog.go +++ b/catalog/resource_catalog.go @@ -148,6 +148,10 @@ func ResourceCatalog() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + updateCatalogRequest.Owner = "" ci, err := w.Catalogs.Update(ctx, updateCatalogRequest) diff --git a/catalog/resource_external_location.go b/catalog/resource_external_location.go index dcaf85a674..6b14f1c62f 100644 --- a/catalog/resource_external_location.go +++ b/catalog/resource_external_location.go @@ -108,6 +108,10 @@ func ResourceExternalLocation() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + updateExternalLocationRequest.Owner = "" _, err = w.ExternalLocations.Update(ctx, updateExternalLocationRequest) if err != nil { diff --git a/catalog/resource_metastore.go b/catalog/resource_metastore.go index c45dfee673..541dace596 100644 --- a/catalog/resource_metastore.go +++ b/catalog/resource_metastore.go @@ -138,6 +138,11 @@ func ResourceMetastore() *schema.Resource { return err } } + + if !d.HasChangeExcept("owner") { + return nil + } + update.Owner = "" _, err := acc.Metastores.Update(ctx, catalog.AccountsUpdateMetastore{ MetastoreId: d.Id(), @@ -171,6 +176,11 @@ func ResourceMetastore() *schema.Resource { return err } } + + if !d.HasChangeExcept("owner") { + return nil + } + update.Owner = "" _, err := w.Metastores.Update(ctx, update) if err != nil { diff --git a/catalog/resource_schema.go b/catalog/resource_schema.go index f3cf93832c..4952f8688d 100644 --- a/catalog/resource_schema.go +++ b/catalog/resource_schema.go @@ -99,6 +99,10 @@ func ResourceSchema() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + updateSchemaRequest.Owner = "" schema, err := w.Schemas.Update(ctx, updateSchemaRequest) if err != nil { diff --git a/catalog/resource_share.go b/catalog/resource_share.go index 8cd209a919..f3873c753d 100644 --- a/catalog/resource_share.go +++ b/catalog/resource_share.go @@ -237,6 +237,10 @@ func ResourceShare() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + err = NewSharesAPI(ctx, c).update(d.Id(), ShareUpdates{ Updates: changes, }) diff --git a/catalog/resource_storage_credential.go b/catalog/resource_storage_credential.go index 02c71e1f5f..a4f94baec6 100644 --- a/catalog/resource_storage_credential.go +++ b/catalog/resource_storage_credential.go @@ -152,6 +152,11 @@ func ResourceStorageCredential() *schema.Resource { return err } } + + if !d.HasChangeExcept("owner") { + return nil + } + update.Owner = "" _, err := acc.StorageCredentials.Update(ctx, catalog.AccountsUpdateStorageCredential{ CredentialInfo: &update, diff --git a/catalog/resource_volume.go b/catalog/resource_volume.go index f4b6871684..4c13e8c201 100644 --- a/catalog/resource_volume.go +++ b/catalog/resource_volume.go @@ -93,6 +93,10 @@ func ResourceVolume() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + updateVolumeRequestContent.Owner = "" v, err := w.Volumes.Update(ctx, updateVolumeRequestContent) if err != nil { diff --git a/sharing/resource_recipient.go b/sharing/resource_recipient.go index dce799d533..640e18bb74 100644 --- a/sharing/resource_recipient.go +++ b/sharing/resource_recipient.go @@ -85,6 +85,10 @@ func ResourceRecipient() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + updateRecipientRequest.Owner = "" err = w.Recipients.Update(ctx, updateRecipientRequest) if err != nil { From 4e5e8af5bcda57408f3c23cc274ca2ac0a9348f0 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Wed, 31 Jan 2024 17:35:55 +0100 Subject: [PATCH 02/14] fix tests --- catalog/resource_metastore_test.go | 63 ++---------------------------- 1 file changed, 4 insertions(+), 59 deletions(-) diff --git a/catalog/resource_metastore_test.go b/catalog/resource_metastore_test.go index 0a4e1940cf..7eb998b48e 100644 --- a/catalog/resource_metastore_test.go +++ b/catalog/resource_metastore_test.go @@ -172,17 +172,6 @@ func TestUpdateMetastore_NoChanges(t *testing.T) { StorageRoot: "s3://b/abc", Name: "abc", }, nil) - e.Update(mock.Anything, catalog.UpdateMetastore{ - Id: "abc", - Name: "abc", - DeltaSharingScope: "INTERNAL_AND_EXTERNAL", - DeltaSharingRecipientTokenLifetimeInSeconds: 1002, - ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, - }).Return(&catalog.MetastoreInfo{ - Name: "a", - DeltaSharingScope: "INTERNAL_AND_EXTERNAL", - DeltaSharingRecipientTokenLifetimeInSeconds: 1002, - }, nil) }, Resource: ResourceMetastore(), ID: "abc", @@ -216,18 +205,6 @@ func TestUpdateMetastore_OwnerChanges(t *testing.T) { Name: "abc", Owner: "updatedOwner", }, nil) - e.Update(mock.Anything, catalog.UpdateMetastore{ - Id: "abc", - Name: "abc", - DeltaSharingScope: "INTERNAL_AND_EXTERNAL", - DeltaSharingRecipientTokenLifetimeInSeconds: 1002, - ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, - }).Return(&catalog.MetastoreInfo{ - Name: "a", - Owner: "updatedOwner", - DeltaSharingScope: "INTERNAL_AND_EXTERNAL", - DeltaSharingRecipientTokenLifetimeInSeconds: 1002, - }, nil) e.GetById(mock.Anything, "abc").Return(&catalog.MetastoreInfo{ Name: "abc", Owner: "updatedOwner", @@ -269,7 +246,7 @@ func TestUpdateMetastore_Rollback(t *testing.T) { }, nil) e.Update(mock.Anything, catalog.UpdateMetastore{ Id: "abc", - Name: "abc", + Name: "abcd", DeltaSharingScope: "INTERNAL_AND_EXTERNAL", DeltaSharingRecipientTokenLifetimeInSeconds: 1002, ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, @@ -294,7 +271,7 @@ func TestUpdateMetastore_Rollback(t *testing.T) { "delta_sharing_recipient_token_lifetime_in_seconds": "1002", }, HCL: ` - name = "abc" + name = "abcd" storage_root = "s3:/a" owner = "updatedOwner" delta_sharing_scope = "INTERNAL_AND_EXTERNAL" @@ -507,22 +484,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", @@ -569,22 +530,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", @@ -635,7 +580,7 @@ func TestUpdateAccountMetastore_Rollback(t *testing.T) { MetastoreId: "abc", MetastoreInfo: &catalog.UpdateMetastore{ Id: "abc", - Name: "abc", + Name: "abcd", DeltaSharingScope: "INTERNAL_AND_EXTERNAL", DeltaSharingRecipientTokenLifetimeInSeconds: 1002, ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, @@ -667,7 +612,7 @@ func TestUpdateAccountMetastore_Rollback(t *testing.T) { "delta_sharing_recipient_token_lifetime_in_seconds": "1002", }, HCL: ` - name = "abc" + name = "abcd" storage_root = "s3:/a" owner = "updatedOwner" delta_sharing_scope = "INTERNAL_AND_EXTERNAL" From 08369168c2d256579a683e7a655ab907af16fe70 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Thu, 1 Feb 2024 15:04:11 +0100 Subject: [PATCH 03/14] - --- catalog/resource_metastore_test.go | 53 ++++++++++++++++++++++++++- internal/acceptance/metastore_test.go | 7 ++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/catalog/resource_metastore_test.go b/catalog/resource_metastore_test.go index 7eb998b48e..6b590ba8c1 100644 --- a/catalog/resource_metastore_test.go +++ b/catalog/resource_metastore_test.go @@ -194,7 +194,7 @@ func TestUpdateMetastore_NoChanges(t *testing.T) { }.ApplyNoError(t) } -func TestUpdateMetastore_OwnerChanges(t *testing.T) { +func TestUpdateMetastore_OnlyOwnerChanges(t *testing.T) { qa.ResourceFixture{ MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { e := w.GetMockMetastoresAPI().EXPECT() @@ -233,6 +233,57 @@ func TestUpdateMetastore_OwnerChanges(t *testing.T) { }.ApplyNoError(t) } +func TestUpdateMetastore_OwnerAndOtherChanges(t *testing.T) { + qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockMetastoresAPI().EXPECT() + e.Update(mock.Anything, catalog.UpdateMetastore{ + Id: "abc", + Owner: "updatedOwner", + }).Return(&catalog.MetastoreInfo{ + Name: "abc", + Owner: "updatedOwner", + }, nil) + e.Update(mock.Anything, catalog.UpdateMetastore{ + Id: "abc", + Name: "abc", + DeltaSharingScope: "INTERNAL_AND_EXTERNAL", + DeltaSharingRecipientTokenLifetimeInSeconds: 1004, + ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, + }).Return(&catalog.MetastoreInfo{ + Name: "abc", + Owner: "updatedOwner", + DeltaSharingScope: "INTERNAL_AND_EXTERNAL", + DeltaSharingRecipientTokenLifetimeInSeconds: 1004, + }, nil) + e.GetById(mock.Anything, "abc").Return(&catalog.MetastoreInfo{ + Name: "abc", + Owner: "updatedOwner", + DeltaSharingScope: "INTERNAL_AND_EXTERNAL", + DeltaSharingRecipientTokenLifetimeInSeconds: 1004, + }, 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 = "updatedOwner" + delta_sharing_scope = "INTERNAL_AND_EXTERNAL" + delta_sharing_recipient_token_lifetime_in_seconds = 1004 + `, + }.ApplyNoError(t) +} + func TestUpdateMetastore_Rollback(t *testing.T) { _, err := qa.ResourceFixture{ MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { diff --git a/internal/acceptance/metastore_test.go b/internal/acceptance/metastore_test.go index 5861a158fa..bf0f50a101 100644 --- a/internal/acceptance/metastore_test.go +++ b/internal/acceptance/metastore_test.go @@ -117,5 +117,12 @@ func runMetastoreTestWithOwnerUpdates(t *testing.T, extraAttributes map[string]a owner = "{env.TEST_METASTORE_ADMIN_GROUP_NAME}" %s }`, template), + }, step{ + Template: fmt.Sprintf(`resource "databricks_metastore" "this" { + name = "{var.STICKY_RANDOM}-updated" + force_destroy = true + owner = "{env.TEST_DATA_ENG_GROUP}" + %s + }`, template), }) } From 45a9901c0c0dfcd42699a1129634342bfa3f4848 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Thu, 1 Feb 2024 15:11:42 +0100 Subject: [PATCH 04/14] test --- catalog/resource_metastore_test.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/catalog/resource_metastore_test.go b/catalog/resource_metastore_test.go index 6b590ba8c1..50512798b6 100644 --- a/catalog/resource_metastore_test.go +++ b/catalog/resource_metastore_test.go @@ -251,7 +251,6 @@ func TestUpdateMetastore_OwnerAndOtherChanges(t *testing.T) { DeltaSharingRecipientTokenLifetimeInSeconds: 1004, ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, }).Return(&catalog.MetastoreInfo{ - Name: "abc", Owner: "updatedOwner", DeltaSharingScope: "INTERNAL_AND_EXTERNAL", DeltaSharingRecipientTokenLifetimeInSeconds: 1004, @@ -297,9 +296,9 @@ func TestUpdateMetastore_Rollback(t *testing.T) { }, nil) e.Update(mock.Anything, catalog.UpdateMetastore{ Id: "abc", - Name: "abcd", + 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{ @@ -322,11 +321,11 @@ func TestUpdateMetastore_Rollback(t *testing.T) { "delta_sharing_recipient_token_lifetime_in_seconds": "1002", }, HCL: ` - name = "abcd" + name = "abc" 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") @@ -631,9 +630,9 @@ func TestUpdateAccountMetastore_Rollback(t *testing.T) { MetastoreId: "abc", MetastoreInfo: &catalog.UpdateMetastore{ Id: "abc", - Name: "abcd", + Name: "abc", DeltaSharingScope: "INTERNAL_AND_EXTERNAL", - DeltaSharingRecipientTokenLifetimeInSeconds: 1002, + DeltaSharingRecipientTokenLifetimeInSeconds: 1004, ForceSendFields: []string{"DeltaSharingRecipientTokenLifetimeInSeconds"}, }, }).Return(nil, errors.New("Something unexpected happened")) @@ -663,11 +662,11 @@ func TestUpdateAccountMetastore_Rollback(t *testing.T) { "delta_sharing_recipient_token_lifetime_in_seconds": "1002", }, HCL: ` - name = "abcd" + name = "abc" 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") From 402d119818591436bfa5135e31fdd43be0a82192 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Thu, 1 Feb 2024 15:24:38 +0100 Subject: [PATCH 05/14] integration test --- catalog/resource_connection.go | 4 +++ catalog/resource_storage_credential.go | 5 ++++ internal/acceptance/connection_test.go | 2 ++ internal/acceptance/external_location_test.go | 4 +++ internal/acceptance/schema_test.go | 2 ++ internal/acceptance/volume_test.go | 29 ++++++++++++++++--- 6 files changed, 42 insertions(+), 4 deletions(-) diff --git a/catalog/resource_connection.go b/catalog/resource_connection.go index 3f556f1631..975893e02a 100644 --- a/catalog/resource_connection.go +++ b/catalog/resource_connection.go @@ -126,6 +126,10 @@ func ResourceConnection() *schema.Resource { } } + if !d.HasChangeExcept("owner") { + return nil + } + updateConnectionRequest.Owner = "" _, err = w.Connections.Update(ctx, updateConnectionRequest) if err != nil { diff --git a/catalog/resource_storage_credential.go b/catalog/resource_storage_credential.go index a4f94baec6..4e5ceb272f 100644 --- a/catalog/resource_storage_credential.go +++ b/catalog/resource_storage_credential.go @@ -196,6 +196,11 @@ func ResourceStorageCredential() *schema.Resource { return err } } + + if !d.HasChangeExcept("owner") { + return nil + } + update.Owner = "" _, err = w.StorageCredentials.Update(ctx, update) if err != nil { diff --git a/internal/acceptance/connection_test.go b/internal/acceptance/connection_test.go index 714ea33a53..b1c493c7d7 100644 --- a/internal/acceptance/connection_test.go +++ b/internal/acceptance/connection_test.go @@ -26,6 +26,8 @@ func TestUcAccConnectionsResourceFullLifecycle(t *testing.T) { Template: connectionTemplateWithOwner("test.mysql.database.azure.com", "account users"), }, step{ Template: connectionTemplateWithOwner("test.mysql.database.aws.com", "account users"), + }, step{ + Template: connectionTemplateWithOwner("test.mysql.database.aws.com", "{env.TEST_DATA_ENG_GROUP}"), }, step{ Template: connectionTemplateWithOwner("test.mysql.database.azure.com", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}"), }) diff --git a/internal/acceptance/external_location_test.go b/internal/acceptance/external_location_test.go index 0352382129..7e219bed2a 100644 --- a/internal/acceptance/external_location_test.go +++ b/internal/acceptance/external_location_test.go @@ -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}") + diff --git a/internal/acceptance/schema_test.go b/internal/acceptance/schema_test.go index df2098ceb8..246668d440 100644 --- a/internal/acceptance/schema_test.go +++ b/internal/acceptance/schema_test.go @@ -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}"), }) diff --git a/internal/acceptance/volume_test.go b/internal/acceptance/volume_test.go index c2ba2e5d0f..9fa4348983 100644 --- a/internal/acceptance/volume_test.go +++ b/internal/acceptance/volume_test.go @@ -46,9 +46,8 @@ func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) { volume_type = "EXTERNAL" storage_location = databricks_external_location.some.url }`, - }, - step{ - Template: prefixTestTemplate + ` + }, step{ + Template: prefixTestTemplate + ` resource "databricks_volume" "this" { name = "name-def" comment = "comment-def" @@ -58,7 +57,18 @@ func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) { volume_type = "EXTERNAL" storage_location = databricks_external_location.some.url }`, - }) + }, step{ + Template: prefixTestTemplate + ` + resource "databricks_volume" "this" { + name = "name-def" + comment = "comment-def" + owner = "{env.TEST_DATA_ENG_GROUP}" + catalog_name = "main" + schema_name = databricks_schema.this.name + volume_type = "EXTERNAL" + storage_location = databricks_external_location.some.url + }`, + }) } func TestUcAccVolumesResourceWithInitialOwnerAWSFullLifecycle(t *testing.T) { @@ -84,5 +94,16 @@ func TestUcAccVolumesResourceWithInitialOwnerAWSFullLifecycle(t *testing.T) { volume_type = "EXTERNAL" storage_location = databricks_external_location.some.url }`, + }, step{ + Template: prefixTestTemplate + ` + resource "databricks_volume" "this" { + name = "name-def" + comment = "comment-def" + owner = "{env.TEST_DATA_ENG_GROUP}" + catalog_name = "main" + schema_name = databricks_schema.this.name + volume_type = "EXTERNAL" + storage_location = databricks_external_location.some.url + }`, }) } From 3ddc078716beb2c5565d80a25ff7897209deba08 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Thu, 1 Feb 2024 15:34:15 +0100 Subject: [PATCH 06/14] test --- internal/acceptance/catalog_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/acceptance/catalog_test.go b/internal/acceptance/catalog_test.go index 9fdf29a247..ae0bbe3b65 100644 --- a/internal/acceptance/catalog_test.go +++ b/internal/acceptance/catalog_test.go @@ -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" { From 1c67861650e23f527f7021b6843e1267ea9dc49d Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Thu, 1 Feb 2024 15:37:02 +0100 Subject: [PATCH 07/14] Test --- internal/acceptance/recipient_test.go | 2 ++ internal/acceptance/share_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/internal/acceptance/recipient_test.go b/internal/acceptance/recipient_test.go index 10a91177fa..e59bf90d1c 100644 --- a/internal/acceptance/recipient_test.go +++ b/internal/acceptance/recipient_test.go @@ -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}"), }) diff --git a/internal/acceptance/share_test.go b/internal/acceptance/share_test.go index 2ce09769bf..afc3667ca4 100644 --- a/internal/acceptance/share_test.go +++ b/internal/acceptance/share_test.go @@ -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}"), }) From d0b4a2d9d897428a8dc08c3022e11e5140fb31e2 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 10:36:21 +0100 Subject: [PATCH 08/14] Update connection resource --- catalog/resource_connection.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/catalog/resource_connection.go b/catalog/resource_connection.go index 0ff7f24240..9b7b4c3aa7 100644 --- a/catalog/resource_connection.go +++ b/catalog/resource_connection.go @@ -120,7 +120,6 @@ func ResourceConnection() common.Resource { if d.HasChange("owner") { _, err = w.Connections.Update(ctx, catalog.UpdateConnection{ - Name: updateConnectionRequest.Name, NameArg: updateConnectionRequest.Name, Owner: updateConnectionRequest.Owner, }) @@ -140,7 +139,6 @@ func ResourceConnection() common.Resource { // Rollback old, new := d.GetChange("owner") _, rollbackErr := w.Connections.Update(ctx, catalog.UpdateConnection{ - Name: updateConnectionRequest.Name, NameArg: updateConnectionRequest.Name, Owner: old.(string), }) From 4bf42eaac528c75de0076a9f1798beded6c75915 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 11:20:15 +0100 Subject: [PATCH 09/14] restore connection --- catalog/resource_connection.go | 33 ++++++++++------------- internal/acceptance/connection_test.go | 37 -------------------------- 2 files changed, 14 insertions(+), 56 deletions(-) diff --git a/catalog/resource_connection.go b/catalog/resource_connection.go index 9b7b4c3aa7..3f556f1631 100644 --- a/catalog/resource_connection.go +++ b/catalog/resource_connection.go @@ -36,7 +36,7 @@ type ConnectionInfo struct { var sensitiveOptions = []string{"user", "password", "personalAccessToken", "access_token", "client_secret", "OAuthPvtKey", "GoogleServiceAccountKeyJson"} -func ResourceConnection() common.Resource { +func ResourceConnection() *schema.Resource { s := common.StructToSchema(ConnectionInfo{}, common.NoCustomize) pi := common.NewPairID("metastore_id", "name").Schema( @@ -56,19 +56,20 @@ func ResourceConnection() common.Resource { } var createConnectionRequest catalog.CreateConnection common.DataToStructPointer(d, s, &createConnectionRequest) - conn, err := w.Connections.Create(ctx, createConnectionRequest) + _, err = w.Connections.Create(ctx, createConnectionRequest) if err != nil { return err } // Update owner if it is provided - if d.Get("owner") != "" { - var updateConnectionRequest catalog.UpdateConnection - common.DataToStructPointer(d, s, &updateConnectionRequest) - updateConnectionRequest.NameArg = updateConnectionRequest.Name - conn, err = w.Connections.Update(ctx, updateConnectionRequest) - if err != nil { - return err - } + if d.Get("owner") == "" { + return nil + } + var updateConnectionRequest catalog.UpdateConnection + common.DataToStructPointer(d, s, &updateConnectionRequest) + updateConnectionRequest.NameArg = updateConnectionRequest.Name + conn, err := w.Connections.Update(ctx, updateConnectionRequest) + if err != nil { + return err } d.Set("metastore_id", conn.MetastoreId) pi.Pack(d) @@ -90,10 +91,6 @@ func ResourceConnection() common.Resource { // We need to preserve original sensitive options as API doesn't return them var cOrig catalog.CreateConnection common.DataToStructPointer(d, s, &cOrig) - // If there are no options returned, need to initialize the map - if conn.Options == nil { - conn.Options = map[string]string{} - } for key, element := range cOrig.Options { if slices.Contains(sensitiveOptions, key) { conn.Options[key] = element @@ -120,6 +117,7 @@ func ResourceConnection() common.Resource { if d.HasChange("owner") { _, err = w.Connections.Update(ctx, catalog.UpdateConnection{ + Name: updateConnectionRequest.Name, NameArg: updateConnectionRequest.Name, Owner: updateConnectionRequest.Owner, }) @@ -128,10 +126,6 @@ func ResourceConnection() common.Resource { } } - if !d.HasChangeExcept("owner") { - return nil - } - updateConnectionRequest.Owner = "" _, err = w.Connections.Update(ctx, updateConnectionRequest) if err != nil { @@ -139,6 +133,7 @@ func ResourceConnection() common.Resource { // Rollback old, new := d.GetChange("owner") _, rollbackErr := w.Connections.Update(ctx, catalog.UpdateConnection{ + Name: updateConnectionRequest.Name, NameArg: updateConnectionRequest.Name, Owner: old.(string), }) @@ -161,5 +156,5 @@ func ResourceConnection() common.Resource { } return w.Connections.DeleteByNameArg(ctx, connName) }, - } + }.ToResource() } diff --git a/internal/acceptance/connection_test.go b/internal/acceptance/connection_test.go index 31d0c9f9eb..714ea33a53 100644 --- a/internal/acceptance/connection_test.go +++ b/internal/acceptance/connection_test.go @@ -21,49 +21,12 @@ func connectionTemplateWithOwner(host string, owner string) string { } `, host, owner) } - -func connectionTemplateWithoutOwner() string { - return ` - resource "databricks_connection" "this" { - name = "name-{var.STICKY_RANDOM}" - connection_type = "BIGQUERY" - comment = "test" - options = { - GoogleServiceAccountKeyJson = <<-EOT - { - "type": "service_account", - "project_id": "PROJECT_ID", - "private_key_id": "KEY_ID", - "private_key": "-----BEGIN PRIVATE KEY-----\nPRIVATE_KEY\n-----END PRIVATE KEY-----\n", - "client_email": "SERVICE_ACCOUNT_EMAIL", - "client_id": "CLIENT_ID", - "auth_uri": "https://accounts.google.com/o/oauth2/auth", - "token_uri": "https://accounts.google.com/o/oauth2/token", - "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", - "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/SERVICE_ACCOUNT_EMAIL", - "universe_domain": "googleapis.com" - } - EOT - } - } - ` -} func TestUcAccConnectionsResourceFullLifecycle(t *testing.T) { unityWorkspaceLevel(t, step{ Template: connectionTemplateWithOwner("test.mysql.database.azure.com", "account users"), }, step{ Template: connectionTemplateWithOwner("test.mysql.database.aws.com", "account users"), - }, step{ - Template: connectionTemplateWithOwner("test.mysql.database.aws.com", "{env.TEST_DATA_ENG_GROUP}"), }, step{ Template: connectionTemplateWithOwner("test.mysql.database.azure.com", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}"), }) } - -func TestUcAccConnectionsWithoutOwnerResourceFullLifecycle(t *testing.T) { - unityWorkspaceLevel(t, step{ - Template: connectionTemplateWithoutOwner(), - }, step{ - Template: connectionTemplateWithoutOwner(), - }) -} From 51e868e3c54bcca35cf212206e71ec618514dbaf Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 11:21:27 +0100 Subject: [PATCH 10/14] restore connection --- catalog/resource_connection.go | 27 +++++++++++--------- internal/acceptance/connection_test.go | 35 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/catalog/resource_connection.go b/catalog/resource_connection.go index 3f556f1631..978e213348 100644 --- a/catalog/resource_connection.go +++ b/catalog/resource_connection.go @@ -36,7 +36,7 @@ type ConnectionInfo struct { var sensitiveOptions = []string{"user", "password", "personalAccessToken", "access_token", "client_secret", "OAuthPvtKey", "GoogleServiceAccountKeyJson"} -func ResourceConnection() *schema.Resource { +func ResourceConnection() common.Resource { s := common.StructToSchema(ConnectionInfo{}, common.NoCustomize) pi := common.NewPairID("metastore_id", "name").Schema( @@ -56,20 +56,19 @@ func ResourceConnection() *schema.Resource { } var createConnectionRequest catalog.CreateConnection common.DataToStructPointer(d, s, &createConnectionRequest) - _, err = w.Connections.Create(ctx, createConnectionRequest) + conn, err := w.Connections.Create(ctx, createConnectionRequest) if err != nil { return err } // Update owner if it is provided - if d.Get("owner") == "" { - return nil - } - var updateConnectionRequest catalog.UpdateConnection - common.DataToStructPointer(d, s, &updateConnectionRequest) - updateConnectionRequest.NameArg = updateConnectionRequest.Name - conn, err := w.Connections.Update(ctx, updateConnectionRequest) - if err != nil { - return err + if d.Get("owner") != "" { + var updateConnectionRequest catalog.UpdateConnection + common.DataToStructPointer(d, s, &updateConnectionRequest) + updateConnectionRequest.NameArg = updateConnectionRequest.Name + conn, err = w.Connections.Update(ctx, updateConnectionRequest) + if err != nil { + return err + } } d.Set("metastore_id", conn.MetastoreId) pi.Pack(d) @@ -91,6 +90,10 @@ func ResourceConnection() *schema.Resource { // We need to preserve original sensitive options as API doesn't return them var cOrig catalog.CreateConnection common.DataToStructPointer(d, s, &cOrig) + // If there are no options returned, need to initialize the map + if conn.Options == nil { + conn.Options = map[string]string{} + } for key, element := range cOrig.Options { if slices.Contains(sensitiveOptions, key) { conn.Options[key] = element @@ -156,5 +159,5 @@ func ResourceConnection() *schema.Resource { } return w.Connections.DeleteByNameArg(ctx, connName) }, - }.ToResource() + } } diff --git a/internal/acceptance/connection_test.go b/internal/acceptance/connection_test.go index 714ea33a53..7cf6ec5093 100644 --- a/internal/acceptance/connection_test.go +++ b/internal/acceptance/connection_test.go @@ -21,6 +21,33 @@ func connectionTemplateWithOwner(host string, owner string) string { } `, host, owner) } + +func connectionTemplateWithoutOwner() string { + return ` + resource "databricks_connection" "this" { + name = "name-{var.STICKY_RANDOM}" + connection_type = "BIGQUERY" + comment = "test" + options = { + GoogleServiceAccountKeyJson = <<-EOT + { + "type": "service_account", + "project_id": "PROJECT_ID", + "private_key_id": "KEY_ID", + "private_key": "-----BEGIN PRIVATE KEY-----\nPRIVATE_KEY\n-----END PRIVATE KEY-----\n", + "client_email": "SERVICE_ACCOUNT_EMAIL", + "client_id": "CLIENT_ID", + "auth_uri": "https://accounts.google.com/o/oauth2/auth", + "token_uri": "https://accounts.google.com/o/oauth2/token", + "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", + "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/SERVICE_ACCOUNT_EMAIL", + "universe_domain": "googleapis.com" + } + EOT + } + } + ` +} func TestUcAccConnectionsResourceFullLifecycle(t *testing.T) { unityWorkspaceLevel(t, step{ Template: connectionTemplateWithOwner("test.mysql.database.azure.com", "account users"), @@ -30,3 +57,11 @@ func TestUcAccConnectionsResourceFullLifecycle(t *testing.T) { Template: connectionTemplateWithOwner("test.mysql.database.azure.com", "{env.TEST_METASTORE_ADMIN_GROUP_NAME}"), }) } + +func TestUcAccConnectionsWithoutOwnerResourceFullLifecycle(t *testing.T) { + unityWorkspaceLevel(t, step{ + Template: connectionTemplateWithoutOwner(), + }, step{ + Template: connectionTemplateWithoutOwner(), + }) +} From df401b519ebcfde4cd167c85e29f49ecc61930ff Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 11:30:14 +0100 Subject: [PATCH 11/14] update --- internal/acceptance/metastore_test.go | 6 +++--- internal/acceptance/volume_test.go | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/acceptance/metastore_test.go b/internal/acceptance/metastore_test.go index bf0f50a101..2250567bff 100644 --- a/internal/acceptance/metastore_test.go +++ b/internal/acceptance/metastore_test.go @@ -112,16 +112,16 @@ func runMetastoreTestWithOwnerUpdates(t *testing.T, extraAttributes map[string]a }`, template), }, step{ Template: fmt.Sprintf(`resource "databricks_metastore" "this" { - name = "{var.STICKY_RANDOM}-updated" + name = "{var.STICKY_RANDOM}" force_destroy = true - owner = "{env.TEST_METASTORE_ADMIN_GROUP_NAME}" + owner = "{env.TEST_DATA_ENG_GROUP}" %s }`, template), }, step{ Template: fmt.Sprintf(`resource "databricks_metastore" "this" { name = "{var.STICKY_RANDOM}-updated" force_destroy = true - owner = "{env.TEST_DATA_ENG_GROUP}" + owner = "{env.TEST_METASTORE_ADMIN_GROUP_NAME}" %s }`, template), }) diff --git a/internal/acceptance/volume_test.go b/internal/acceptance/volume_test.go index 9fa4348983..8ffa282358 100644 --- a/internal/acceptance/volume_test.go +++ b/internal/acceptance/volume_test.go @@ -49,9 +49,9 @@ func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) { }, step{ Template: prefixTestTemplate + ` resource "databricks_volume" "this" { - name = "name-def" - comment = "comment-def" - owner = "account users" + name = "name-abc" + comment = "comment-pqr" + owner = "{env.TEST_DATA_ENG_GROUP}" catalog_name = "main" schema_name = databricks_schema.this.name volume_type = "EXTERNAL" @@ -62,7 +62,7 @@ func TestUcAccVolumesResourceWithoutInitialOwnerAWSFullLifecycle(t *testing.T) { resource "databricks_volume" "this" { name = "name-def" comment = "comment-def" - owner = "{env.TEST_DATA_ENG_GROUP}" + owner = "account users" catalog_name = "main" schema_name = databricks_schema.this.name volume_type = "EXTERNAL" @@ -86,9 +86,9 @@ func TestUcAccVolumesResourceWithInitialOwnerAWSFullLifecycle(t *testing.T) { }, step{ Template: prefixTestTemplate + ` resource "databricks_volume" "this" { - name = "name-def" - comment = "comment-def" - owner = "{env.TEST_METASTORE_ADMIN_GROUP_NAME}" + name = "name-abc" + comment = "comment-abc" + owner = "{env.TEST_DATA_ENG_GROUP}" catalog_name = "main" schema_name = databricks_schema.this.name volume_type = "EXTERNAL" @@ -99,7 +99,7 @@ func TestUcAccVolumesResourceWithInitialOwnerAWSFullLifecycle(t *testing.T) { resource "databricks_volume" "this" { name = "name-def" comment = "comment-def" - owner = "{env.TEST_DATA_ENG_GROUP}" + owner = "{env.TEST_METASTORE_ADMIN_GROUP_NAME}" catalog_name = "main" schema_name = databricks_schema.this.name volume_type = "EXTERNAL" From a8a10009daf85a1498c80d8b2c081f2d7c196d31 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 14:53:51 +0100 Subject: [PATCH 12/14] docs --- docs/index.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/index.md b/docs/index.md index 85038d3980..a6029bade1 100644 --- a/docs/index.md +++ b/docs/index.md @@ -355,6 +355,8 @@ 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. +Note: 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. From acf95bbc34c829e946fa34dff5cfeaeba8697f05 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 14:56:14 +0100 Subject: [PATCH 13/14] docs --- docs/index.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index a6029bade1..7c1909e57a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -355,7 +355,9 @@ 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. -Note: 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. +## Special considerations for Unity Catalog + +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 From 07ae5bd0d2fe0b9e028fb65fcf7dd721163d0a22 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 2 Feb 2024 14:58:23 +0100 Subject: [PATCH 14/14] docs --- docs/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 7c1909e57a..20ddfd1137 100644 --- a/docs/index.md +++ b/docs/index.md @@ -355,7 +355,7 @@ 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 +## 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.