diff --git a/.nextchanges/bundles/permissions-empty-list.md b/.nextchanges/bundles/permissions-empty-list.md new file mode 100644 index 00000000000..a42cb5eb14e --- /dev/null +++ b/.nextchanges/bundles/permissions-empty-list.md @@ -0,0 +1 @@ +An explicit `permissions: []` on a resource is now managed like a non-empty list: the current user is added as the default owner (`IS_OWNER`), matching Terraform. Previously an empty list was treated as if the block were absent. diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/databricks.yml.tmpl b/acceptance/bundle/resources/grants/schemas/empty_list/databricks.yml.tmpl new file mode 100644 index 00000000000..ba68e8b85fe --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/databricks.yml.tmpl @@ -0,0 +1,12 @@ +bundle: + name: schema-grants-empty-list-$UNIQUE_NAME + +resources: + schemas: + grants_schema: + name: schema_grants_empty_list_$UNIQUE_NAME + catalog_name: main + # Unlike permissions, grants have no default owner: an empty list grants + # nothing. Note the engines diverge on create: direct emits a no-op PATCH + # with an empty body, terraform emits no request at all. + grants: [] diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/out.plan.direct.txt b/acceptance/bundle/resources/grants/schemas/empty_list/out.plan.direct.txt new file mode 100644 index 00000000000..4f432da715f --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/out.plan.direct.txt @@ -0,0 +1,33 @@ +{ + "plan_version": 2, + "cli_version": "[CLI_VERSION]", + "plan": { + "resources.schemas.grants_schema": { + "action": "create", + "new_state": { + "value": { + "catalog_name": "main", + "name": "schema_grants_empty_list_[UNIQUE_NAME]" + } + } + }, + "resources.schemas.grants_schema.grants": { + "depends_on": [ + { + "node": "resources.schemas.grants_schema", + "label": "${resources.schemas.grants_schema.id}" + } + ], + "action": "create", + "new_state": { + "value": { + "securable_type": "schema", + "full_name": "" + }, + "vars": { + "full_name": "${resources.schemas.grants_schema.id}" + } + } + } + } +} diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/out.plan.terraform.txt b/acceptance/bundle/resources/grants/schemas/empty_list/out.plan.terraform.txt new file mode 100644 index 00000000000..f0c57994804 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/out.plan.terraform.txt @@ -0,0 +1,8 @@ +{ + "cli_version": "[CLI_VERSION]", + "plan": { + "resources.schemas.grants_schema": { + "action": "create" + } + } +} diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/out.requests.deploy.direct.json b/acceptance/bundle/resources/grants/schemas/empty_list/out.requests.deploy.direct.json new file mode 100644 index 00000000000..f2fd13ab2d2 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/out.requests.deploy.direct.json @@ -0,0 +1,5 @@ +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.schema_grants_empty_list_[UNIQUE_NAME]", + "body": {} +} diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/out.requests.deploy.terraform.json b/acceptance/bundle/resources/grants/schemas/empty_list/out.requests.deploy.terraform.json new file mode 100644 index 00000000000..e69de29bb2d diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/out.test.toml b/acceptance/bundle/resources/grants/schemas/empty_list/out.test.toml new file mode 100644 index 00000000000..e849ec85ace --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/out.test.toml @@ -0,0 +1,4 @@ +Local = true +Cloud = true +RequiresUnityCatalog = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/output.txt b/acceptance/bundle/resources/grants/schemas/empty_list/output.txt new file mode 100644 index 00000000000..14a0763d9c5 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/output.txt @@ -0,0 +1,21 @@ + +>>> [CLI] bundle validate -o json +[] + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-empty-list-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> errcode [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.grants_schema + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.grants_schema + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/schema-grants-empty-list-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/script b/acceptance/bundle/resources/grants/schemas/empty_list/script new file mode 100644 index 00000000000..251df2a4af2 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/script @@ -0,0 +1,15 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace errcode $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT + +trace $CLI bundle validate -o json | jq .resources.schemas.grants_schema.grants + +errcode $CLI bundle plan -o json &> out.plan.$DATABRICKS_BUNDLE_ENGINE.txt +rm -f out.requests.txt + +trace $CLI bundle deploy +print_requests.py //permissions > out.requests.deploy.$DATABRICKS_BUNDLE_ENGINE.json diff --git a/acceptance/bundle/resources/grants/schemas/empty_list/test.toml b/acceptance/bundle/resources/grants/schemas/empty_list/test.toml new file mode 100644 index 00000000000..ab196c758c8 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/empty_list/test.toml @@ -0,0 +1 @@ +Timeout = "5m" diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/databricks.yml b/acceptance/bundle/resources/permissions/jobs/empty_list/databricks.yml index 79f03b68bd7..56475c0d2e4 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/databricks.yml +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/databricks.yml @@ -10,5 +10,5 @@ resources: notebook_task: notebook_path: /Workspace/Users/user@example.com/notebook source: WORKSPACE - # this is interpreted the same as nil, but it's IMO a bug; it should do default IS_OWNER setting + # An explicit empty list is managed: FixPermissions adds the default IS_OWNER for the current user. permissions: [] diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.direct.json b/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.direct.json index 0ec914589b1..88e1e115a9c 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.direct.json @@ -28,6 +28,29 @@ ] } } + }, + "resources.jobs.foo.permissions": { + "depends_on": [ + { + "node": "resources.jobs.foo", + "label": "${resources.jobs.foo.id}" + } + ], + "action": "create", + "new_state": { + "value": { + "object_id": "", + "__embed__": [ + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + }, + "vars": { + "object_id": "/jobs/${resources.jobs.foo.id}" + } + } } } } diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.terraform.json b/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.terraform.json index 5a250db78de..0db00e350f8 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/out.plan.terraform.json @@ -3,6 +3,9 @@ "plan": { "resources.jobs.foo": { "action": "create" + }, + "resources.jobs.foo.permissions": { + "action": "create" } } } diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.direct.json b/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.direct.json index e69de29bb2d..f7aa2dbaa0d 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.direct.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/jobs/[NUMID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.terraform.json index e69de29bb2d..f7aa2dbaa0d 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.deploy.terraform.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/jobs/[NUMID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.destroy.terraform.json b/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.destroy.terraform.json index e69de29bb2d..f7aa2dbaa0d 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.destroy.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/out.requests.destroy.terraform.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/jobs/[NUMID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/jobs/empty_list/output.txt b/acceptance/bundle/resources/permissions/jobs/empty_list/output.txt index ec71aa04be9..1e0de881407 100644 --- a/acceptance/bundle/resources/permissions/jobs/empty_list/output.txt +++ b/acceptance/bundle/resources/permissions/jobs/empty_list/output.txt @@ -1,6 +1,11 @@ >>> [CLI] bundle validate -o json -null +[ + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } +] >>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json index 57ee99c53f4..d19fda4235d 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json @@ -78,7 +78,18 @@ "label": "${resources.jobs.job_with_permissions.id}" } ], - "action": "delete", + "action": "update", + "new_state": { + "value": { + "object_id": "/jobs/[JOB_WITH_PERMISSIONS_ID]", + "__embed__": [ + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } + }, "remote_state": { "object_id": "/jobs/[JOB_WITH_PERMISSIONS_ID]", "__embed__": [ @@ -95,6 +106,30 @@ "user_name": "[USERNAME]" } ] + }, + "changes": { + "[group_name='data-team']": { + "action": "update", + "old": { + "level": "CAN_MANAGE", + "group_name": "data-team" + }, + "remote": { + "level": "CAN_MANAGE", + "group_name": "data-team" + } + }, + "[user_name='viewer@example.com']": { + "action": "update", + "old": { + "level": "CAN_VIEW", + "user_name": "viewer@example.com" + }, + "remote": { + "level": "CAN_VIEW", + "user_name": "viewer@example.com" + } + } } } } diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json index dea726555ec..cc4ceee2774 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json @@ -5,7 +5,7 @@ "action": "skip" }, "resources.jobs.job_with_permissions.permissions": { - "action": "delete" + "action": "update" } } } diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.requests_destroy.terraform.json b/acceptance/bundle/resources/permissions/jobs/update/out.requests_destroy.terraform.json index 73495bdc38f..f66a05564c6 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.requests_destroy.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.requests_destroy.terraform.json @@ -1,3 +1,15 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/jobs/[JOB_WITH_PERMISSIONS_ID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} { "method": "POST", "path": "/api/2.2/jobs/delete", diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.requests_set_empty.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.requests_set_empty.direct.json index e69de29bb2d..95d354f99a4 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.requests_set_empty.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.requests_set_empty.direct.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/jobs/[JOB_WITH_PERMISSIONS_ID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/output.txt b/acceptance/bundle/resources/permissions/output.txt index 85a16ad6a38..7090203247e 100644 --- a/acceptance/bundle/resources/permissions/output.txt +++ b/acceptance/bundle/resources/permissions/output.txt @@ -180,7 +180,24 @@ DIFF jobs/delete_one/out.requests_destroy.direct.json "body": { "job_id": "[JOB_WITH_PERMISSIONS_ID]" EXACT jobs/empty_list/out.requests.deploy.direct.json -EXACT jobs/empty_list/out.requests.destroy.direct.json +DIFF jobs/empty_list/out.requests.destroy.direct.json +--- jobs/empty_list/out.requests.destroy.direct.json ++++ jobs/empty_list/out.requests.destroy.terraform.json +@@ -1 +1,14 @@ +-[]+[ ++ { ++ "body": { ++ "access_control_list": [ ++ { ++ "permission_level": "IS_OWNER", ++ "user_name": "[USERNAME]" ++ } ++ ] ++ }, ++ "method": "PUT", ++ "path": "/api/2.0/permissions/jobs/[NUMID]" ++ } ++] MATCH jobs/other_can_manage/out.requests.deploy.direct.json DIFF jobs/other_can_manage/out.requests.destroy.direct.json --- jobs/other_can_manage/out.requests.destroy.direct.json @@ -257,12 +274,11 @@ DIFF jobs/update/out.requests_delete_all.direct.json + "path": "/api/2.0/permissions/jobs/[JOB_WITH_PERMISSIONS_ID]" + } +] -EXACT jobs/update/out.requests_destroy.direct.json -DIFF jobs/update/out.requests_set_empty.direct.json ---- jobs/update/out.requests_set_empty.direct.json -+++ jobs/update/out.requests_set_empty.terraform.json -@@ -1 +1,14 @@ --[]+[ +DIFF jobs/update/out.requests_destroy.direct.json +--- jobs/update/out.requests_destroy.direct.json ++++ jobs/update/out.requests_destroy.terraform.json +@@ -1,4 +1,16 @@ + [ + { + "body": { + "access_control_list": [ @@ -274,8 +290,11 @@ DIFF jobs/update/out.requests_set_empty.direct.json + }, + "method": "PUT", + "path": "/api/2.0/permissions/jobs/[JOB_WITH_PERMISSIONS_ID]" -+ } -+] ++ }, + { + "body": { + "job_id": "[JOB_WITH_PERMISSIONS_ID]" +EXACT jobs/update/out.requests_set_empty.direct.json MATCH jobs/viewers/out.requests.deploy.direct.json DIFF jobs/viewers/out.requests.destroy.direct.json --- jobs/viewers/out.requests.destroy.direct.json diff --git a/acceptance/bundle/resources/permissions/pipelines/empty_list/databricks.yml b/acceptance/bundle/resources/permissions/pipelines/empty_list/databricks.yml index 6089d045ffd..6db75dbfda1 100644 --- a/acceptance/bundle/resources/permissions/pipelines/empty_list/databricks.yml +++ b/acceptance/bundle/resources/permissions/pipelines/empty_list/databricks.yml @@ -5,5 +5,5 @@ resources: pipelines: foo: name: foo - # this is interpreted the same as nil, but it's IMO a bug; it should do default IS_OWNER setting + # An explicit empty list is managed: FixPermissions adds the default IS_OWNER for the current user. permissions: [] diff --git a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.direct.json b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.direct.json index 97493239403..029250bd21a 100644 --- a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.direct.json +++ b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.direct.json @@ -15,6 +15,29 @@ "name": "foo" } } + }, + "resources.pipelines.foo.permissions": { + "depends_on": [ + { + "node": "resources.pipelines.foo", + "label": "${resources.pipelines.foo.id}" + } + ], + "action": "create", + "new_state": { + "value": { + "object_id": "", + "__embed__": [ + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + }, + "vars": { + "object_id": "/pipelines/${resources.pipelines.foo.id}" + } + } } } } diff --git a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.terraform.json b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.terraform.json index 4305cb583e4..58cb00a270d 100644 --- a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.terraform.json +++ b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.plan.terraform.json @@ -3,6 +3,9 @@ "plan": { "resources.pipelines.foo": { "action": "create" + }, + "resources.pipelines.foo.permissions": { + "action": "create" } } } diff --git a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.direct.json b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.direct.json index e69de29bb2d..578163835c1 100644 --- a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.direct.json +++ b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.direct.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/pipelines/[UUID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.terraform.json index e69de29bb2d..578163835c1 100644 --- a/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.terraform.json +++ b/acceptance/bundle/resources/permissions/pipelines/empty_list/out.requests.deploy.terraform.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/pipelines/[UUID]", + "body": { + "access_control_list": [ + { + "permission_level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/pipelines/empty_list/output.txt b/acceptance/bundle/resources/permissions/pipelines/empty_list/output.txt index 2f3ce99457f..89f7921475b 100644 --- a/acceptance/bundle/resources/permissions/pipelines/empty_list/output.txt +++ b/acceptance/bundle/resources/permissions/pipelines/empty_list/output.txt @@ -1,6 +1,11 @@ >>> [CLI] bundle validate -o json -null +[ + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } +] >>> [CLI] bundle deploy Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... diff --git a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go index f2a6d857090..794035c8016 100644 --- a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go +++ b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go @@ -122,6 +122,7 @@ func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Di v, err = dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { var permissions []resources.Permission pv, err := dyn.Get(v, "permissions") + present := err == nil // If the permissions field is not found, we set to an empty array if err != nil { pv = dyn.V([]dyn.Value{}) @@ -140,7 +141,11 @@ func (m *bundlePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Di levelsMap[key], )...) - if len(permissions) == 0 { + // Collapse to nil only when the block was absent, leaving the + // resource unmanaged. An explicit `permissions: []` must survive as + // an empty sequence so FixPermissions adds the default IS_OWNER, + // matching both a non-empty list and Terraform. + if len(permissions) == 0 && !present { permissions = nil }