From 0148dbb39677b5cd95b7232d42dedcaa2b51ab90 Mon Sep 17 00:00:00 2001 From: Alex Ott Date: Mon, 3 Nov 2025 15:25:38 +0100 Subject: [PATCH 1/4] * Relaxed `force_new` constraint on `catalog` attribute in `databricks_pipeline` resource to allow changing the default catalog for existing pipelines Pipelines will still be recreated when switching between `storage` and `catalog` modes, but changing the catalog value in an existing catalog-based pipeline no longer requires recreation. Resolves #4692 --- NEXT_CHANGELOG.md | 3 +- docs/resources/pipeline.md | 2 +- pipelines/resource_pipeline.go | 39 ++++++- pipelines/resource_pipeline_test.go | 171 ++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+), 3 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 2c8a461ab1..5a3f4afe5f 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -6,7 +6,8 @@ ### New Features and Improvements -* Improve `databricks_service_principals` data source ([#5164](https://github.com/databricks/terraform-provider-databricks/pull/5164)) +* Improve `databricks_service_principals` data source ([#5164](https://github.com/databricks/terraform-provider-databricks/pull/5164)). +* Relaxed `force_new` constraint on `catalog` attribute in `databricks_pipeline` resource to allow changing the default catalog for existing pipelines ([#5180](https://github.com/databricks/terraform-provider-databricks/issues/5180)). ### Bug Fixes diff --git a/docs/resources/pipeline.md b/docs/resources/pipeline.md index a5abf40637..ebf4c427c1 100644 --- a/docs/resources/pipeline.md +++ b/docs/resources/pipeline.md @@ -81,7 +81,7 @@ resource "databricks_pipeline" "this" { The following arguments are supported: * `name` - A user-friendly name for this pipeline. The name can be used to identify pipeline jobs in the UI. -* `catalog` - The name of catalog in Unity Catalog. *Change of this parameter forces recreation of the pipeline.* (Conflicts with `storage`). +* `catalog` - The name of default catalog in Unity Catalog. *Change of this parameter forces recreation of the pipeline if you switch from `storage` to `catalog` or vice versa. If pipeline was already created with `catalog` set, the value could be changed.* (Conflicts with `storage`). * `schema` - (Optional, String, Conflicts with `target`) The default schema (database) where tables are read from or published to. The presence of this attribute implies that the pipeline is in direct publishing mode. * `storage` - A location on cloud storage where output data and metadata required for pipeline execution are stored. By default, tables are stored in a subdirectory of this location. *Change of this parameter forces recreation of the pipeline.* (Conflicts with `catalog`). * `target` - (Optional, String, Conflicts with `schema`) The name of a database (in either the Hive metastore or in a UC catalog) for persisting pipeline output data. Configuring the target setting allows you to view and query the pipeline output data from the Databricks UI. diff --git a/pipelines/resource_pipeline.go b/pipelines/resource_pipeline.go index ff568e66e6..c4369f327f 100644 --- a/pipelines/resource_pipeline.go +++ b/pipelines/resource_pipeline.go @@ -198,7 +198,8 @@ func (Pipeline) CustomizeSchema(s *common.CustomizableSchema) *common.Customizab // ForceNew fields s.SchemaPath("storage").SetForceNew() - s.SchemaPath("catalog").SetForceNew() + // catalog can be updated in-place, but switching between storage and catalog requires recreation + // (handled in CustomizeDiff) s.SchemaPath("gateway_definition", "connection_id").SetForceNew() s.SchemaPath("gateway_definition", "gateway_storage_catalog").SetForceNew() s.SchemaPath("gateway_definition", "gateway_storage_schema").SetForceNew() @@ -335,5 +336,41 @@ func ResourcePipeline() common.Resource { Timeouts: &schema.ResourceTimeout{ Default: schema.DefaultTimeout(DefaultTimeout), }, + CustomizeDiff: func(ctx context.Context, d *schema.ResourceDiff) error { + // Allow changing catalog value in existing pipelines, but force recreation + // when switching between storage and catalog (or vice versa). + // This should only run on update, thus we skip this check if the ID is not known. + if d.Id() != "" { + storageChanged := d.HasChange("storage") + catalogChanged := d.HasChange("catalog") + + // If both changed, it means we're switching between storage and catalog modes + if storageChanged && catalogChanged { + oldStorage, newStorage := d.GetChange("storage") + oldCatalog, newCatalog := d.GetChange("catalog") + + // Force new if switching from storage to catalog + if oldStorage != "" && oldStorage != nil && newCatalog != "" && newCatalog != nil { + if err := d.ForceNew("catalog"); err != nil { + return err + } + if err := d.ForceNew("storage"); err != nil { + return err + } + } + + // Force new if switching from catalog to storage + if oldCatalog != "" && oldCatalog != nil && newStorage != "" && newStorage != nil { + if err := d.ForceNew("catalog"); err != nil { + return err + } + if err := d.ForceNew("storage"); err != nil { + return err + } + } + } + } + return nil + }, } } diff --git a/pipelines/resource_pipeline_test.go b/pipelines/resource_pipeline_test.go index aae0e8f350..f9c7740657 100644 --- a/pipelines/resource_pipeline_test.go +++ b/pipelines/resource_pipeline_test.go @@ -806,3 +806,174 @@ func TestDefault(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "abcd", d.Id()) } + +func TestUpdatePipelineCatalogInPlace(t *testing.T) { + state := pipelines.PipelineStateRunning + spec := pipelines.PipelineSpec{ + Id: "abcd", + Name: "test", + Catalog: "new_catalog", + Libraries: []pipelines.PipelineLibrary{ + { + Notebook: &pipelines.NotebookLibrary{ + Path: "/Test", + }, + }, + }, + Filters: &pipelines.Filters{ + Include: []string{"com.databricks.include"}, + }, + Channel: "CURRENT", + Edition: "ADVANCED", + } + qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockPipelinesAPI().EXPECT() + e.Update(mock.Anything, pipelines.EditPipeline{ + Id: "abcd", + PipelineId: "abcd", + Name: "test", + Catalog: "new_catalog", + Libraries: []pipelines.PipelineLibrary{ + { + Notebook: &pipelines.NotebookLibrary{ + Path: "/Test", + }, + }, + }, + Filters: &pipelines.Filters{ + Include: []string{"com.databricks.include"}, + }, + Channel: "CURRENT", + Edition: "ADVANCED", + }).Return(nil) + e.Get(mock.Anything, pipelines.GetPipelineRequest{ + PipelineId: "abcd", + }).Return(&pipelines.GetPipelineResponse{ + PipelineId: "abcd", + Spec: &spec, + State: state, + }, nil).Twice() + }, + Resource: ResourcePipeline(), + HCL: `name = "test" + catalog = "new_catalog" + library { + notebook { + path = "/Test" + } + } + filters { + include = [ "com.databricks.include" ] + }`, + InstanceState: map[string]string{ + "name": "test", + "catalog": "old_catalog", + }, + Update: true, + ID: "abcd", + }.ApplyAndExpectData(t, map[string]any{ + "id": "abcd", + "catalog": "new_catalog", + }) +} + +func TestUpdatePipelineStorageToCatalogForceNew(t *testing.T) { + state := pipelines.PipelineStateRunning + spec := pipelines.PipelineSpec{ + Id: "abcd", + Name: "test", + Storage: "/test/storage", + Libraries: []pipelines.PipelineLibrary{ + { + Notebook: &pipelines.NotebookLibrary{ + Path: "/Test", + }, + }, + }, + Filters: &pipelines.Filters{ + Include: []string{"com.databricks.include"}, + }, + Channel: "CURRENT", + Edition: "ADVANCED", + } + qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockPipelinesAPI().EXPECT() + e.Update(mock.Anything, mock.Anything).Return(nil) + e.Get(mock.Anything, pipelines.GetPipelineRequest{ + PipelineId: "abcd", + }).Return(&pipelines.GetPipelineResponse{ + PipelineId: "abcd", + Spec: &spec, + State: state, + }, nil) + }, + RequiresNew: true, + Resource: ResourcePipeline(), + Update: true, + ID: "abcd", + InstanceState: map[string]string{ + "name": "test", + "storage": "/test/storage", + }, + HCL: ` + name = "test" + catalog = "new_catalog" + library { + notebook { + path = "/Test" + } + }`, + }.ApplyNoError(t) +} + +func TestUpdatePipelineCatalogToStorageForceNew(t *testing.T) { + state := pipelines.PipelineStateRunning + spec := pipelines.PipelineSpec{ + Id: "abcd", + Name: "test", + Catalog: "old_catalog", + Libraries: []pipelines.PipelineLibrary{ + { + Notebook: &pipelines.NotebookLibrary{ + Path: "/Test", + }, + }, + }, + Filters: &pipelines.Filters{ + Include: []string{"com.databricks.include"}, + }, + Channel: "CURRENT", + Edition: "ADVANCED", + } + qa.ResourceFixture{ + MockWorkspaceClientFunc: func(w *mocks.MockWorkspaceClient) { + e := w.GetMockPipelinesAPI().EXPECT() + e.Update(mock.Anything, mock.Anything).Return(nil) + e.Get(mock.Anything, pipelines.GetPipelineRequest{ + PipelineId: "abcd", + }).Return(&pipelines.GetPipelineResponse{ + PipelineId: "abcd", + Spec: &spec, + State: state, + }, nil) + }, + RequiresNew: true, + Resource: ResourcePipeline(), + Update: true, + ID: "abcd", + InstanceState: map[string]string{ + "name": "test", + "catalog": "old_catalog", + }, + HCL: ` + name = "test" + storage = "/test/storage" + library { + notebook { + path = "/Test" + } + }`, + }.ApplyNoError(t) +} From 1d2ab24ed4c97f769a1262c1df8543baf66b3f7e Mon Sep 17 00:00:00 2001 From: Alex Ott Date: Tue, 4 Nov 2025 09:16:18 +0100 Subject: [PATCH 2/4] Address review comments --- pipelines/resource_pipeline.go | 39 ++++++++++------------------------ 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/pipelines/resource_pipeline.go b/pipelines/resource_pipeline.go index c4369f327f..532e550feb 100644 --- a/pipelines/resource_pipeline.go +++ b/pipelines/resource_pipeline.go @@ -340,34 +340,17 @@ func ResourcePipeline() common.Resource { // Allow changing catalog value in existing pipelines, but force recreation // when switching between storage and catalog (or vice versa). // This should only run on update, thus we skip this check if the ID is not known. - if d.Id() != "" { - storageChanged := d.HasChange("storage") - catalogChanged := d.HasChange("catalog") - - // If both changed, it means we're switching between storage and catalog modes - if storageChanged && catalogChanged { - oldStorage, newStorage := d.GetChange("storage") - oldCatalog, newCatalog := d.GetChange("catalog") - - // Force new if switching from storage to catalog - if oldStorage != "" && oldStorage != nil && newCatalog != "" && newCatalog != nil { - if err := d.ForceNew("catalog"); err != nil { - return err - } - if err := d.ForceNew("storage"); err != nil { - return err - } - } - - // Force new if switching from catalog to storage - if oldCatalog != "" && oldCatalog != nil && newStorage != "" && newStorage != nil { - if err := d.ForceNew("catalog"); err != nil { - return err - } - if err := d.ForceNew("storage"); err != nil { - return err - } - } + if d.Id() == "" { + return nil + } + + // If both storage and catalog changed, it means we're switching between storage and catalog modes + if d.HasChange("storage") && d.HasChange("catalog") { + if err := d.ForceNew("catalog"); err != nil { + return err + } + if err := d.ForceNew("storage"); err != nil { + return err } } return nil From 20cd67b107ece7796d8fe7815487ec0c309a7b10 Mon Sep 17 00:00:00 2001 From: Alex Ott Date: Tue, 4 Nov 2025 09:48:54 +0100 Subject: [PATCH 3/4] simplify the code --- pipelines/resource_pipeline.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pipelines/resource_pipeline.go b/pipelines/resource_pipeline.go index 532e550feb..4b85988d31 100644 --- a/pipelines/resource_pipeline.go +++ b/pipelines/resource_pipeline.go @@ -349,9 +349,7 @@ func ResourcePipeline() common.Resource { if err := d.ForceNew("catalog"); err != nil { return err } - if err := d.ForceNew("storage"); err != nil { - return err - } + return d.ForceNew("storage") } return nil }, From 259d086dc8d1900181b03ef65eb27249d898712c Mon Sep 17 00:00:00 2001 From: Alex Ott Date: Fri, 7 Nov 2025 12:53:46 +0100 Subject: [PATCH 4/4] fix --- pipelines/resource_pipeline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipelines/resource_pipeline.go b/pipelines/resource_pipeline.go index 4b85988d31..4c2d9ec66b 100644 --- a/pipelines/resource_pipeline.go +++ b/pipelines/resource_pipeline.go @@ -336,7 +336,7 @@ func ResourcePipeline() common.Resource { Timeouts: &schema.ResourceTimeout{ Default: schema.DefaultTimeout(DefaultTimeout), }, - CustomizeDiff: func(ctx context.Context, d *schema.ResourceDiff) error { + CustomizeDiff: func(ctx context.Context, d *schema.ResourceDiff, c *common.DatabricksClient) error { // Allow changing catalog value in existing pipelines, but force recreation // when switching between storage and catalog (or vice versa). // This should only run on update, thus we skip this check if the ID is not known.