fix(sync): three ways a UserGroup sync loses a permission silently

bin/core/src/api/execute/sync.rs

@@ -266,10 +266,12 @@ impl Resolve<ExecuteArgs> for RunSync {
     } else {
       Default::default()
     };
+    let user_groups_toml = resources.user_groups.clone();
     let (
       user_groups_to_create,
       user_groups_to_update,
       user_groups_to_delete,
+      dropped_permission_targets,
     ) = if match_resource_type.is_none()
       && match_resources.is_none()
       && sync.config.include_user_groups
@@ -299,6 +301,7 @@ impl Resolve<ExecuteArgs> for RunSync {
       && user_groups_to_create.is_empty()
       && user_groups_to_update.is_empty()
       && user_groups_to_delete.is_empty()
+      && dropped_permission_targets.is_empty()
       && variables_to_create.is_empty()
       && variables_to_update.is_empty()
       && variables_to_delete.is_empty()
@@ -335,15 +338,6 @@ impl Resolve<ExecuteArgs> for RunSync {
       )
       .await,
     );
-    maybe_extend(
-      &mut update.logs,
-      crate::sync::user_groups::run_updates(
-        user_groups_to_create,
-        user_groups_to_update,
-        user_groups_to_delete,
-      )
-      .await,
-    );
 
     maybe_extend(
       &mut update.logs,
@@ -400,6 +394,37 @@ impl Resolve<ExecuteArgs> for RunSync {
       Procedure::execute_sync_updates(procedure_deltas).await,
     );
 
+    if match_resource_type.is_none()
+      && match_resources.is_none()
+      && sync.config.include_user_groups
+    {
+      match crate::sync::user_groups::get_updates_for_execution(
+        user_groups_toml,
+        delete,
+      )
+      .await
+      {
+        Ok((to_create, to_update, to_delete, dropped_targets)) => {
+          maybe_extend(
+            &mut update.logs,
+            crate::sync::user_groups::run_updates(
+              to_create,
+              to_update,
+              to_delete,
+              dropped_targets,
+            )
+            .await,
+          )
+        }
+        Err(e) => update.push_error_log(
+          "Update UserGroups",
+          format_serror(
+            &e.context("failed to compute UserGroup updates").into(),
+          ),
+        ),
+      }
+    }
+
     // Execute the deploy cache
     deploy_from_cache(deploy_cache, &mut update.logs).await;
 

bin/core/src/sync/user_groups.rs

@@ -173,15 +173,18 @@ pub async fn get_updates_for_view(
       .permissions
       .retain(|p| p.level > PermissionLevel::None);
 
-    user_group.permissions =
-      expand_user_group_permissions(user_group.permissions)
-        .await
-        .with_context(|| {
-          format!(
-            "failed to expand user group {} permissions",
-            user_group.name
-          )
-        })?;
+    user_group.permissions = expand_user_group_permissions(
+      user_group.permissions,
+      &user_group.name,
+      &mut Vec::new(),
+    )
+    .await
+    .with_context(|| {
+      format!(
+        "failed to expand user group {} permissions",
+        user_group.name
+      )
+    })?;
 
     let (_original_id, original) =
       match map.get(&user_group.name).cloned() {
@@ -229,6 +232,7 @@ pub async fn get_updates_for_execution(
   Vec<UserGroupToml>,
   Vec<UpdateItem>,
   Vec<DeleteItem>,
+  Vec<String>,
 )> {
   let map = find_collect(&db_client().user_groups, None, None)
     .await
@@ -248,6 +252,7 @@ pub async fn get_updates_for_execution(
   let mut to_create = Vec::<UserGroupToml>::new();
   let mut to_update = Vec::<UpdateItem>::new();
   let mut to_delete = Vec::<DeleteItem>::new();
+  let mut dropped = Vec::<String>::new();
 
   if delete {
     for user_group in map.values() {
@@ -261,7 +266,7 @@ pub async fn get_updates_for_execution(
   }
 
   if user_groups.is_empty() {
-    return Ok((to_create, to_update, to_delete));
+    return Ok((to_create, to_update, to_delete, dropped));
   }
 
   let id_to_user = find_collect(&db_client().users, None, None)
@@ -280,15 +285,18 @@ pub async fn get_updates_for_execution(
       .permissions
       .retain(|p| p.level > PermissionLevel::None);
 
-    user_group.permissions =
-      expand_user_group_permissions(user_group.permissions)
-        .await
-        .with_context(|| {
-          format!(
-            "Failed to expand user group {} permissions",
-            user_group.name
-          )
-        })?;
+    user_group.permissions = expand_user_group_permissions(
+      user_group.permissions,
+      &user_group.name,
+      &mut dropped,
+    )
+    .await
+    .with_context(|| {
+      format!(
+        "Failed to expand user group {} permissions",
+        user_group.name
+      )
+    })?;
 
     let original = match map.get(&user_group.name).cloned() {
       Some(original) => original,
@@ -472,7 +480,10 @@ pub async fn get_updates_for_execution(
     }
   }
 
-  Ok((to_create, to_update, to_delete))
+  dropped.sort_unstable();
+  dropped.dedup();
+
+  Ok((to_create, to_update, to_delete, dropped))
 }
 
 /// order permissions in deterministic way
@@ -495,17 +506,27 @@ pub async fn run_updates(
   to_create: Vec<UserGroupToml>,
   to_update: Vec<UpdateItem>,
   to_delete: Vec<DeleteItem>,
+  dropped_targets: Vec<String>,
 ) -> Option<Log> {
   if to_create.is_empty()
     && to_update.is_empty()
     && to_delete.is_empty()
+    && dropped_targets.is_empty()
   {
     return None;
   }
 
   let mut has_error = false;
   let mut log = String::from("running updates on UserGroups");
 
+  for target in dropped_targets {
+    log.push_str(&format!(
+      "\n{}: permission target matched no existing resource, so it was NOT applied | {}",
+      colored("WARN", Color::Red),
+      bold(&target)
+    ));
+  }
+
   // Create the non-existant user groups
   for user_group in to_create {
     // Create the user group
@@ -791,6 +812,8 @@ async fn run_update_permissions(
 /// Expands any regex defined targets into the full list
 async fn expand_user_group_permissions(
   permissions: Vec<PermissionToml>,
+  user_group: &str,
+  dropped: &mut Vec<String>,
 ) -> anyhow::Result<Vec<PermissionToml>> {
   let mut expanded =
     Vec::<PermissionToml>::with_capacity(permissions.capacity());
@@ -799,9 +822,13 @@ async fn expand_user_group_permissions(
   for permission in permissions {
     let (variant, id) = permission.target.extract_variant_id();
     if id.is_empty() {
+      dropped.push(format!(
+        "user group: {user_group} | target: {variant} with an empty id"
+      ));
       continue;
     }
     let matcher = Matcher::new(id)?;
+    let before = expanded.len();
     match variant {
       ResourceTargetVariant::Swarm => {
         let permissions = all_resources
@@ -937,7 +964,15 @@ async fn expand_user_group_permissions(
           });
         expanded.extend(permissions);
       }
-      ResourceTargetVariant::System => {}
+      ResourceTargetVariant::System => {
+        expanded.push(permission);
+        continue;
+      }
+    }
+    if expanded.len() == before {
+      dropped.push(format!(
+        "user group: {user_group} | target: {variant} {id}"
+      ));
     }
   }