Re: [PR] Fix role_permission description edits and authoritative drift detection [cloudstack-terraform-provider]

["Copilot (via GitHub)" <gi...@apache.org>]

Copilot commented on code in PR #350:
URL: https://github.com/apache/cloudstack-terraform-provider/pull/350#discussion_r4013050359


##########
cloudstack/resource_cloudstack_role_permission.go:
##########
@@ -205,24 +234,14 @@ func resourceCloudStackRolePermissionDelete(d *schema.ResourceData, meta interfa
 		return nil
 	}
 
-	rolePermissionsByID := make(map[string]*cloudstack.RolePermission)
-	for _, rp := range rolePermissions {
-		rolePermissionsByID[rp.Id] = rp
-	}
-
-	used := make(map[string]bool)
-	for _, permission := range rolePermissionSpecs(d.Get("permission").([]interface{})) {
-		ruleID := permission.ID
-		if ruleID == "" {
-			if rp := findMatchingRolePermission(rolePermissions, permission, used); rp != nil {
-				ruleID = rp.Id
-			}
-		}
-		if ruleID == "" || rolePermissionsByID[ruleID] == nil {
+	// Not authoritative: remove only the permissions this resource manages and
+	// leave anything added outside Terraform in place.
+	desiredPermissions := rolePermissionSpecs(d.Get("permission").([]interface{}))
+	for _, rp := range matchCloudStackRolePermissions(rolePermissions, desiredPermissions) {

Review Comment:
   Destroy currently matches only by rule and can delete the wrong permission: if the state ID is gone but an externally recreated permission has the same rule, this resource will delete that external permission even though `authoritative = false` is supposed to preserve undeclared permissions. Prefer the persisted `permission.ID` when it is present and only use rule matching as a fallback for ID-less entries, as the previous implementation did.
   
   This issue also appears on line 330 of the same file.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org