From f01e32c2006f5b0efd29100c8eac5cd33faf2525 Mon Sep 17 00:00:00 2001 From: Tanmay Rustagi <88379306+tanmay-db@users.noreply.github.com> Date: Tue, 27 Aug 2024 16:15:28 +0200 Subject: [PATCH] [Internal] Refactor provider and related packages (#3940) ## Changes ## Package organization for Providers We are migrating the resource from SDKv2 to Plugin Framework provider and hence both of them exist in the codebase. For uniform code convention, readability and development, they are organized in the `internal/providers` directory under root as follows: - `providers`: Contains the changes that `depends` on both internal/providers/sdkv2 and internal/providers/pluginfw packages, eg: `GetProviderServer`. - `common`: Contains the changes `used by` both internal/providers/sdkv2 and internal/providers/pluginfw packages, eg: `ProviderName`. - `pluginfw`: Contains the changes specific to Plugin Framework. This package shouldn't depend on sdkv2 or common. - `sdkv2`: Contains the changes specific to SDKv2. This package shouldn't depend on pluginfw or common. This has been updated in CONTRIBUTING.md Package comments have been added, also some tests have been refactored to reduce redundancy example: we define test data path at multiple places. ## Tests Modified existing tests - [ ] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] relevant acceptance tests are passing - [ ] using Go SDK --- CONTRIBUTING.md | 8 +++ exporter/context.go | 4 +- exporter/context_test.go | 4 +- exporter/importables_test.go | 4 +- internal/acceptance/init_test.go | 4 +- internal/acceptance/mws_workspaces_test.go | 6 +-- internal/acceptance/tf_provider_test.go | 4 +- internal/providers/common/common.go | 8 +++ .../pluginfw}/converters/converters_test.go | 0 .../pluginfw}/converters/go_to_tf.go | 0 .../pluginfw}/converters/tf_to_go.go | 0 .../pluginfw/pluginfw.go} | 12 ++--- .../pluginfw}/tfschema/attribute_builder.go | 0 .../pluginfw}/tfschema/bool_attribute.go | 0 .../pluginfw}/tfschema/customizable_schema.go | 0 .../tfschema/customizable_schema_test.go | 0 .../pluginfw}/tfschema/float64_attribute.go | 0 .../pluginfw}/tfschema/int64_attribute.go | 0 .../pluginfw}/tfschema/list_attribute.go | 0 .../tfschema/list_nested_attribute.go | 0 .../pluginfw}/tfschema/map_attribute.go | 0 .../tfschema/map_nested_attribute.go | 0 .../tfschema/nested_attribute_object.go | 0 .../tfschema/single_nested_attribute.go | 0 .../pluginfw}/tfschema/string_attribute.go | 0 .../pluginfw}/tfschema/struct_to_schema.go | 0 .../tfschema/struct_to_schema_test.go | 0 .../providers/providers.go | 12 +++-- .../providers/providers_test.go | 49 ++++++++++--------- .../providers/providers_test_utils.go | 11 +++-- .../providers/sdkv2}/gen/data_x.go.tmpl | 0 .../providers/sdkv2}/gen/data_x.md.go.tmpl | 0 .../sdkv2}/gen/data_x_acctest.go.tmpl | 0 .../sdkv2}/gen/data_x_manual.go.tmpl | 0 .../providers/sdkv2}/gen/data_x_test.go.tmpl | 0 .../providers/sdkv2}/gen/main.go | 0 .../providers/sdkv2/sdkv2.go | 19 ++++--- .../providers/sdkv2/tests}/coverage_test.go | 5 +- .../providers/sdkv2/tests}/generate_test.go | 5 +- main.go | 4 +- 40 files changed, 94 insertions(+), 65 deletions(-) create mode 100644 internal/providers/common/common.go rename internal/{pluginframework => providers/pluginfw}/converters/converters_test.go (100%) rename internal/{pluginframework => providers/pluginfw}/converters/go_to_tf.go (100%) rename internal/{pluginframework => providers/pluginfw}/converters/tf_to_go.go (100%) rename internal/{pluginframework/provider/provider_plugin_framework.go => providers/pluginfw/pluginfw.go} (94%) rename internal/{pluginframework => providers/pluginfw}/tfschema/attribute_builder.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/bool_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/customizable_schema.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/customizable_schema_test.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/float64_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/int64_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/list_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/list_nested_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/map_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/map_nested_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/nested_attribute_object.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/single_nested_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/string_attribute.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/struct_to_schema.go (100%) rename internal/{pluginframework => providers/pluginfw}/tfschema/struct_to_schema_test.go (100%) rename provider/provider_factory.go => internal/providers/providers.go (71%) rename provider/provider_test.go => internal/providers/providers_test.go (92%) rename provider/test_utils.go => internal/providers/providers_test_utils.go (94%) rename {provider => internal/providers/sdkv2}/gen/data_x.go.tmpl (100%) rename {provider => internal/providers/sdkv2}/gen/data_x.md.go.tmpl (100%) rename {provider => internal/providers/sdkv2}/gen/data_x_acctest.go.tmpl (100%) rename {provider => internal/providers/sdkv2}/gen/data_x_manual.go.tmpl (100%) rename {provider => internal/providers/sdkv2}/gen/data_x_test.go.tmpl (100%) rename {provider => internal/providers/sdkv2}/gen/main.go (100%) rename provider/provider.go => internal/providers/sdkv2/sdkv2.go (97%) rename {provider => internal/providers/sdkv2/tests}/coverage_test.go (98%) rename {provider => internal/providers/sdkv2/tests}/generate_test.go (98%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 753594307f..ac42672749 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -109,6 +109,14 @@ provider_installation { After installing the necessary software for building provider from sources, you should be able to run `make coverage` to run the tests and see the coverage. +## Package organization for Providers +We are migrating the resource from SDKv2 to Plugin Framework provider and hence both of them exist in the codebase. For uniform code convention, readability and development, they are organized in the `internal/providers` directory under root as follows: +- `providers`: Contains the changes that `depends` on both internal/providers/sdkv2 and internal/providers/pluginfw packages, eg: `GetProviderServer`. +- `common`: Contains the changes `used by` both internal/providers/sdkv2 and internal/providers/pluginfw packages, eg: `ProviderName`. +- `pluginfw`: Contains the changes specific to Plugin Framework. This package shouldn't depend on sdkv2 or common. +- `sdkv2`: Contains the changes specific to SDKv2. This package shouldn't depend on pluginfw or common. + + ## Debugging **TF_LOG_PROVIDER=DEBUG terraform apply** allows you to see the internal logs from `terraform apply`. diff --git a/exporter/context.go b/exporter/context.go index 122f43ad87..9fbfcb20d6 100644 --- a/exporter/context.go +++ b/exporter/context.go @@ -22,7 +22,7 @@ import ( "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" - "github.com/databricks/terraform-provider-databricks/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/databricks/terraform-provider-databricks/scim" "github.com/databricks/terraform-provider-databricks/workspace" @@ -221,7 +221,7 @@ func makeResourcesChannels() map[string]resourceChannel { } func newImportContext(c *common.DatabricksClient) *importContext { - p := provider.DatabricksProvider() + p := sdkv2.DatabricksProvider() p.TerraformVersion = "exporter" p.SetMeta(c) ctx := context.WithValue(context.Background(), common.Provider, p) diff --git a/exporter/context_test.go b/exporter/context_test.go index d27e35925e..8d581c0cc8 100644 --- a/exporter/context_test.go +++ b/exporter/context_test.go @@ -7,7 +7,7 @@ import ( "sync" "testing" - "github.com/databricks/terraform-provider-databricks/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/databricks/terraform-provider-databricks/qa" "github.com/databricks/terraform-provider-databricks/workspace" "github.com/hashicorp/hcl/v2/hclwrite" @@ -373,7 +373,7 @@ func TestLoadingLastRun(t *testing.T) { } func TestGenerateResourceIdForWsObject(t *testing.T) { - p := provider.DatabricksProvider() + p := sdkv2.DatabricksProvider() ic := &importContext{ Importables: resourcesMap, Resources: p.ResourcesMap, diff --git a/exporter/importables_test.go b/exporter/importables_test.go index 61bc06d998..e18d9d163e 100644 --- a/exporter/importables_test.go +++ b/exporter/importables_test.go @@ -26,10 +26,10 @@ import ( "github.com/databricks/terraform-provider-databricks/jobs" "github.com/databricks/terraform-provider-databricks/permissions" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" dlt_pipelines "github.com/databricks/terraform-provider-databricks/pipelines" "github.com/databricks/terraform-provider-databricks/policies" "github.com/databricks/terraform-provider-databricks/pools" - "github.com/databricks/terraform-provider-databricks/provider" "github.com/databricks/terraform-provider-databricks/qa" "github.com/databricks/terraform-provider-databricks/repos" "github.com/databricks/terraform-provider-databricks/scim" @@ -45,7 +45,7 @@ import ( ) func importContextForTest() *importContext { - p := provider.DatabricksProvider() + p := sdkv2.DatabricksProvider() supportedResources := maps.Keys(resourcesMap) return &importContext{ Importables: resourcesMap, diff --git a/internal/acceptance/init_test.go b/internal/acceptance/init_test.go index 39cf736812..01fdc90c56 100644 --- a/internal/acceptance/init_test.go +++ b/internal/acceptance/init_test.go @@ -21,8 +21,8 @@ import ( "github.com/databricks/databricks-sdk-go/logger" "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" dbproviderlogger "github.com/databricks/terraform-provider-databricks/logger" - "github.com/databricks/terraform-provider-databricks/provider" "github.com/databricks/terraform-provider-databricks/qa" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -138,7 +138,7 @@ func run(t *testing.T, steps []step) { t.Skip("Acceptance tests skipped unless env 'CLOUD_ENV' is set") } t.Parallel() - provider := provider.DatabricksProvider() + provider := sdkv2.DatabricksProvider() cwd, err := os.Getwd() if err != nil { t.Skip(err.Error()) diff --git a/internal/acceptance/mws_workspaces_test.go b/internal/acceptance/mws_workspaces_test.go index b289dca063..08e60f9842 100644 --- a/internal/acceptance/mws_workspaces_test.go +++ b/internal/acceptance/mws_workspaces_test.go @@ -11,7 +11,7 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/logger" "github.com/databricks/terraform-provider-databricks/common" - "github.com/databricks/terraform-provider-databricks/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/databricks/terraform-provider-databricks/tokens" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -378,14 +378,14 @@ func TestMwsAccAwsChangeToServicePrincipal(t *testing.T) { spId := s.RootModule().Resources["databricks_service_principal.this"].Primary.ID spAppId := s.RootModule().Resources["databricks_service_principal.this"].Primary.Attributes["application_id"] spSecret := s.RootModule().Resources["databricks_service_principal_secret.this"].Primary.Attributes["secret"] - pr = provider.DatabricksProvider() + pr = sdkv2.DatabricksProvider() rd := schema.TestResourceDataRaw(t, pr.Schema, map[string]interface{}{ "client_id": spAppId, "client_secret": spSecret, }) fmt.Printf("client_id: %s, client_secret: %s\n", spAppId, spSecret) pr.ConfigureContextFunc = func(ctx context.Context, c *schema.ResourceData) (interface{}, diag.Diagnostics) { - return provider.ConfigureDatabricksClient(ctx, rd) + return sdkv2.ConfigureDatabricksClient(ctx, rd) } logger.DefaultLogger = &logger.SimpleLogger{ Level: logger.LevelDebug, diff --git a/internal/acceptance/tf_provider_test.go b/internal/acceptance/tf_provider_test.go index d2f83ebbb8..7f29105137 100644 --- a/internal/acceptance/tf_provider_test.go +++ b/internal/acceptance/tf_provider_test.go @@ -3,7 +3,7 @@ package acceptance import ( "testing" - "github.com/databricks/terraform-provider-databricks/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -28,7 +28,7 @@ func TestAccProviderPlanShouldSucceedWithIncompleteConfiguration(t *testing.T) { resource.Test(t, resource.TestCase{ IsUnitTest: true, ProviderFactories: map[string]func() (*schema.Provider, error){ - "databricks": func() (*schema.Provider, error) { return provider.DatabricksProvider(), nil }, + "databricks": func() (*schema.Provider, error) { return sdkv2.DatabricksProvider(), nil }, "noop": func() (*schema.Provider, error) { return noOpProvider(), nil }, }, Steps: []resource.TestStep{ diff --git a/internal/providers/common/common.go b/internal/providers/common/common.go new file mode 100644 index 0000000000..7be08e9302 --- /dev/null +++ b/internal/providers/common/common.go @@ -0,0 +1,8 @@ +// Package common contains the changes used by both internal/providers/sdkv2 and internal/providers/pluginfw packages. +// +// Note: This is different from internal/providers which contains the changes that *depends* on both: +// internal/providers/sdkv2 and internal/providers/pluginfw packages. Whereas, internal/providers/common package contains +// the changes *used* by both internal/providers/sdkv2 and internal/providers/pluginfw packages. +package internal + +const ProviderName = "databricks-tf-provider" diff --git a/internal/pluginframework/converters/converters_test.go b/internal/providers/pluginfw/converters/converters_test.go similarity index 100% rename from internal/pluginframework/converters/converters_test.go rename to internal/providers/pluginfw/converters/converters_test.go diff --git a/internal/pluginframework/converters/go_to_tf.go b/internal/providers/pluginfw/converters/go_to_tf.go similarity index 100% rename from internal/pluginframework/converters/go_to_tf.go rename to internal/providers/pluginfw/converters/go_to_tf.go diff --git a/internal/pluginframework/converters/tf_to_go.go b/internal/providers/pluginfw/converters/tf_to_go.go similarity index 100% rename from internal/pluginframework/converters/tf_to_go.go rename to internal/providers/pluginfw/converters/tf_to_go.go diff --git a/internal/pluginframework/provider/provider_plugin_framework.go b/internal/providers/pluginfw/pluginfw.go similarity index 94% rename from internal/pluginframework/provider/provider_plugin_framework.go rename to internal/providers/pluginfw/pluginfw.go index 7f04921189..77de6d0b3c 100644 --- a/internal/pluginframework/provider/provider_plugin_framework.go +++ b/internal/providers/pluginfw/pluginfw.go @@ -1,4 +1,7 @@ -package provider +// Package pluginfw contains the changes specific to the plugin framework +// +// Note: This shouldn't depend on internal/providers/sdkv2 or internal/providers +package pluginfw import ( "context" @@ -12,6 +15,7 @@ import ( "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" + providercommon "github.com/databricks/terraform-provider-databricks/internal/providers/common" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -23,10 +27,6 @@ import ( "github.com/hashicorp/terraform-plugin-log/tflog" ) -func GetProviderName() string { - return "databricks-tf-provider" -} - func GetDatabricksProviderPluginFramework() provider.Provider { p := &DatabricksProviderPluginFramework{} return p @@ -50,7 +50,7 @@ func (p *DatabricksProviderPluginFramework) Schema(ctx context.Context, req prov } func (p *DatabricksProviderPluginFramework) Metadata(ctx context.Context, req provider.MetadataRequest, resp *provider.MetadataResponse) { - resp.TypeName = GetProviderName() + resp.TypeName = providercommon.ProviderName resp.Version = common.Version() } diff --git a/internal/pluginframework/tfschema/attribute_builder.go b/internal/providers/pluginfw/tfschema/attribute_builder.go similarity index 100% rename from internal/pluginframework/tfschema/attribute_builder.go rename to internal/providers/pluginfw/tfschema/attribute_builder.go diff --git a/internal/pluginframework/tfschema/bool_attribute.go b/internal/providers/pluginfw/tfschema/bool_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/bool_attribute.go rename to internal/providers/pluginfw/tfschema/bool_attribute.go diff --git a/internal/pluginframework/tfschema/customizable_schema.go b/internal/providers/pluginfw/tfschema/customizable_schema.go similarity index 100% rename from internal/pluginframework/tfschema/customizable_schema.go rename to internal/providers/pluginfw/tfschema/customizable_schema.go diff --git a/internal/pluginframework/tfschema/customizable_schema_test.go b/internal/providers/pluginfw/tfschema/customizable_schema_test.go similarity index 100% rename from internal/pluginframework/tfschema/customizable_schema_test.go rename to internal/providers/pluginfw/tfschema/customizable_schema_test.go diff --git a/internal/pluginframework/tfschema/float64_attribute.go b/internal/providers/pluginfw/tfschema/float64_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/float64_attribute.go rename to internal/providers/pluginfw/tfschema/float64_attribute.go diff --git a/internal/pluginframework/tfschema/int64_attribute.go b/internal/providers/pluginfw/tfschema/int64_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/int64_attribute.go rename to internal/providers/pluginfw/tfschema/int64_attribute.go diff --git a/internal/pluginframework/tfschema/list_attribute.go b/internal/providers/pluginfw/tfschema/list_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/list_attribute.go rename to internal/providers/pluginfw/tfschema/list_attribute.go diff --git a/internal/pluginframework/tfschema/list_nested_attribute.go b/internal/providers/pluginfw/tfschema/list_nested_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/list_nested_attribute.go rename to internal/providers/pluginfw/tfschema/list_nested_attribute.go diff --git a/internal/pluginframework/tfschema/map_attribute.go b/internal/providers/pluginfw/tfschema/map_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/map_attribute.go rename to internal/providers/pluginfw/tfschema/map_attribute.go diff --git a/internal/pluginframework/tfschema/map_nested_attribute.go b/internal/providers/pluginfw/tfschema/map_nested_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/map_nested_attribute.go rename to internal/providers/pluginfw/tfschema/map_nested_attribute.go diff --git a/internal/pluginframework/tfschema/nested_attribute_object.go b/internal/providers/pluginfw/tfschema/nested_attribute_object.go similarity index 100% rename from internal/pluginframework/tfschema/nested_attribute_object.go rename to internal/providers/pluginfw/tfschema/nested_attribute_object.go diff --git a/internal/pluginframework/tfschema/single_nested_attribute.go b/internal/providers/pluginfw/tfschema/single_nested_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/single_nested_attribute.go rename to internal/providers/pluginfw/tfschema/single_nested_attribute.go diff --git a/internal/pluginframework/tfschema/string_attribute.go b/internal/providers/pluginfw/tfschema/string_attribute.go similarity index 100% rename from internal/pluginframework/tfschema/string_attribute.go rename to internal/providers/pluginfw/tfschema/string_attribute.go diff --git a/internal/pluginframework/tfschema/struct_to_schema.go b/internal/providers/pluginfw/tfschema/struct_to_schema.go similarity index 100% rename from internal/pluginframework/tfschema/struct_to_schema.go rename to internal/providers/pluginfw/tfschema/struct_to_schema.go diff --git a/internal/pluginframework/tfschema/struct_to_schema_test.go b/internal/providers/pluginfw/tfschema/struct_to_schema_test.go similarity index 100% rename from internal/pluginframework/tfschema/struct_to_schema_test.go rename to internal/providers/pluginfw/tfschema/struct_to_schema_test.go diff --git a/provider/provider_factory.go b/internal/providers/providers.go similarity index 71% rename from provider/provider_factory.go rename to internal/providers/providers.go index 2e9f5ef32b..a439c02471 100644 --- a/provider/provider_factory.go +++ b/internal/providers/providers.go @@ -1,10 +1,14 @@ -package provider +// Package providers contains the changes for both SDKv2 and Plugin Framework which are defined in their respective sub packages. +// +// Note: Top level files under providers package only contains the changes that depends on both internal/providers/sdkv2 and internal/providers/pluginfw packages +package providers import ( "context" "log" - pluginframeworkprovider "github.com/databricks/terraform-provider-databricks/internal/pluginframework/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-framework/providerserver" "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-mux/tf5to6server" @@ -20,7 +24,7 @@ import ( // Protocol v6 providers to be served together. The function returns the multiplexed // ProviderServer, or an error if any part of the process fails. func GetProviderServer(ctx context.Context) (tfprotov6.ProviderServer, error) { - sdkPluginProvider := DatabricksProvider() + sdkPluginProvider := sdkv2.DatabricksProvider() upgradedSdkPluginProvider, err := tf5to6server.UpgradeServer( context.Background(), @@ -30,7 +34,7 @@ func GetProviderServer(ctx context.Context) (tfprotov6.ProviderServer, error) { log.Fatal(err) } - pluginFrameworkProvider := pluginframeworkprovider.GetDatabricksProviderPluginFramework() + pluginFrameworkProvider := pluginfw.GetDatabricksProviderPluginFramework() providers := []func() tfprotov6.ProviderServer{ func() tfprotov6.ProviderServer { diff --git a/provider/provider_test.go b/internal/providers/providers_test.go similarity index 92% rename from provider/provider_test.go rename to internal/providers/providers_test.go index 6c1c0bbf34..856f579e64 100644 --- a/provider/provider_test.go +++ b/internal/providers/providers_test.go @@ -1,4 +1,4 @@ -package provider +package providers import ( "context" @@ -12,6 +12,7 @@ import ( "time" "github.com/databricks/terraform-provider-databricks/common" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -179,7 +180,7 @@ func TestConfig_PatFromDatabricksCfg(t *testing.T) { providerFixture{ // loading with DEFAULT profile in databrickscfs env: map[string]string{ - "HOME": "../common/testdata", + "HOME": testDataPath, }, assertHost: "https://dbc-XXXXXXXX-YYYY.cloud.databricks.com", assertAuth: "pat", @@ -190,7 +191,7 @@ func TestConfig_PatFromDatabricksCfg_NohostProfile(t *testing.T) { providerFixture{ // loading with nohost profile in databrickscfs env: map[string]string{ - "HOME": "../common/testdata", + "HOME": testDataPath, "DATABRICKS_CONFIG_PROFILE": "nohost", }, assertError: common.NoAuth + @@ -203,7 +204,7 @@ func TestConfig_ConfigProfileAndToken(t *testing.T) { env: map[string]string{ "DATABRICKS_TOKEN": "x", "DATABRICKS_CONFIG_PROFILE": "nohost", - "HOME": "../common/testdata", + "HOME": testDataPath, }, assertError: common.NoAuth + ". Config: token=***, profile=nohost. Env: DATABRICKS_TOKEN, DATABRICKS_CONFIG_PROFILE", @@ -215,7 +216,7 @@ func TestConfig_ConfigProfileAndPassword(t *testing.T) { env: map[string]string{ "DATABRICKS_USERNAME": "x", "DATABRICKS_CONFIG_PROFILE": "nohost", - "HOME": "../common/testdata", + "HOME": testDataPath, }, assertError: "validate: more than one authorization method configured: basic and pat. " + "Config: token=***, username=x, profile=nohost. Env: DATABRICKS_USERNAME, DATABRICKS_CONFIG_PROFILE", @@ -225,7 +226,7 @@ func TestConfig_ConfigProfileAndPassword(t *testing.T) { var azResourceID = "/subscriptions/a/resourceGroups/b/providers/Microsoft.Databricks/workspaces/c" func TestConfig_AzureCliHost(t *testing.T) { - p, _ := filepath.Abs("../common/testdata") + p, _ := filepath.Abs(testDataPath) providerFixture{ // this test will skip ensureWorkspaceUrl host: "x", @@ -243,7 +244,7 @@ func TestConfig_AzureCliHost(t *testing.T) { } func TestConfig_AzureCliHost_Fail(t *testing.T) { - p, _ := filepath.Abs("../common/testdata") + p, _ := filepath.Abs(testDataPath) providerFixture{ azureResourceID: azResourceID, env: map[string]string{ @@ -262,14 +263,14 @@ func TestConfig_AzureCliHost_AzNotInstalled(t *testing.T) { azureResourceID: azResourceID, env: map[string]string{ "PATH": "whatever", - "HOME": "../common/testdata", + "HOME": "../../common/testdata", }, assertError: common.NoAuth, }.apply(t) } func TestConfig_AzureCliHost_PatConflict(t *testing.T) { - p, _ := filepath.Abs("../common/testdata") + p, _ := filepath.Abs(testDataPath) providerFixture{ azureResourceID: azResourceID, token: "x", @@ -283,7 +284,7 @@ func TestConfig_AzureCliHost_PatConflict(t *testing.T) { } func TestConfig_AzureCliHostAndResourceID(t *testing.T) { - p, _ := filepath.Abs("../common/testdata") + p, _ := filepath.Abs(testDataPath) providerFixture{ // omit request to management endpoint to get workspace properties azureResourceID: azResourceID, @@ -301,7 +302,7 @@ func TestConfig_AzureCliHostAndResourceID(t *testing.T) { } func TestConfig_AzureAndPasswordConflict(t *testing.T) { - p, _ := filepath.Abs("../common/testdata") + p, _ := filepath.Abs(testDataPath) providerFixture{ host: "x", azureResourceID: azResourceID, @@ -318,7 +319,7 @@ func TestConfig_AzureAndPasswordConflict(t *testing.T) { func TestConfig_CorruptConfig(t *testing.T) { providerFixture{ env: map[string]string{ - "HOME": "../common/testdata/corrupt", + "HOME": testDataPath + "/corrupt", }, assertError: common.NoAuth, }.apply(t) @@ -390,7 +391,7 @@ type parseUserAgentTestCase struct { name string env string err error - out []userAgentExtra + out []sdkv2.UserAgentExtra } func Test_ParseUserAgentExtra(t *testing.T) { @@ -399,34 +400,34 @@ func Test_ParseUserAgentExtra(t *testing.T) { name: "single product", env: "databricks-cli/0.1.2", err: nil, - out: []userAgentExtra{ - {"databricks-cli", "0.1.2"}, + out: []sdkv2.UserAgentExtra{ + {Key: "databricks-cli", Value: "0.1.2"}, }, }, { name: "multiple products", env: "databricks-cli/0.1.2 custom-thing/0.0.1", err: nil, - out: []userAgentExtra{ - {"databricks-cli", "0.1.2"}, - {"custom-thing", "0.0.1"}, + out: []sdkv2.UserAgentExtra{ + {Key: "databricks-cli", Value: "0.1.2"}, + {Key: "custom-thing", Value: "0.0.1"}, }, }, { name: "multiple products with many separators", env: "\ta/0.0.1\tb/0.0.2 \t c/0.0.3", err: nil, - out: []userAgentExtra{ - {"a", "0.0.1"}, - {"b", "0.0.2"}, - {"c", "0.0.3"}, + out: []sdkv2.UserAgentExtra{ + {Key: "a", Value: "0.0.1"}, + {Key: "b", Value: "0.0.2"}, + {Key: "c", Value: "0.0.3"}, }, }, { name: "empty string", env: "", err: nil, - out: []userAgentExtra{}, + out: []sdkv2.UserAgentExtra{}, }, { name: "product with comment", @@ -450,7 +451,7 @@ func Test_ParseUserAgentExtra(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - out, err := parseUserAgentExtra(tc.env) + out, err := sdkv2.ParseUserAgentExtra(tc.env) assert.Equal(t, tc.err, err) assert.Equal(t, tc.out, out) diff --git a/provider/test_utils.go b/internal/providers/providers_test_utils.go similarity index 94% rename from provider/test_utils.go rename to internal/providers/providers_test_utils.go index 5193835f33..a3e9cfdd51 100644 --- a/provider/test_utils.go +++ b/internal/providers/providers_test_utils.go @@ -1,4 +1,4 @@ -package provider +package providers import ( "context" @@ -8,7 +8,8 @@ import ( "testing" "github.com/databricks/terraform-provider-databricks/common" - pluginframeworkprovider "github.com/databricks/terraform-provider-databricks/internal/pluginframework/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-framework/provider" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -36,6 +37,8 @@ type providerFixture struct { assertAzure bool } +const testDataPath = "../../common/testdata" + func (pf providerFixture) rawConfig() map[string]string { rawConfig := map[string]string{} if pf.host != "" { @@ -138,7 +141,7 @@ func (pf providerFixture) configureProviderAndReturnClient_SDKv2(t *testing.T) ( for k, v := range pf.env { t.Setenv(k, v) } - p := DatabricksProvider() + p := sdkv2.DatabricksProvider() ctx := context.Background() diags := p.Configure(ctx, terraform.NewResourceConfigRaw(pf.rawConfigSDKv2())) if len(diags) > 0 { @@ -164,7 +167,7 @@ func (pf providerFixture) configureProviderAndReturnClient_PluginFramework(t *te for k, v := range pf.env { t.Setenv(k, v) } - p := pluginframeworkprovider.GetDatabricksProviderPluginFramework() + p := pluginfw.GetDatabricksProviderPluginFramework() ctx := context.Background() rawConfig := pf.rawConfigPluginFramework() var providerSchemaResponse provider.SchemaResponse diff --git a/provider/gen/data_x.go.tmpl b/internal/providers/sdkv2/gen/data_x.go.tmpl similarity index 100% rename from provider/gen/data_x.go.tmpl rename to internal/providers/sdkv2/gen/data_x.go.tmpl diff --git a/provider/gen/data_x.md.go.tmpl b/internal/providers/sdkv2/gen/data_x.md.go.tmpl similarity index 100% rename from provider/gen/data_x.md.go.tmpl rename to internal/providers/sdkv2/gen/data_x.md.go.tmpl diff --git a/provider/gen/data_x_acctest.go.tmpl b/internal/providers/sdkv2/gen/data_x_acctest.go.tmpl similarity index 100% rename from provider/gen/data_x_acctest.go.tmpl rename to internal/providers/sdkv2/gen/data_x_acctest.go.tmpl diff --git a/provider/gen/data_x_manual.go.tmpl b/internal/providers/sdkv2/gen/data_x_manual.go.tmpl similarity index 100% rename from provider/gen/data_x_manual.go.tmpl rename to internal/providers/sdkv2/gen/data_x_manual.go.tmpl diff --git a/provider/gen/data_x_test.go.tmpl b/internal/providers/sdkv2/gen/data_x_test.go.tmpl similarity index 100% rename from provider/gen/data_x_test.go.tmpl rename to internal/providers/sdkv2/gen/data_x_test.go.tmpl diff --git a/provider/gen/main.go b/internal/providers/sdkv2/gen/main.go similarity index 100% rename from provider/gen/main.go rename to internal/providers/sdkv2/gen/main.go diff --git a/provider/provider.go b/internal/providers/sdkv2/sdkv2.go similarity index 97% rename from provider/provider.go rename to internal/providers/sdkv2/sdkv2.go index 0b954fc620..b9ee686121 100644 --- a/provider/provider.go +++ b/internal/providers/sdkv2/sdkv2.go @@ -1,4 +1,7 @@ -package provider +// Package sdkv2 contains the changes specific to the SDKv2 +// +// Note: This package shouldn't depend on internal/providers/pluginfw or internal/providers +package sdkv2 import ( "context" @@ -26,7 +29,7 @@ import ( "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" "github.com/databricks/terraform-provider-databricks/dashboards" - pluginframeworkprovider "github.com/databricks/terraform-provider-databricks/internal/pluginframework/provider" + providercommon "github.com/databricks/terraform-provider-databricks/internal/providers/common" "github.com/databricks/terraform-provider-databricks/jobs" "github.com/databricks/terraform-provider-databricks/logger" "github.com/databricks/terraform-provider-databricks/mlflow" @@ -51,10 +54,10 @@ import ( func init() { // IMPORTANT: this line cannot be changed, because it's used for // internal purposes at Databricks. - useragent.WithProduct(pluginframeworkprovider.GetProviderName(), common.Version()) + useragent.WithProduct(providercommon.ProviderName, common.Version()) userAgentExtraEnv := os.Getenv("DATABRICKS_USER_AGENT_EXTRA") - out, err := parseUserAgentExtra(userAgentExtraEnv) + out, err := ParseUserAgentExtra(userAgentExtraEnv) if err != nil { panic(fmt.Errorf("failed to parse DATABRICKS_USER_AGENT_EXTRA: %s", err)) @@ -310,7 +313,7 @@ func ConfigureDatabricksClient(ctx context.Context, d *schema.ResourceData) (any return pc, nil } -type userAgentExtra struct { +type UserAgentExtra struct { Key string Value string } @@ -323,8 +326,8 @@ type userAgentExtra struct { // tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA var productRegexRfc9110 = regexp.MustCompile("^([!#$%&'*+\\-.^_`|~0-9A-Za-z]+)(/([!#$%&'*+\\-.^_`|~0-9A-Za-z]+))?$") -func parseUserAgentExtra(env string) ([]userAgentExtra, error) { - out := []userAgentExtra{} +func ParseUserAgentExtra(env string) ([]UserAgentExtra, error) { + out := []UserAgentExtra{} products := strings.FieldsFunc(env, func(r rune) bool { return unicode.IsSpace(r) @@ -341,7 +344,7 @@ func parseUserAgentExtra(env string) ([]userAgentExtra, error) { return nil, fmt.Errorf("product string must include version: %s", product) } - out = append(out, userAgentExtra{ + out = append(out, UserAgentExtra{ Key: match[1], Value: match[3], }) diff --git a/provider/coverage_test.go b/internal/providers/sdkv2/tests/coverage_test.go similarity index 98% rename from provider/coverage_test.go rename to internal/providers/sdkv2/tests/coverage_test.go index 52030cb640..a9fff7ff25 100644 --- a/provider/coverage_test.go +++ b/internal/providers/sdkv2/tests/coverage_test.go @@ -1,4 +1,4 @@ -package provider +package tests import ( "fmt" @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/stretchr/testify/assert" ) @@ -143,7 +144,7 @@ func TestCoverageReport(t *testing.T) { files, err := recursiveChildren("..") assert.NoError(t, err) - p := DatabricksProvider() + p := sdkv2.DatabricksProvider() var cr CoverageReport var longestResourceName, longestFieldName int diff --git a/provider/generate_test.go b/internal/providers/sdkv2/tests/generate_test.go similarity index 98% rename from provider/generate_test.go rename to internal/providers/sdkv2/tests/generate_test.go index 4bba20c1f1..4b12513670 100644 --- a/provider/generate_test.go +++ b/internal/providers/sdkv2/tests/generate_test.go @@ -1,4 +1,4 @@ -package provider +package tests import ( "fmt" @@ -11,6 +11,7 @@ import ( "testing" "text/template" + "github.com/databricks/terraform-provider-databricks/internal/providers/sdkv2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/stretchr/testify/assert" ) @@ -233,7 +234,7 @@ func TestGenerateTestCodeStubs(t *testing.T) { t.Logf("Got %d unit tests in total. %v", len(funcs), resourceTestStub{}) t.Skip() - p := DatabricksProvider() + p := sdkv2.DatabricksProvider() for name, resource := range p.ResourcesMap { if name != "databricks_group_instance_profile" { continue diff --git a/main.go b/main.go index 47825aad22..a615e3f235 100644 --- a/main.go +++ b/main.go @@ -8,7 +8,7 @@ import ( "github.com/databricks/terraform-provider-databricks/common" "github.com/databricks/terraform-provider-databricks/exporter" - "github.com/databricks/terraform-provider-databricks/provider" + "github.com/databricks/terraform-provider-databricks/internal/providers" "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tfprotov6/tf6server" ) @@ -38,7 +38,7 @@ func main() { log.Printf(startMessageFormat, common.Version()) ctx := context.Background() - providerServer, err := provider.GetProviderServer(ctx) + providerServer, err := providers.GetProviderServer(ctx) if err != nil { log.Fatal(err) }