From 69f04e6aa870b1b95ff192f36b8226b78d8a6512 Mon Sep 17 00:00:00 2001 From: "pieter.noordhuis" Date: Wed, 4 Nov 2026 09:09:59 +0000 Subject: [PATCH 1/2] Skip the deploying identity when listing snapshot CAN_MANAGE principals BuildCanManage always adds the deploying identity, and its doc comment says that identity is skipped when it also shows up in the top-level permissions section. The loop had no such check, so the recommended setup (deployer listed with CAN_MANAGE) sent the same principal twice in can_manage_principals. Match the deployer by name in either the user_name or service_principal_name field, as the permissions diagnostics do, and send other repeated principals only once. Bundles deployed with v1.23.0 that list the deployer with CAN_MANAGE have the duplicate in state, so their next deploy uploads the snapshot to a new path once. The changelog entry says so. Cover this in TestBuildCanManage, a new test for the deployer listed in the other name field, and the immutable/can-manage acceptance test, which now lists the deployer in its permissions section. Co-authored-by: Isaac --- .../snapshot-can-manage-deployer-duplicate.md | 1 + .../immutable/can-manage/databricks.yml.tmpl | 2 ++ .../deploy/immutable/can-manage/output.txt | 24 ---------------- bundle/deploy/snapshot/upload.go | 28 +++++++++++-------- bundle/deploy/snapshot/upload_warning_test.go | 18 +++++++++++- 5 files changed, 37 insertions(+), 36 deletions(-) create mode 100644 .nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md diff --git a/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md b/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md new file mode 100644 index 00000000000..86c81d77a63 --- /dev/null +++ b/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md @@ -0,0 +1 @@ +* Fix immutable-folder deploys sending the deploying identity twice in the snapshot's CAN_MANAGE principals when the top-level `permissions` section lists it with `CAN_MANAGE`. Affected bundles upload their snapshot to a new path once on the next deploy. ([#7332](https://github.com/databricks/cli/pull/7332)) diff --git a/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl index 3247180b9e9..43c834e898c 100644 --- a/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl +++ b/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl @@ -5,6 +5,8 @@ workspace: sync_mode: immutable permissions: + - level: CAN_MANAGE + service_principal_name: ${workspace.current_user.userName} - level: CAN_MANAGE user_name: $TEST_USER_EMAIL diff --git a/acceptance/bundle/deploy/immutable/can-manage/output.txt b/acceptance/bundle/deploy/immutable/can-manage/output.txt index cdcca39708d..3fa11a0fcd1 100644 --- a/acceptance/bundle/deploy/immutable/can-manage/output.txt +++ b/acceptance/bundle/deploy/immutable/can-manage/output.txt @@ -1,18 +1,6 @@ === Deploy with a CAN_MANAGE permission entry >>> [CLI] bundle deploy -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: - - service_principal_name: [USERNAME] - level: CAN_MANAGE - -See https://docs.databricks.com/dev-tools/bundles/permissions.html to learn more about permission configuration. - in databricks.yml:8:3 - Created internal_immutable_snapshots.immutable Created jobs.my_job Created jobs.my_job.permissions @@ -31,18 +19,6 @@ Resources: 3 created, 0 changed, 0 deleted, 0 unchanged ] >>> [CLI] bundle destroy --auto-approve -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: - - service_principal_name: [USERNAME] - level: CAN_MANAGE - -See https://docs.databricks.com/dev-tools/bundles/permissions.html to learn more about permission configuration. - in databricks.yml:8:3 - The following resources will be deleted: delete resources.jobs.my_job diff --git a/bundle/deploy/snapshot/upload.go b/bundle/deploy/snapshot/upload.go index cdfb6d538d3..a4b01c190e2 100644 --- a/bundle/deploy/snapshot/upload.go +++ b/bundle/deploy/snapshot/upload.go @@ -204,28 +204,34 @@ func BuildACL(b *bundle.Bundle) []snapshot.ACLEntry { // CAN_MANAGE in the top-level permissions section. The deploying identity is always included // because the API rejects an empty list, and it is the one principal always available. The // recommended permissions section names it with CAN_MANAGE as well, so skip it in the loop -// rather than sending it twice. +// rather than sending it twice. Other principals listed more than once are sent once. func BuildCanManage(b *bundle.Bundle) []snapshot.ManagePrincipal { currentUser := b.Config.Workspace.CurrentUser.User - var canManage []snapshot.ManagePrincipal + deployer := snapshot.ManagePrincipal{UserName: currentUser.UserName} if iamutil.IsServicePrincipal(currentUser) { - canManage = append(canManage, snapshot.ManagePrincipal{ - ServicePrincipalName: currentUser.UserName, - }) - } else { - canManage = append(canManage, snapshot.ManagePrincipal{ - UserName: currentUser.UserName, - }) + deployer = snapshot.ManagePrincipal{ServicePrincipalName: currentUser.UserName} } + canManage := []snapshot.ManagePrincipal{deployer} + seen := map[snapshot.ManagePrincipal]bool{} for _, p := range b.Config.Permissions { if p.Level != "CAN_MANAGE" { continue } - canManage = append(canManage, snapshot.ManagePrincipal{ + // Match the deployer by name in either field, as the permissions diagnostics do, so a + // config that satisfies their recommendation never sends the deployer twice. + if p.UserName == currentUser.UserName || p.ServicePrincipalName == currentUser.UserName { + continue + } + principal := snapshot.ManagePrincipal{ UserName: p.UserName, GroupName: p.GroupName, ServicePrincipalName: p.ServicePrincipalName, - }) + } + if seen[principal] { + continue + } + seen[principal] = true + canManage = append(canManage, principal) } return canManage } diff --git a/bundle/deploy/snapshot/upload_warning_test.go b/bundle/deploy/snapshot/upload_warning_test.go index 254d7a70aa6..4567964bb49 100644 --- a/bundle/deploy/snapshot/upload_warning_test.go +++ b/bundle/deploy/snapshot/upload_warning_test.go @@ -109,13 +109,14 @@ func canManageBundle(permissions []resources.Permission) *bundle.Bundle { func TestBuildCanManage(t *testing.T) { b := canManageBundle([]resources.Permission{ + {Level: "CAN_MANAGE", UserName: "deployer@example.test"}, {Level: "CAN_MANAGE", UserName: "manager@example.test"}, {Level: "CAN_VIEW", UserName: "viewer@example.test"}, {Level: "CAN_MANAGE", GroupName: "admins"}, {Level: "CAN_MANAGE", ServicePrincipalName: "sp-1"}, }) - // The deploying identity, then only the CAN_MANAGE principals. + // The deploying identity once, then only the other CAN_MANAGE principals. assert.Equal(t, []snapshot.ManagePrincipal{ {UserName: "deployer@example.test"}, {UserName: "manager@example.test"}, @@ -124,6 +125,21 @@ func TestBuildCanManage(t *testing.T) { }, BuildCanManage(b)) } +func TestBuildCanManageSkipsDeployerListedInOtherField(t *testing.T) { + // The permissions diagnostics accept the deployer under either name field, so a user + // deployer listed as service_principal_name is still the deployer. + b := canManageBundle([]resources.Permission{ + {Level: "CAN_MANAGE", ServicePrincipalName: "deployer@example.test"}, + {Level: "CAN_MANAGE", GroupName: "admins"}, + {Level: "CAN_MANAGE", GroupName: "admins"}, + }) + + assert.Equal(t, []snapshot.ManagePrincipal{ + {UserName: "deployer@example.test"}, + {GroupName: "admins"}, + }, BuildCanManage(b)) +} + func TestBuildCanManageWithoutManagers(t *testing.T) { b := canManageBundle([]resources.Permission{ {Level: "CAN_VIEW", UserName: "viewer@example.test"}, From 2c28abf0716cdf23571fcdd7e01c727bb175b15f Mon Sep 17 00:00:00 2001 From: "pieter.noordhuis" Date: Wed, 4 Nov 2026 16:06:28 +0000 Subject: [PATCH 2/2] Describe the user-visible effect in the changelog entry Co-authored-by: Isaac --- .nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md b/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md index 86c81d77a63..a3a96d72889 100644 --- a/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md +++ b/.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md @@ -1 +1 @@ -* Fix immutable-folder deploys sending the deploying identity twice in the snapshot's CAN_MANAGE principals when the top-level `permissions` section lists it with `CAN_MANAGE`. Affected bundles upload their snapshot to a new path once on the next deploy. ([#7332](https://github.com/databricks/cli/pull/7332)) +* Immutable-folder snapshots no longer list the deploying identity twice in `can_manage_principals` when the top-level `permissions` section grants it `CAN_MANAGE`. Affected bundles upload their snapshot to a new path once on the next deploy. ([#7332](https://github.com/databricks/cli/pull/7332))