You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ma...@apache.org on 2026/08/28 12:08:03 UTC

(cloudstack-terraform-provider) branch fix/role-permission-reorder-drift created (now e1f2374)

This is an automated email from the ASF dual-hosted git repository.

sudo87 pushed a change to branch fix/role-permission-reorder-drift
in repository https://gitbox.apache.org/repos/asf/cloudstack-terraform-provider.git


      at e1f2374  Fix role_permission reorder collision and authoritative drift detection

This branch includes the following new commits:

     new e1f2374  Fix role_permission reorder collision and authoritative drift detection

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(cloudstack-terraform-provider) 01/01: Fix role_permission reorder collision and authoritative drift detection

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

sudo87 pushed a commit to branch fix/role-permission-reorder-drift
in repository https://gitbox.apache.org/repos/asf/cloudstack-terraform-provider.git

commit e1f2374757758e9b254f498c069ce8978e3c16dd
Author: Manoj Kumar <ma...@gmail.com>
AuthorDate: Fri Aug 28 17:30:58 2026 +0530

    Fix role_permission reorder collision and authoritative drift detection
    
    Reordering permission list entries deleted and recreated them one at a
    time in the same pass, which could collide with a not-yet-processed
    entry still holding the target rule and fail with API error 531.
    reconcileCloudStackRolePermissions now deletes every stale permission
    first, then creates/updates the desired set in a second pass.
    
    authoritative = true also never surfaced drift: Read only considered
    permissions declared in config, so an out-of-band permission produced
    no diff and Update (where deletion happens) never ran. Read now
    appends undeclared permissions to state when authoritative is true so
    they show up as a plan diff.
    
    Non-authoritative shrink-deletion previously read its "previously
    managed" set from the same permission list Read populates, so a
    permission surfaced only for authoritative drift visibility could get
    misread as user-declared-then-removed and deleted even after flipping
    authoritative back to false. Track genuinely-managed IDs in a separate
    managed_ids field so drift visibility and shrink-deletion don't share
    state.
---
 cloudstack/resource_cloudstack_role_permission.go | 111 +++++++++++++++-------
 1 file changed, 78 insertions(+), 33 deletions(-)

diff --git a/cloudstack/resource_cloudstack_role_permission.go b/cloudstack/resource_cloudstack_role_permission.go
index 1769f8d..a0760d9 100644
--- a/cloudstack/resource_cloudstack_role_permission.go
+++ b/cloudstack/resource_cloudstack_role_permission.go
@@ -57,6 +57,11 @@ func resourceCloudStackRolePermission() *schema.Resource {
 				Default:     false,
 				Description: "Whether permissions not declared in this resource should be deleted.",
 			},
+			"managed_ids": {
+				Type:     schema.TypeSet,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
+			},
 			"permission": {
 				Type:        schema.TypeList,
 				Optional:    true,
@@ -99,11 +104,15 @@ func resourceCloudStackRolePermissionCreate(d *schema.ResourceData, meta interfa
 	roleLock.Lock()
 	defer roleLock.Unlock()
 
-	if err := reconcileCloudStackRolePermissions(d, meta, nil); err != nil {
-		return err
+	reconcileErr := reconcileCloudStackRolePermissions(d, meta, nil)
+	if readErr := resourceCloudStackRolePermissionRead(d, meta); readErr != nil {
+		if reconcileErr != nil {
+			return reconcileErr
+		}
+		return readErr
 	}
 
-	return resourceCloudStackRolePermissionRead(d, meta)
+	return reconcileErr
 }
 
 func resourceCloudStackRolePermissionRead(d *schema.ResourceData, meta interface{}) error {
@@ -150,6 +159,28 @@ func resourceCloudStackRolePermissionRead(d *schema.ResourceData, meta interface
 		}))
 	}
 
+	managedIDs := make([]interface{}, 0, len(used))
+	for id := range used {
+		managedIDs = append(managedIDs, id)
+	}
+	if err := d.Set("managed_ids", managedIDs); err != nil {
+		return fmt.Errorf("Error setting Role Permission managed IDs: %s", err)
+	}
+
+	if d.Get("authoritative").(bool) {
+		for _, rp := range rolePermissions {
+			if used[rp.Id] {
+				continue
+			}
+			readPermissions = append(readPermissions, rolePermissionState(rolePermissionSpec{
+				ID:          rp.Id,
+				Rule:        rp.Rule,
+				Permission:  rp.Permission,
+				Description: rp.Description,
+			}))
+		}
+	}
+
 	if err := d.Set("permission", readPermissions); err != nil {
 		return fmt.Errorf("Error setting Role Permissions: %s", err)
 	}
@@ -168,17 +199,23 @@ func resourceCloudStackRolePermissionUpdate(d *schema.ResourceData, meta interfa
 	roleLock.Lock()
 	defer roleLock.Unlock()
 
-	var oldPermissions []rolePermissionSpec
-	if d.HasChange("permission") {
-		oldRaw, _ := d.GetChange("permission")
-		oldPermissions = rolePermissionSpecs(oldRaw.([]interface{}))
+	oldManagedIDs := make(map[string]bool)
+	if d.HasChange("managed_ids") {
+		oldRaw, _ := d.GetChange("managed_ids")
+		for _, id := range oldRaw.(*schema.Set).List() {
+			oldManagedIDs[id.(string)] = true
+		}
 	}
 
-	if err := reconcileCloudStackRolePermissions(d, meta, oldPermissions); err != nil {
-		return err
+	reconcileErr := reconcileCloudStackRolePermissions(d, meta, oldManagedIDs)
+	if readErr := resourceCloudStackRolePermissionRead(d, meta); readErr != nil {
+		if reconcileErr != nil {
+			return reconcileErr
+		}
+		return readErr
 	}
 
-	return resourceCloudStackRolePermissionRead(d, meta)
+	return reconcileErr
 }
 
 func resourceCloudStackRolePermissionDelete(d *schema.ResourceData, meta interface{}) error {
@@ -228,7 +265,7 @@ func resourceCloudStackRolePermissionDelete(d *schema.ResourceData, meta interfa
 	return nil
 }
 
-func reconcileCloudStackRolePermissions(d *schema.ResourceData, meta interface{}, oldPermissions []rolePermissionSpec) error {
+func reconcileCloudStackRolePermissions(d *schema.ResourceData, meta interface{}, oldManagedIDs map[string]bool) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	roleID := d.Get("role_id").(string)
 
@@ -242,28 +279,44 @@ func reconcileCloudStackRolePermissions(d *schema.ResourceData, meta interface{}
 		rolePermissionsByID[rp.Id] = rp
 	}
 
+	desiredPermissions := rolePermissionSpecs(d.Get("permission").([]interface{}))
+
 	used := make(map[string]bool)
 	deleted := make(map[string]bool)
-	managedIDs := make([]string, 0)
-	managedIDSet := make(map[string]bool)
+	matches := make([]*cloudstack.RolePermission, len(desiredPermissions))
+	toDelete := make([]string, 0)
 
-	for _, desired := range rolePermissionSpecs(d.Get("permission").([]interface{})) {
+	for i, desired := range desiredPermissions {
 		rp := rolePermissionsByID[desired.ID]
 		if rp != nil && (rp.Rule != desired.Rule || rp.Description != desired.Description) {
-			if exactMatch := findMatchingRolePermission(rolePermissions, desired, used); exactMatch != nil {
-				rp = exactMatch
-			} else {
-				if err := deleteCloudStackRolePermission(cs, rp.Id); err != nil {
-					return err
-				}
-				deleted[rp.Id] = true
-				used[rp.Id] = true
-				rp = nil
-			}
-		} else if rp == nil {
+			rp = nil
+		}
+		if rp == nil {
 			rp = findMatchingRolePermission(rolePermissions, desired, used)
 		}
+		if rp == nil {
+			if stale := rolePermissionsByID[desired.ID]; stale != nil && !used[stale.Id] {
+				toDelete = append(toDelete, stale.Id)
+				used[stale.Id] = true
+			}
+			continue
+		}
+		used[rp.Id] = true
+		matches[i] = rp
+	}
+
+	for _, id := range toDelete {
+		if err := deleteCloudStackRolePermission(cs, id); err != nil {
+			return err
+		}
+		deleted[id] = true
+	}
+
+	managedIDs := make([]string, 0, len(desiredPermissions))
+	managedIDSet := make(map[string]bool)
 
+	for i, desired := range desiredPermissions {
+		rp := matches[i]
 		if rp == nil {
 			rp, err = createCloudStackRolePermission(cs, roleID, desired)
 			if err != nil {
@@ -275,18 +328,10 @@ func reconcileCloudStackRolePermissions(d *schema.ResourceData, meta interface{}
 			}
 		}
 
-		used[rp.Id] = true
 		managedIDs = append(managedIDs, rp.Id)
 		managedIDSet[rp.Id] = true
 	}
 
-	oldManagedIDs := make(map[string]bool)
-	for _, oldPermission := range oldPermissions {
-		if oldPermission.ID != "" {
-			oldManagedIDs[oldPermission.ID] = true
-		}
-	}
-
 	if d.Get("authoritative").(bool) {
 		for _, rp := range rolePermissions {
 			if managedIDSet[rp.Id] || deleted[rp.Id] {