From bfe688442b64f013a041be73074e3189489c89cc Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 26 Jun 2025 14:50:59 +0200 Subject: [PATCH 01/14] WIP new terraform.Load --- .../check_dashboards_modified_remotely.go | 21 +- .../terraform/check_running_resources.go | 70 +++-- bundle/deploy/terraform/convert.go | 256 +++--------------- bundle/deploy/terraform/load.go | 11 +- bundle/deploy/terraform/util.go | 53 +++- 5 files changed, 133 insertions(+), 278 deletions(-) diff --git a/bundle/deploy/terraform/check_dashboards_modified_remotely.go b/bundle/deploy/terraform/check_dashboards_modified_remotely.go index d0f22c51ee..0cdd9f5e1c 100644 --- a/bundle/deploy/terraform/check_dashboards_modified_remotely.go +++ b/bundle/deploy/terraform/check_dashboards_modified_remotely.go @@ -7,7 +7,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" - tfjson "github.com/hashicorp/terraform-json" ) type dashboardState struct { @@ -23,20 +22,12 @@ func collectDashboardsFromState(ctx context.Context, b *bundle.Bundle) ([]dashbo } var dashboards []dashboardState - for _, resource := range state.Resources { - if resource.Mode != tfjson.ManagedResourceMode { - continue - } - for _, instance := range resource.Instances { - switch resource.Type { - case "databricks_dashboard": - dashboards = append(dashboards, dashboardState{ - Name: resource.Name, - ID: instance.Attributes.ID, - ETag: instance.Attributes.ETag, - }) - } - } + for resourceName, instance := range state["dashboards"] { + dashboards = append(dashboards, dashboardState{ + Name: resourceName, + ID: instance.ID, + ETag: instance.ETag, + }) } return dashboards, nil diff --git a/bundle/deploy/terraform/check_running_resources.go b/bundle/deploy/terraform/check_running_resources.go index 5b3a704083..f84429aab1 100644 --- a/bundle/deploy/terraform/check_running_resources.go +++ b/bundle/deploy/terraform/check_running_resources.go @@ -10,7 +10,6 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/databricks/databricks-sdk-go/service/pipelines" - tfjson "github.com/hashicorp/terraform-json" "golang.org/x/sync/errgroup" ) @@ -51,50 +50,45 @@ func CheckRunningResource() *checkRunningResources { return &checkRunningResources{} } -func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state *resourcesState) error { - if state == nil { - return nil - } - +func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state map[string]map[string]ExportedStateAttributes) error { errs, errCtx := errgroup.WithContext(ctx) - for _, resource := range state.Resources { - if resource.Mode != tfjson.ManagedResourceMode { + for _, jobAttrs := range state["jobs"] { + id := jobAttrs.ID + if id == "" { continue } - for _, instance := range resource.Instances { - id := instance.Attributes.ID - if id == "" { - continue - } - switch resource.Type { - case "databricks_job": - errs.Go(func() error { - isRunning, err := IsJobRunning(errCtx, w, id) - // If there's an error retrieving the job, we assume it's not running - if err != nil { - return err - } - if isRunning { - return &ErrResourceIsRunning{resourceType: "job", resourceId: id} - } - return nil - }) - case "databricks_pipeline": - errs.Go(func() error { - isRunning, err := IsPipelineRunning(errCtx, w, id) - // If there's an error retrieving the pipeline, we assume it's not running - if err != nil { - return nil - } - if isRunning { - return &ErrResourceIsRunning{resourceType: "pipeline", resourceId: id} - } - return nil - }) + errs.Go(func() error { + isRunning, err := IsJobRunning(errCtx, w, id) + // If there's an error retrieving the job, we assume it's not running + if err != nil { + return err + } + if isRunning { + return &ErrResourceIsRunning{resourceType: "job", resourceId: id} } + return nil + }) + } + + for _, pipelineAttrs := range state["pipelines"] { + id := pipelineAttrs.ID + if id == "" { + continue } + + errs.Go(func() error { + isRunning, err := IsPipelineRunning(errCtx, w, id) + // If there's an error retrieving the pipeline, we assume it's not running + if err != nil { + return nil + } + if isRunning { + return &ErrResourceIsRunning{resourceType: "pipeline", resourceId: id} + } + return nil + }) } return errs.Wait() diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index cbbcf9592c..f6bc554fba 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -9,7 +9,7 @@ import ( "github.com/databricks/cli/bundle/deploy/terraform/tfdyn" "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" - tfjson "github.com/hashicorp/terraform-json" + "github.com/databricks/cli/libs/log" ) // BundleToTerraformWithDynValue converts resources in a bundle configuration @@ -79,222 +79,50 @@ func BundleToTerraformWithDynValue(ctx context.Context, root dyn.Value) (*schema return tfroot, nil } -func TerraformToBundle(state *resourcesState, config *config.Root) error { - for _, resource := range state.Resources { - if resource.Mode != tfjson.ManagedResourceMode { - continue - } - for _, instance := range resource.Instances { - switch resource.Type { - case "databricks_job": - if config.Resources.Jobs == nil { - config.Resources.Jobs = make(map[string]*resources.Job) - } - cur := config.Resources.Jobs[resource.Name] - if cur == nil { - cur = &resources.Job{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Jobs[resource.Name] = cur - case "databricks_pipeline": - if config.Resources.Pipelines == nil { - config.Resources.Pipelines = make(map[string]*resources.Pipeline) - } - cur := config.Resources.Pipelines[resource.Name] - if cur == nil { - cur = &resources.Pipeline{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Pipelines[resource.Name] = cur - case "databricks_mlflow_model": - if config.Resources.Models == nil { - config.Resources.Models = make(map[string]*resources.MlflowModel) - } - cur := config.Resources.Models[resource.Name] - if cur == nil { - cur = &resources.MlflowModel{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Models[resource.Name] = cur - case "databricks_mlflow_experiment": - if config.Resources.Experiments == nil { - config.Resources.Experiments = make(map[string]*resources.MlflowExperiment) - } - cur := config.Resources.Experiments[resource.Name] - if cur == nil { - cur = &resources.MlflowExperiment{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Experiments[resource.Name] = cur - case "databricks_model_serving": - if config.Resources.ModelServingEndpoints == nil { - config.Resources.ModelServingEndpoints = make(map[string]*resources.ModelServingEndpoint) - } - cur := config.Resources.ModelServingEndpoints[resource.Name] - if cur == nil { - cur = &resources.ModelServingEndpoint{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.ModelServingEndpoints[resource.Name] = cur - case "databricks_registered_model": - if config.Resources.RegisteredModels == nil { - config.Resources.RegisteredModels = make(map[string]*resources.RegisteredModel) - } - cur := config.Resources.RegisteredModels[resource.Name] - if cur == nil { - cur = &resources.RegisteredModel{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.RegisteredModels[resource.Name] = cur - case "databricks_quality_monitor": - if config.Resources.QualityMonitors == nil { - config.Resources.QualityMonitors = make(map[string]*resources.QualityMonitor) - } - cur := config.Resources.QualityMonitors[resource.Name] - if cur == nil { - cur = &resources.QualityMonitor{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.QualityMonitors[resource.Name] = cur - case "databricks_schema": - if config.Resources.Schemas == nil { - config.Resources.Schemas = make(map[string]*resources.Schema) - } - cur := config.Resources.Schemas[resource.Name] - if cur == nil { - cur = &resources.Schema{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Schemas[resource.Name] = cur - case "databricks_volume": - if config.Resources.Volumes == nil { - config.Resources.Volumes = make(map[string]*resources.Volume) - } - cur := config.Resources.Volumes[resource.Name] - if cur == nil { - cur = &resources.Volume{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Volumes[resource.Name] = cur - case "databricks_cluster": - if config.Resources.Clusters == nil { - config.Resources.Clusters = make(map[string]*resources.Cluster) - } - cur := config.Resources.Clusters[resource.Name] - if cur == nil { - cur = &resources.Cluster{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Clusters[resource.Name] = cur - case "databricks_dashboard": - if config.Resources.Dashboards == nil { - config.Resources.Dashboards = make(map[string]*resources.Dashboard) - } - cur := config.Resources.Dashboards[resource.Name] - if cur == nil { - cur = &resources.Dashboard{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.ID - config.Resources.Dashboards[resource.Name] = cur - case "databricks_app": - if config.Resources.Apps == nil { - config.Resources.Apps = make(map[string]*resources.App) - } - cur := config.Resources.Apps[resource.Name] - if cur == nil { - cur = &resources.App{ModifiedStatus: resources.ModifiedStatusDeleted} +func TerraformToBundle(ctx context.Context, state map[string]map[string]ExportedStateAttributes, config *config.Root) error { + return config.Mutate(func(v dyn.Value) (dyn.Value, error) { + for groupName, group := range state { + for resourceName, attrs := range group { + path := dyn.Path{dyn.Key("resources"), dyn.Key(groupName), dyn.Key(resourceName)} + resource, err := dyn.GetByPath(v, path) + if !resource.IsValid() { + m := dyn.NewMapping() + m.SetLoc("id", nil, dyn.V(attrs.ID)) + m.SetLoc("modified_status", nil, dyn.V(resources.ModifiedStatusDeleted)) + v, err = dyn.SetByPath(v, path, dyn.V(m)) + if err != nil { + return dyn.InvalidValue, err + } + } else if err != nil { + return dyn.InvalidValue, err } else { - // If the app exists in terraform and bundle, we always set modified status to updated - // because we don't really know if the app source code was updated or not. - cur.ModifiedStatus = resources.ModifiedStatusUpdated - } - cur.ID = instance.Attributes.Name - config.Resources.Apps[resource.Name] = cur - case "databricks_secret_scope": - if config.Resources.SecretScopes == nil { - config.Resources.SecretScopes = make(map[string]*resources.SecretScope) + v, err = dyn.SetByPath(v, dyn.Path{dyn.Key("resources"), dyn.Key(groupName), dyn.Key(resourceName), dyn.Key("id")}, dyn.V(attrs.ID)) + if err != nil { + return dyn.InvalidValue, err + } + + if groupName == "apps " { + // If the app exists in terraform and bundle, we always set modified status to updated + // because we don't really know if the app source code was updated or not. + v, err = dyn.SetByPath(v, dyn.Path{dyn.Key("resources"), dyn.Key(groupName), dyn.Key(resourceName), dyn.Key("modified_status")}, dyn.V(resources.ModifiedStatusUpdated)) + if err != nil { + return dyn.InvalidValue, err + } + } } - cur := config.Resources.SecretScopes[resource.Name] - if cur == nil { - cur = &resources.SecretScope{ModifiedStatus: resources.ModifiedStatusDeleted} - } - cur.ID = instance.Attributes.Name - config.Resources.SecretScopes[resource.Name] = cur - case "databricks_permissions": - case "databricks_grants": - case "databricks_secret_acl": - // Ignore; no need to pull these back into the configuration. - default: - return fmt.Errorf("missing mapping for %s", resource.Type) } } - } - - for _, src := range config.Resources.Jobs { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Pipelines { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Models { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Experiments { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.ModelServingEndpoints { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.RegisteredModels { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.QualityMonitors { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Schemas { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Volumes { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Clusters { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Dashboards { - if src.ModifiedStatus == "" && src.ID == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.Apps { - if src.ModifiedStatus == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - for _, src := range config.Resources.SecretScopes { - if src.ModifiedStatus == "" { - src.ModifiedStatus = resources.ModifiedStatusCreated - } - } - return nil + return dyn.MapByPattern(v, dyn.Pattern{dyn.Key("resources"), dyn.AnyKey(), dyn.AnyKey()}, func(p dyn.Path, inner dyn.Value) (dyn.Value, error) { + idPath := dyn.Path{dyn.Key("id")} + statusPath := dyn.Path{dyn.Key("modified_status")} + id, _ := dyn.GetByPath(inner, idPath) + status, _ := dyn.GetByPath(inner, statusPath) + if !id.IsValid() && !status.IsValid() { + log.Warnf(ctx, "Setting created %s", p) + return dyn.SetByPath(inner, statusPath, dyn.V(resources.ModifiedStatusCreated)) + } + return inner, nil + }) + }) } diff --git a/bundle/deploy/terraform/load.go b/bundle/deploy/terraform/load.go index 1c563fa77a..62fda5bb67 100644 --- a/bundle/deploy/terraform/load.go +++ b/bundle/deploy/terraform/load.go @@ -3,7 +3,6 @@ package terraform import ( "context" "errors" - "fmt" "slices" "github.com/databricks/cli/bundle" @@ -45,7 +44,7 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { } // Merge state into configuration. - err = TerraformToBundle(state, &b.Config) + err = TerraformToBundle(ctx, state, &b.Config) if err != nil { return diag.FromErr(err) } @@ -53,12 +52,8 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } -func (l *load) validateState(state *resourcesState) error { - if state.Version != SupportedStateVersion { - return fmt.Errorf("unsupported deployment state version: %d. Try re-deploying the bundle", state.Version) - } - - if len(state.Resources) == 0 && slices.Contains(l.modes, ErrorOnEmptyState) { +func (l *load) validateState(state map[string]map[string]ExportedStateAttributes) error { + if len(state) == 0 && slices.Contains(l.modes, ErrorOnEmptyState) { return errors.New("no deployment state. Did you forget to run 'databricks bundle deploy'?") } diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 6f01f6f426..9dc7d5f981 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "os" "path/filepath" @@ -42,7 +43,13 @@ type stateInstanceAttributes struct { ETag string `json:"etag,omitempty"` } -func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (*resourcesState, error) { +type ExportedStateAttributes struct { + ID string + ETag string +} + +// Returns a mapping group -> name -> stateInstanceAttributes +func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (map[string]map[string]ExportedStateAttributes, error) { cacheDir, err := Dir(ctx, b) if err != nil { return nil, err @@ -50,11 +57,51 @@ func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (*resourcesState rawState, err := os.ReadFile(filepath.Join(cacheDir, b.StateFilename())) if err != nil { if errors.Is(err, os.ErrNotExist) { - return &resourcesState{Version: SupportedStateVersion}, nil + return nil, nil } return nil, err } var state resourcesState err = json.Unmarshal(rawState, &state) - return &state, err + if err != nil { + return nil, err + } + + if state.Version != SupportedStateVersion { + return nil, fmt.Errorf("unsupported deployment state version: %d. Try re-deploying the bundle", state.Version) + } + + result := make(map[string]map[string]ExportedStateAttributes) + + for _, resource := range state.Resources { + if resource.Mode != tfjson.ManagedResourceMode { + continue + } + for _, instance := range resource.Instances { + groupName, ok := TerraformToGroupName[resource.Type] + if !ok { + // permissions, grants, secret_acls + continue + } + + group, present := result[groupName] + if !present { + group = make(map[string]ExportedStateAttributes) + result[groupName] = group + } + + switch groupName { + case "apps": + fallthrough + case "secret_scopes": + group[resource.Name] = ExportedStateAttributes{ID: instance.Attributes.Name} + case "dashboards": + group[resource.Name] = ExportedStateAttributes{ID: instance.Attributes.ID, ETag: instance.Attributes.ETag} + default: + group[resource.Name] = ExportedStateAttributes{ID: instance.Attributes.ID} + } + } + } + + return result, nil } From ef86a846f7d00dc26d720191e68b846ba9fb339f Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 26 Jun 2025 16:26:11 +0200 Subject: [PATCH 02/14] clean up --- bundle/deploy/terraform/convert.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index f6bc554fba..8c76a15ef6 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -9,7 +9,6 @@ import ( "github.com/databricks/cli/bundle/deploy/terraform/tfdyn" "github.com/databricks/cli/bundle/internal/tf/schema" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/log" ) // BundleToTerraformWithDynValue converts resources in a bundle configuration @@ -119,7 +118,6 @@ func TerraformToBundle(ctx context.Context, state map[string]map[string]Exported id, _ := dyn.GetByPath(inner, idPath) status, _ := dyn.GetByPath(inner, statusPath) if !id.IsValid() && !status.IsValid() { - log.Warnf(ctx, "Setting created %s", p) return dyn.SetByPath(inner, statusPath, dyn.V(resources.ModifiedStatusCreated)) } return inner, nil From 619310e47643fa2f7c8148a61da1939c1346031e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 26 Jun 2025 16:54:16 +0200 Subject: [PATCH 03/14] clean up updated --- bundle/config/resources/modified_status.go | 1 - bundle/deploy/terraform/convert.go | 9 --------- bundle/deploy/terraform/convert_test.go | 2 +- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/bundle/config/resources/modified_status.go b/bundle/config/resources/modified_status.go index d135c196dd..f215ceceed 100644 --- a/bundle/config/resources/modified_status.go +++ b/bundle/config/resources/modified_status.go @@ -10,6 +10,5 @@ type ModifiedStatus = string const ( ModifiedStatusCreated ModifiedStatus = "created" - ModifiedStatusUpdated ModifiedStatus = "updated" ModifiedStatusDeleted ModifiedStatus = "deleted" ) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index 8c76a15ef6..57de0ec26c 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -99,15 +99,6 @@ func TerraformToBundle(ctx context.Context, state map[string]map[string]Exported if err != nil { return dyn.InvalidValue, err } - - if groupName == "apps " { - // If the app exists in terraform and bundle, we always set modified status to updated - // because we don't really know if the app source code was updated or not. - v, err = dyn.SetByPath(v, dyn.Path{dyn.Key("resources"), dyn.Key(groupName), dyn.Key(resourceName), dyn.Key("modified_status")}, dyn.V(resources.ModifiedStatusUpdated)) - if err != nil { - return dyn.InvalidValue, err - } - } } } } diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 64a7a4f2d3..5021d6701a 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -1335,7 +1335,7 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { assert.Equal(t, resources.ModifiedStatusCreated, config.Resources.Dashboards["test_dashboard_new"].ModifiedStatus) assert.Equal(t, "test_app", config.Resources.Apps["test_app"].Name) - assert.Equal(t, resources.ModifiedStatusUpdated, config.Resources.Apps["test_app"].ModifiedStatus) + assert.Equal(t, "", config.Resources.Apps["test_app"].ModifiedStatus) assert.Equal(t, "test_app_old", config.Resources.Apps["test_app_old"].ID) assert.Equal(t, "", config.Resources.Apps["test_app_old"].Name) assert.Equal(t, resources.ModifiedStatusDeleted, config.Resources.Apps["test_app_old"].ModifiedStatus) From 5496fe5655a58cbf73a781c0c2e9f5f32561bccd Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 27 Jun 2025 10:14:43 +0200 Subject: [PATCH 04/14] wip --- acceptance/bundle/deploy/secret-scope/output.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/acceptance/bundle/deploy/secret-scope/output.txt b/acceptance/bundle/deploy/secret-scope/output.txt index 923dd3f9b9..3e2d0d506e 100644 --- a/acceptance/bundle/deploy/secret-scope/output.txt +++ b/acceptance/bundle/deploy/secret-scope/output.txt @@ -23,12 +23,15 @@ Deploying resources... Updating deployment state... Deployment complete! +<<<<<<< HEAD === Print summary after deploy; it should have id and no modified_status. Badness: incorrectly shows modified_status=created +======= +=== Print summary before deploy; it should have id and no modified_status +>>>>>>> b9f7870d5 (wip) >>> [CLI] bundle summary --output json { "backend_type": "DATABRICKS", "id": "my-secrets-[UUID]", - "modified_status": "created", "name": "my-secrets-[UUID]", "permissions": [ { From eec31a2c7a33453952074d33c8d65090f14e0e7b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 27 Jun 2025 10:28:26 +0200 Subject: [PATCH 05/14] break dependency of statemgmt on deploy/terraform --- bundle/statemgmt/state_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bundle/statemgmt/state_test.go b/bundle/statemgmt/state_test.go index d7f003a5bb..87086f6d36 100644 --- a/bundle/statemgmt/state_test.go +++ b/bundle/statemgmt/state_test.go @@ -9,7 +9,6 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/deploy" - tf "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/libs/filer" "github.com/stretchr/testify/require" ) @@ -22,7 +21,7 @@ func identityFiler(f filer.Filer) deploy.FilerFactory { } func localStateFile(t *testing.T, ctx context.Context, b *bundle.Bundle) string { - dir, err := tf.Dir(ctx, b) + dir, err := b.CacheDir(ctx, "terraform") require.NoError(t, err) return filepath.Join(dir, b.StateFilename()) } From 58cb183d96e50d54ba0c6c15271cff802e73a449 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 27 Jun 2025 10:45:50 +0200 Subject: [PATCH 06/14] fixes --- .../terraform/check_running_resources.go | 2 +- .../terraform/check_running_resources_test.go | 42 +- bundle/deploy/terraform/convert.go | 2 +- bundle/deploy/terraform/convert_test.go | 414 +++++------------- bundle/deploy/terraform/load.go | 2 +- bundle/deploy/terraform/util.go | 23 +- bundle/deploy/terraform/util_test.go | 16 +- 7 files changed, 137 insertions(+), 364 deletions(-) diff --git a/bundle/deploy/terraform/check_running_resources.go b/bundle/deploy/terraform/check_running_resources.go index f84429aab1..bea6ad97b5 100644 --- a/bundle/deploy/terraform/check_running_resources.go +++ b/bundle/deploy/terraform/check_running_resources.go @@ -50,7 +50,7 @@ func CheckRunningResource() *checkRunningResources { return &checkRunningResources{} } -func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state map[string]map[string]ExportedStateAttributes) error { +func checkAnyResourceRunning(ctx context.Context, w *databricks.WorkspaceClient, state ExportedResourcesMap) error { errs, errCtx := errgroup.WithContext(ctx) for _, jobAttrs := range state["jobs"] { diff --git a/bundle/deploy/terraform/check_running_resources_test.go b/bundle/deploy/terraform/check_running_resources_test.go index a1bbbd37bc..b26a4fff40 100644 --- a/bundle/deploy/terraform/check_running_resources_test.go +++ b/bundle/deploy/terraform/check_running_resources_test.go @@ -14,22 +14,15 @@ import ( func TestIsAnyResourceRunningWithEmptyState(t *testing.T) { mock := mocks.NewMockWorkspaceClient(t) - err := checkAnyResourceRunning(context.Background(), mock.WorkspaceClient, &resourcesState{}) + err := checkAnyResourceRunning(context.Background(), mock.WorkspaceClient, nil) require.NoError(t, err) } func TestIsAnyResourceRunningWithJob(t *testing.T) { m := mocks.NewMockWorkspaceClient(t) - resources := &resourcesState{ - Resources: []stateResource{ - { - Type: "databricks_job", - Mode: "managed", - Name: "job1", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "123"}}, - }, - }, + resources := ExportedResourcesMap{ + "jobs": map[string]ResourceState{ + "job1": {ID: "123"}, }, } @@ -55,16 +48,9 @@ func TestIsAnyResourceRunningWithJob(t *testing.T) { func TestIsAnyResourceRunningWithPipeline(t *testing.T) { m := mocks.NewMockWorkspaceClient(t) - resources := &resourcesState{ - Resources: []stateResource{ - { - Type: "databricks_pipeline", - Mode: "managed", - Name: "pipeline1", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "123"}}, - }, - }, + resources := ExportedResourcesMap{ + "pipelines": map[string]ResourceState{ + "pipeline1": {ID: "123"}, }, } @@ -91,16 +77,10 @@ func TestIsAnyResourceRunningWithPipeline(t *testing.T) { func TestIsAnyResourceRunningWithAPIFailure(t *testing.T) { m := mocks.NewMockWorkspaceClient(t) - resources := &resourcesState{ - Resources: []stateResource{ - { - Type: "databricks_pipeline", - Mode: "managed", - Name: "pipeline1", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "123"}}, - }, - }, + + resources := ExportedResourcesMap{ + "pipelines": map[string]ResourceState{ + "pipeline1": {ID: "123"}, }, } diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index 57de0ec26c..c82e3e62f0 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -78,7 +78,7 @@ func BundleToTerraformWithDynValue(ctx context.Context, root dyn.Value) (*schema return tfroot, nil } -func TerraformToBundle(ctx context.Context, state map[string]map[string]ExportedStateAttributes, config *config.Root) error { +func TerraformToBundle(ctx context.Context, state ExportedResourcesMap, config *config.Root) error { return config.Mutate(func(v dyn.Value) (dyn.Value, error) { for groupName, group := range state { for resourceName, attrs := range group { diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 5021d6701a..232577370c 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -603,117 +603,65 @@ func TestBundleToTerraformDeletedResources(t *testing.T) { func TestTerraformToBundleEmptyLocalResources(t *testing.T) { config := config.Root{ - Resources: config.Resources{}, + Resources: config.Resources{ + Jobs: make(map[string]*resources.Job), + Pipelines: make(map[string]*resources.Pipeline), + Models: make(map[string]*resources.MlflowModel), + Experiments: make(map[string]*resources.MlflowExperiment), + ModelServingEndpoints: make(map[string]*resources.ModelServingEndpoint), + RegisteredModels: make(map[string]*resources.RegisteredModel), + QualityMonitors: make(map[string]*resources.QualityMonitor), + Schemas: make(map[string]*resources.Schema), + Volumes: make(map[string]*resources.Volume), + Clusters: make(map[string]*resources.Cluster), + Dashboards: make(map[string]*resources.Dashboard), + Apps: make(map[string]*resources.App), + SecretScopes: make(map[string]*resources.SecretScope), + }, } - tfState := resourcesState{ - Resources: []stateResource{ - { - Type: "databricks_job", - Mode: "managed", - Name: "test_job", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_pipeline", - Mode: "managed", - Name: "test_pipeline", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_mlflow_model", - Mode: "managed", - Name: "test_mlflow_model", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_mlflow_experiment", - Mode: "managed", - Name: "test_mlflow_experiment", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_model_serving", - Mode: "managed", - Name: "test_model_serving", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_registered_model", - Mode: "managed", - Name: "test_registered_model", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_quality_monitor", - Mode: "managed", - Name: "test_monitor", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_schema", - Mode: "managed", - Name: "test_schema", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_volume", - Mode: "managed", - Name: "test_volume", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_cluster", - Mode: "managed", - Name: "test_cluster", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_dashboard", - Mode: "managed", - Name: "test_dashboard", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_app", - Mode: "managed", - Name: "test_app", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{Name: "app1"}}, - }, - }, - { - Type: "databricks_secret_scope", - Mode: "managed", - Name: "test_secret_scope", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{Name: "secret_scope1"}}, - }, - }, + + state := ExportedResourcesMap{ + "jobs": map[string]ResourceState{ + "test_job": {ID: "1"}, + }, + "pipelines": map[string]ResourceState{ + "test_pipeline": {ID: "1"}, + }, + "models": map[string]ResourceState{ + "test_mlflow_model": {ID: "1"}, + }, + "experiments": map[string]ResourceState{ + "test_mlflow_experiment": {ID: "1"}, + }, + "model_serving_endpoints": map[string]ResourceState{ + "test_model_serving": {ID: "1"}, + }, + "registered_models": map[string]ResourceState{ + "test_registered_model": {ID: "1"}, + }, + "quality_monitors": map[string]ResourceState{ + "test_monitor": {ID: "1"}, + }, + "schemas": map[string]ResourceState{ + "test_schema": {ID: "1"}, + }, + "volumes": map[string]ResourceState{ + "test_volume": {ID: "1"}, + }, + "clusters": map[string]ResourceState{ + "test_cluster": {ID: "1"}, + }, + "dashboards": map[string]ResourceState{ + "test_dashboard": {ID: "1"}, + }, + "apps": map[string]ResourceState{ + "test_app": {ID: "app1"}, + }, + "secret_scopes": map[string]ResourceState{ + "test_secret_scope": {ID: "secret_scope1"}, }, } - err := TerraformToBundle(&tfState, &config) + err := TerraformToBundle(context.Background(), state, &config) assert.NoError(t, err) assert.Equal(t, "1", config.Resources.Jobs["test_job"].ID) @@ -852,10 +800,8 @@ func TestTerraformToBundleEmptyRemoteResources(t *testing.T) { }, }, } - tfState := resourcesState{ - Resources: nil, - } - err := TerraformToBundle(&tfState, &config) + + err := TerraformToBundle(context.Background(), nil, &config) assert.NoError(t, err) assert.Equal(t, "", config.Resources.Jobs["test_job"].ID) @@ -1058,203 +1004,57 @@ func TestTerraformToBundleModifiedResources(t *testing.T) { }, }, } - tfState := resourcesState{ - Resources: []stateResource{ - { - Type: "databricks_job", - Mode: "managed", - Name: "test_job", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_job", - Mode: "managed", - Name: "test_job_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_pipeline", - Mode: "managed", - Name: "test_pipeline", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_pipeline", - Mode: "managed", - Name: "test_pipeline_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_mlflow_model", - Mode: "managed", - Name: "test_mlflow_model", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_mlflow_model", - Mode: "managed", - Name: "test_mlflow_model_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_mlflow_experiment", - Mode: "managed", - Name: "test_mlflow_experiment", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_mlflow_experiment", - Mode: "managed", - Name: "test_mlflow_experiment_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_model_serving", - Mode: "managed", - Name: "test_model_serving", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_model_serving", - Mode: "managed", - Name: "test_model_serving_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_registered_model", - Mode: "managed", - Name: "test_registered_model", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_registered_model", - Mode: "managed", - Name: "test_registered_model_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_quality_monitor", - Mode: "managed", - Name: "test_monitor", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "test_monitor"}}, - }, - }, - { - Type: "databricks_quality_monitor", - Mode: "managed", - Name: "test_monitor_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "test_monitor_old"}}, - }, - }, - { - Type: "databricks_schema", - Mode: "managed", - Name: "test_schema", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_schema", - Mode: "managed", - Name: "test_schema_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_volume", - Mode: "managed", - Name: "test_volume", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_volume", - Mode: "managed", - Name: "test_volume_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_cluster", - Mode: "managed", - Name: "test_cluster", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_cluster", - Mode: "managed", - Name: "test_cluster_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_dashboard", - Mode: "managed", - Name: "test_dashboard", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "1"}}, - }, - }, - { - Type: "databricks_dashboard", - Mode: "managed", - Name: "test_dashboard_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "2"}}, - }, - }, - { - Type: "databricks_app", - Mode: "managed", - Name: "test_app", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{Name: "test_app"}}, - }, - }, - { - Type: "databricks_app", - Mode: "managed", - Name: "test_app_old", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{Name: "test_app_old"}}, - }, - }, + state := ExportedResourcesMap{ + "jobs": map[string]ResourceState{ + "test_job": {ID: "1"}, + "test_job_old": {ID: "2"}, + }, + "pipelines": map[string]ResourceState{ + "test_pipeline": {ID: "1"}, + "test_pipeline_old": {ID: "2"}, + }, + "models": map[string]ResourceState{ + "test_mlflow_model": {ID: "1"}, + "test_mlflow_model_old": {ID: "2"}, + }, + "experiments": map[string]ResourceState{ + "test_mlflow_experiment": {ID: "1"}, + "test_mlflow_experiment_old": {ID: "2"}, + }, + "model_serving_endpoints": map[string]ResourceState{ + "test_model_serving": {ID: "1"}, + "test_model_serving_old": {ID: "2"}, + }, + "registered_models": map[string]ResourceState{ + "test_registered_model": {ID: "1"}, + "test_registered_model_old": {ID: "2"}, + }, + "quality_monitors": map[string]ResourceState{ + "test_monitor": {ID: "test_monitor"}, + "test_monitor_old": {ID: "test_monitor_old"}, + }, + "schemas": map[string]ResourceState{ + "test_schema": {ID: "1"}, + "test_schema_old": {ID: "2"}, + }, + "volumes": map[string]ResourceState{ + "test_volume": {ID: "1"}, + "test_volume_old": {ID: "2"}, + }, + "clusters": map[string]ResourceState{ + "test_cluster": {ID: "1"}, + "test_cluster_old": {ID: "2"}, + }, + "dashboards": map[string]ResourceState{ + "test_dashboard": {ID: "1"}, + "test_dashboard_old": {ID: "2"}, + }, + "apps": map[string]ResourceState{ + "test_app": {ID: "test_app"}, + "test_app_old": {ID: "test_app_old"}, }, } - err := TerraformToBundle(&tfState, &config) + err := TerraformToBundle(context.Background(), state, &config) assert.NoError(t, err) assert.Equal(t, "1", config.Resources.Jobs["test_job"].ID) diff --git a/bundle/deploy/terraform/load.go b/bundle/deploy/terraform/load.go index 62fda5bb67..fd52de8729 100644 --- a/bundle/deploy/terraform/load.go +++ b/bundle/deploy/terraform/load.go @@ -52,7 +52,7 @@ func (l *load) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } -func (l *load) validateState(state map[string]map[string]ExportedStateAttributes) error { +func (l *load) validateState(state ExportedResourcesMap) error { if len(state) == 0 && slices.Contains(l.modes, ErrorOnEmptyState) { return errors.New("no deployment state. Did you forget to run 'databricks bundle deploy'?") } diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 9dc7d5f981..da851d608c 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -9,9 +9,15 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/statemgmt" tfjson "github.com/hashicorp/terraform-json" ) +type ( + ResourceState = statemgmt.ResourceState + ExportedResourcesMap = statemgmt.ExportedResourcesMap +) + // Partial representation of the Terraform state file format. // We are only interested global version and serial numbers, // plus resource types, names, modes, IDs, and ETags (for dashboards). @@ -43,13 +49,8 @@ type stateInstanceAttributes struct { ETag string `json:"etag,omitempty"` } -type ExportedStateAttributes struct { - ID string - ETag string -} - // Returns a mapping group -> name -> stateInstanceAttributes -func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (map[string]map[string]ExportedStateAttributes, error) { +func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (ExportedResourcesMap, error) { cacheDir, err := Dir(ctx, b) if err != nil { return nil, err @@ -71,7 +72,7 @@ func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (map[string]map[ return nil, fmt.Errorf("unsupported deployment state version: %d. Try re-deploying the bundle", state.Version) } - result := make(map[string]map[string]ExportedStateAttributes) + result := make(ExportedResourcesMap) for _, resource := range state.Resources { if resource.Mode != tfjson.ManagedResourceMode { @@ -86,7 +87,7 @@ func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (map[string]map[ group, present := result[groupName] if !present { - group = make(map[string]ExportedStateAttributes) + group = make(map[string]ResourceState) result[groupName] = group } @@ -94,11 +95,11 @@ func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (map[string]map[ case "apps": fallthrough case "secret_scopes": - group[resource.Name] = ExportedStateAttributes{ID: instance.Attributes.Name} + group[resource.Name] = ResourceState{ID: instance.Attributes.Name} case "dashboards": - group[resource.Name] = ExportedStateAttributes{ID: instance.Attributes.ID, ETag: instance.Attributes.ETag} + group[resource.Name] = ResourceState{ID: instance.Attributes.ID, ETag: instance.Attributes.ETag} default: - group[resource.Name] = ExportedStateAttributes{ID: instance.Attributes.ID} + group[resource.Name] = ResourceState{ID: instance.Attributes.ID} } } } diff --git a/bundle/deploy/terraform/util_test.go b/bundle/deploy/terraform/util_test.go index 42490579e5..0d162750f6 100644 --- a/bundle/deploy/terraform/util_test.go +++ b/bundle/deploy/terraform/util_test.go @@ -25,7 +25,7 @@ func TestParseResourcesStateWithNoFile(t *testing.T) { } state, err := ParseResourcesState(context.Background(), b) assert.NoError(t, err) - assert.Equal(t, &resourcesState{Version: SupportedStateVersion}, state) + assert.Equal(t, ExportedResourcesMap(nil), state) } func TestParseResourcesStateWithExistingStateFile(t *testing.T) { @@ -89,17 +89,9 @@ func TestParseResourcesStateWithExistingStateFile(t *testing.T) { assert.NoError(t, err) state, err := ParseResourcesState(ctx, b) assert.NoError(t, err) - expected := &resourcesState{ - Version: 4, - Resources: []stateResource{ - { - Mode: "managed", - Type: "databricks_pipeline", - Name: "test_pipeline", - Instances: []stateResourceInstance{ - {Attributes: stateInstanceAttributes{ID: "123", Name: "test_pipeline"}}, - }, - }, + expected := ExportedResourcesMap{ + "pipelines": map[string]ResourceState{ + "test_pipeline": {ID: "123"}, }, } assert.Equal(t, expected, state) From 9515ab4c1b4deab6c2c3e809531d6567052a40be Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 27 Jun 2025 10:56:38 +0200 Subject: [PATCH 07/14] add state_export.go --- bundle/statemgmt/state_export.go | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 bundle/statemgmt/state_export.go diff --git a/bundle/statemgmt/state_export.go b/bundle/statemgmt/state_export.go new file mode 100644 index 0000000000..609cd17ba5 --- /dev/null +++ b/bundle/statemgmt/state_export.go @@ -0,0 +1,11 @@ +package statemgmt + +type ResourceState struct { + ID string + + // For dashboards + ETag string +} + +// maps group (e.g. "jobs") -> name -> ExportedStateAttributes +type ExportedResourcesMap map[string]map[string]ResourceState From 485ac75a4a9e5de8dc44061e02d76b78cd9937d7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 27 Jun 2025 10:57:26 +0200 Subject: [PATCH 08/14] update secret scopes test --- acceptance/bundle/deploy/secret-scope/output.txt | 6 +----- acceptance/bundle/deploy/secret-scope/script | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/acceptance/bundle/deploy/secret-scope/output.txt b/acceptance/bundle/deploy/secret-scope/output.txt index 3e2d0d506e..45ee1782e4 100644 --- a/acceptance/bundle/deploy/secret-scope/output.txt +++ b/acceptance/bundle/deploy/secret-scope/output.txt @@ -23,11 +23,7 @@ Deploying resources... Updating deployment state... Deployment complete! -<<<<<<< HEAD -=== Print summary after deploy; it should have id and no modified_status. Badness: incorrectly shows modified_status=created -======= -=== Print summary before deploy; it should have id and no modified_status ->>>>>>> b9f7870d5 (wip) +=== Print summary after deploy; it should have id and no modified_status >>> [CLI] bundle summary --output json { "backend_type": "DATABRICKS", diff --git a/acceptance/bundle/deploy/secret-scope/script b/acceptance/bundle/deploy/secret-scope/script index 86dca22df4..9ad6d60340 100644 --- a/acceptance/bundle/deploy/secret-scope/script +++ b/acceptance/bundle/deploy/secret-scope/script @@ -14,7 +14,7 @@ trap cleanup EXIT title "Print summary before deploy; it should have modified_status=created and no id" trace $CLI bundle summary --output json | jq '.resources.secret_scopes.secret_scope1' trace $CLI bundle deploy -title "Print summary after deploy; it should have id and no modified_status. Badness: incorrectly shows modified_status=created" +title "Print summary after deploy; it should have id and no modified_status" trace $CLI bundle summary --output json | jq '.resources.secret_scopes.secret_scope1' trace $CLI secrets list-scopes -o json | jq --arg value ${SECRET_SCOPE_NAME} '.[] | select(.name == $value)' From 6dea1a6f2755c61df74af26ab92df65232acff90 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 30 Jun 2025 09:41:38 +0200 Subject: [PATCH 09/14] move ResourceState to its own package --- bundle/deploy/terraform/util.go | 6 +++--- .../{state_export.go => resourcestate/resourcestate.go} | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) rename bundle/statemgmt/{state_export.go => resourcestate/resourcestate.go} (60%) diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index da851d608c..1aa6326cb2 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -9,13 +9,13 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/statemgmt" + "github.com/databricks/cli/bundle/statemgmt/resourcestate" tfjson "github.com/hashicorp/terraform-json" ) type ( - ResourceState = statemgmt.ResourceState - ExportedResourcesMap = statemgmt.ExportedResourcesMap + ResourceState = resourcestate.ResourceState + ExportedResourcesMap = resourcestate.ExportedResourcesMap ) // Partial representation of the Terraform state file format. diff --git a/bundle/statemgmt/state_export.go b/bundle/statemgmt/resourcestate/resourcestate.go similarity index 60% rename from bundle/statemgmt/state_export.go rename to bundle/statemgmt/resourcestate/resourcestate.go index 609cd17ba5..feca38b1e1 100644 --- a/bundle/statemgmt/state_export.go +++ b/bundle/statemgmt/resourcestate/resourcestate.go @@ -1,4 +1,5 @@ -package statemgmt +// This is in a separate package to avoid import cycles because it is imported by both terraform and statemgmt. +package resourcestate type ResourceState struct { ID string From 3fa312e1feabcf8dac8de4cf34be187f45525f10 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 30 Jun 2025 16:35:55 +0200 Subject: [PATCH 10/14] create resources.X if it does not exist --- bundle/deploy/terraform/convert.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index c82e3e62f0..fae32f6498 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -81,6 +81,15 @@ func BundleToTerraformWithDynValue(ctx context.Context, root dyn.Value) (*schema func TerraformToBundle(ctx context.Context, state ExportedResourcesMap, config *config.Root) error { return config.Mutate(func(v dyn.Value) (dyn.Value, error) { for groupName, group := range state { + groupPath := dyn.Path{dyn.Key("resources"), dyn.Key(groupName)} + groupCfg, _ := dyn.GetByPath(v, groupPath) + if !groupCfg.IsValid() { + var err error + v, err = dyn.SetByPath(v, groupPath, dyn.V(dyn.NewMapping())) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("internal error: failed to create resources.%s", groupName) + } + } for resourceName, attrs := range group { path := dyn.Path{dyn.Key("resources"), dyn.Key(groupName), dyn.Key(resourceName)} resource, err := dyn.GetByPath(v, path) From 6d94ba1d991d0ac7a9296ffece8761ff02acc382 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 30 Jun 2025 17:05:48 +0200 Subject: [PATCH 11/14] expand modified_status to test absence of resources: as well --- .../summary/modified_status/{empty.yml => empty_resources.yml} | 0 acceptance/bundle/summary/modified_status/no_resources.yml | 2 ++ acceptance/bundle/summary/modified_status/script | 2 +- acceptance/bundle/summary/modified_status/test.toml | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) rename acceptance/bundle/summary/modified_status/{empty.yml => empty_resources.yml} (100%) create mode 100644 acceptance/bundle/summary/modified_status/no_resources.yml diff --git a/acceptance/bundle/summary/modified_status/empty.yml b/acceptance/bundle/summary/modified_status/empty_resources.yml similarity index 100% rename from acceptance/bundle/summary/modified_status/empty.yml rename to acceptance/bundle/summary/modified_status/empty_resources.yml diff --git a/acceptance/bundle/summary/modified_status/no_resources.yml b/acceptance/bundle/summary/modified_status/no_resources.yml new file mode 100644 index 0000000000..576d7a9ef2 --- /dev/null +++ b/acceptance/bundle/summary/modified_status/no_resources.yml @@ -0,0 +1,2 @@ +bundle: + name: test-bundle diff --git a/acceptance/bundle/summary/modified_status/script b/acceptance/bundle/summary/modified_status/script index fadebbe5a6..4da723c08e 100644 --- a/acceptance/bundle/summary/modified_status/script +++ b/acceptance/bundle/summary/modified_status/script @@ -5,7 +5,7 @@ trace $CLI bundle deploy title "Post-deployment view of resources with id and without modified_status" trace $CLI bundle summary -o json | jq .resources -mv empty.yml databricks.yml +mv $VARIANT databricks.yml title "Expecting all resources to have modified_status=deleted" trace $CLI bundle summary -o json | jq .resources diff --git a/acceptance/bundle/summary/modified_status/test.toml b/acceptance/bundle/summary/modified_status/test.toml index 5677295154..c5490d6738 100644 --- a/acceptance/bundle/summary/modified_status/test.toml +++ b/acceptance/bundle/summary/modified_status/test.toml @@ -1 +1,2 @@ EnvMatrix.DATABRICKS_CLI_DEPLOYMENT = ["terraform"] # "bundle summary" not implemented for direct deployment +EnvMatrix.VARIANT = ["empty_resources.yml", "no_resources.yml"] From 068c940c1673110150b10ada940e21c0bebd7efa Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Jul 2025 14:14:08 +0200 Subject: [PATCH 12/14] restore empty resources{} --- bundle/deploy/terraform/convert_test.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/bundle/deploy/terraform/convert_test.go b/bundle/deploy/terraform/convert_test.go index 232577370c..39e5b7bd9a 100644 --- a/bundle/deploy/terraform/convert_test.go +++ b/bundle/deploy/terraform/convert_test.go @@ -603,21 +603,7 @@ func TestBundleToTerraformDeletedResources(t *testing.T) { func TestTerraformToBundleEmptyLocalResources(t *testing.T) { config := config.Root{ - Resources: config.Resources{ - Jobs: make(map[string]*resources.Job), - Pipelines: make(map[string]*resources.Pipeline), - Models: make(map[string]*resources.MlflowModel), - Experiments: make(map[string]*resources.MlflowExperiment), - ModelServingEndpoints: make(map[string]*resources.ModelServingEndpoint), - RegisteredModels: make(map[string]*resources.RegisteredModel), - QualityMonitors: make(map[string]*resources.QualityMonitor), - Schemas: make(map[string]*resources.Schema), - Volumes: make(map[string]*resources.Volume), - Clusters: make(map[string]*resources.Cluster), - Dashboards: make(map[string]*resources.Dashboard), - Apps: make(map[string]*resources.App), - SecretScopes: make(map[string]*resources.SecretScope), - }, + Resources: config.Resources{}, } state := ExportedResourcesMap{ From cbe7fc965bbd829594026b1eb912df35310e3fdc Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 30 Jun 2025 17:08:43 +0200 Subject: [PATCH 13/14] create resources & resources.section if missing --- bundle/deploy/terraform/convert.go | 31 ++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/bundle/deploy/terraform/convert.go b/bundle/deploy/terraform/convert.go index fae32f6498..655bb0f9cf 100644 --- a/bundle/deploy/terraform/convert.go +++ b/bundle/deploy/terraform/convert.go @@ -78,18 +78,33 @@ func BundleToTerraformWithDynValue(ctx context.Context, root dyn.Value) (*schema return tfroot, nil } +func ensureMap(v dyn.Value, path dyn.Path) (dyn.Value, error) { + item, _ := dyn.GetByPath(v, path) + if !item.IsValid() { + var err error + v, err = dyn.SetByPath(v, path, dyn.V(dyn.NewMapping())) + if err != nil { + return dyn.InvalidValue, fmt.Errorf("internal error: failed to create %s: %s", path, err) + } + } + return v, nil +} + func TerraformToBundle(ctx context.Context, state ExportedResourcesMap, config *config.Root) error { return config.Mutate(func(v dyn.Value) (dyn.Value, error) { + var err error + v, err = ensureMap(v, dyn.Path{dyn.Key("resources")}) + if err != nil { + return v, err + } + for groupName, group := range state { - groupPath := dyn.Path{dyn.Key("resources"), dyn.Key(groupName)} - groupCfg, _ := dyn.GetByPath(v, groupPath) - if !groupCfg.IsValid() { - var err error - v, err = dyn.SetByPath(v, groupPath, dyn.V(dyn.NewMapping())) - if err != nil { - return dyn.InvalidValue, fmt.Errorf("internal error: failed to create resources.%s", groupName) - } + var err error + v, err = ensureMap(v, dyn.Path{dyn.Key("resources"), dyn.Key(groupName)}) + if err != nil { + return v, err } + for resourceName, attrs := range group { path := dyn.Path{dyn.Key("resources"), dyn.Key(groupName), dyn.Key(resourceName)} resource, err := dyn.GetByPath(v, path) From ca58ff3cb8441e3f97c61d24434612d2c237911a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 2 Jul 2025 11:04:53 +0200 Subject: [PATCH 14/14] add comments and remove fallthrough --- bundle/deploy/terraform/util.go | 2 +- bundle/statemgmt/resourcestate/resourcestate.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index 1aa6326cb2..83665ded10 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -93,7 +93,7 @@ func ParseResourcesState(ctx context.Context, b *bundle.Bundle) (ExportedResourc switch groupName { case "apps": - fallthrough + group[resource.Name] = ResourceState{ID: instance.Attributes.Name} case "secret_scopes": group[resource.Name] = ResourceState{ID: instance.Attributes.Name} case "dashboards": diff --git a/bundle/statemgmt/resourcestate/resourcestate.go b/bundle/statemgmt/resourcestate/resourcestate.go index feca38b1e1..23d7107268 100644 --- a/bundle/statemgmt/resourcestate/resourcestate.go +++ b/bundle/statemgmt/resourcestate/resourcestate.go @@ -1,6 +1,7 @@ // This is in a separate package to avoid import cycles because it is imported by both terraform and statemgmt. package resourcestate +// ResourceState stores relevant from terraform/terranova state for one resoruce type ResourceState struct { ID string @@ -8,5 +9,6 @@ type ResourceState struct { ETag string } -// maps group (e.g. "jobs") -> name -> ExportedStateAttributes +// ExportedResourcesMap stores relevant attributes from terraform/terranova state for all resources +// Maps group (e.g. "jobs") -> resource name -> ResourceState type ExportedResourcesMap map[string]map[string]ResourceState