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] {