diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 55159800e5..2c13750d4b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -14,6 +14,7 @@ * engine/direct: Fix unwanted recreation of secret scopes when scope_backend_type is not set ([#4834](https://github.com/databricks/cli/pull/4834)) * engine/direct: Fix bind and unbind for non-Terraform resources ([#4850](https://github.com/databricks/cli/pull/4850)) * engine/direct: Fix deploying removed principals ([#4824](https://github.com/databricks/cli/pull/4824)) +* engine/direct: Fix secret scope permissions migration from Terraform to Direct engine ([#4866](https://github.com/databricks/cli/pull/4866)) ### Dependency updates diff --git a/acceptance/bundle/invariant/configs/secret_scope_with_permissions.yml.tmpl b/acceptance/bundle/invariant/configs/secret_scope_with_permissions.yml.tmpl new file mode 100644 index 0000000000..daa61aaaaa --- /dev/null +++ b/acceptance/bundle/invariant/configs/secret_scope_with_permissions.yml.tmpl @@ -0,0 +1,13 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + secret_scopes: + foo: + name: test-scope-$UNIQUE_NAME + backend_type: DATABRICKS + permissions: + - level: READ + group_name: users + - level: WRITE + group_name: admins diff --git a/acceptance/bundle/invariant/continue_293/out.test.toml b/acceptance/bundle/invariant/continue_293/out.test.toml index 711e04631f..199859bd7c 100644 --- a/acceptance/bundle/invariant/continue_293/out.test.toml +++ b/acceptance/bundle/invariant/continue_293/out.test.toml @@ -4,4 +4,4 @@ RequiresUnityCatalog = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct"] - INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] + INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] diff --git a/acceptance/bundle/invariant/migrate/out.test.toml b/acceptance/bundle/invariant/migrate/out.test.toml index c12dd02d1f..7da28d9c55 100644 --- a/acceptance/bundle/invariant/migrate/out.test.toml +++ b/acceptance/bundle/invariant/migrate/out.test.toml @@ -4,4 +4,4 @@ RequiresUnityCatalog = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct"] - INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] + INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] diff --git a/acceptance/bundle/invariant/migrate/test.toml b/acceptance/bundle/invariant/migrate/test.toml index d73fd65904..5ffa32ae13 100644 --- a/acceptance/bundle/invariant/migrate/test.toml +++ b/acceptance/bundle/invariant/migrate/test.toml @@ -2,10 +2,6 @@ EnvMatrixExclude.no_catalog = ["INPUT_CONFIG=catalog.yml.tmpl"] EnvMatrixExclude.no_external_location = ["INPUT_CONFIG=external_location.yml.tmpl"] -# Unexpected action='create' for resources.secret_scopes.foo.permissions -EnvMatrixExclude.no_secret_scope = ["INPUT_CONFIG=secret_scope.yml.tmpl"] -EnvMatrixExclude.no_secret_scope2 = ["INPUT_CONFIG=secret_scope_default_backend_type.yml.tmpl"] - # Cross-resource permission references (e.g. ${resources.jobs.job_b.permissions[0].level}) # don't work in terraform mode: the terraform interpolator converts the path to # ${databricks_job.job_b.permissions[0].level}, but Terraform's databricks_job resource diff --git a/acceptance/bundle/invariant/no_drift/out.test.toml b/acceptance/bundle/invariant/no_drift/out.test.toml index c12dd02d1f..7da28d9c55 100644 --- a/acceptance/bundle/invariant/no_drift/out.test.toml +++ b/acceptance/bundle/invariant/no_drift/out.test.toml @@ -4,4 +4,4 @@ RequiresUnityCatalog = true [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct"] - INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] + INPUT_CONFIG = ["alert.yml.tmpl", "app.yml.tmpl", "catalog.yml.tmpl", "cluster.yml.tmpl", "dashboard.yml.tmpl", "database_catalog.yml.tmpl", "database_instance.yml.tmpl", "experiment.yml.tmpl", "external_location.yml.tmpl", "job.yml.tmpl", "job_pydabs_10_tasks.yml.tmpl", "job_pydabs_1000_tasks.yml.tmpl", "job_cross_resource_ref.yml.tmpl", "job_permission_ref.yml.tmpl", "job_with_permissions.yml.tmpl", "job_with_task.yml.tmpl", "model.yml.tmpl", "model_serving_endpoint.yml.tmpl", "pipeline.yml.tmpl", "postgres_branch.yml.tmpl", "postgres_endpoint.yml.tmpl", "postgres_project.yml.tmpl", "registered_model.yml.tmpl", "schema.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl"] diff --git a/acceptance/bundle/invariant/test.toml b/acceptance/bundle/invariant/test.toml index 00b826c9cf..cf7cd12c30 100644 --- a/acceptance/bundle/invariant/test.toml +++ b/acceptance/bundle/invariant/test.toml @@ -49,6 +49,7 @@ EnvMatrix.INPUT_CONFIG = [ "schema_with_grants.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", + "secret_scope_with_permissions.yml.tmpl", "synced_database_table.yml.tmpl", "volume.yml.tmpl", ] diff --git a/acceptance/bundle/resources/secret_scopes/basic/out.plan1.terraform.json b/acceptance/bundle/resources/secret_scopes/basic/out.plan1.terraform.json index f10683b207..0266fceefc 100644 --- a/acceptance/bundle/resources/secret_scopes/basic/out.plan1.terraform.json +++ b/acceptance/bundle/resources/secret_scopes/basic/out.plan1.terraform.json @@ -3,6 +3,9 @@ "plan": { "resources.secret_scopes.my_scope": { "action": "create" + }, + "resources.secret_scopes.my_scope.permissions": { + "action": "create" } } } diff --git a/acceptance/bundle/resources/secret_scopes/basic/out.plan2.terraform.json b/acceptance/bundle/resources/secret_scopes/basic/out.plan2.terraform.json index 299892e6dd..d61ac7b77a 100644 --- a/acceptance/bundle/resources/secret_scopes/basic/out.plan2.terraform.json +++ b/acceptance/bundle/resources/secret_scopes/basic/out.plan2.terraform.json @@ -3,6 +3,9 @@ "plan": { "resources.secret_scopes.my_scope": { "action": "recreate" + }, + "resources.secret_scopes.my_scope.permissions": { + "action": "recreate" } } } diff --git a/acceptance/bundle/resources/secret_scopes/basic/out.plan_verify_no_drift.terraform.json b/acceptance/bundle/resources/secret_scopes/basic/out.plan_verify_no_drift.terraform.json index a5a4c6840f..2bf6a06f48 100644 --- a/acceptance/bundle/resources/secret_scopes/basic/out.plan_verify_no_drift.terraform.json +++ b/acceptance/bundle/resources/secret_scopes/basic/out.plan_verify_no_drift.terraform.json @@ -3,6 +3,9 @@ "plan": { "resources.secret_scopes.my_scope": { "action": "skip" + }, + "resources.secret_scopes.my_scope.permissions": { + "action": "skip" } } } diff --git a/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt b/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt index 989400461f..1307df4e1a 100644 --- a/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt +++ b/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt @@ -1,5 +1,6 @@ >>> [CLI] bundle plan delete secret_scopes.second +delete secret_scopes.second.permissions -Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged +Plan: 0 to add, 0 to change, 2 to delete, 1 unchanged diff --git a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.terraform.txt b/acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.terraform.txt deleted file mode 100644 index 9057d991b2..0000000000 --- a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.terraform.txt +++ /dev/null @@ -1,3 +0,0 @@ -create secret_scopes.my_scope - -Plan: 1 to add, 0 to change, 0 to delete, 0 unchanged diff --git a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.direct.txt b/acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.txt similarity index 100% rename from acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.direct.txt rename to acceptance/bundle/resources/secret_scopes/permissions/out.plan.create.txt diff --git a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.recreate.terraform.txt b/acceptance/bundle/resources/secret_scopes/permissions/out.plan.recreate.terraform.txt index a619e377e8..6126244cbe 100644 --- a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.recreate.terraform.txt +++ b/acceptance/bundle/resources/secret_scopes/permissions/out.plan.recreate.terraform.txt @@ -1,3 +1,4 @@ recreate secret_scopes.my_scope +recreate secret_scopes.my_scope.permissions -Plan: 1 to add, 0 to change, 1 to delete, 0 unchanged +Plan: 2 to add, 0 to change, 2 to delete, 0 unchanged diff --git a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.terraform.txt b/acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.terraform.txt deleted file mode 100644 index c54c9d511c..0000000000 --- a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.terraform.txt +++ /dev/null @@ -1 +0,0 @@ -Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged diff --git a/acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.direct.txt b/acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.txt similarity index 100% rename from acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.direct.txt rename to acceptance/bundle/resources/secret_scopes/permissions/out.plan.update.txt diff --git a/acceptance/bundle/resources/secret_scopes/permissions/script b/acceptance/bundle/resources/secret_scopes/permissions/script index 7c261749a1..2b866baa75 100755 --- a/acceptance/bundle/resources/secret_scopes/permissions/script +++ b/acceptance/bundle/resources/secret_scopes/permissions/script @@ -36,7 +36,7 @@ cleanup() { trap cleanup EXIT title "create secret scope with permissions" -trace $CLI bundle plan > out.plan.create.$DATABRICKS_BUNDLE_ENGINE.txt +trace $CLI bundle plan > out.plan.create.txt trace $CLI bundle deploy scope_name=$($CLI bundle summary --output json | jq -r '.resources.secret_scopes.my_scope.name') @@ -60,7 +60,7 @@ resources: level: MANAGE EOF envsubst < databricks.yml.tmpl > databricks.yml -trace $CLI bundle plan > out.plan.update.$DATABRICKS_BUNDLE_ENGINE.txt +trace $CLI bundle plan > out.plan.update.txt trace $CLI bundle deploy trace $CLI secrets list-acls $scope_name | jq -c '.[]' | sort diff --git a/bundle/deploy/terraform/pkg.go b/bundle/deploy/terraform/pkg.go index 83ea796024..a66e5cb6a0 100644 --- a/bundle/deploy/terraform/pkg.go +++ b/bundle/deploy/terraform/pkg.go @@ -133,6 +133,7 @@ var GroupToTerraformName = map[string]string{ // 3 level groups: resources.*.GROUP "permissions": "databricks_permissions", "grants": "databricks_grants", + "secret_acls": "databricks_secret_acl", } var TerraformToGroupName = func() map[string]string { diff --git a/bundle/deploy/terraform/showplanfile.go b/bundle/deploy/terraform/showplanfile.go index 23e9436b34..c0cc7a4e0c 100644 --- a/bundle/deploy/terraform/showplanfile.go +++ b/bundle/deploy/terraform/showplanfile.go @@ -10,12 +10,6 @@ import ( tfjson "github.com/hashicorp/terraform-json" ) -// silentlyUpdatedResources contains resource types that are automatically created by DABs, -// no need to show them in the plan -var silentlyUpdatedResources = map[string]bool{ - "databricks_secret_acl": true, -} - var prefixToGroup = []struct{ prefix, group string }{ {"job_", "jobs"}, {"pipeline_", "pipelines"}, @@ -63,6 +57,17 @@ func convertGrantsResourceNameToKey(terraformName string) string { return "" } +// convertSecretAclNameToScopeKey converts terraform secret ACL resource names to scope permission keys. +// ACL names have format "secret_acl__" (see convert_secret_scope.go). +// e.g., "secret_acl_my_scope_0" -> "resources.secret_scopes.my_scope.permissions" +func convertSecretAclNameToScopeKey(name string) string { + name, _ = strings.CutPrefix(name, "secret_acl_") + if i := strings.LastIndex(name, "_"); i >= 0 { + name = name[:i] + } + return "resources.secret_scopes." + name + ".permissions" +} + // populatePlan populates a deployplan.Plan from Terraform resource changes. func populatePlan(ctx context.Context, plan *deployplan.Plan, changes []*tfjson.ResourceChange) { for _, rc := range changes { @@ -88,25 +93,35 @@ func populatePlan(ctx context.Context, plan *deployplan.Plan, changes []*tfjson. group, ok := TerraformToGroupName[rc.Type] if !ok { - if !silentlyUpdatedResources[rc.Type] { - log.Warnf(ctx, "unknown resource type '%s'", rc.Type) - } + log.Warnf(ctx, "unknown resource type '%s'", rc.Type) continue } var key string switch group { case "permissions": - // Convert terraform permission resource name back to hierarchical resource key key = convertPermissionsResourceNameToKey(rc.Name) case "grants": - // Convert terraform grants resource name back to hierarchical resource key key = convertGrantsResourceNameToKey(rc.Name) + case "secret_acls": + key = convertSecretAclNameToScopeKey(rc.Name) default: key = "resources." + group + "." + rc.Name } - plan.Plan[key] = &deployplan.PlanEntry{Action: actionType} + if existing, ok := plan.Plan[key]; ok { + // For secret ACLs, multiple individual ACL changes are merged into a single + // scope-level permissions entry. When the actions differ (e.g., some ACLs are + // recreated while others are deleted), it means permissions are being updated, + // not deleted entirely. + if group == "secret_acls" && existing.Action != actionType { + existing.Action = deployplan.Update + } else { + existing.Action = deployplan.GetHigherAction(existing.Action, actionType) + } + } else { + plan.Plan[key] = &deployplan.PlanEntry{Action: actionType} + } } } diff --git a/bundle/deploy/terraform/showplanfile_test.go b/bundle/deploy/terraform/showplanfile_test.go index 75a2cc6f43..5c4426f665 100644 --- a/bundle/deploy/terraform/showplanfile_test.go +++ b/bundle/deploy/terraform/showplanfile_test.go @@ -76,3 +76,89 @@ func TestPopulatePlan(t *testing.T) { // Unknown resource type should not be in the plan assert.NotContains(t, plan.Plan, "resources.recreate whatever") } + +func TestPopulatePlanSecretAcl(t *testing.T) { + ctx := t.Context() + changes := []*tfjson.ResourceChange{ + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionCreate}}, + Name: "secret_acl_my_scope_0", + }, + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionDelete, tfjson.ActionCreate}}, + Name: "secret_acl_my_scope_1", + }, + } + + plan := deployplan.NewPlanTerraform() + populatePlan(ctx, plan, changes) + + // Multiple ACL changes for the same scope with different actions are merged as Update. + assert.Equal(t, map[string]*deployplan.PlanEntry{ + "resources.secret_scopes.my_scope.permissions": {Action: deployplan.Update}, + }, plan.Plan) +} + +func TestPopulatePlanSecretAclMixedCreateDelete(t *testing.T) { + ctx := t.Context() + changes := []*tfjson.ResourceChange{ + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionDelete}}, + Name: "secret_acl_my_scope_0", + }, + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionCreate}}, + Name: "secret_acl_my_scope_1", + }, + } + + plan := deployplan.NewPlanTerraform() + populatePlan(ctx, plan, changes) + + assert.Equal(t, map[string]*deployplan.PlanEntry{ + "resources.secret_scopes.my_scope.permissions": {Action: deployplan.Update}, + }, plan.Plan) +} + +func TestPopulatePlanSecretAclMixedRecreateDelete(t *testing.T) { + ctx := t.Context() + // Simulates a permission update where some ACLs are recreated (principal changed) + // and some are deleted (principal removed). This is the typical Terraform plan shape + // when updating secret scope permissions. + changes := []*tfjson.ResourceChange{ + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionDelete, tfjson.ActionCreate}}, + Name: "secret_acl_my_scope_0", + }, + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionDelete, tfjson.ActionCreate}}, + Name: "secret_acl_my_scope_1", + }, + { + Type: "databricks_secret_acl", + Change: &tfjson.Change{Actions: tfjson.Actions{tfjson.ActionDelete}}, + Name: "secret_acl_my_scope_2", + }, + } + + plan := deployplan.NewPlanTerraform() + populatePlan(ctx, plan, changes) + + // When permissions are being updated (some ACLs recreated, some deleted), + // the aggregated action should be Update, not Delete. + assert.Equal(t, map[string]*deployplan.PlanEntry{ + "resources.secret_scopes.my_scope.permissions": {Action: deployplan.Update}, + }, plan.Plan) +} + +func TestConvertSecretAclNameToScopeKey(t *testing.T) { + assert.Equal(t, "resources.secret_scopes.my_scope.permissions", convertSecretAclNameToScopeKey("secret_acl_my_scope_0")) + assert.Equal(t, "resources.secret_scopes.my_scope.permissions", convertSecretAclNameToScopeKey("secret_acl_my_scope_1")) + assert.Equal(t, "resources.secret_scopes.scope_123.permissions", convertSecretAclNameToScopeKey("secret_acl_scope_123_2")) +} diff --git a/bundle/deploy/terraform/util.go b/bundle/deploy/terraform/util.go index ff083995cf..632d32bca1 100644 --- a/bundle/deploy/terraform/util.go +++ b/bundle/deploy/terraform/util.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/statemgmt/resourcestate" + "github.com/databricks/cli/libs/log" tfjson "github.com/hashicorp/terraform-json" ) @@ -75,18 +76,27 @@ func parseResourcesState(ctx context.Context, path string) (ExportedResourcesMap continue } for _, instance := range resource.Instances { - groupName, ok := TerraformToGroupName[resource.Type] + var resourceKey string + var resourceState ResourceState + groupName, ok := TerraformToGroupName[resource.Type] if !ok { - // secret_acls + log.Warnf(ctx, "Unknown Terraform resource type: %s", resource.Type) continue } - var resourceKey string - var resourceState ResourceState - switch groupName { - case "apps", "secret_scopes", "database_instances", "database_catalogs", "synced_database_tables", "postgres_projects", "postgres_branches", "postgres_endpoints": + case "secret_acls": + // Secret ACLs don't have their own state entries; permissions are + // created alongside the scope in the "secret_scopes" case below. + continue + case "secret_scopes": + resourceKey = "resources." + groupName + "." + resource.Name + resourceState = ResourceState{ID: instance.Attributes.Name} + // The direct engine manages permissions as a sub-resource + // (SecretScopeFixups adds MANAGE ACL for the current user). + result[resourceKey+".permissions"] = ResourceState{ID: instance.Attributes.Name} + case "apps", "database_instances", "database_catalogs", "synced_database_tables", "postgres_projects", "postgres_branches", "postgres_endpoints": resourceKey = "resources." + groupName + "." + resource.Name resourceState = ResourceState{ID: instance.Attributes.Name} case "dashboards": diff --git a/bundle/deploy/terraform/util_test.go b/bundle/deploy/terraform/util_test.go index 3e4e6eb636..59b0f03635 100644 --- a/bundle/deploy/terraform/util_test.go +++ b/bundle/deploy/terraform/util_test.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParseResourcesStateWithNoFile(t *testing.T) { @@ -94,3 +95,67 @@ func TestParseResourcesStateWithExistingStateFile(t *testing.T) { } assert.Equal(t, expected, state) } + +func TestParseResourcesStateSecretScopeWithAcls(t *testing.T) { + ctx := t.Context() + data := []byte(`{ + "version": 4, + "resources": [ + { + "mode": "managed", + "type": "databricks_secret_scope", + "name": "my_scope", + "instances": [{"attributes": {"id": "123", "name": "actual-scope-name"}}] + }, + { + "mode": "managed", + "type": "databricks_secret_acl", + "name": "secret_acl_my_scope_0", + "instances": [{"attributes": {"id": "actual-scope-name|||user@example.com"}}] + }, + { + "mode": "managed", + "type": "databricks_secret_acl", + "name": "secret_acl_my_scope_1", + "instances": [{"attributes": {"id": "actual-scope-name|||data-team"}}] + } + ] + }`) + path := filepath.Join(t.TempDir(), "state.json") + require.NoError(t, os.WriteFile(path, data, 0o600)) + + state, err := parseResourcesState(ctx, path) + require.NoError(t, err) + + assert.Equal(t, ExportedResourcesMap{ + "resources.secret_scopes.my_scope": {ID: "actual-scope-name"}, + "resources.secret_scopes.my_scope.permissions": {ID: "actual-scope-name"}, + }, state) +} + +func TestParseResourcesStateSecretScopeWithoutAcls(t *testing.T) { + ctx := t.Context() + data := []byte(`{ + "version": 4, + "resources": [ + { + "mode": "managed", + "type": "databricks_secret_scope", + "name": "my_scope", + "instances": [{"attributes": {"id": "123", "name": "my-scope-name"}}] + } + ] + }`) + path := filepath.Join(t.TempDir(), "state.json") + require.NoError(t, os.WriteFile(path, data, 0o600)) + + state, err := parseResourcesState(ctx, path) + require.NoError(t, err) + + // Even without ACLs, a .permissions entry is created for every secret scope, + // so the direct engine doesn't plan a phantom "create" after migration. + assert.Equal(t, ExportedResourcesMap{ + "resources.secret_scopes.my_scope": {ID: "my-scope-name"}, + "resources.secret_scopes.my_scope.permissions": {ID: "my-scope-name"}, + }, state) +} diff --git a/bundle/statemgmt/upload_state_for_yaml_sync.go b/bundle/statemgmt/upload_state_for_yaml_sync.go index 7d7c766743..425cf67574 100644 --- a/bundle/statemgmt/upload_state_for_yaml_sync.go +++ b/bundle/statemgmt/upload_state_for_yaml_sync.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/engine" + "github.com/databricks/cli/bundle/config/mutator/resourcemutator" "github.com/databricks/cli/bundle/deploy" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/deployplan" @@ -22,6 +23,7 @@ import ( "github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" + "github.com/databricks/cli/libs/logdiag" "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structpath" ) @@ -135,6 +137,14 @@ func (m *uploadStateForYamlSync) convertState(ctx context.Context, b *bundle.Bun }, } + // Apply SecretScopeFixups so the config matches what the direct engine expects. + // This adds MANAGE ACL for the current user to all secret scopes, ensuring + // the migrated state and config agree on .permissions entries. + bundle.ApplyContext(ctx, b, resourcemutator.SecretScopeFixups(engine.EngineDirect)) + if logdiag.HasError(ctx) { + return diag.Errorf("failed to apply secret scope fixups") + } + // Get the dynamic value from b.Config and reverse the interpolation // b.Config has been modified by terraform.Interpolate which converts bundle-style // references (${resources.pipelines.x.id}) to terraform-style (${databricks_pipeline.x.id}) diff --git a/cmd/bundle/deployment/migrate.go b/cmd/bundle/deployment/migrate.go index 88e44e024e..07951f6bf6 100644 --- a/cmd/bundle/deployment/migrate.go +++ b/cmd/bundle/deployment/migrate.go @@ -12,7 +12,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/engine" - + "github.com/databricks/cli/bundle/config/mutator/resourcemutator" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/direct" @@ -244,6 +244,14 @@ To start using direct engine, set "engine: direct" under bundle in your databric } }() + // Apply SecretScopeFixups so the config matches what the direct engine expects. + // This adds MANAGE ACL for the current user to all secret scopes, ensuring + // the migrated state and config agree on .permissions entries. + bundle.ApplyContext(ctx, b, resourcemutator.SecretScopeFixups(engine.EngineDirect)) + if logdiag.HasError(ctx) { + return root.ErrAlreadyPrinted + } + plan, err := deploymentBundle.CalculatePlan(ctx, b.WorkspaceClient(), &b.Config, tempStatePath) if err != nil { return err