Skip to content

Commit de465e0

Browse files
authored
Fix databricks#5682: add IS_OWNER for permissions added by Python mutators (databricks#5821)
Permissions added to an existing job/pipeline by a PyDABs mutator went through `NormalizeResources`, which skipped `FixPermissions`. The deploying user was never added as `IS_OWNER`, so the direct engine sent an ownerless permissions PUT and the API rejected it with `The <resource> must have exactly one owner` (databricks#5682). The terraform provider re-injects the owner at PUT time, which is why the same bundle worked on terraform but failed on the direct engine. Fix: run `FixPermissions` in `NormalizeResources` so permissions added by a Python mutator get the same owner treatment as permissions declared in YAML. `FixPermissions` is idempotent, so it is a no-op for resources that already have an owner; `ApplyBundlePermissions` is intentionally not re-run (not idempotent, already applied earlier). `FixPermissions` also now skips when `CurrentUser` is unset. Includes an acceptance test that deploys a pipeline whose only permission is added by a Python mutator; verified against real AWS on both engines. This pull request and its description were written by Isaac.
1 parent 1026506 commit de465e0

12 files changed

Lines changed: 115 additions & 11 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Bundles
1212

1313
* `bundle generate job` now downloads workspace files referenced by `spark_python_task`, rewriting them to a relative path like it already does for notebooks. Git-sourced files and cloud URIs are left untouched ([#5799](https://github.com/databricks/cli/pull/5799)).
14+
* Fix permissions added to a job or pipeline by a Python (PyDABs) mutator failing to deploy with "must have exactly one owner"; the deploying identity is now set as owner, matching resources whose permissions are declared in YAML ([#5821](https://github.com/databricks/cli/pull/5821)).
1415

1516
### Dependency updates
1617

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
bundle:
2+
name: test-bundle-$UNIQUE_NAME
3+
4+
variables:
5+
grantee_group:
6+
default: some-group
7+
8+
python:
9+
mutators:
10+
- "mutators:add_pipeline_permission"
11+
12+
resources:
13+
pipelines:
14+
my_pipeline:
15+
name: test-pipeline-$UNIQUE_NAME
16+
catalog: main
17+
schema: default
18+
serverless: true
19+
libraries:
20+
- notebook:
21+
path: ./nb.py
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
from dataclasses import replace
2+
from databricks.bundles.pipelines import Pipeline, PipelinePermission
3+
from databricks.bundles.core import pipeline_mutator, Bundle
4+
5+
6+
# Regression test for https://github.com/databricks/cli/issues/5682.
7+
# Adds a permission to a pipeline that is already defined in YAML (mirrors the
8+
# reporter's mutator, which reads a bundle variable). Resources updated by a
9+
# PythonMutator go through NormalizeResources; that path now runs FixPermissions,
10+
# so the deploying user is added as IS_OWNER and the permissions PUT succeeds.
11+
# Before the fix the ACL had no owner and the backend rejected the PUT with
12+
# "The pipeline must have exactly one owner".
13+
@pipeline_mutator
14+
def add_pipeline_permission(bundle: Bundle, pipeline: Pipeline) -> Pipeline:
15+
group = bundle.resolve_variable(bundle.variables["grantee_group"])
16+
permissions = [
17+
*pipeline.permissions,
18+
PipelinePermission.from_dict({"group_name": group, "level": "CAN_RUN"}),
19+
]
20+
return replace(pipeline, permissions=permissions)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Databricks notebook source

acceptance/bundle/python/mutator-permissions-owner-5682/out.test.toml

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
2+
>>> uv run [UV_ARGS] -q [CLI] bundle validate --output json
3+
[
4+
"CAN_RUN",
5+
"IS_OWNER"
6+
]
7+
8+
>>> uv run [UV_ARGS] -q [CLI] bundle deploy
9+
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files...
10+
Deploying resources...
11+
Updating deployment state...
12+
Deployment complete!
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
envsubst < databricks.yml.tmpl > databricks.yml
2+
3+
if [ -n "$CLOUD_ENV" ]; then
4+
# Unique group so parallel cloud runs don't collide; must exist for the PUT to
5+
# reach the owner check rather than failing on an unknown principal.
6+
export BUNDLE_VAR_grantee_group="dabs-5682-$UNIQUE_NAME"
7+
$CLI groups create --display-name "$BUNDLE_VAR_grantee_group" &> /dev/null || true
8+
fi
9+
10+
cleanup() {
11+
uv run $UV_ARGS -q $CLI bundle destroy --auto-approve &> LOG.destroy
12+
rm -fr .databricks __pycache__
13+
}
14+
trap cleanup EXIT
15+
16+
# The Python mutator adds a CAN_RUN permission; FixPermissions then adds IS_OWNER for
17+
# the current user (issue #5682 - previously it did not run on Python-updated resources).
18+
trace uv run $UV_ARGS -q $CLI bundle validate --output json | \
19+
jq ".resources.pipelines.my_pipeline.permissions | map(.level)"
20+
21+
# With the owner present, the deploy succeeds on both engines.
22+
trace uv run $UV_ARGS -q $CLI bundle deploy
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Cloud = true
2+
RequiresUnityCatalog = true

bundle/config/mutator/resourcemutator/fix_permissions.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,12 @@ func createPermissionFromPrincipal(principal, level string) dyn.Value {
211211
}
212212

213213
func (m *fixPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
214+
// CurrentUser is populated by PopulateCurrentUser early in the initialize phase.
215+
// It can be nil when this mutator runs outside that phase (e.g. NormalizeResources
216+
// after PythonMutator); there is no user to add as owner, so skip.
217+
if b.Config.Workspace.CurrentUser == nil {
218+
return nil
219+
}
214220
currentUser := b.Config.Workspace.CurrentUser.UserName
215221

216222
err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {

bundle/config/mutator/resourcemutator/resource_mutator.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,17 @@ func NormalizeResources(
295295
return
296296
}
297297

298+
// Permissions added to an existing resource by a Python mutator must still get the
299+
// deploying user as IS_OWNER, otherwise the Permissions API rejects the PUT with
300+
// "must have exactly one owner" (#5682). FixPermissions is idempotent, so re-running
301+
// it on resources that already have an owner is a no-op. ApplyBundlePermissions is
302+
// intentionally not re-run here: it is not idempotent (it appends bundle-level
303+
// permissions) and already ran for these resources in ProcessStaticResources.
304+
bundle.ApplyContext(ctx, b, FixPermissions())
305+
if logdiag.HasError(ctx) {
306+
return
307+
}
308+
298309
// after mutators, we merge updated resources back to snapshot to preserve non-selected resources
299310
err = b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) {
300311
return mergeResources(root, snapshot)

0 commit comments

Comments
 (0)