[fix][GOV] Update Read() method to update state with permissions provisioned only via terraform

internal/provider/permissions/resource.go

@@ -260,76 +260,25 @@ func (r *permissionResource) grantPermission(ctx context.Context, subject permis
 	return diags
 }
 
-func (r *permissionResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
-	// Retrieve values from plan.
-	var plan permissionsResourceModel
-	var planSubject permissionSubjectModel
-
-	diags := req.Plan.Get(ctx, &plan)
-	resp.Diagnostics.Append(diags...)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	diags = plan.Subject.As(ctx, &planSubject, basetypes.ObjectAsOptions{})
-	resp.Diagnostics.Append(diags...)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	for _, planPermission := range plan.Permissions {
-		diags = r.grantPermission(ctx, planSubject, planPermission)
-
-		resp.Diagnostics.Append(diags...)
-		if resp.Diagnostics.HasError() {
-			return
-		}
-	}
-
-	// Set state to fully populated data.
-	diags = resp.State.Set(ctx, plan)
-	resp.Diagnostics.Append(diags...)
-	if resp.Diagnostics.HasError() {
-		tflog.Error(ctx, "Error creating permissions", map[string]interface{}{"error": "Could not set state"})
-		return
-	}
-}
-
-func (r *permissionResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
-	var state permissionsResourceModel
-
-	diags := req.State.Get(ctx, &state)
-	resp.Diagnostics.Append(diags...)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
-	var stateSubject permissionSubjectModel
-
-	diags = state.Subject.As(ctx, &stateSubject, basetypes.ObjectAsOptions{})
-	resp.Diagnostics.Append(diags...)
-	if resp.Diagnostics.HasError() {
-		return
-	}
-
+func (r *permissionResource) fetchPermissionsForSubject(ctx context.Context, subject permissionSubjectModel) ([]permissionModel, diag.Diagnostics) {
 	var permissions []permissionModel
+	var allDiags diag.Diagnostics
 
-	subjectID := stateSubject.ID.ValueString() + "|" + stateSubject.Type.ValueString()
+	subjectID := subject.ID.ValueString() + "|" + subject.Type.ValueString()
 
-	// We'll need to get all the permissions for the given subject.
 	for _, objectType := range []string{"app", "folder", "resource", "resource_configuration"} {
-		request := api.NewPermissionsListObjectsPostRequest(createNewAPIPermissionsSubject(stateSubject), objectType)
+		request := api.NewPermissionsListObjectsPostRequest(createNewAPIPermissionsSubject(subject), objectType)
 
-		tflog.Info(ctx, "Reading permission", map[string]interface{}{"subjectId": subjectID})
+		tflog.Info(ctx, "Fetching permissions", map[string]interface{}{"subjectId": subjectID, "objectType": objectType})
 
 		permissionsResponse, httpResponse, err := r.client.PermissionsAPI.PermissionsListObjectsPost(ctx).PermissionsListObjectsPostRequest(*request).Execute()
 		if err != nil {
-			resp.Diagnostics.AddError(
+			allDiags.AddError(
 				"Error reading permission",
 				fmt.Sprintf("Could not read permissions for id: %s, object type: %s, error: %s", subjectID, objectType, err.Error()),
 			)
 			tflog.Error(ctx, "Error reading group", utils.AddHTTPStatusCode(map[string]any{"permissionId": subjectID, "objectType": objectType, "error": err.Error()}, httpResponse))
-			return
+			return nil, allDiags
 		}
 
 		// Now let's populate the state with permissions based on our API response.
@@ -372,18 +321,103 @@ func (r *permissionResource) Read(ctx context.Context, req resource.ReadRequest,
 				Type: types.StringValue(objectType),
 			}
 			object, diags := types.ObjectValueFrom(ctx, objValue.AttributeTypes(), objValue)
-			resp.Diagnostics.Append(diags...)
-			if resp.Diagnostics.HasError() {
-				return
+			allDiags.Append(diags...)
+			if allDiags.HasError() {
+				return nil, allDiags
 			}
 			permissions = append(permissions, permissionModel{
 				Object:      object,
 				AccessLevel: types.StringValue(accessLevel),
 			})
 		}
 	}
+	return permissions, allDiags
+}
 
-	state.Permissions = permissions
+func (r *permissionResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
+	// Retrieve values from plan.
+	var plan permissionsResourceModel
+	var planSubject permissionSubjectModel
+
+	diags := req.Plan.Get(ctx, &plan)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	diags = plan.Subject.As(ctx, &planSubject, basetypes.ObjectAsOptions{})
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	for _, planPermission := range plan.Permissions {
+		diags = r.grantPermission(ctx, planSubject, planPermission)
+
+		resp.Diagnostics.Append(diags...)
+		if resp.Diagnostics.HasError() {
+			return
+		}
+	}
+
+	// Set state to fully populated data.
+	diags = resp.State.Set(ctx, plan)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		tflog.Error(ctx, "Error creating permissions", map[string]interface{}{"error": "Could not set state"})
+		return
+	}
+}
+
+func (r *permissionResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
+	var state permissionsResourceModel
+
+	diags := req.State.Get(ctx, &state)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	var stateSubject permissionSubjectModel
+	var managedPermissionKeys = make(map[string]bool)
+
+	for _, permission := range state.Permissions {
+		var obj permissionObjectModel
+		diags := permission.Object.As(ctx, &obj, basetypes.ObjectAsOptions{})
+		if diags.HasError() {
+			return
+		}
+		key := obj.ID.ValueString() + "|" + obj.Type.ValueString()
+		managedPermissionKeys[key] = true
+	}
+
+	diags = state.Subject.As(ctx, &stateSubject, basetypes.ObjectAsOptions{})
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	allPermissions, diags := r.fetchPermissionsForSubject(ctx, stateSubject)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	var filteredPermissions []permissionModel
+	for _, perm := range allPermissions {
+		var obj permissionObjectModel
+		diags := perm.Object.As(ctx, &obj, basetypes.ObjectAsOptions{})
+		if diags.HasError() {
+			resp.Diagnostics.Append(diags...)
+			return
+		}
+		key := obj.ID.ValueString() + "|" + obj.Type.ValueString()
+		if managedPermissionKeys[key] {
+			filteredPermissions = append(filteredPermissions, perm)
+		}
+	}
+
+	state.Permissions = filteredPermissions
 
 	diags = resp.State.Set(ctx, &state)
 	resp.Diagnostics.Append(diags...)
@@ -548,4 +582,11 @@ func (r *permissionResource) ImportState(ctx context.Context, req resource.Impor
 		Type: types.StringValue(subjType),
 	}
 	resp.State.SetAttribute(ctx, path.Root("subject"), subject)
+
+	allPermissions, diags := r.fetchPermissionsForSubject(ctx, subject)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+	resp.State.SetAttribute(ctx, path.Root("permissions"), allPermissions)
 }

internal/provider/permissions/resource_test.go

@@ -1,13 +1,16 @@
 package permissions_test
 
 import (
+	// "context".
 	"fmt"
+	// "strconv".
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 
 	"github.com/tryretool/terraform-provider-retool/internal/acctest"
+	// "github.com/tryretool/terraform-provider-retool/internal/sdk/api".
 )
 
 const testPermissionsConfig = `
@@ -43,8 +46,7 @@ resource "retool_permissions" "test_permissions" {
 }
 `
 
-const testUpdatedPermissionsConfig = `
-resource "retool_group" "test_group" {
+const testUpdatedPermissionsConfig = `resource "retool_group" "test_group" {
 	name = "tf-acc-test-group"
 }
 
@@ -127,3 +129,168 @@ func TestAccPermissions(t *testing.T) {
 		},
 	})
 }
+
+// TestAccPermissions_ManagedDeletion verifies that the permissions resource properly
+// manages only the permissions it creates by testing updates and removals.
+func TestAccPermissions_ManagedDeletion(t *testing.T) {
+	// Step 1: Create permissions for two folders.
+	configWithTwoPerms := `
+resource "retool_group" "test_group" {
+	name = "tf-acc-test-group-managed-del"
+}
+
+resource "retool_folder" "test_folder1" {
+	name = "tf-acc-test-folder-managed-1"
+	folder_type = "app"
+}
+
+resource "retool_folder" "test_folder2" {
+	name = "tf-acc-test-folder-managed-2"
+	folder_type = "app"
+}
+
+resource "retool_permissions" "test_permissions" {
+	subject = {
+		type = "group"
+		id = retool_group.test_group.id
+	}
+	permissions = [
+		{
+			object = {
+				type = "folder"
+				id = retool_folder.test_folder1.id
+			}
+			access_level = "use"
+		},
+		{
+			object = {
+				type = "folder"
+				id = retool_folder.test_folder2.id
+			}
+			access_level = "edit"
+		},
+	]
+}
+`
+
+	// Step 2: Remove one permission from config.
+	configWithOnePerm := `
+resource "retool_group" "test_group" {
+	name = "tf-acc-test-group-managed-del"
+}
+
+resource "retool_folder" "test_folder1" {
+	name = "tf-acc-test-folder-managed-1"
+	folder_type = "app"
+}
+
+resource "retool_folder" "test_folder2" {
+	name = "tf-acc-test-folder-managed-2"
+	folder_type = "app"
+}
+
+resource "retool_permissions" "test_permissions" {
+	subject = {
+		type = "group"
+		id = retool_group.test_group.id
+	}
+	permissions = [
+		{
+			object = {
+				type = "folder"
+				id = retool_folder.test_folder2.id
+			}
+			access_level = "edit"
+		},
+	]
+}
+`
+
+	acctest.Test(t, resource.TestCase{
+		Steps: []resource.TestStep{
+			// Create permissions for two folders.
+			{
+				Config: configWithTwoPerms,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions", "permissions.#", "2"),
+					// Don't check specific indices as order may vary.
+				),
+			},
+			// Remove one permission from config.
+			{
+				Config: configWithOnePerm,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					// Verify only one permission remains.
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions", "permissions.#", "1"),
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions", "permissions.0.access_level", "edit"),
+				),
+			},
+		},
+	})
+}
+
+// TestAccPermissions_ImportAndRead verifies that importing permissions and subsequent
+// reads correctly maintain the imported permissions in state.
+func TestAccPermissions_ImportAndRead(t *testing.T) {
+	config := `
+resource "retool_group" "test_group_import" {
+	name = "tf-acc-test-group-import"
+}
+
+resource "retool_folder" "test_folder_import" {
+	name = "tf-acc-test-folder-import"
+	folder_type = "app"
+}
+
+resource "retool_permissions" "test_permissions_import" {
+	subject = {
+		type = "group"
+		id = retool_group.test_group_import.id
+	}
+	permissions = [
+		{
+			object = {
+				type = "folder"
+				id = retool_folder.test_folder_import.id
+			}
+			access_level = "own"
+		},
+	]
+}
+`
+
+	acctest.Test(t, resource.TestCase{
+		Steps: []resource.TestStep{
+			// Create the permissions.
+			{
+				Config: config,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions_import", "permissions.#", "1"),
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions_import", "permissions.0.access_level", "own"),
+				),
+			},
+			// Import the permissions.
+			{
+				ResourceName: "retool_permissions.test_permissions_import",
+				ImportState:  true,
+				ImportStateIdFunc: func(state *terraform.State) (string, error) {
+					permissions, ok := state.RootModule().Resources["retool_permissions.test_permissions_import"]
+					if !ok {
+						return "", fmt.Errorf("Resource not found")
+					}
+					return "group|" + permissions.Primary.Attributes["subject.id"], nil
+				},
+				ImportStateVerify:                    true,
+				ImportStateVerifyIdentifierAttribute: "subject.id",
+			},
+			// Re-apply config to trigger Read() and verify state is maintained.
+			{
+				Config: config,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions_import", "permissions.#", "1"),
+					resource.TestCheckResourceAttr("retool_permissions.test_permissions_import", "permissions.0.access_level", "own"),
+				),
+			},
+		},
+	})
+}