diff --git a/catalog/resource_registered_model.go b/catalog/resource_registered_model.go index 644850c706..fa2d6b3634 100644 --- a/catalog/resource_registered_model.go +++ b/catalog/resource_registered_model.go @@ -17,6 +17,11 @@ func ResourceRegisteredModel() common.Resource { m["schema_name"].ForceNew = true m["storage_location"].ForceNew = true m["storage_location"].Computed = true + m["owner"] = &schema.Schema{ + Type: schema.TypeString, + Computed: true, + Optional: true, + } return m }) @@ -34,6 +39,18 @@ func ResourceRegisteredModel() common.Resource { return err } d.SetId(model.FullName) + // Don't update owner if it is not provided + if d.Get("owner") == "" { + return nil + } + + var update catalog.UpdateRegisteredModelRequest + common.DataToStructPointer(d, s, &update) + update.FullName = d.Id() + _, err = w.RegisteredModels.Update(ctx, update) + if err != nil { + return err + } return nil }, Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error { @@ -52,9 +69,41 @@ func ResourceRegisteredModel() common.Resource { if err != nil { return err } + var u catalog.UpdateRegisteredModelRequest common.DataToStructPointer(d, s, &u) u.FullName = d.Id() + + if d.HasChange("owner") { + _, err := w.RegisteredModels.Update(ctx, catalog.UpdateRegisteredModelRequest{ + FullName: u.FullName, + Owner: u.Owner, + }) + if err != nil { + return err + } + } + + if !d.HasChangeExcept("owner") { + return nil + } + + u.Owner = "" + _, err = w.RegisteredModels.Update(ctx, u) + if err != nil { + if d.HasChange("owner") { + // Rollback + old, new := d.GetChange("owner") + _, rollbackErr := w.RegisteredModels.Update(ctx, catalog.UpdateRegisteredModelRequest{ + FullName: u.FullName, + Owner: old.(string), + }) + if rollbackErr != nil { + return common.OwnerRollbackError(err, rollbackErr, old.(string), new.(string)) + } + } + return err + } _, err = w.RegisteredModels.Update(ctx, u) return err }, diff --git a/catalog/resource_registered_model_test.go b/catalog/resource_registered_model_test.go index 93e2f671c0..712abaf666 100644 --- a/catalog/resource_registered_model_test.go +++ b/catalog/resource_registered_model_test.go @@ -1,12 +1,13 @@ package catalog import ( - "net/http" + "errors" "testing" - "github.com/databricks/databricks-sdk-go/apierr" + "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/databricks/databricks-sdk-go/service/catalog" "github.com/databricks/terraform-provider-databricks/qa" + "github.com/stretchr/testify/mock" ) func TestRegisteredModelCornerCases(t *testing.T) { @@ -15,35 +16,29 @@ func TestRegisteredModelCornerCases(t *testing.T) { func TestRegisteredModelCreate(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodPost, - Resource: "/api/2.1/unity-catalog/models", - ExpectedRequest: catalog.CreateRegisteredModelRequest{ - Name: "model", - CatalogName: "catalog", - SchemaName: "schema", - Comment: "comment", - }, - Response: catalog.RegisteredModelInfo{ - Name: "model", - CatalogName: "catalog", - SchemaName: "schema", - FullName: "catalog.schema.model", - Comment: "comment", - }, - }, - { - Method: http.MethodGet, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model?", - Response: catalog.RegisteredModelInfo{ - Name: "model", - CatalogName: "catalog", - SchemaName: "schema", - FullName: "catalog.schema.model", - Comment: "comment", - }, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Create(mock.Anything, catalog.CreateRegisteredModelRequest{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + Comment: "comment", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + Owner: "owner", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + e.GetByFullName(mock.Anything, "catalog.schema.model").Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + Owner: "owner", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) }, Resource: ResourceRegisteredModel(), HCL: ` @@ -55,23 +50,72 @@ func TestRegisteredModelCreate(t *testing.T) { Create: true, }.ApplyAndExpectData(t, map[string]any{ - "id": "catalog.schema.model", + "id": "catalog.schema.model", + "owner": "owner", + }, + ) +} + +func TestRegisteredModelCreateWithOwner(t *testing.T) { + qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Create(mock.Anything, catalog.CreateRegisteredModelRequest{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + Comment: "comment", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + Owner: "old_owner", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + Owner: "owner", + FullName: "catalog.schema.model", + Comment: "comment", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + Owner: "owner", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + e.GetByFullName(mock.Anything, "catalog.schema.model").Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + Owner: "owner", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + }, + Resource: ResourceRegisteredModel(), + HCL: ` + name = "model" + owner = "owner" + catalog_name = "catalog" + schema_name = "schema" + comment = "comment" + `, + Create: true, + }.ApplyAndExpectData(t, + map[string]any{ + "id": "catalog.schema.model", + "owner": "owner", }, ) } func TestRegisteredModelCreate_Error(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodPost, - Resource: "/api/2.1/unity-catalog/models", - Response: apierr.APIErrorBody{ - ErrorCode: "INVALID_REQUEST", - Message: "Internal error happened", - }, - Status: 400, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Create(mock.Anything, catalog.CreateRegisteredModelRequest{}).Return(nil, errors.New("Internal error happened")) }, Resource: ResourceRegisteredModel(), Create: true, @@ -80,18 +124,15 @@ func TestRegisteredModelCreate_Error(t *testing.T) { func TestRegisteredModelRead(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodGet, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model?", - Response: catalog.RegisteredModelInfo{ - Name: "model", - CatalogName: "catalog", - SchemaName: "schema", - FullName: "catalog.schema.model", - Comment: "comment", - }, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.GetByFullName(mock.Anything, "catalog.schema.model").Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) }, Resource: ResourceRegisteredModel(), Read: true, @@ -105,16 +146,9 @@ func TestRegisteredModelRead(t *testing.T) { func TestRegisteredModelRead_Error(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodGet, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model?", - Response: apierr.APIErrorBody{ - ErrorCode: "INVALID_REQUEST", - Message: "Internal error happened", - }, - Status: 400, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.GetByFullName(mock.Anything, "catalog.schema.model").Return(nil, errors.New("Internal error happened")) }, Resource: ResourceRegisteredModel(), Read: true, @@ -124,33 +158,25 @@ func TestRegisteredModelRead_Error(t *testing.T) { func TestRegisteredModelUpdate(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodPatch, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model", - ExpectedRequest: catalog.UpdateRegisteredModelRequest{ - FullName: "catalog.schema.model", - Comment: "new comment", - }, - Response: catalog.RegisteredModelInfo{ - Name: "model", - CatalogName: "catalog", - SchemaName: "schema", - FullName: "catalog.schema.model", - Comment: "new comment", - }, - }, - { - Method: http.MethodGet, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model?", - Response: catalog.RegisteredModelInfo{ - Name: "model", - CatalogName: "catalog", - SchemaName: "schema", - FullName: "catalog.schema.model", - Comment: "new comment", - }, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Comment: "new comment", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "new comment", + }, nil) + e.GetByFullName(mock.Anything, "catalog.schema.model").Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "new comment", + }, nil) }, Resource: ResourceRegisteredModel(), Update: true, @@ -170,18 +196,127 @@ func TestRegisteredModelUpdate(t *testing.T) { }.ApplyNoError(t) } +func TestRegisteredModelUpdateOwner(t *testing.T) { + qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Owner: "new_owner", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + Owner: "new_owner", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Comment: "new comment", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + FullName: "catalog.schema.model", + Comment: "new comment", + }, nil) + e.GetByFullName(mock.Anything, "catalog.schema.model").Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + Owner: "new_owner", + FullName: "catalog.schema.model", + Comment: "new comment", + }, nil) + }, + Resource: ResourceRegisteredModel(), + Update: true, + ID: "catalog.schema.model", + InstanceState: map[string]string{ + "name": "model", + "catalog_name": "catalog", + "owner": "owner", + "schema_name": "schema", + "comment": "comment", + }, + HCL: ` + name = "model" + catalog_name = "catalog" + owner = "new_owner" + schema_name = "schema" + comment = "new comment" + `, + }.ApplyAndExpectData(t, map[string]any{ + "id": "catalog.schema.model", + "owner": "new_owner", + "comment": "new comment", + "name": "model", + "catalog_name": "catalog", + "schema_name": "schema", + }) +} + +func TestRegisteredModelUpdateRollback(t *testing.T) { + _, err := qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Owner: "new_owner", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + Owner: "new_owner", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Comment: "new comment", + }).Return(nil, errors.New("Something unexpected")) + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Owner: "owner", + }).Return(&catalog.RegisteredModelInfo{ + Name: "model", + CatalogName: "catalog", + SchemaName: "schema", + Owner: "owner", + FullName: "catalog.schema.model", + Comment: "comment", + }, nil) + }, + Resource: ResourceRegisteredModel(), + Update: true, + ID: "catalog.schema.model", + InstanceState: map[string]string{ + "name": "model", + "catalog_name": "catalog", + "owner": "owner", + "schema_name": "schema", + "comment": "comment", + }, + HCL: ` + name = "model" + catalog_name = "catalog" + owner = "new_owner" + schema_name = "schema" + comment = "new comment" + `, + }.Apply(t) + qa.AssertErrorStartsWith(t, err, "Something unexpected") +} + func TestRegisteredModelUpdate_Error(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodPatch, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model", - Response: apierr.APIErrorBody{ - ErrorCode: "INVALID_REQUEST", - Message: "Internal error happened", - }, - Status: 400, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.Update(mock.Anything, catalog.UpdateRegisteredModelRequest{ + FullName: "catalog.schema.model", + Comment: "new comment", + }).Return(nil, errors.New("Internal error happened")) }, Resource: ResourceRegisteredModel(), Update: true, @@ -203,12 +338,9 @@ func TestRegisteredModelUpdate_Error(t *testing.T) { func TestRegisteredModelDelete(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodDelete, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model?", - Response: "", - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.DeleteByFullName(mock.Anything, "catalog.schema.model").Return(nil) }, Resource: ResourceRegisteredModel(), Delete: true, @@ -218,16 +350,9 @@ func TestRegisteredModelDelete(t *testing.T) { func TestRegisteredModelDelete_Error(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - { - Method: http.MethodDelete, - Resource: "/api/2.1/unity-catalog/models/catalog.schema.model?", - Response: apierr.APIErrorBody{ - ErrorCode: "INVALID_REQUEST", - Message: "Internal error happened", - }, - Status: 400, - }, + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockRegisteredModelsAPI().EXPECT() + e.DeleteByFullName(mock.Anything, "catalog.schema.model").Return(errors.New("Internal error happened")) }, Resource: ResourceRegisteredModel(), Delete: true, diff --git a/docs/resources/registered_model.md b/docs/resources/registered_model.md index 56d7195e15..90628839c9 100644 --- a/docs/resources/registered_model.md +++ b/docs/resources/registered_model.md @@ -24,8 +24,9 @@ The following arguments are supported: * `name` - (Required) The name of the registered model. *Change of this parameter forces recreation of the resource.* * `catalog_name` - (Required) The name of the catalog where the schema and the registered model reside. *Change of this parameter forces recreation of the resource.* * `schema_name` - (Required) The name of the schema where the registered model resides. *Change of this parameter forces recreation of the resource.* -* `comment` - The comment attached to the registered model. -* `storage_location` - The storage location under which model version data files are stored. *Change of this parameter forces recreation of the resource.* +* `owner` - (Optional) Name of the registered model owner. +* `comment` - (Optional) The comment attached to the registered model. +* `storage_location` - (Optional) The storage location under which model version data files are stored. *Change of this parameter forces recreation of the resource.* ## Attribute Reference diff --git a/internal/acceptance/registered_model_test.go b/internal/acceptance/registered_model_test.go index 98dbbd31d7..9da0873aa2 100644 --- a/internal/acceptance/registered_model_test.go +++ b/internal/acceptance/registered_model_test.go @@ -40,6 +40,7 @@ func TestUcAccRegisteredModel(t *testing.T) { name = "terraform-test-registered-model-update-{var.STICKY_RANDOM}" catalog_name = "main" schema_name = "default" + owner = "account users" comment = "new comment" } `,