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

[Internal] Migrate Share Data Source to Plugin Framework #4161

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internal/providers/pluginfw/pluginfw.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ func (p *DatabricksProviderPluginFramework) DataSources(ctx context.Context) []f
volume.DataSourceVolumes,
registered_model.DataSourceRegisteredModel,
notificationdestinations.DataSourceNotificationDestinations,
sharing.DataSourceShare,
sharing.DataSourceShares,
}
}

Expand Down
79 changes: 79 additions & 0 deletions internal/providers/pluginfw/resources/sharing/data_share.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package sharing

import (
"context"

"github.com/databricks/databricks-sdk-go/apierr"
"github.com/databricks/databricks-sdk-go/service/sharing"
"github.com/databricks/terraform-provider-databricks/common"
pluginfwcommon "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/common"
"github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/converters"
"github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/tfschema"
"github.com/databricks/terraform-provider-databricks/internal/service/sharing_tf"
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
)

func DataSourceShare() datasource.DataSource {
return &ShareDataSource{}
}

var _ datasource.DataSourceWithConfigure = &ShareDataSource{}

type ShareDataSource struct {
Client *common.DatabricksClient
}

func (d *ShareDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) {
resp.TypeName = pluginfwcommon.GetDatabricksStagingName("share")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment below.

}

func (d *ShareDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) {
attrs, blocks := tfschema.DataSourceStructToSchemaMap(sharing_tf.ShareInfo{}, nil)
resp.Schema = schema.Schema{
Attributes: attrs,
Blocks: blocks,
}
}

func (d *ShareDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) {
if d.Client == nil {
d.Client = pluginfwcommon.ConfigureDataSource(req, resp)
}
}

func (d *ShareDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
w, diags := d.Client.GetWorkspaceClient()
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

var config sharing_tf.ShareInfo
diags = req.Config.Get(ctx, &config)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

share, err := w.Shares.Get(ctx, sharing.GetShareRequest{
Name: config.Name.ValueString(),
IncludeSharedData: true,
})
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove from state if missing:


		if apierr.IsMissing(err) {
			resp.State.RemoveResource(ctx)
		}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know that's needed :) b7c32b7

if apierr.IsMissing(err) {
resp.State.RemoveResource(ctx)
}

resp.Diagnostics.AddError("Failed to fetch share", err.Error())
return
}

var shareInfoTfSdk sharing_tf.ShareInfo
resp.Diagnostics.Append(converters.GoSdkToTfSdkStruct(ctx, share, &shareInfoTfSdk)...)
if resp.Diagnostics.HasError() {
return
}

resp.Diagnostics.Append(resp.State.Set(ctx, shareInfoTfSdk)...)
}
67 changes: 67 additions & 0 deletions internal/providers/pluginfw/resources/sharing/data_shares.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package sharing

import (
"context"

"github.com/hashicorp/terraform-plugin-framework/types"

"github.com/databricks/databricks-sdk-go/service/sharing"
"github.com/databricks/terraform-provider-databricks/common"
pluginfwcommon "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/common"
"github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/tfschema"
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/datasource/schema"
)

type SharesList struct {
Shares []types.String `tfsdk:"shares" tf:"computed,optional,slice_set"`
}

func DataSourceShares() datasource.DataSource {
return &SharesDataSource{}
}

var _ datasource.DataSourceWithConfigure = &SharesDataSource{}

type SharesDataSource struct {
Client *common.DatabricksClient
}

func (d *SharesDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) {
resp.TypeName = pluginfwcommon.GetDatabricksStagingName("shares")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I planned to use a dataSourceName constant for both the share and shares data sources. However, since they share the same namespace, that wasn’t an option. So, I just included the constant within the Metadata function, particularly since it’s only used once in each data source. Alternatively, I could assign a dataSourceName constant to one data source and rename it for the other, but that didn't feel quite right.

Let me know if you have a preferred approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yeah we could use dataSourceNameShare, dataSourceNameShares, mentioning this because we would also need to set the useragent (eg: https://github.com/databricks/terraform-provider-databricks/blob/main/internal/providers/pluginfw/resources/volume/data_volumes.go#L56) and use the same name there as well so might be better to abstract this in a single variable.

}

func (d *SharesDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) {
attrs, blocks := tfschema.DataSourceStructToSchemaMap(SharesList{}, nil)
resp.Schema = schema.Schema{
Attributes: attrs,
Blocks: blocks,
}
}

func (d *SharesDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) {
if d.Client == nil {
d.Client = pluginfwcommon.ConfigureDataSource(req, resp)
}
}

func (d *SharesDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
w, diags := d.Client.GetWorkspaceClient()
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

shares, err := w.Shares.ListAll(ctx, sharing.ListSharesRequest{})
if err != nil {
resp.Diagnostics.AddError("Failed to fetch shares", err.Error())
return
}

shareNames := make([]types.String, len(shares))
for i, share := range shares {
shareNames[i] = types.StringValue(share.Name)
}

resp.Diagnostics.Append(resp.State.Set(ctx, SharesList{Shares: shareNames})...)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package sharing_test

import (
"strconv"
"testing"

"github.com/databricks/terraform-provider-databricks/internal/acceptance"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func checkSharesDataSourcePopulated(t *testing.T) func(s *terraform.State) error {
return func(s *terraform.State) error {
_, ok := s.Modules[0].Resources["data.databricks_shares_pluginframework.this"]
require.True(t, ok, "data.databricks_shares_pluginframework.this has to be there")
num_shares, _ := strconv.Atoi(s.Modules[0].Outputs["shares"].Value.(string))
assert.GreaterOrEqual(t, num_shares, 1)
return nil
}
}
func TestUcAccDataSourceShares(t *testing.T) {
acceptance.UnityWorkspaceLevel(t, acceptance.Step{
Template: `
resource "databricks_catalog" "sandbox" {
name = "sandbox{var.RANDOM}"
comment = "this catalog is managed by terraform"
properties = {
purpose = "testing"
}
}

resource "databricks_schema" "things" {
catalog_name = databricks_catalog.sandbox.id
name = "things{var.RANDOM}"
comment = "this database is managed by terraform"
properties = {
kind = "various"
}
}

resource "databricks_table" "mytable" {
catalog_name = databricks_catalog.sandbox.id
schema_name = databricks_schema.things.name
name = "bar"
table_type = "MANAGED"
data_source_format = "DELTA"

column {
name = "id"
position = 0
type_name = "INT"
type_text = "int"
type_json = "{\"name\":\"id\",\"type\":\"integer\",\"nullable\":true,\"metadata\":{}}"
}
}

resource "databricks_table" "mytable_2" {
catalog_name = databricks_catalog.sandbox.id
schema_name = databricks_schema.things.name
name = "bar_2"
table_type = "MANAGED"
data_source_format = "DELTA"

column {
name = "id"
position = 0
type_name = "INT"
type_text = "int"
type_json = "{\"name\":\"id\",\"type\":\"integer\",\"nullable\":true,\"metadata\":{}}"
}
}

resource "databricks_share_pluginframework" "myshare" {
name = "{var.RANDOM}-terraform-delta-share"
object {
name = databricks_table.mytable.id
comment = "c"
data_object_type = "TABLE"
}
object {
name = databricks_table.mytable_2.id
cdf_enabled = false
comment = "c"
data_object_type = "TABLE"
}
}

data "databricks_shares_pluginframework" "this" {
depends_on = [databricks_share_pluginframework.myshare]
}
output "shares" {
value = length(data.databricks_shares_pluginframework.this.shares)
}
`,
Check: checkSharesDataSourcePopulated(t),
})
}
Loading