Fix permissions logic not to add 2nd IS_OWNER (#3850)

denik · web-flow · commit fb3b2700ae01 · 2025-10-31T11:07:09.000Z
## Changes When processing permissions, do not add 2nd IS_OWNER for current user. Bug introduced in #3780 ## Why It was always the intention. #3849 ## Tests Existing tests.
diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md
@@ -9,5 +9,6 @@
 ### Dependency updates
 
 ### Bundles
+* Fix a permissions bug adding second IS\_OWNER and causing "The job must have exactly one owner." error. Introduced in 0.274.0. ([#3850](https://github.com/databricks/cli/pull/3850))
 
 ### API Changes
diff --git a/acceptance/bundle/resources/permissions/jobs/other_is_owner/out.plan.direct.json b/acceptance/bundle/resources/permissions/jobs/other_is_owner/out.plan.direct.json
@@ -44,7 +44,7 @@
               "user_name": "other_user@databricks.com"
             },
             {
-              "permission_level": "IS_OWNER",
+              "permission_level": "CAN_MANAGE",
               "user_name": "[USERNAME]"
             }
           ]
diff --git a/acceptance/bundle/resources/permissions/jobs/other_is_owner/out.requests.deploy.direct.json b/acceptance/bundle/resources/permissions/jobs/other_is_owner/out.requests.deploy.direct.json
@@ -8,7 +8,7 @@
         "user_name": "other_user@databricks.com"
       },
       {
-        "permission_level": "IS_OWNER",
+        "permission_level": "CAN_MANAGE",
         "user_name": "[USERNAME]"
       }
     ]
diff --git a/acceptance/bundle/resources/permissions/jobs/other_is_owner/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/jobs/other_is_owner/out.requests.deploy.terraform.json
@@ -8,7 +8,7 @@
         "user_name": "other_user@databricks.com"
       },
       {
-        "permission_level": "IS_OWNER",
+        "permission_level": "CAN_MANAGE",
         "user_name": "[USERNAME]"
       }
     ]
diff --git a/acceptance/bundle/resources/permissions/jobs/other_is_owner/output.txt b/acceptance/bundle/resources/permissions/jobs/other_is_owner/output.txt
@@ -6,7 +6,7 @@
     "user_name": "other_user@databricks.com"
   },
   {
-    "level": "IS_OWNER",
+    "level": "CAN_MANAGE",
     "user_name": "[USERNAME]"
   }
 ]
diff --git a/acceptance/bundle/resources/permissions/pipelines/other_is_owner/out.plan.direct.json b/acceptance/bundle/resources/permissions/pipelines/other_is_owner/out.plan.direct.json
@@ -31,7 +31,7 @@
               "user_name": "other_user@databricks.com"
             },
             {
-              "permission_level": "IS_OWNER",
+              "permission_level": "CAN_MANAGE",
               "user_name": "[USERNAME]"
             }
           ]
diff --git a/acceptance/bundle/resources/permissions/pipelines/other_is_owner/out.requests.deploy.direct.json b/acceptance/bundle/resources/permissions/pipelines/other_is_owner/out.requests.deploy.direct.json
@@ -8,7 +8,7 @@
         "user_name": "other_user@databricks.com"
       },
       {
-        "permission_level": "IS_OWNER",
+        "permission_level": "CAN_MANAGE",
         "user_name": "[USERNAME]"
       }
     ]
diff --git a/acceptance/bundle/resources/permissions/pipelines/other_is_owner/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/pipelines/other_is_owner/out.requests.deploy.terraform.json
@@ -8,7 +8,7 @@
         "user_name": "other_user@databricks.com"
       },
       {
-        "permission_level": "IS_OWNER",
+        "permission_level": "CAN_MANAGE",
         "user_name": "[USERNAME]"
       }
     ]
diff --git a/acceptance/bundle/resources/permissions/pipelines/other_is_owner/output.txt b/acceptance/bundle/resources/permissions/pipelines/other_is_owner/output.txt
@@ -6,7 +6,7 @@
     "user_name": "other_user@databricks.com"
   },
   {
-    "level": "IS_OWNER",
+    "level": "CAN_MANAGE",
     "user_name": "[USERNAME]"
   }
 ]
diff --git a/acceptance/bundle/run_as/pipelines_legacy/output.txt b/acceptance/bundle/run_as/pipelines_legacy/output.txt
@@ -33,7 +33,7 @@ Warning: You are using the legacy mode of run_as. The support for this mode is e
     "service_principal_name": "my_service_principal"
   },
   {
-    "level": "IS_OWNER",
+    "level": "CAN_MANAGE",
     "user_name": "[USERNAME]"
   }
 ]
diff --git a/bundle/config/mutator/resourcemutator/filter_current_user.go b/bundle/config/mutator/resourcemutator/filter_current_user.go
@@ -2,7 +2,6 @@ package resourcemutator
 
 import (
 	"context"
-	"slices"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
@@ -15,26 +14,26 @@ const (
 	canManage = "CAN_MANAGE"
 )
 
-// This defines which permissions are considered management permissions.
-// Current user must have one management permission of themselves; if they don't have any,
-// we'll add the first in slice;
-// if they end up having both CAN_MANAGE and IS_OWNER, backend may fail with
-//
-//	Error: cannot create permissions: Permissions being set for UserName([USERNAME]) are ambiguous
-//
-// Since terraform adds IS_OWNER permission when there is not one, regardless of CAN_MANAGE presence,
-// the above error can occur.
-// We thus add another bit of logic: we upgrade CAN_MANAGE to IS_OWNER when we can.
-var managementPermissions = map[string][]string{
-	"jobs":           {isOwner, canManage},
-	"pipelines":      {isOwner, canManage},
-	"sql_warehouses": {isOwner, canManage},
-
-	// nil means "do nothing"
-	"secret_scopes": nil,
+// Which resources support IS_OWNER permission.
+var hasIsOwner = map[string]bool{
+	"jobs":           true,
+	"pipelines":      true,
+	"sql_warehouses": true,
 }
 
-var defaultManagementPermissions = []string{canManage}
+var ignoredResources = map[string]bool{
+	"secret_scopes": true,
+}
+
+// When processing permissions, we need to implement these constraints:
+// 1. There should be no more than one IS_OWNER settings for a given resource.
+// 2. We should automatically add IS_OWNER for current user (for resources that support it) or CAN_MANAGE permission.
+//    Terraform will do this, so doing this early makes request equal between terraform and direct.
+// 3. We prefer to add IS_OWNER, unless there already is one, in which case we fallback to CAN_MANAGE.
+// 4. Current user cannot have both CAN_MANAGE and IS_OWNER, we've seen backend failing with
+//    "Error: cannot create permissions: Permissions being set for UserName([USERNAME]) are ambiguous"
+//    Since terraform adds IS_OWNER permission when there is not one, regardless of CAN_MANAGE presence,
+//    he above error can occur. We thus add another bit of logic: we upgrade CAN_MANAGE to IS_OWNER when we can.
 
 type ensureOwnerPermissions struct{}
 
@@ -55,19 +54,11 @@ func ensureCurrentUserPermission(currentUser string) dyn.MapFunc {
 		}
 
 		resourceType := p[1].Key()
-
-		// Determine the required permission level
-		mgmtPerms, ok := managementPermissions[resourceType]
-		if !ok {
-			mgmtPerms = defaultManagementPermissions
-		}
-
-		if len(mgmtPerms) == 0 {
+		if ignoredResources[resourceType] {
 			return v, nil
 		}
 
-		// Process permissions array
-		return processPermissions(v, currentUser, mgmtPerms)
+		return processPermissions(v, currentUser, resourceType)
 	}
 }
 
@@ -80,9 +71,10 @@ func readUser(v dyn.Value) string {
 	return servicePrincipalName
 }
 
-func processPermissions(permissions dyn.Value, currentUser string, mgmtPerms []string) (dyn.Value, error) {
-	priorityPermission := mgmtPerms[0]
-	permissionToUpgrade := -1
+func processPermissions(permissions dyn.Value, currentUser, resourceType string) (dyn.Value, error) {
+	currentUserHasIsOwner := false
+	currentUserIndCanManage := -1
+	canAddIsOwner := hasIsOwner[resourceType]
 
 	permissionArray, ok := permissions.AsSequence()
 	if !ok {
@@ -95,25 +87,35 @@ func processPermissions(permissions dyn.Value, currentUser string, mgmtPerms []s
 			continue
 		}
 		user := readUser(permission)
-		if user == "" || user != currentUser {
-			continue
+		if level == isOwner {
+			canAddIsOwner = false
+			if user == currentUser {
+				currentUserHasIsOwner = true
+			}
 		}
-		if level == priorityPermission {
-			// already have required permission (IS_OWNER)
-			return permissions, nil
+		if user == currentUser && level == canManage {
+			currentUserIndCanManage = ind
 		}
-		if slices.Contains(mgmtPerms, level) {
-			// has management permission that needs to be upgraded to IS_OWNER
-			// continue the loop; if we is IS_OWNER we will still bail
-			permissionToUpgrade = ind
+	}
+
+	if currentUserHasIsOwner {
+		return dyn.V(permissionArray), nil
+	}
+
+	if canAddIsOwner {
+		if currentUserIndCanManage >= 0 {
+			// Upgrade current user's CAN_MANAGE to IS_OWNER. We do this because terraform will add IS_OWNER if it does not see one
+			// and that may confuse backend. We can stop doing it when removed terraform.
+			v, _ := dyn.Set(permissionArray[currentUserIndCanManage], "level", dyn.V(isOwner))
+			permissionArray[currentUserIndCanManage] = v
+		} else {
+			permissionArray = append(permissionArray, createPermission(currentUser, isOwner))
 		}
+		return dyn.V(permissionArray), nil
 	}
 
-	if permissionToUpgrade >= 0 {
-		v, _ := dyn.Set(permissionArray[permissionToUpgrade], "level", dyn.V(priorityPermission))
-		permissionArray[permissionToUpgrade] = v
-	} else {
-		permissionArray = append(permissionArray, createPermission(currentUser, priorityPermission))
+	if currentUserIndCanManage < 0 {
+		permissionArray = append(permissionArray, createPermission(currentUser, canManage))
 	}
 
 	return dyn.V(permissionArray), nil

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@`
`44`	`44`	`"user_name": "[email protected]"`
`45`	`45`	`},`
`46`	`46`	`{`
`47`		`- "permission_level": "IS_OWNER",`
	`47`	`+ "permission_level": "CAN_MANAGE",`
`48`	`48`	`"user_name": "[USERNAME]"`
`49`	`49`	`}`
`50`	`50`	`]`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`"user_name": "[email protected]"`
`9`	`9`	`},`
`10`	`10`	`{`
`11`		`- "permission_level": "IS_OWNER",`
	`11`	`+ "permission_level": "CAN_MANAGE",`
`12`	`12`	`"user_name": "[USERNAME]"`
`13`	`13`	`}`
`14`	`14`	`]`
Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@`
`6`	`6`	`"user_name": "[email protected]"`
`7`	`7`	`},`
`8`	`8`	`{`
`9`		`- "level": "IS_OWNER",`
	`9`	`+ "level": "CAN_MANAGE",`
`10`	`10`	`"user_name": "[USERNAME]"`
`11`	`11`	`}`
`12`	`12`	`]`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@`
`31`	`31`	`"user_name": "[email protected]"`
`32`	`32`	`},`
`33`	`33`	`{`
`34`		`- "permission_level": "IS_OWNER",`
	`34`	`+ "permission_level": "CAN_MANAGE",`
`35`	`35`	`"user_name": "[USERNAME]"`
`36`	`36`	`}`
`37`	`37`	`]`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ Warning: You are using the legacy mode of run_as. The support for this mode is e`
`33`	`33`	`"service_principal_name": "my_service_principal"`
`34`	`34`	`},`
`35`	`35`	`{`
`36`		`- "level": "IS_OWNER",`
	`36`	`+ "level": "CAN_MANAGE",`
`37`	`37`	`"user_name": "[USERNAME]"`
`38`	`38`	`}`
`39`	`39`	`]`