Fixes #19114: Overridden directives in the same rule are missing (not…

… even \"skipped\")
Normation · Apr 1, 2021 · c364e72 · c364e72
1 parent 0c5c550
commit c364e72
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 6 deletions.
diff --git a/...dder-core/src/main/scala/com/normation/rudder/services/reports/ReportingServiceImpl.scala b/...dder-core/src/main/scala/com/normation/rudder/services/reports/ReportingServiceImpl.scala
@@ -66,14 +66,16 @@ import zio._
 object ReportingServiceUtils {
 
   /*
-   * Build rule status reports from node reports, decide=ing which directive should be "skipped"
+   * Build rule status reports from node reports, deciding which directives should be "skipped"
    */
   def buildRuleStatusReport(ruleId: RuleId, nodeReports: Map[NodeId, NodeStatusReport]): RuleStatusReport = {
     val toKeep = nodeReports.values.flatMap( _.reports ).filter(_.ruleId == ruleId).toList
     // we don't keep overrides for a directive which is already in "toKeep" or that don't target that rule
     val toKeepDir = toKeep.map(_.directives.keySet).toSet.flatten
     val overrides = nodeReports.values.flatMap( _.overrides.filterNot(r => r.policy.ruleId != ruleId || toKeepDir.contains(r.policy.directiveId))).toList.distinct
-    RuleStatusReport(ruleId, toKeep, overrides)
+    // and we must make overrides unique - ie, we don't keep overridden that are overridden by directive themselve in the overridden list
+    val overrides2 = overrides.filterNot(o => overrides.exists(_.policy == o.overridenBy))
+    RuleStatusReport(ruleId, toKeep, overrides2)
   }
 }
 

diff --git a/...core/src/test/scala/com/normation/rudder/services/reports/ReportingServiceUtilsTest.scala b/...core/src/test/scala/com/normation/rudder/services/reports/ReportingServiceUtilsTest.scala
@@ -70,6 +70,7 @@ class ReportingServiceUtilsTest extends Specification {
   val rule3 = RuleId("rule3")
   val dir1  = DirectiveId("dir1")
   val dir2  = DirectiveId("dir2")
+  val dir3  = DirectiveId("dir3")
 
   val expiration = new DateTime(0) // not used
 
@@ -184,4 +185,46 @@ class ReportingServiceUtilsTest extends Specification {
       RuleStatusReport(rule2, List(rnReport(node2, rule2, dir2)), noOverrides)
     )
   }
+
+  /*
+   * More complexe for one node:
+   * - 3 directives: dir1 (most prioritary), dir2 and dir3 (less)
+   * - 3 rules: rule1 has dir2, dir3 (skipped),  rule2 has all 3 (so dir1 ok, other skipped), rule3 has all 3 (skipped)
+   * There is no ducplication of reports.
+   */
+  "a rule not overriden on all nodes is not written overriden" in {
+    val reports = List(
+      NodeStatusReport(node1, NoRunNoExpectedReport, RunComplianceInfo.OK
+        , List(
+              // on rule1, both dir2 and dir3 are overridden by rule2/dir1
+              thisOverrideThatOn2(rule2, dir1, rule1, dir2)
+            , thisOverrideThatOn2(rule2, dir1, rule1, dir3)
+              // on rule2, both dir2 and dir3 are overridden by rule2/dir1
+            , thisOverrideThatOn2(rule2, dir1, rule2, dir2)
+            , thisOverrideThatOn2(rule2, dir1, rule2, dir3)
+              // on rule3, dir2, dir2 and dir3 are overridden by rule2/dir1
+            , thisOverrideThatOn2(rule2, dir1, rule3, dir1)
+            , thisOverrideThatOn2(rule2, dir1, rule3, dir2)
+            , thisOverrideThatOn2(rule2, dir1, rule3, dir3)
+              // and these one are real but should not be kept to avoid having several time the same "skipped"
+            , thisOverrideThatOn2(rule1, dir2, rule1, dir3)
+            , thisOverrideThatOn2(rule2, dir2, rule2, dir3)
+            , thisOverrideThatOn2(rule3, dir1, rule3, dir2)
+            , thisOverrideThatOn2(rule3, dir1, rule3, dir3)
+            , thisOverrideThatOn2(rule3, dir2, rule3, dir3)
+          )
+          // only one expected report: dir1 on rule2
+        , Set(rnReport(node1, rule2, dir1))
+      )
+    ).map(r => (r.nodeId, r)).toMap
+
+    ReportingServiceUtils.buildRuleStatusReport(rule1, reports).isSameReportAs(
+      RuleStatusReport(rule1, List(), List(thisOverrideThatOn2(rule2, dir1, rule1, dir2), thisOverrideThatOn2(rule2, dir1, rule1, dir3)))
+    ) and ReportingServiceUtils.buildRuleStatusReport(rule2, reports).isSameReportAs(
+      RuleStatusReport(rule2, List(rnReport(node1, rule2, dir1)), List(thisOverrideThatOn2(rule2, dir1, rule2, dir2), thisOverrideThatOn2(rule2, dir1, rule2, dir3)))
+    ) and ReportingServiceUtils.buildRuleStatusReport(rule3, reports).isSameReportAs(
+      RuleStatusReport(rule3, List(), List(thisOverrideThatOn2(rule2, dir1, rule3, dir1), thisOverrideThatOn2(rule2, dir1, rule3, dir2), thisOverrideThatOn2(rule2, dir1, rule3, dir3)))
+    )
+  }
+
 }
diff --git a/...s/rudder/rudder-web/src/main/scala/com/normation/rudder/web/services/ComplianceData.scala b/...s/rudder/rudder-web/src/main/scala/com/normation/rudder/web/services/ComplianceData.scala
@@ -552,9 +552,7 @@ object ComplianceData extends Loggable {
         , explanation
         , JsonTagSerialisation.serializeTags(rule.tags)
       )
-
     }
-
     JsTableData(ruleComplianceLine.toList)
   }
 
@@ -565,8 +563,7 @@ object ComplianceData extends Loggable {
     , onRuleScreen: Option[Rule] // if we are on a rule, we want to adapt message
   ) : List[DirectiveComplianceLine] = {
     val overridesData = for {
-      // we don't want to write an overriden directive several time for the same overriding rule/directive.
-      over                            <- overrides.groupBy(_.overridenBy).map(_._2.head)
+      over                            <- overrides
       (overridenTech , overridenDir)  <- directiveLib.allDirectives.get(over.policy.directiveId)
       overridingRule                  <- rules.find( _.id == over.overridenBy.ruleId)
       (overridingTech, overridingDir) <- directiveLib.allDirectives.get(over.overridenBy.directiveId)