diff --git a/.nextchanges/bundles/permissions-rekey.md b/.nextchanges/bundles/permissions-rekey.md new file mode 100644 index 00000000000..f2943ab76c1 --- /dev/null +++ b/.nextchanges/bundles/permissions-rekey.md @@ -0,0 +1 @@ +* Fix permissions of a recreated pipeline, app, dashboard, model serving endpoint or other resource staying recorded under the old ID, which made every later `bundle plan` propose to apply them again. ([#7361](https://github.com/databricks/cli/pull/7361)) diff --git a/acceptance/bundle/resources/model_serving_endpoints/basic/out.second-plan.txt b/acceptance/bundle/resources/model_serving_endpoints/basic/out.second-plan.txt index 60bca95b9ff..f8508239cfc 100644 --- a/acceptance/bundle/resources/model_serving_endpoints/basic/out.second-plan.txt +++ b/acceptance/bundle/resources/model_serving_endpoints/basic/out.second-plan.txt @@ -31,7 +31,7 @@ json.plan.resources.model_serving_endpoints.my_endpoint.changes.route_optimized. json.plan.resources.model_serving_endpoints.my_endpoint.changes.route_optimized.remote = false; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.depends_on[0].node = "resources.model_serving_endpoints.my_endpoint"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.depends_on[0].label = "${resources.model_serving_endpoints.my_endpoint.endpoint_id}"; -json.plan.resources.model_serving_endpoints.my_endpoint.permissions.action = "update"; +json.plan.resources.model_serving_endpoints.my_endpoint.permissions.action = "update_id"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.new_state.value.object_id = ""; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.new_state.value.__embed__[0].level = "CAN_MANAGE"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.new_state.value.__embed__[0].service_principal_name = "[USERNAME]"; @@ -43,7 +43,8 @@ json.plan.resources.model_serving_endpoints.my_endpoint.permissions.remote_state json.plan.resources.model_serving_endpoints.my_endpoint.permissions.remote_state.__embed__[0].service_principal_name = "[USERNAME]"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.remote_state.__embed__[1].level = "CAN_VIEW"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.remote_state.__embed__[1].user_name = "deco-test-user@databricks.com"; -json.plan.resources.model_serving_endpoints.my_endpoint.permissions.changes.object_id.action = "update"; +json.plan.resources.model_serving_endpoints.my_endpoint.permissions.changes.object_id.action = "update_id"; +json.plan.resources.model_serving_endpoints.my_endpoint.permissions.changes.object_id.reason = "id_changes"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.changes.object_id.old = "/serving-endpoints/[ENDPOINT_ID_1]"; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.changes.object_id.new = ""; json.plan.resources.model_serving_endpoints.my_endpoint.permissions.changes.object_id.remote = "/serving-endpoints/[ENDPOINT_ID_1]"; diff --git a/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/output.txt b/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/output.txt index fdeba3e5dc4..6c9c004b74e 100644 --- a/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/output.txt +++ b/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/output.txt @@ -48,11 +48,9 @@ Resources: 0 created, 1 changed, 0 deleted, 1 unchanged "creator": "[USERNAME]" } -=== Persistent drift after deploy: V1 permissions API leaves an ACL on the deleted endpoint id, so plan keeps showing an update. +=== After deploy the permissions are recorded under the new endpoint id, so the plan is clean >>> [CLI] bundle plan -update model_serving_endpoints.my_endpoint.permissions - -Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged +Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: diff --git a/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/script b/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/script index cdc3cb1de33..5b6f5db4b58 100644 --- a/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/script +++ b/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/script @@ -36,5 +36,5 @@ trace $CLI bundle plan trace $CLI bundle deploy trace $CLI serving-endpoints get "${endpoint_name}" | jq '{name, creator}' -title "Persistent drift after deploy: V1 permissions API leaves an ACL on the deleted endpoint id, so plan keeps showing an update." +title "After deploy the permissions are recorded under the new endpoint id, so the plan is clean" trace $CLI bundle plan diff --git a/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/test.toml b/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/test.toml index 0423113d9fa..c7fc201902b 100644 --- a/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/test.toml +++ b/acceptance/bundle/resources/model_serving_endpoints/drift/recreated_same_name/test.toml @@ -1,4 +1,2 @@ -Badness = "After deleting and recreating a model serving endpoint remotely with the same name but a different endpoint_id, bundle plan/deploy ends up with a permanent update on permissions because the V1 permissions API does not delete ACLs immediately when the parent is gone." - Cloud = true RecordRequests = false diff --git a/acceptance/bundle/resources/permissions/pipelines/recreate/databricks.yml.tmpl b/acceptance/bundle/resources/permissions/pipelines/recreate/databricks.yml.tmpl new file mode 100644 index 00000000000..867479a2531 --- /dev/null +++ b/acceptance/bundle/resources/permissions/pipelines/recreate/databricks.yml.tmpl @@ -0,0 +1,20 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + pipelines: + foo: + name: test-pipeline-$UNIQUE_NAME + libraries: + - notebook: + path: ./nb.sql + catalog: main + target: ${resources.schemas.bar.name} + permissions: + - level: CAN_VIEW + group_name: users + + schemas: + bar: + catalog_name: main + name: test-schema-$UNIQUE_NAME diff --git a/acceptance/bundle/resources/permissions/pipelines/recreate/nb.sql b/acceptance/bundle/resources/permissions/pipelines/recreate/nb.sql new file mode 100644 index 00000000000..199ff507884 --- /dev/null +++ b/acceptance/bundle/resources/permissions/pipelines/recreate/nb.sql @@ -0,0 +1,2 @@ +-- Databricks notebook source +select 1 diff --git a/acceptance/bundle/resources/permissions/pipelines/recreate/out.test.toml b/acceptance/bundle/resources/permissions/pipelines/recreate/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/resources/permissions/pipelines/recreate/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/permissions/pipelines/recreate/output.txt b/acceptance/bundle/resources/permissions/pipelines/recreate/output.txt new file mode 100644 index 00000000000..09eb1af7f79 --- /dev/null +++ b/acceptance/bundle/resources/permissions/pipelines/recreate/output.txt @@ -0,0 +1,101 @@ + +=== Deploy a pipeline with permissions +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Created pipelines.foo +Created pipelines.foo.permissions +Created schemas.bar +Files: 3 uploaded, 0 deleted +Resources: 3 created, 0 changed, 0 deleted, 0 unchanged + +>>> print_requests.py //permissions +{ + "method": "PUT", + "path": "/api/2.0/permissions/pipelines/[OLD_PIPELINE_ID]", + "body": { + "access_control_list": [ + { + "group_name": "users", + "permission_level": "CAN_VIEW" + }, + { + "permission_level": "IS_OWNER", + "service_principal_name": "[USERNAME]" + } + ] + } +} + +>>> read_state.py pipelines foo.permissions id +pipelines foo.permissions id='/pipelines/[OLD_PIPELINE_ID]' + +=== Switch from catalog to storage: the pipeline is recreated with a new ID +>>> update_file.py databricks.yml catalog: main storage: dbfs:/my-storage + +>>> update_file.py databricks.yml target: ${resources.schemas.bar.name} #target: ${resources.schemas.bar.name} + +>>> [CLI] bundle plan --output json +{ + "object_id": { + "action": "update_id", + "reason": "id_changes", + "old": "/pipelines/[OLD_PIPELINE_ID]", + "new": "", + "remote": "/pipelines/[OLD_PIPELINE_ID]" + } +} + +=== Deploy recreates the pipeline and applies the permissions to the new one +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... + +This action will result in the deletion or recreation of the following Lakeflow Spark Declarative Pipelines along with the +Streaming Tables (STs) and Materialized Views (MVs) managed by them. Recreating the pipelines will +restore the defined STs and MVs through full refresh. Note that recreation is necessary when pipeline +properties such as the 'catalog' or 'storage' are changed: + recreate resources.pipelines.foo +Recreated pipelines.foo +Updated pipelines.foo.permissions +Files: 1 uploaded, 0 deleted +Resources: 1 created, 1 changed, 1 deleted, 1 unchanged + +>>> print_requests.py //permissions +{ + "method": "PUT", + "path": "/api/2.0/permissions/pipelines/[NEW_PIPELINE_ID]", + "body": { + "access_control_list": [ + { + "group_name": "users", + "permission_level": "CAN_VIEW" + }, + { + "permission_level": "IS_OWNER", + "service_principal_name": "[USERNAME]" + } + ] + } +} + +=== The permissions are recorded under the new ID, so the next plan is a no-op +>>> read_state.py pipelines foo.permissions id +pipelines foo.permissions id='/pipelines/[NEW_PIPELINE_ID]' + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 3 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.pipelines.foo + delete resources.schemas.bar + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.bar + +This action will result in the deletion of the following Lakeflow Spark Declarative Pipelines along with the +Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascade_on_destroy: false' on a pipeline to retain datasets on pipeline deletion: + delete resources.pipelines.foo + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 2 deleted diff --git a/acceptance/bundle/resources/permissions/pipelines/recreate/script b/acceptance/bundle/resources/permissions/pipelines/recreate/script new file mode 100644 index 00000000000..44da8809b53 --- /dev/null +++ b/acceptance/bundle/resources/permissions/pipelines/recreate/script @@ -0,0 +1,29 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve + + # Consume the destroy's requests so none are left recorded. + print_requests.py //permissions &> LOG.destroy_requests +} +trap cleanup EXIT + +title "Deploy a pipeline with permissions" +trace $CLI bundle deploy +add_repl.py "$($CLI bundle summary -o json | jq -r .resources.pipelines.foo.id)" OLD_PIPELINE_ID +trace print_requests.py //permissions +trace read_state.py pipelines foo.permissions id + +title "Switch from catalog to storage: the pipeline is recreated with a new ID" +trace update_file.py databricks.yml "catalog: main" "storage: dbfs:/my-storage" +trace update_file.py databricks.yml 'target: ${resources.schemas.bar.name}' '#target: ${resources.schemas.bar.name}' +trace $CLI bundle plan --output json | jq '.plan["resources.pipelines.foo.permissions"].changes' + +title "Deploy recreates the pipeline and applies the permissions to the new one" +trace $CLI bundle deploy --auto-approve +add_repl.py "$($CLI bundle summary -o json | jq -r .resources.pipelines.foo.id)" NEW_PIPELINE_ID +trace print_requests.py //permissions + +title "The permissions are recorded under the new ID, so the next plan is a no-op" +trace read_state.py pipelines foo.permissions id +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/permissions/pipelines/recreate/test.toml b/acceptance/bundle/resources/permissions/pipelines/recreate/test.toml new file mode 100644 index 00000000000..1e6f401e694 --- /dev/null +++ b/acceptance/bundle/resources/permissions/pipelines/recreate/test.toml @@ -0,0 +1,12 @@ +Cloud = true + +Ignore = [ + ".databricks", + ".gitignore", + "databricks.yml", +] + +# The deployer is added as IS_OWNER: a service principal on AWS and Azure, a user locally and on GCP. +[[Repls]] +Old = '"user_name": "\[USERNAME\]"' +New = '"service_principal_name": "[USERNAME]"' diff --git a/acceptance/bundle/state/permission_level_migration/out.plan.direct.json b/acceptance/bundle/state/permission_level_migration/out.plan.direct.json index e02d691b171..76eb0731217 100644 --- a/acceptance/bundle/state/permission_level_migration/out.plan.direct.json +++ b/acceptance/bundle/state/permission_level_migration/out.plan.direct.json @@ -29,7 +29,7 @@ "label": "${resources.jobs.my_job.id}" } ], - "action": "update", + "action": "update_id", "new_state": { "value": { "object_id": "", @@ -79,7 +79,8 @@ ] }, "object_id": { - "action": "update", + "action": "update_id", + "reason": "id_changes", "old": "/jobs/123", "new": "", "remote": "/jobs/123" diff --git a/acceptance/bundle/state/permission_level_migration/output.txt b/acceptance/bundle/state/permission_level_migration/output.txt index c642fb01f52..2091843525f 100644 --- a/acceptance/bundle/state/permission_level_migration/output.txt +++ b/acceptance/bundle/state/permission_level_migration/output.txt @@ -35,7 +35,7 @@ Resources: 1 created, 1 changed, 0 deleted, 0 unchanged } }, "resources.jobs.my_job.permissions": { - "__id__": "/jobs/123", + "__id__": "/jobs/[MY_JOB_ID]", "state": { "object_id": "/jobs/[MY_JOB_ID]", "__embed__": [ diff --git a/bundle/direct/dresources/README.md b/bundle/direct/dresources/README.md index 0262a2d11e0..78952537943 100644 --- a/bundle/direct/dresources/README.md +++ b/bundle/direct/dresources/README.md @@ -17,7 +17,7 @@ Each field with special plan/deploy behavior must be declared in the YAML file of its resource type, under `configs/`. Both files of a resource type are optional and are omitted when they would be empty: - - `configs/.yml` (e.g. `configs/jobs.yml`) — hand-written rules. A file named after a sub-resource (`configs/grants.yml`) applies to that sub-resource of every parent. + - `configs/.yml` (e.g. `configs/jobs.yml`) — hand-written rules. A file named after a sub-resource (`configs/grants.yml`) applies to that sub-resource of every parent without a file of its own. - `configs/.generated.yml` — API field behaviors from the OpenAPI schema. Generated by `./task generate-direct-resources`, do not edit. Choose the right category: diff --git a/bundle/direct/dresources/config.go b/bundle/direct/dresources/config.go index 9552f935d1c..9b3ae9080b8 100644 --- a/bundle/direct/dresources/config.go +++ b/bundle/direct/dresources/config.go @@ -139,7 +139,7 @@ type Config struct { // OpenAPI spec. A resource type without rules has no file. The file name is the // resource type, so each file holds the rules directly with no enclosing keys. // A file named after a sub-resource (grants.yml) applies to that sub-resource of -// every parent (catalogs.grants, schemas.grants, ...). +// every parent (catalogs.grants, schemas.grants, ...) that has no file of its own. // //go:embed configs/*.yml var configFS embed.FS @@ -193,9 +193,11 @@ var loadConfigs = sync.OnceValues(func() (*Config, *Config) { panic(fmt.Errorf("%s: %w", name, err)) } + _, isResourceType := SupportedResources[resourceType] for _, rt := range configResourceTypes(resourceType) { - if _, ok := dst.Resources[rt]; ok { - panic(fmt.Errorf("%s: rules for %s are already defined by another file", name, rt)) + // A resource type's own file replaces the shared one, whichever is read first. + if _, ok := dst.Resources[rt]; ok && !isResourceType { + continue } dst.Resources[rt] = rc } diff --git a/bundle/direct/dresources/config_test.go b/bundle/direct/dresources/config_test.go index 4cf0d7d22be..53279eed6c6 100644 --- a/bundle/direct/dresources/config_test.go +++ b/bundle/direct/dresources/config_test.go @@ -43,12 +43,24 @@ func TestGetResourceConfig(t *testing.T) { assert.Empty(t, GetResourceConfig("nonexistent").RecreateOnChanges) } -// TestSubResourceConfig checks that grants.yml applies to the grants of every parent. +// TestSubResourceConfig checks that grants.yml and permissions.yml apply to the grants and +// permissions of every parent, except where a parent has its own file. func TestSubResourceConfig(t *testing.T) { for resourceType := range SupportedResources { - if strings.HasSuffix(resourceType, ".grants") { - assert.NotEmptyf(t, GetResourceConfig(resourceType).UpdatableIDFields, "%s: grants.yml does not apply", resourceType) + var field string + switch { + case resourceType == "secret_scopes.permissions": + field = "scope_name" // secret_scopes.permissions.yml + case strings.HasSuffix(resourceType, ".grants"): + field = "full_name" // grants.yml + case strings.HasSuffix(resourceType, ".permissions"): + field = "object_id" // permissions.yml + default: + continue } + rules := GetResourceConfig(resourceType).UpdatableIDFields + require.Lenf(t, rules, 1, "%s", resourceType) + assert.Equalf(t, field, rules[0].Field.String(), "%s", resourceType) } } diff --git a/bundle/direct/dresources/configs/permissions.yml b/bundle/direct/dresources/configs/permissions.yml new file mode 100644 index 00000000000..b74426fb1cd --- /dev/null +++ b/bundle/direct/dresources/configs/permissions.yml @@ -0,0 +1,6 @@ +updatable_id_fields: + # The ID is the parent's object ID, which changes when the parent is recreated. + # UpdateWithID records the new ID, so DoRead does not keep reading the permissions + # of the deleted object. + - field: object_id + reason: id_changes diff --git a/bundle/direct/dresources/permissions.go b/bundle/direct/dresources/permissions.go index c2aebb4d0ac..7f2eb0dbf04 100644 --- a/bundle/direct/dresources/permissions.go +++ b/bundle/direct/dresources/permissions.go @@ -237,6 +237,16 @@ func (r *ResourcePermissions) DoCreate(ctx context.Context, newState *Permission return newState.ObjectID, nil, nil } +// DoUpdateWithID records the new ID when the parent is recreated under a new ID. Otherwise +// DoRead keeps reading the permissions of the old object and every later plan reports an update. +func (r *ResourcePermissions) DoUpdateWithID(ctx context.Context, id string, newState *PermissionsState, entry *PlanEntry) (string, *PermissionsState, error) { + _, err := r.DoUpdate(ctx, id, newState, entry) + if err != nil { + return "", nil, err + } + return newState.ObjectID, nil, nil +} + // DoUpdate calls https://docs.databricks.com/api/workspace/jobs/setjobpermissions. func (r *ResourcePermissions) DoUpdate(ctx context.Context, _ string, newState *PermissionsState, _ *PlanEntry) (*PermissionsState, error) { extractedType, extractedID, err := parsePermissionsID(newState.ObjectID)