From c97ebae22918369896890a8b0f86e876cb5df1b1 Mon Sep 17 00:00:00 2001 From: Vuong Date: Wed, 31 May 2023 15:38:29 +0100 Subject: [PATCH] Migrate resource `databricks_catalog` to Go SDK --- catalog/data_catalogs_test.go | 5 +- catalog/resource_catalog.go | 108 +++++++++++++++---------------- catalog/resource_catalog_test.go | 95 +++++++++++++++++++-------- catalog/resource_schema.go | 7 -- docs/resources/catalog.md | 2 +- 5 files changed, 125 insertions(+), 92 deletions(-) diff --git a/catalog/data_catalogs_test.go b/catalog/data_catalogs_test.go index 15e3d74f7c..369126ca98 100644 --- a/catalog/data_catalogs_test.go +++ b/catalog/data_catalogs_test.go @@ -3,6 +3,7 @@ package catalog import ( "testing" + "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/terraform-provider-databricks/qa" ) @@ -12,8 +13,8 @@ func TestCatalogsData(t *testing.T) { { Method: "GET", Resource: "/api/2.1/unity-catalog/catalogs", - Response: Catalogs{ - Catalogs: []CatalogInfo{ + Response: catalog.ListCatalogsResponse{ + Catalogs: []catalog.CatalogInfo{ { Name: "b", }, diff --git a/catalog/resource_catalog.go b/catalog/resource_catalog.go index d77ec8841c..19b07ca49e 100644 --- a/catalog/resource_catalog.go +++ b/catalog/resource_catalog.go @@ -5,6 +5,7 @@ import ( "fmt" "log" + "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/terraform-provider-databricks/common" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -17,17 +18,8 @@ func ucDirectoryPathSuppressDiff(k, old, new string, d *schema.ResourceData) boo return false } -type CatalogsAPI struct { - client *common.DatabricksClient - context context.Context -} - -func NewCatalogsAPI(ctx context.Context, m any) CatalogsAPI { - return CatalogsAPI{m.(*common.DatabricksClient), context.WithValue(ctx, common.Api, common.API_2_1)} -} - type CatalogInfo struct { - Name string `json:"name" tf:"force_new"` + Name string `json:"name"` Comment string `json:"comment,omitempty"` StorageRoot string `json:"storage_root,omitempty" tf:"force_new"` ProviderName string `json:"provider_name,omitempty" tf:"force_new,conflicts:storage_root"` @@ -37,38 +29,6 @@ type CatalogInfo struct { MetastoreID string `json:"metastore_id,omitempty" tf:"computed"` } -type Catalogs struct { - Catalogs []CatalogInfo `json:"catalogs"` -} - -func (a CatalogsAPI) createCatalog(ci *CatalogInfo) error { - return a.client.Post(a.context, "/unity-catalog/catalogs", ci, ci) -} - -func (a CatalogsAPI) getCatalog(name string) (ci CatalogInfo, err error) { - err = a.client.Get(a.context, "/unity-catalog/catalogs/"+name, nil, &ci) - return -} - -func (a CatalogsAPI) deleteCatalog(name string) error { - return a.client.Delete(a.context, "/unity-catalog/catalogs/"+name, nil) -} - -func (a CatalogsAPI) forceDeleteCatalog(name string) error { - schemasAPI := NewSchemasAPI(a.context, a.client) - schemas, err := schemasAPI.listByCatalog(name) - if err != nil { - return err - } - for _, s := range schemas.Schemas { - if s.Name != name+".information_schema" { - schemasAPI.forceDeleteSchema(s.FullName) - } - } - - return a.client.Delete(a.context, "/unity-catalog/catalogs/"+name, nil) -} - func ResourceCatalog() *schema.Resource { catalogSchema := common.StructToSchema(CatalogInfo{}, func(m map[string]*schema.Schema) map[string]*schema.Schema { @@ -80,39 +40,77 @@ func ResourceCatalog() *schema.Resource { m["storage_root"].DiffSuppressFunc = ucDirectoryPathSuppressDiff return m }) - update := updateFunctionFactory("/unity-catalog/catalogs", []string{"owner", "comment", "properties"}) return common.Resource{ Schema: catalogSchema, Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { - var ci CatalogInfo - common.DataToStructPointer(d, catalogSchema, &ci) - if err := NewCatalogsAPI(ctx, c).createCatalog(&ci); err != nil { + w, err := c.WorkspaceClient() + if err != nil { + return err + } + + var createCatalogRequest catalog.CreateCatalog + common.DataToStructPointer(d, catalogSchema, &createCatalogRequest) + ci, err := w.Catalogs.Create(ctx, createCatalogRequest) + if err != nil { return err } // only remove catalog default schema for non-Delta Sharing catalog if ci.ShareName == "" { - if err := NewSchemasAPI(ctx, c).deleteSchema(ci.Name + ".default"); err != nil { + if err := w.Schemas.DeleteByFullName(ctx, ci.Name+".default"); err != nil { return fmt.Errorf("cannot remove new catalog default schema: %w", err) } } + d.SetId(ci.Name) - return update(ctx, d, c) + + // Update owner if it is provided + if d.Get("owner") == "" { + return nil + } + var updateCatalogRequest catalog.UpdateCatalog + common.DataToStructPointer(d, catalogSchema, &updateCatalogRequest) + _, err = w.Catalogs.Update(ctx, updateCatalogRequest) + if err != nil { + return err + } + + return nil }, Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { - ci, err := NewCatalogsAPI(ctx, c).getCatalog(d.Id()) + w, err := c.WorkspaceClient() + if err != nil { + return err + } + + ci, err := w.Catalogs.GetByName(ctx, d.Id()) if err != nil { return err } return common.StructToData(ci, catalogSchema, d) }, - Update: update, + Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { + w, err := c.WorkspaceClient() + if err != nil { + return err + } + var updateCatalogRequest catalog.UpdateCatalog + common.DataToStructPointer(d, catalogSchema, &updateCatalogRequest) + ci, err := w.Catalogs.Update(ctx, updateCatalogRequest) + // We need to update the resource data because Name is updatable + // So if we don't update the field then the requests would be made to old Name which doesn't exists. + d.SetId(ci.Name) + if err != nil { + return err + } + return nil + }, Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { - force := d.Get("force_destroy").(bool) - if force { - return NewCatalogsAPI(ctx, c).forceDeleteCatalog(d.Id()) - } else { - return NewCatalogsAPI(ctx, c).deleteCatalog(d.Id()) + w, err := c.WorkspaceClient() + if err != nil { + return err } + force := d.Get("force_destroy").(bool) + return w.Catalogs.Delete(ctx, catalog.DeleteCatalogRequest{Force: force, Name: d.Id()}) }, }.ToResource() } diff --git a/catalog/resource_catalog_test.go b/catalog/resource_catalog_test.go index e487b38414..a0572c4c29 100644 --- a/catalog/resource_catalog_test.go +++ b/catalog/resource_catalog_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/terraform-provider-databricks/qa" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,28 +21,37 @@ func TestCatalogCreateAlsoDeletesDefaultSchema(t *testing.T) { { Method: "POST", Resource: "/api/2.1/unity-catalog/catalogs", - ExpectedRequest: CatalogInfo{ + ExpectedRequest: catalog.CreateCatalog{ Name: "a", Comment: "b", Properties: map[string]string{ "c": "d", }, }, + Response: catalog.CatalogInfo{ + Name: "a", + Comment: "b", + Properties: map[string]string{ + "c": "d", + }, + MetastoreId: "e", + Owner: "f", + }, }, { Method: "DELETE", - Resource: "/api/2.1/unity-catalog/schemas/a.default", + Resource: "/api/2.1/unity-catalog/schemas/a.default?", }, { Method: "GET", - Resource: "/api/2.1/unity-catalog/catalogs/a", - Response: CatalogInfo{ + Resource: "/api/2.1/unity-catalog/catalogs/a?", + Response: catalog.CatalogInfo{ Name: "a", Comment: "b", Properties: map[string]string{ "c": "d", }, - MetastoreID: "e", + MetastoreId: "e", Owner: "f", }, }, @@ -64,15 +74,14 @@ func TestCatalogCreateWithOwnerAlsoDeletesDefaultSchema(t *testing.T) { { Method: "POST", Resource: "/api/2.1/unity-catalog/catalogs", - ExpectedRequest: CatalogInfo{ + ExpectedRequest: catalog.CreateCatalog{ Name: "a", Comment: "b", Properties: map[string]string{ "c": "d", }, - Owner: "administrators", }, - Response: CatalogInfo{ + Response: catalog.CatalogInfo{ Name: "a", Comment: "b", Properties: map[string]string{ @@ -83,25 +92,30 @@ func TestCatalogCreateWithOwnerAlsoDeletesDefaultSchema(t *testing.T) { }, { Method: "DELETE", - Resource: "/api/2.1/unity-catalog/schemas/a.default", + Resource: "/api/2.1/unity-catalog/schemas/a.default?", }, { Method: "PATCH", Resource: "/api/2.1/unity-catalog/catalogs/a", - ExpectedRequest: map[string]any{ - "owner": "administrators", + ExpectedRequest: catalog.UpdateCatalog{ + Name: "a", + Comment: "b", + Properties: map[string]string{ + "c": "d", + }, + Owner: "administrators", }, }, { Method: "GET", - Resource: "/api/2.1/unity-catalog/catalogs/a", - Response: CatalogInfo{ + Resource: "/api/2.1/unity-catalog/catalogs/a?", + Response: catalog.CatalogInfo{ Name: "a", Comment: "b", Properties: map[string]string{ "c": "d", }, - MetastoreID: "e", + MetastoreId: "e", Owner: "administrators", }, }, @@ -125,21 +139,28 @@ func TestCatalogCreateCannotDeleteDefaultSchema(t *testing.T) { { Method: "POST", Resource: "/api/2.1/unity-catalog/catalogs", - ExpectedRequest: CatalogInfo{ + ExpectedRequest: catalog.CatalogInfo{ + Name: "a", + Comment: "b", + Properties: map[string]string{ + "c": "d", + }, + }, + Response: catalog.CatalogInfo{ Name: "a", Comment: "b", Properties: map[string]string{ "c": "d", }, + Owner: "testers", }, }, { Method: "DELETE", - Resource: "/api/2.1/unity-catalog/schemas/a.default", + Resource: "/api/2.1/unity-catalog/schemas/a.default?", Status: 400, Response: apierr.APIErrorBody{ - ScimDetail: "Something", - ScimStatus: "Else", + Message: "Something", }, }, }, @@ -155,6 +176,7 @@ func TestCatalogCreateCannotDeleteDefaultSchema(t *testing.T) { }.Apply(t) require.Error(t, err) assert.Equal(t, "cannot remove new catalog default schema: Something", fmt.Sprint(err)) + } func TestUpdateCatalog(t *testing.T) { @@ -163,16 +185,24 @@ func TestUpdateCatalog(t *testing.T) { { Method: "PATCH", Resource: "/api/2.1/unity-catalog/catalogs/a", - ExpectedRequest: map[string]any{ - "owner": "administrators", + ExpectedRequest: catalog.UpdateCatalog{ + Name: "a", + Comment: "c", + Owner: "administrators", + }, + Response: catalog.CatalogInfo{ + Name: "a", + MetastoreId: "d", + Comment: "c", + Owner: "administrators", }, }, { Method: "GET", - Resource: "/api/2.1/unity-catalog/catalogs/a", - Response: CatalogInfo{ + Resource: "/api/2.1/unity-catalog/catalogs/a?", + Response: catalog.CatalogInfo{ Name: "a", - MetastoreID: "d", + MetastoreId: "d", Comment: "c", Owner: "administrators", }, @@ -264,7 +294,7 @@ func TestCatalogCreateDeltaSharing(t *testing.T) { { Method: "POST", Resource: "/api/2.1/unity-catalog/catalogs", - ExpectedRequest: CatalogInfo{ + ExpectedRequest: catalog.CatalogInfo{ Name: "a", Comment: "b", Properties: map[string]string{ @@ -273,11 +303,22 @@ func TestCatalogCreateDeltaSharing(t *testing.T) { ProviderName: "foo", ShareName: "bar", }, + Response: catalog.CatalogInfo{ + Name: "a", + Comment: "b", + Properties: map[string]string{ + "c": "d", + }, + ProviderName: "foo", + ShareName: "bar", + MetastoreId: "e", + Owner: "f", + }, }, { Method: "GET", - Resource: "/api/2.1/unity-catalog/catalogs/a", - Response: CatalogInfo{ + Resource: "/api/2.1/unity-catalog/catalogs/a?", + Response: catalog.CatalogInfo{ Name: "a", Comment: "b", Properties: map[string]string{ @@ -285,7 +326,7 @@ func TestCatalogCreateDeltaSharing(t *testing.T) { }, ProviderName: "foo", ShareName: "bar", - MetastoreID: "e", + MetastoreId: "e", Owner: "f", }, }, diff --git a/catalog/resource_schema.go b/catalog/resource_schema.go index f90baaad03..b65ea83c11 100644 --- a/catalog/resource_schema.go +++ b/catalog/resource_schema.go @@ -32,13 +32,6 @@ type Schemas struct { Schemas []SchemaInfo `json:"schemas"` } -func (a SchemasAPI) listByCatalog(catalogName string) (schemas Schemas, err error) { - err = a.client.Get(a.context, "/unity-catalog/schemas", map[string]string{ - "catalog_name": catalogName, - }, &schemas) - return -} - func (a SchemasAPI) createSchema(si *SchemaInfo) error { return a.client.Post(a.context, "/unity-catalog/schemas", si, si) } diff --git a/docs/resources/catalog.md b/docs/resources/catalog.md index 85680c9a01..e51b0960d1 100644 --- a/docs/resources/catalog.md +++ b/docs/resources/catalog.md @@ -24,7 +24,7 @@ resource "databricks_catalog" "sandbox" { The following arguments are required: -* `name` - Name of Catalog relative to parent metastore. Change forces creation of a new resource. +* `name` - Name of Catalog relative to parent metastore. * `storage_root` - (Optional) Managed location of the catalog. Location in cloud storage where data for managed tables will be stored. If not specified, the location will default to the metastore root location. Change forces creation of a new resource. * `provider_name` - (Optional) For Delta Sharing Catalogs: the name of the delta sharing provider. Change forces creation of a new resource. * `share_name` - (Optional) For Delta Sharing Catalogs: the name of the share under the share provider. Change forces creation of a new resource.