Skip the deploying identity when listing snapshot CAN_MANAGE principals

.nextchanges/bundles/snapshot-can-manage-deployer-duplicate.md

@@ -0,0 +1 @@
+* 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))

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
 

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
 

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
 }

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"},