From c5c9974e83244efce4ba8f1cb59768c132f7cb38 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 11:00:47 +0200 Subject: [PATCH 1/7] [Internal] Make plugin framework implementation of share resource as SDKv2 compatible --- .../providers/pluginfw/products/sharing/resource_share.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/providers/pluginfw/products/sharing/resource_share.go b/internal/providers/pluginfw/products/sharing/resource_share.go index f451c90cf5..179c6dd937 100644 --- a/internal/providers/pluginfw/products/sharing/resource_share.go +++ b/internal/providers/pluginfw/products/sharing/resource_share.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" ) const resourceName = "share" @@ -31,6 +32,7 @@ func ResourceShare() resource.Resource { type ShareInfoExtended struct { sharing_tf.ShareInfo_SdkV2 + ID types.String `tfsdk:"id"` // Adding ID field to stay compatible with SDKv2 } var _ pluginfwcommon.ComplexFieldTypeProvider = ShareInfoExtended{} @@ -156,6 +158,8 @@ func (r *ShareResource) Schema(ctx context.Context, req resource.SchemaRequest, c.SetRequired("object", "partition", "value", "op") c.SetRequired("object", "partition", "value", "name") + c.SetComputed("id") + return c }) resp.Schema = schema.Schema{ @@ -228,6 +232,8 @@ func (r *ShareResource) Create(ctx context.Context, req resource.CreateRequest, return } + newState.ID = newState.Name + resp.Diagnostics.Append(resp.State.Set(ctx, newState)...) } From f343207c643b8bc07ea428c673df8d21083f46b0 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 12:55:19 +0200 Subject: [PATCH 2/7] tests --- .../products/sharing/resource_acc_test.go | 39 +++++++++++++++++++ .../products/sharing/resource_share.go | 1 + 2 files changed, 40 insertions(+) diff --git a/internal/providers/pluginfw/products/sharing/resource_acc_test.go b/internal/providers/pluginfw/products/sharing/resource_acc_test.go index cce7c0a551..ec711559bc 100644 --- a/internal/providers/pluginfw/products/sharing/resource_acc_test.go +++ b/internal/providers/pluginfw/products/sharing/resource_acc_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/databricks/terraform-provider-databricks/internal/acceptance" + "github.com/hashicorp/terraform-plugin-testing/terraform" ) const preTestTemplate = ` @@ -234,3 +235,41 @@ func TestUcAccUpdateShareReorderObject(t *testing.T) { }`, }) } + +func shareUpdateWithName(name string) string { + return fmt.Sprintf(`resource "databricks_share_pluginframework" "myshare" { + name = "%s" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "A" + data_object_type = "TABLE" + history_data_sharing_status = "DISABLED" + } + }`, name) +} + +func shareCheckStateforID() func(s *terraform.State) error { + return func(s *terraform.State) error { + r, ok := s.RootModule().Resources["databricks_share_pluginframework.myshare"] + if !ok { + return fmt.Errorf("resource not found in state") + } + id := r.Primary.Attributes["id"] + name := r.Primary.Attributes["name"] + if id != name { + return fmt.Errorf("resource ID is not equal to the name. Attributes: %v", r.Primary.Attributes) + } + return nil + } +} + +func TestUcAccUpdateShareName(t *testing.T) { + acceptance.UnityWorkspaceLevel(t, acceptance.Step{ + Template: preTestTemplate + shareUpdateWithName("{var.STICKY_RANDOM}-terraform-delta-share-before"), + Check: shareCheckStateforID(), + }, acceptance.Step{ + Template: preTestTemplate + shareUpdateWithName("{var.STICKY_RANDOM}-terraform-delta-share-after"), + Check: shareCheckStateforID(), + }) +} diff --git a/internal/providers/pluginfw/products/sharing/resource_share.go b/internal/providers/pluginfw/products/sharing/resource_share.go index 179c6dd937..a841628db2 100644 --- a/internal/providers/pluginfw/products/sharing/resource_share.go +++ b/internal/providers/pluginfw/products/sharing/resource_share.go @@ -396,6 +396,7 @@ func (r *ShareResource) Update(ctx context.Context, req resource.UpdateRequest, return } + state.ID = state.Name resp.Diagnostics.Append(resp.State.Set(ctx, state)...) } From 1dc98259a39b3483584ca9285876bf6154c582a4 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 12:55:53 +0200 Subject: [PATCH 3/7] - --- NEXT_CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e66e9e624d..3d204f1b6b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -9,7 +9,7 @@ * Document and handle additional Slack options in `databricks_notification_destination` ([#4933](https://github.com/databricks/terraform-provider-databricks/pull/4933)) ### Bug Fixes - + * Add support for share resource in plugin framework implementation to be SDKv2 compatible ([#4965](https://github.com/databricks/terraform-provider-databricks/pull/4965)) ### Documentation * Improve documentation for grant resource ([#4906](https://github.com/databricks/terraform-provider-databricks/pull/4935)) From 9d209981e07dcb4d09e87c2d33d985785a5d3e08 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 12:58:39 +0200 Subject: [PATCH 4/7] work --- NEXT_CHANGELOG.md | 2 - exporter/importables.go | 37 ++- .../pluginfw/pluginfw_rollout_utils.go | 6 +- .../pluginfw/products/sharing/data_share.go | 2 +- .../pluginfw/products/sharing/data_shares.go | 2 +- .../products/sharing/data_shares_acc_test.go | 10 +- .../products/sharing/resource_acc_test.go | 156 +++++++++- .../products/sharing/resource_share.go | 2 +- .../products/sharing/resource_share_test.go | 273 ++++++++++++++++++ 9 files changed, 468 insertions(+), 22 deletions(-) create mode 100644 internal/providers/pluginfw/products/sharing/resource_share_test.go diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e66e9e624d..ea3856280d 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -25,5 +25,3 @@ * Resolve references also for map values ([#4944](https://github.com/databricks/terraform-provider-databricks/pull/4944)) ### Internal Changes - -* Replaced `common.APIErrorBody` with corresponding structs in Go SDK ([#4936](https://github.com/databricks/terraform-provider-databricks/pull/4936)) diff --git a/exporter/importables.go b/exporter/importables.go index e6167b31f4..cb57556415 100644 --- a/exporter/importables.go +++ b/exporter/importables.go @@ -2221,8 +2221,42 @@ var resourcesMap map[string]importable = map[string]importable{ return nil }, Import: func(ic *importContext, r *resource) error { + resourceInfo := ic.Resources["databricks_share"] + if resourceInfo == nil { + // Fallback to direct data access if schema is not available + objectsList := r.Data.Get("object").([]any) + ic.emitUCGrantsWithOwner("share/"+r.ID, r) + for _, objRaw := range objectsList { + obj := objRaw.(map[string]any) + dataObjectType := obj["data_object_type"].(string) + name := obj["name"].(string) + + switch dataObjectType { + case "TABLE": + ic.Emit(&resource{ + Resource: "databricks_sql_table", + ID: name, + }) + case "VOLUME": + ic.Emit(&resource{ + Resource: "databricks_volume", + ID: name, + }) + case "MODEL": + ic.Emit(&resource{ + Resource: "databricks_registered_model", + ID: name, + }) + default: + log.Printf("[INFO] Object type '%s' (name: '%s') isn't supported in share '%s'", + dataObjectType, name, r.ID) + } + } + return nil + } + var share tf_sharing.ShareInfo - s := ic.Resources["databricks_share"].Schema + s := resourceInfo.Schema common.DataToStructPointer(r.Data, s, &share) // TODO: how to link recipients to share? ic.emitUCGrantsWithOwner("share/"+r.ID, r) @@ -2248,7 +2282,6 @@ var resourcesMap map[string]importable = map[string]importable{ obj.DataObjectType, obj.Name, r.ID) } } - return nil }, ShouldOmitField: shouldOmitForUnityCatalog, diff --git a/internal/providers/pluginfw/pluginfw_rollout_utils.go b/internal/providers/pluginfw/pluginfw_rollout_utils.go index 32e40996de..575f2507c4 100644 --- a/internal/providers/pluginfw/pluginfw_rollout_utils.go +++ b/internal/providers/pluginfw/pluginfw_rollout_utils.go @@ -33,11 +33,14 @@ import ( var migratedResources = []func() resource.Resource{ library.ResourceLibrary, qualitymonitor.ResourceQualityMonitor, + sharing.ResourceShare, } // List of data sources that have been migrated from SDK V2 to plugin framework // Keep this list sorted. var migratedDataSources = []func() datasource.DataSource{ + sharing.DataSourceShare, + sharing.DataSourceShares, volume.DataSourceVolumes, } @@ -46,7 +49,6 @@ var migratedDataSources = []func() datasource.DataSource{ var pluginFwOnlyResources = append( []func() resource.Resource{ app.ResourceApp, - sharing.ResourceShare, }, autoGeneratedResources..., ) @@ -65,8 +67,6 @@ var pluginFwOnlyDataSources = append( serving.DataSourceServingEndpoints, // TODO: Add DataSourceCluster into migratedDataSources after fixing unit tests. cluster.DataSourceCluster, // Using the staging name (with pluginframework suffix) - sharing.DataSourceShare, // Using the staging name (with pluginframework suffix) - sharing.DataSourceShares, // Using the staging name (with pluginframework suffix) }, autoGeneratedDataSources..., ) diff --git a/internal/providers/pluginfw/products/sharing/data_share.go b/internal/providers/pluginfw/products/sharing/data_share.go index bfb9cbb9d4..7395102aaf 100644 --- a/internal/providers/pluginfw/products/sharing/data_share.go +++ b/internal/providers/pluginfw/products/sharing/data_share.go @@ -28,7 +28,7 @@ type ShareDataSource struct { } func (d *ShareDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { - resp.TypeName = pluginfwcommon.GetDatabricksStagingName(dataSourceNameShare) + resp.TypeName = pluginfwcommon.GetDatabricksProductionName(dataSourceNameShare) } func (d *ShareDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { diff --git a/internal/providers/pluginfw/products/sharing/data_shares.go b/internal/providers/pluginfw/products/sharing/data_shares.go index bdc5301d0e..01fa3b2157 100644 --- a/internal/providers/pluginfw/products/sharing/data_shares.go +++ b/internal/providers/pluginfw/products/sharing/data_shares.go @@ -52,7 +52,7 @@ type SharesDataSource struct { } func (d *SharesDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { - resp.TypeName = pluginfwcommon.GetDatabricksStagingName(dataSourceNameShares) + resp.TypeName = pluginfwcommon.GetDatabricksProductionName(dataSourceNameShares) } func (d *SharesDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { diff --git a/internal/providers/pluginfw/products/sharing/data_shares_acc_test.go b/internal/providers/pluginfw/products/sharing/data_shares_acc_test.go index e47a401887..d6aec9a506 100644 --- a/internal/providers/pluginfw/products/sharing/data_shares_acc_test.go +++ b/internal/providers/pluginfw/products/sharing/data_shares_acc_test.go @@ -12,8 +12,8 @@ import ( func checkSharesDataSourcePopulated(t *testing.T) func(s *terraform.State) error { return func(s *terraform.State) error { - ds, ok := s.Modules[0].Resources["data.databricks_shares_pluginframework.this"] - require.True(t, ok, "data.databricks_shares_pluginframework.this has to be there") + ds, ok := s.Modules[0].Resources["data.databricks_shares.this"] + require.True(t, ok, "data.databricks_shares.this has to be there") num_shares, _ := strconv.Atoi(ds.Primary.Attributes["shares.#"]) assert.GreaterOrEqual(t, num_shares, 1) return nil @@ -71,7 +71,7 @@ func TestUcAccDataSourceShares(t *testing.T) { } } - resource "databricks_share_pluginframework" "myshare" { + resource "databricks_share" "myshare" { name = "{var.RANDOM}-terraform-delta-share" object { name = databricks_table.mytable.id @@ -86,8 +86,8 @@ func TestUcAccDataSourceShares(t *testing.T) { } } - data "databricks_shares_pluginframework" "this" { - depends_on = [databricks_share_pluginframework.myshare] + data "databricks_shares" "this" { + depends_on = [databricks_share.myshare] } `, Check: checkSharesDataSourcePopulated(t), diff --git a/internal/providers/pluginfw/products/sharing/resource_acc_test.go b/internal/providers/pluginfw/products/sharing/resource_acc_test.go index cce7c0a551..3127c8d480 100644 --- a/internal/providers/pluginfw/products/sharing/resource_acc_test.go +++ b/internal/providers/pluginfw/products/sharing/resource_acc_test.go @@ -1,10 +1,13 @@ package sharing_test import ( + "context" "fmt" "testing" "github.com/databricks/terraform-provider-databricks/internal/acceptance" + "github.com/databricks/terraform-provider-databricks/internal/providers" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" ) const preTestTemplate = ` @@ -91,7 +94,7 @@ const preTestTemplateUpdate = ` func TestUcAccCreateShare(t *testing.T) { acceptance.UnityWorkspaceLevel(t, acceptance.Step{ Template: preTestTemplate + ` - resource "databricks_share_pluginframework" "myshare" { + resource "databricks_share" "myshare" { name = "{var.STICKY_RANDOM}-terraform-delta-share" owner = "account users" object { @@ -119,7 +122,7 @@ func TestUcAccCreateShare(t *testing.T) { } resource "databricks_grants" "some" { - share = databricks_share_pluginframework.myshare.name + share = databricks_share.myshare.name grant { principal = databricks_recipient.db2open.name privileges = ["SELECT"] @@ -131,7 +134,7 @@ func TestUcAccCreateShare(t *testing.T) { func shareTemplateWithOwner(comment string, owner string) string { return fmt.Sprintf(` - resource "databricks_share_pluginframework" "myshare" { + resource "databricks_share" "myshare" { name = "{var.STICKY_RANDOM}-terraform-delta-share" owner = "%s" object { @@ -159,7 +162,7 @@ func TestUcAccUpdateShare(t *testing.T) { func TestUcAccUpdateShareAddObject(t *testing.T) { acceptance.UnityWorkspaceLevel(t, acceptance.Step{ Template: preTestTemplate + preTestTemplateUpdate + - `resource "databricks_share_pluginframework" "myshare" { + `resource "databricks_share" "myshare" { name = "{var.STICKY_RANDOM}-terraform-delta-share" owner = "account users" object { @@ -178,7 +181,7 @@ func TestUcAccUpdateShareAddObject(t *testing.T) { }`, }, acceptance.Step{ Template: preTestTemplate + preTestTemplateUpdate + - `resource "databricks_share_pluginframework" "myshare" { + `resource "databricks_share" "myshare" { name = "{var.STICKY_RANDOM}-terraform-delta-share" owner = "account users" object { @@ -206,7 +209,7 @@ func TestUcAccUpdateShareAddObject(t *testing.T) { func TestUcAccUpdateShareReorderObject(t *testing.T) { acceptance.UnityWorkspaceLevel(t, acceptance.Step{ Template: preTestTemplate + preTestTemplateUpdate + - `resource "databricks_share_pluginframework" "myshare" { + `resource "databricks_share" "myshare" { name = "{var.STICKY_RANDOM}-terraform-delta-share" owner = "account users" object { @@ -220,7 +223,7 @@ func TestUcAccUpdateShareReorderObject(t *testing.T) { }`, }, acceptance.Step{ Template: preTestTemplate + preTestTemplateUpdate + - `resource "databricks_share_pluginframework" "myshare" { + `resource "databricks_share" "myshare" { name = "{var.STICKY_RANDOM}-terraform-delta-share" owner = "account users" object { @@ -234,3 +237,142 @@ func TestUcAccUpdateShareReorderObject(t *testing.T) { }`, }) } + +// TestUcAccUpdateShareNoChanges tests that updating a share with no actual changes doesn't cause issues +func TestUcAccUpdateShareNoChanges(t *testing.T) { + shareConfig := preTestTemplate + preTestTemplateUpdate + + `resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-delta-share" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "stable comment" + data_object_type = "TABLE" + } + }` + + acceptance.UnityWorkspaceLevel(t, acceptance.Step{ + Template: shareConfig, + }, acceptance.Step{ + Template: shareConfig, // Same config - should not trigger any updates + }) +} + +// TestUcAccUpdateShareComplexObjectChanges tests complex scenarios with multiple object updates +func TestUcAccUpdateShareComplexObjectChanges(t *testing.T) { + acceptance.UnityWorkspaceLevel(t, acceptance.Step{ + Template: preTestTemplate + preTestTemplateUpdate + + `resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-delta-share" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "original comment" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_2.id + comment = "second table" + data_object_type = "TABLE" + } + }`, + }, acceptance.Step{ + // Remove one object, add another, and update comment on existing + Template: preTestTemplate + preTestTemplateUpdate + + `resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-delta-share" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "updated comment" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_3.id + comment = "third table" + data_object_type = "TABLE" + } + }`, + }) +} + +// TestUcAccUpdateShareRemoveAllObjects tests removing all objects from a share +func TestUcAccUpdateShareRemoveAllObjects(t *testing.T) { + acceptance.UnityWorkspaceLevel(t, acceptance.Step{ + Template: preTestTemplate + preTestTemplateUpdate + + `resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-delta-share" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "to be removed" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_2.id + comment = "also to be removed" + data_object_type = "TABLE" + } + }`, + }, acceptance.Step{ + Template: preTestTemplate + preTestTemplateUpdate + + `resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-delta-share" + owner = "account users" + }`, + }) +} + +// TestUcAccShareMigrationFromSDKv2 tests the transition from sdkv2 to plugin framework. +// This test verifies that existing state created by SDK v2 implementation can be +// successfully managed by the plugin framework implementation without any changes. +func TestUcAccShareMigrationFromSDKv2(t *testing.T) { + acceptance.UnityWorkspaceLevel(t, + // Step 1: Create share using SDK v2 implementation + acceptance.Step{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "databricks": func() (tfprotov6.ProviderServer, error) { + sdkv2Provider, pluginfwProvider := acceptance.ProvidersWithResourceFallbacks([]string{"databricks_share"}) + return providers.GetProviderServer(context.Background(), providers.WithSdkV2Provider(sdkv2Provider), providers.WithPluginFrameworkProvider(pluginfwProvider)) + }, + }, + Template: preTestTemplate + preTestTemplateUpdate + ` + resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-migration-share" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "Shared table for migration test" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_2.id + comment = "Second shared table" + data_object_type = "TABLE" + cdf_enabled = false + } + }`, + }, + // Step 2: Update the share using plugin framework implementation (default) + // This verifies no changes are needed when switching implementations + acceptance.Step{ + ExpectNonEmptyPlan: false, + Template: preTestTemplate + preTestTemplateUpdate + ` + resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-migration-share" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "Updated comment after migration" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_2.id + comment = "Second shared table" + data_object_type = "TABLE" + cdf_enabled = false + } + }`, + }, + ) +} diff --git a/internal/providers/pluginfw/products/sharing/resource_share.go b/internal/providers/pluginfw/products/sharing/resource_share.go index f451c90cf5..312be3b7bf 100644 --- a/internal/providers/pluginfw/products/sharing/resource_share.go +++ b/internal/providers/pluginfw/products/sharing/resource_share.go @@ -140,7 +140,7 @@ type ShareResource struct { } func (r *ShareResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { - resp.TypeName = pluginfwcommon.GetDatabricksStagingName(resourceName) + resp.TypeName = pluginfwcommon.GetDatabricksProductionName(resourceName) } func (r *ShareResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { diff --git a/internal/providers/pluginfw/products/sharing/resource_share_test.go b/internal/providers/pluginfw/products/sharing/resource_share_test.go new file mode 100644 index 0000000000..d7d58093a2 --- /dev/null +++ b/internal/providers/pluginfw/products/sharing/resource_share_test.go @@ -0,0 +1,273 @@ +package sharing + +import ( + "testing" + + "github.com/databricks/databricks-sdk-go/service/sharing" + "github.com/stretchr/testify/assert" +) + +// TestDiff tests the diff function that compares two ShareInfo states +func TestDiff(t *testing.T) { + empty := sharing.ShareInfo{ + Name: "test-share", + Objects: []sharing.SharedDataObject{}, + } + + firstShare := sharing.ShareInfo{ + Name: "test-share", + Objects: []sharing.SharedDataObject{ + { + Name: "main.b", + DataObjectType: "TABLE", + Comment: "c", + }, + { + Name: "main.a", + DataObjectType: "TABLE", + Comment: "c", + }, + }, + } + + secondShare := sharing.ShareInfo{ + Name: "test-share", + Objects: []sharing.SharedDataObject{ + { + Name: "main.c", + DataObjectType: "TABLE", + Comment: "d", + }, + { + Name: "main.a", + DataObjectType: "TABLE", + Comment: "c", + }, + }, + } + + thirdShare := sharing.ShareInfo{ + Name: "test-share", + Objects: []sharing.SharedDataObject{ + { + Name: "main.c", + DataObjectType: "TABLE", + Comment: "d", + }, + { + Name: "main.b", + DataObjectType: "TABLE", + Comment: "d", + }, + }, + } + + fourthShare := sharing.ShareInfo{ + Name: "test-share", + Objects: []sharing.SharedDataObject{ + { + Name: "main.b", + DataObjectType: "TABLE", + Comment: "d", + }, + { + Name: "main.a", + DataObjectType: "TABLE", + Comment: "c", + }, + }, + } + + // Test: No difference when comparing same share + assert.Equal(t, []sharing.SharedDataObjectUpdate{}, diff(firstShare, firstShare), "Should not have difference") + + // Test: Adding objects to empty share + diffAdd := diff(empty, firstShare) + assert.Len(t, diffAdd, 2, "Should have 2 ADDs") + for _, update := range diffAdd { + assert.Equal(t, sharing.SharedDataObjectUpdateActionAdd, update.Action) + } + + // Test: Removing all objects + diffRemove := diff(firstShare, empty) + assert.Len(t, diffRemove, 2, "Should have 2 REMOVEs") + for _, update := range diffRemove { + assert.Equal(t, sharing.SharedDataObjectUpdateActionRemove, update.Action) + } + + // Test: One ADD and one REMOVE + diff12 := diff(firstShare, secondShare) + assert.Len(t, diff12, 2, "Should have 2 changes") + var hasAdd, hasRemove bool + for _, update := range diff12 { + if update.Action == sharing.SharedDataObjectUpdateActionAdd { + hasAdd = true + assert.Equal(t, "main.c", update.DataObject.Name) + } + if update.Action == sharing.SharedDataObjectUpdateActionRemove { + hasRemove = true + assert.Equal(t, "main.b", update.DataObject.Name) + } + } + assert.True(t, hasAdd, "Should have ADD action") + assert.True(t, hasRemove, "Should have REMOVE action") + + // Test: One ADD, one REMOVE, one UPDATE + diff13 := diff(firstShare, thirdShare) + assert.Len(t, diff13, 3, "Should have 3 changes") + var hasUpdate bool + hasAdd, hasRemove = false, false + for _, update := range diff13 { + switch update.Action { + case sharing.SharedDataObjectUpdateActionAdd: + hasAdd = true + assert.Equal(t, "main.c", update.DataObject.Name) + case sharing.SharedDataObjectUpdateActionRemove: + hasRemove = true + assert.Equal(t, "main.a", update.DataObject.Name) + case sharing.SharedDataObjectUpdateActionUpdate: + hasUpdate = true + assert.Equal(t, "main.b", update.DataObject.Name) + assert.Equal(t, "d", update.DataObject.Comment) + } + } + assert.True(t, hasAdd, "Should have ADD action") + assert.True(t, hasRemove, "Should have REMOVE action") + assert.True(t, hasUpdate, "Should have UPDATE action") + + // Test: Only UPDATE + diff14 := diff(firstShare, fourthShare) + assert.Len(t, diff14, 1, "Should have 1 UPDATE") + assert.Equal(t, sharing.SharedDataObjectUpdateActionUpdate, diff14[0].Action) + assert.Equal(t, "main.b", diff14[0].DataObject.Name) + assert.Equal(t, "d", diff14[0].DataObject.Comment) +} + +// TestEqual tests the equal function that compares SharedDataObjects +func TestEqual(t *testing.T) { + obj1 := sharing.SharedDataObject{ + Name: "main.table", + DataObjectType: "TABLE", + Comment: "test comment", + SharedAs: "alias", + AddedAt: 123456, + AddedBy: "user@example.com", + Status: "ACTIVE", + } + + obj2 := sharing.SharedDataObject{ + Name: "main.table", + DataObjectType: "TABLE", + Comment: "test comment", + SharedAs: "", // Empty SharedAs should be considered equal to obj1.SharedAs + AddedAt: 999999, // Different computed fields should be ignored + AddedBy: "other@example.com", + Status: "INACTIVE", + } + + obj3 := sharing.SharedDataObject{ + Name: "main.table", + DataObjectType: "TABLE", + Comment: "different comment", // Different comment + SharedAs: "alias", + AddedAt: 123456, + AddedBy: "user@example.com", + Status: "ACTIVE", + } + + // Test: Objects should be equal when only computed fields differ + assert.True(t, equal(obj1, obj2), "Objects should be equal when only computed fields differ") + + // Test: Objects should not be equal when user-defined fields differ + assert.False(t, equal(obj1, obj3), "Objects should not be equal when comment differs") +} + +// TestMatchOrder tests the matchOrder function +func TestMatchOrder(t *testing.T) { + reference := []sharing.SharedDataObject{ + {Name: "table1"}, + {Name: "table2"}, + {Name: "table3"}, + } + + target := []sharing.SharedDataObject{ + {Name: "table3"}, + {Name: "table1"}, + {Name: "table2"}, + } + + matchOrder(target, reference, func(obj sharing.SharedDataObject) string { + return obj.Name + }) + + // Target should now have the same order as reference + assert.Equal(t, "table1", target[0].Name) + assert.Equal(t, "table2", target[1].Name) + assert.Equal(t, "table3", target[2].Name) +} + +// TestSuppressCDFEnabledDiff tests the suppressCDFEnabledDiff function +func TestSuppressCDFEnabledDiff(t *testing.T) { + si := &sharing.ShareInfo{ + Name: "test-share", + Objects: []sharing.SharedDataObject{ + { + Name: "table1", + CdfEnabled: true, + HistoryDataSharingStatus: "ENABLED", + }, + { + Name: "table2", + CdfEnabled: true, + HistoryDataSharingStatus: "DISABLED", + }, + { + Name: "table3", + CdfEnabled: false, + }, + }, + } + + suppressCDFEnabledDiff(si) + + // CdfEnabled should be false when HistoryDataSharingStatus is ENABLED + assert.False(t, si.Objects[0].CdfEnabled, "CdfEnabled should be false when HistoryDataSharingStatus is ENABLED") + // CdfEnabled should remain true when HistoryDataSharingStatus is DISABLED + assert.True(t, si.Objects[1].CdfEnabled, "CdfEnabled should remain true when HistoryDataSharingStatus is DISABLED") + // CdfEnabled should remain false when already false + assert.False(t, si.Objects[2].CdfEnabled, "CdfEnabled should remain false") +} + +// TestShareChanges tests the shareChanges function +func TestShareChanges(t *testing.T) { + si := sharing.ShareInfo{ + Name: "test-share", + Owner: "test-owner", + Objects: []sharing.SharedDataObject{ + { + Name: "table1", + DataObjectType: "TABLE", + }, + { + Name: "table2", + DataObjectType: "TABLE", + }, + }, + } + + // Test ADD action + result := shareChanges(si, "ADD") + assert.Equal(t, "test-share", result.Name) + assert.Equal(t, "test-owner", result.Owner) + assert.Len(t, result.Updates, 2) + for _, update := range result.Updates { + assert.Equal(t, sharing.SharedDataObjectUpdateActionAdd, update.Action) + } + + // Test REMOVE action + result = shareChanges(si, "REMOVE") + assert.Len(t, result.Updates, 2) + for _, update := range result.Updates { + assert.Equal(t, sharing.SharedDataObjectUpdateActionRemove, update.Action) + } +} From 45b007997acaf0ff8716bfe28ce5f9476dc22dcb Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 12:59:51 +0200 Subject: [PATCH 5/7] - --- .../products/sharing/resource_acc_test.go | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/internal/providers/pluginfw/products/sharing/resource_acc_test.go b/internal/providers/pluginfw/products/sharing/resource_acc_test.go index 3127c8d480..1ff3d8d6f9 100644 --- a/internal/providers/pluginfw/products/sharing/resource_acc_test.go +++ b/internal/providers/pluginfw/products/sharing/resource_acc_test.go @@ -376,3 +376,57 @@ func TestUcAccShareMigrationFromSDKv2(t *testing.T) { }, ) } + +// TestUcAccShareMigrationFromPluginFramework tests the transition from plugin framework to sdkv2. +// This test verifies that existing state created by plugin framework implementation can be +// successfully managed by the SDK v2 implementation without any changes. +func TestUcAccShareMigrationFromPluginFramework(t *testing.T) { + acceptance.UnityWorkspaceLevel(t, + // Step 1: Create share using plugin framework implementation + acceptance.Step{ + Template: preTestTemplate + preTestTemplateUpdate + ` + resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-migration-share-rollback" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "Shared table for migration test" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_2.id + comment = "Second shared table" + data_object_type = "TABLE" + cdf_enabled = false + } + }`, + }, + // Step 2: Update the share using SDK v2 (default) + // This verifies no changes are needed when switching implementations + acceptance.Step{ + ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){ + "databricks": func() (tfprotov6.ProviderServer, error) { + sdkv2Provider, pluginfwProvider := acceptance.ProvidersWithResourceFallbacks([]string{"databricks_share"}) + return providers.GetProviderServer(context.Background(), providers.WithSdkV2Provider(sdkv2Provider), providers.WithPluginFrameworkProvider(pluginfwProvider)) + }, + }, + ExpectNonEmptyPlan: false, + Template: preTestTemplate + preTestTemplateUpdate + ` + resource "databricks_share" "myshare" { + name = "{var.STICKY_RANDOM}-terraform-migration-share-rollback" + owner = "account users" + object { + name = databricks_table.mytable.id + comment = "Shared table for migration test" + data_object_type = "TABLE" + } + object { + name = databricks_table.mytable_2.id + comment = "Second shared table" + data_object_type = "TABLE" + cdf_enabled = false + } + }`, + }, + ) +} From 33ee702fdc7f795a30a35a2adfabe2957bc8dc64 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 14:00:41 +0200 Subject: [PATCH 6/7] - --- NEXT_CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index ea3856280d..17b6cd3f01 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -25,3 +25,5 @@ * Resolve references also for map values ([#4944](https://github.com/databricks/terraform-provider-databricks/pull/4944)) ### Internal Changes + +* Make plugin framework implementation of share resource as default ([#4967](https://github.com/databricks/terraform-provider-databricks/pull/4967)) From 2dce3e7878e2d615005bce44e9471cf7be6528af Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi Date: Fri, 22 Aug 2025 14:04:45 +0200 Subject: [PATCH 7/7] - --- .../providers/pluginfw/products/sharing/resource_acc_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/providers/pluginfw/products/sharing/resource_acc_test.go b/internal/providers/pluginfw/products/sharing/resource_acc_test.go index 11791e98c4..1c46557b1b 100644 --- a/internal/providers/pluginfw/products/sharing/resource_acc_test.go +++ b/internal/providers/pluginfw/products/sharing/resource_acc_test.go @@ -6,12 +6,9 @@ import ( "testing" "github.com/databricks/terraform-provider-databricks/internal/acceptance" -<<<<<<< HEAD "github.com/databricks/terraform-provider-databricks/internal/providers" "github.com/hashicorp/terraform-plugin-go/tfprotov6" -======= "github.com/hashicorp/terraform-plugin-testing/terraform" ->>>>>>> share-resource-sdkv2-compatible ) const preTestTemplate = `