From 5d900c3628ec511664b0f7b70e20c3ebae4a0ca3 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 22:58:19 +0200 Subject: [PATCH] Emit permission overlap warnings instead of discarding them When a bundle-level permission is skipped because the resource already defines a permission for the same principal, the explanatory warnings were computed but dropped (a TODO predating logdiag). Log them via logdiag so users can see why a grant was not applied. Co-authored-by: Isaac --- .../permissions_overlap/databricks.yml | 27 ++++++++++++ .../permissions_overlap/out.test.toml | 3 ++ .../validate/permissions_overlap/output.txt | 44 +++++++++++++++++++ .../validate/permissions_overlap/script | 1 + .../apply_bundle_permissions.go | 8 ++-- .../apply_bundle_permissions_test.go | 5 +++ 6 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 acceptance/bundle/validate/permissions_overlap/databricks.yml create mode 100644 acceptance/bundle/validate/permissions_overlap/out.test.toml create mode 100644 acceptance/bundle/validate/permissions_overlap/output.txt create mode 100644 acceptance/bundle/validate/permissions_overlap/script diff --git a/acceptance/bundle/validate/permissions_overlap/databricks.yml b/acceptance/bundle/validate/permissions_overlap/databricks.yml new file mode 100644 index 00000000000..fe7c061a1f2 --- /dev/null +++ b/acceptance/bundle/validate/permissions_overlap/databricks.yml @@ -0,0 +1,27 @@ +bundle: + name: test-bundle + +permissions: + - level: CAN_MANAGE + user_name: overlap@example.com + - level: CAN_VIEW + group_name: some_group + +resources: + jobs: + overlapping_job: + tasks: + - task_key: main + notebook_task: + notebook_path: /Workspace/Users/user@example.com/notebook + source: WORKSPACE + permissions: + - level: CAN_VIEW + user_name: overlap@example.com + + other_job: + tasks: + - task_key: main + notebook_task: + notebook_path: /Workspace/Users/user@example.com/notebook + source: WORKSPACE diff --git a/acceptance/bundle/validate/permissions_overlap/out.test.toml b/acceptance/bundle/validate/permissions_overlap/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/bundle/validate/permissions_overlap/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/validate/permissions_overlap/output.txt b/acceptance/bundle/validate/permissions_overlap/output.txt new file mode 100644 index 00000000000..43b95ddd440 --- /dev/null +++ b/acceptance/bundle/validate/permissions_overlap/output.txt @@ -0,0 +1,44 @@ +Warning: 'jobs' already has permissions set for 'overlap@example.com' user name + +Recommendation: permissions section should explicitly include the current deployment identity '[USERNAME]' or one of its groups +If it is not included, CAN_MANAGE permissions are only applied if the present identity is used to deploy. + +Consider using a adding a top-level permissions section such as the following: + + permissions: + - user_name: [USERNAME] + level: CAN_MANAGE + +See https://docs.databricks.com/dev-tools/bundles/permissions.html to learn more about permission configuration. + in databricks.yml:5:3 + +{ + "other_job": [ + { + "level": "CAN_MANAGE", + "user_name": "overlap@example.com" + }, + { + "group_name": "some_group", + "level": "CAN_VIEW" + }, + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ], + "overlapping_job": [ + { + "level": "CAN_VIEW", + "user_name": "overlap@example.com" + }, + { + "group_name": "some_group", + "level": "CAN_VIEW" + }, + { + "level": "IS_OWNER", + "user_name": "[USERNAME]" + } + ] +} diff --git a/acceptance/bundle/validate/permissions_overlap/script b/acceptance/bundle/validate/permissions_overlap/script new file mode 100644 index 00000000000..da6c2e46463 --- /dev/null +++ b/acceptance/bundle/validate/permissions_overlap/script @@ -0,0 +1 @@ +$CLI bundle validate -o json | jq '.resources.jobs | map_values(.permissions)' diff --git a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go index fd019479d77..1690485ab95 100644 --- a/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go +++ b/bundle/config/mutator/resourcemutator/apply_bundle_permissions.go @@ -13,6 +13,7 @@ import ( "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/logdiag" "github.com/databricks/databricks-sdk-go/service/iam" ) @@ -236,9 +237,10 @@ func notifyForPermissionOverlap( resourcePermissions []resources.Permission, resourceName string, ) bool { - isOverlap, _ := isPermissionOverlap(permission, resourcePermissions, resourceName) - // TODO: When we start to collect all diagnostics at the top level and visualize jointly, - // use diagnostics returned from isPermissionOverlap to display warnings + isOverlap, diagnostics := isPermissionOverlap(permission, resourcePermissions, resourceName) + for _, d := range diagnostics { + logdiag.LogDiag(ctx, d) + } return isOverlap } diff --git a/bundle/config/mutator/resourcemutator/apply_bundle_permissions_test.go b/bundle/config/mutator/resourcemutator/apply_bundle_permissions_test.go index 3fa97c41a7f..9878c0ffe50 100644 --- a/bundle/config/mutator/resourcemutator/apply_bundle_permissions_test.go +++ b/bundle/config/mutator/resourcemutator/apply_bundle_permissions_test.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -191,6 +192,10 @@ func TestWarningOnOverlapPermission(t *testing.T) { diags := bundle.Apply(t.Context(), b, ApplyBundlePermissions()) require.NoError(t, diags.Error()) + require.Len(t, diags, 1) + require.Equal(t, diag.Warning, diags[0].Severity) + require.Equal(t, "'jobs' already has permissions set for 'TestUser' user name", diags[0].Summary) + require.Contains(t, b.Config.Resources.Jobs["job_1"].Permissions, resources.JobPermission{Level: "CAN_VIEW", UserName: "TestUser"}) require.Contains(t, b.Config.Resources.Jobs["job_1"].Permissions, resources.JobPermission{Level: "CAN_VIEW", GroupName: "TestGroup"}) require.Contains(t, b.Config.Resources.Jobs["job_2"].Permissions, resources.JobPermission{Level: "CAN_VIEW", UserName: "TestUser2"})