Fixes #18952: Static group which depends on dynamic subgroup must be …

…forbiden

webapp/sources/rudder/rudder-web/src/main/scala/com/normation/rudder/web/components/NodeGroupForm.scala

@@ -39,13 +39,17 @@ package com.normation.rudder.web.components
 
 import bootstrap.liftweb.RudderConfig
 import com.normation.box._
+import com.normation.errors.IOResult
 import com.normation.plugins.DefaultExtendableSnippet
 import com.normation.rudder.AuthorizationType
 import com.normation.rudder.domain.nodes._
 import com.normation.rudder.domain.policies._
+import com.normation.rudder.domain.queries.CriterionLine
 import com.normation.rudder.domain.queries.NewQuery
 import com.normation.rudder.domain.queries.Query
+import com.normation.rudder.domain.queries.QueryTrait
 import com.normation.rudder.domain.queries.ResultTransformation
+import com.normation.rudder.domain.queries.SubGroupComparator
 import com.normation.rudder.domain.workflows.ChangeRequestId
 import com.normation.rudder.repository.FullNodeGroupCategory
 import com.normation.rudder.services.workflows.DGModAction
@@ -62,9 +66,10 @@ import net.liftweb.http.LocalSnippet
 import net.liftweb.http.js._
 import net.liftweb.http.js.JE._
 import net.liftweb.http.js.JsCmds._
-import net.liftweb.util.CssSel
+import net.liftweb.util._
 import net.liftweb.util.Helpers._
 import scala.xml._
+import zio.syntax._
 
 object NodeGroupForm {
   val templatePath = "templates-hidden" :: "components" :: "NodeGroupForm" :: Nil
@@ -448,40 +453,148 @@ class NodeGroupForm(
             logger.error(f.messageChain)
         }
 
+        val optContainer = {
+          val c = NodeGroupCategoryId(groupContainer.get)
+          if (c == parentCategoryId) None
+          else Some(c)
+        }
+
+        // submit can be done only for node group, not system one
+        val newGroup = savedGroup.copy(
+          name = groupName.get,
+          description = groupDescription.get, // , container = container
+
+          isDynamic = groupStatic.get match { case "dynamic" => true; case _ => false },
+          query = query,
+          serverList = srvList.getOrElse(Set()).map(_.id).toSet
+        )
+
+        /*
+         * - If a group changes from dynamic to static, we must ensure that it does not refer
+         *   any dynamic subgroup, else raise an error
+         * See https://issues.rudder.io/issues/18952
+         */
+        if (savedGroup.isDynamic == true && newGroup.isDynamic == false) {
+          hasDynamicSubgroups(newGroup.query).either.runNow match {
+            case Left(err)        =>
+              formTracker.addFormError(Text("Error when saving group"))
+              logger.error(
+                s"Error when getting group information for consistency check on static change status: ${err.fullMsg}"
+              )
+            case Right(Some(msg)) =>
+              val m = s"Error when getting group information for consistency check on static change status: you can't change " +
+                s"the nature of current group to static because it uses following dynamic groups as a subgroup criteria: ${msg}"
+              formTracker.addFormError(Text(m))
+              logger.error(m)
+            case Right(None)      => // ok
+          }
+        }
+
+        /*
+         * - If a group changes from static to dynamic, we must ensure that it is not referred in any static
+         *   group target, else raise an error
+         * See https://issues.rudder.io/issues/18952
+         */
+        if (savedGroup.isDynamic == false && newGroup.isDynamic == true) {
+          getDependingGroups(newGroup.id, onlyStatic = true).either.runNow match {
+            case Left(err)        =>
+              formTracker.addFormError(Text("Error when saving group"))
+              logger.error(
+                s"Error when getting group information for consistency check on static change status: ${err.fullMsg}"
+              )
+            case Right(Some(msg)) =>
+              val m = s"Error when getting group information for consistency check on static change status: you " +
+                s"can't make that group dynamic since groups ${msg} are static and use it as a subgroup target."
+              formTracker.addFormError(Text(m))
+              logger.error(m)
+            case Right(None)      => // ok
+          }
+        }
+
+        if (newGroup == savedGroup && optContainer.isEmpty) {
+          formTracker.addFormError(Text("There are no modifications to save"))
+        }
+
         if (formTracker.hasErrors) {
           onFailure & onFailureCallback()
         } else {
-          val optContainer = {
-            val c = NodeGroupCategoryId(groupContainer.get)
-            if (c == parentCategoryId) None
-            else Some(c)
-          }
-
-          // submit can be done only for node group, not system one
-          val newGroup = savedGroup.copy(
-            name = groupName.get,
-            description = groupDescription.get, // , container = container
+          // don't warn on mod of a group for impact on depending groups
+          displayConfirmationPopup(DGModAction.Update, newGroup, optContainer, None)
+        }
+    }
+  }
 
-            isDynamic = groupStatic.get match { case "dynamic" => true; case _ => false },
-            query = query,
-            serverList = srvList.getOrElse(Set()).map(_.id).toSet
-          )
+  // find used subgroup in the query (wherever their place is) if any
+  private[components] def hasDynamicSubgroups(query: Option[QueryTrait]): IOResult[Option[String]] = {
+    query match {
+      case None    => None.succeed
+      case Some(q) =>
+        val subgroups = q.criteria.collect {
+          case CriterionLine(_, a, _, value) if (a.cType.isInstanceOf[SubGroupComparator]) => NodeGroupId(NodeGroupUid(value))
+        }
 
-          if (newGroup == savedGroup && optContainer.isEmpty) {
-            formTracker.addFormError(Text("There are no modifications to save"))
-            onFailure & onFailureCallback()
-          } else {
-            displayConfirmationPopup(DGModAction.Update, newGroup, optContainer)
+        for {
+          groups <- roNodeGroupRepository.getFullGroupLibrary()
+        } yield {
+          val depending = subgroups.flatMap { gid =>
+            groups.allGroups.get(gid) match {
+              case None    => // ? ignore, it's strange but that does not change things
+                None
+              case Some(g) =>
+                if (g.nodeGroup.isDynamic) { Some(g) }
+                else { None }
+            }
+          }
+          depending match {
+            case Nil  =>
+              None
+            case list =>
+              val gs = list.map(g => s"'${g.nodeGroup.name}' [${g.nodeGroup.id.serialize}]").mkString(", ")
+              Some(gs)
           }
         }
     }
+
+  }
+
+  // get the list of group that use that group as a target (optionally: only the static ones)
+  // The returned value is a message with the list of dep groups that can be used in form error or warning pop-up.
+  // If none, no dependent group were found.
+  private[components] def getDependingGroups(id: NodeGroupId, onlyStatic: Boolean): IOResult[Option[String]] = {
+    def queryTargetsSubgroup(query: Option[QueryTrait], id: NodeGroupId): Boolean = {
+      query match {
+        case None    => false
+        case Some(q) =>
+          q.criteria.find {
+            case CriterionLine(_, a, _, value) => a.cType.isInstanceOf[SubGroupComparator] && value == id.serialize
+          }.nonEmpty
+      }
+    }
+    def checkStatic(isDynamic: Boolean, onlyStatic: Boolean) = !onlyStatic || !isDynamic
+
+    roNodeGroupRepository.getFullGroupLibrary().map { groups =>
+      val dependingGroups = groups.allGroups.collect {
+        case (_, g) if (checkStatic(g.nodeGroup.isDynamic, onlyStatic) && queryTargetsSubgroup(g.nodeGroup.query, id)) =>
+          g.nodeGroup
+      }.toList
+      if (dependingGroups.nonEmpty) {
+        val gs = dependingGroups.map(g => s"'${g.name}' [${g.id.serialize}]}").mkString(", ")
+
+        Some(gs)
+      } else None
+    }
   }
 
   private[this] def onSubmitDelete(): JsCmd = {
     nodeGroup match {
       case Left(_)   => Noop
       case Right(ng) =>
-        displayConfirmationPopup(DGModAction.Delete, ng, None)
+        getDependingGroups(ng.id, onlyStatic = false).either.runNow match {
+          case Left(err)  =>
+            onFailure & onFailureCallback()
+          case Right(msg) =>
+            displayConfirmationPopup(DGModAction.Delete, ng, None, msg)
+        }
     }
   }
 
@@ -490,9 +603,10 @@ class NodeGroupForm(
    */
 
   private[this] def displayConfirmationPopup(
-      action:      DGModAction,
-      newGroup:    NodeGroup,
-      newCategory: Option[NodeGroupCategoryId]
+      action:             DGModAction,
+      newGroup:           NodeGroup,
+      newCategory:        Option[NodeGroupCategoryId],
+      dependingSubgroups: Option[String] // if Some, string contains a message with the groups
   ): JsCmd = {
 
     val optOriginal = nodeGroup.toOption
@@ -528,12 +642,22 @@ class NodeGroupForm(
           )
         }
 
-        popup.popupWarningMessages match {
-          case None    =>
-            popup.onSubmit()
-          case Some(_) =>
-            SetHtml("confirmUpdateActionDialog", popup.popupContent()) &
-            JsRaw("""createPopup("confirmUpdateActionDialog")""")
+        if (popup.popupWarningMessages.isEmpty && dependingSubgroups.isEmpty) {
+          popup.onSubmit()
+        } else {
+          val html: NodeSeq = dependingSubgroups match {
+            case None      => popup.popupContent()
+            case Some(msg) =>
+              val cssSel: CssSel = "#explanationMessageZone *+" #>
+                <div id="dialogSubgroupWarning" class="col-lg-12 col-sm-12 col-xs-12 alert alert-warning text-center">
+                  This group is used as a subgroups of group {msg}. If you delete it, they will be impacted.
+                </div>
+
+              cssSel(popup.popupContent())
+          }
+
+          SetHtml("confirmUpdateActionDialog", html) &
+          JsRaw("""createPopup("confirmUpdateActionDialog")""")
         }
     }
   }