Fix edge cases for databricks_permissions resource (#2158)

Author: nkvuong

permissions/resource_permissions.go

@@ -12,8 +12,6 @@ import (
 	"github.com/databricks/databricks-sdk-go/apierr"
 	"github.com/databricks/terraform-provider-databricks/common"
 
-	"github.com/hashicorp/go-cty/cty"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
@@ -170,7 +168,7 @@ func (a PermissionsAPI) put(objectID string, objectACL AccessControlChangeList)
 
 // Update updates object permissions. Technically, it's using method named SetOrDelete, but here we do more
 func (a PermissionsAPI) Update(objectID string, objectACL AccessControlChangeList) error {
-	if objectID == "/authorization/tokens" || objectID == "/registered-models/root" {
+	if objectID == "/authorization/tokens" || objectID == "/registered-models/root" || objectID == "/directories/0" {
 		// Prevent "Cannot change permissions for group 'admins' to None."
 		objectACL.AccessControlList = append(objectACL.AccessControlList, AccessControlChange{
 			GroupName:       "admins",
@@ -372,23 +370,6 @@ func ResourcePermissions() *schema.Resource {
 			}
 		}
 		s["access_control"].MinItems = 1
-		if groupNameSchema, err := common.SchemaPath(s,
-			"access_control", "group_name"); err == nil {
-			groupNameSchema.ValidateDiagFunc = func(i any, p cty.Path) diag.Diagnostics {
-				if v, ok := i.(string); ok {
-					if strings.ToLower(v) == "admins" {
-						return diag.Diagnostics{
-							{
-								Summary:       "It is not possible to restrict any permissions from `admins`.",
-								Severity:      diag.Error,
-								AttributePath: p,
-							},
-						}
-					}
-				}
-				return nil
-			}
-		}
 		return s
 	})
 	return common.Resource{
@@ -447,22 +428,27 @@ func ResourcePermissions() *schema.Resource {
 			if err != nil {
 				return err
 			}
-			// this logic was moved from CustomizeDiff because of undeterministic auth behavior
-			// in the corner-case scenarios.
-			// see https://github.com/databricks/terraform-provider-databricks/issues/2052
-			for _, v := range entity.AccessControlList {
-				if v.UserName == me.UserName {
-					format := "it is not possible to decrease administrative permissions for the current user: %s"
-					return fmt.Errorf(format, me.UserName)
-				}
-			}
 			for _, mapping := range permissionsResourceIDFields() {
 				if v, ok := d.GetOk(mapping.field); ok {
 					id, err := mapping.idRetriever(ctx, w, v.(string))
 					if err != nil {
 						return err
 					}
 					objectID := fmt.Sprintf("/%s/%s", mapping.resourceType, id)
+					// this logic was moved from CustomizeDiff because of undeterministic auth behavior
+					// in the corner-case scenarios.
+					// see https://github.com/databricks/terraform-provider-databricks/issues/2052
+					for _, v := range entity.AccessControlList {
+						if v.UserName == me.UserName {
+							format := "it is not possible to decrease administrative permissions for the current user: %s"
+							return fmt.Errorf(format, me.UserName)
+						}
+
+						if v.GroupName == "admins" && mapping.resourceType != "authorization" {
+							// should allow setting admins permissions for passwords and tokens usage
+							return fmt.Errorf("it is not possible to restrict any permissions from `admins`")
+						}
+					}
 					err = NewPermissionsAPI(ctx, c).Update(objectID, AccessControlChangeList{
 						AccessControlList: entity.AccessControlList,
 					})

permissions/resource_permissions_test.go

@@ -668,7 +668,7 @@ func TestResourcePermissionsCreate_conflicting_fields(t *testing.T) {
 
 func TestResourcePermissionsCreate_AdminsThrowError(t *testing.T) {
 	_, err := qa.ResourceFixture{
-		Fixtures: []qa.HTTPFixture{},
+		Fixtures: []qa.HTTPFixture{me},
 		Resource: ResourcePermissions(),
 		Create:   true,
 		HCL: `
@@ -679,8 +679,7 @@ func TestResourcePermissionsCreate_AdminsThrowError(t *testing.T) {
 		}
 		`,
 	}.Apply(t)
-	assert.EqualError(t, err, "invalid config supplied. [access_control] "+
-		"It is not possible to restrict any permissions from `admins`.")
+	assert.EqualError(t, err, "it is not possible to restrict any permissions from `admins`")
 }
 
 func TestResourcePermissionsCreate(t *testing.T) {
@@ -1590,3 +1589,109 @@ func TestResourcePermissionsCreate_DirectoryPath(t *testing.T) {
 	assert.Equal(t, TestingUser, firstElem["user_name"])
 	assert.Equal(t, "CAN_READ", firstElem["permission_level"])
 }
+
+func TestResourcePermissionsPasswordUsage(t *testing.T) {
+	d, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			me,
+			{
+				Method:   http.MethodPut,
+				Resource: "/api/2.0/permissions/authorization/passwords",
+				ExpectedRequest: AccessControlChangeList{
+					AccessControlList: []AccessControlChange{
+						{
+							GroupName:       "admins",
+							PermissionLevel: "CAN_USE",
+						},
+					},
+				},
+			},
+			{
+				Method:   http.MethodGet,
+				Resource: "/api/2.0/permissions/authorization/passwords",
+				Response: ObjectACL{
+					ObjectID:   "/authorization/passwords",
+					ObjectType: "passwords",
+					AccessControlList: []AccessControl{
+						{
+							GroupName:       "admins",
+							PermissionLevel: "CAN_USE",
+						},
+					},
+				},
+			},
+		},
+		Resource: ResourcePermissions(),
+		HCL: `
+		authorization = "passwords"
+		access_control {
+			group_name       = "admins"
+			permission_level = "CAN_USE"
+		}		
+		`,
+		Create: true,
+	}.Apply(t)
+	assert.NoError(t, err)
+	ac := d.Get("access_control").(*schema.Set)
+	require.Equal(t, 1, len(ac.List()))
+	firstElem := ac.List()[0].(map[string]any)
+	assert.Equal(t, "admins", firstElem["group_name"])
+	assert.Equal(t, "CAN_USE", firstElem["permission_level"])
+}
+
+func TestResourcePermissionsRootDirectory(t *testing.T) {
+	d, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			me,
+			{
+				Method:   http.MethodPut,
+				Resource: "/api/2.0/permissions/directories/0",
+				ExpectedRequest: AccessControlChangeList{
+					AccessControlList: []AccessControlChange{
+						{
+							UserName:        TestingUser,
+							PermissionLevel: "CAN_READ",
+						},
+						{
+							GroupName:       "admins",
+							PermissionLevel: "CAN_MANAGE",
+						},
+					},
+				},
+			},
+			{
+				Method:   http.MethodGet,
+				Resource: "/api/2.0/permissions/directories/0",
+				Response: ObjectACL{
+					ObjectID:   "/directories/0",
+					ObjectType: "directory",
+					AccessControlList: []AccessControl{
+						{
+							UserName:        TestingUser,
+							PermissionLevel: "CAN_READ",
+						},
+						{
+							GroupName:       "admins",
+							PermissionLevel: "CAN_MANAGE",
+						},
+					},
+				},
+			},
+		},
+		Resource: ResourcePermissions(),
+		HCL: `
+		directory_id = "0"
+		access_control {
+			user_name        = "ben"
+			permission_level = "CAN_READ"
+		}	
+		`,
+		Create: true,
+	}.Apply(t)
+	assert.NoError(t, err)
+	ac := d.Get("access_control").(*schema.Set)
+	require.Equal(t, 1, len(ac.List()))
+	firstElem := ac.List()[0].(map[string]any)
+	assert.Equal(t, TestingUser, firstElem["user_name"])
+	assert.Equal(t, "CAN_READ", firstElem["permission_level"])
+}