Combined structured output and updated default rule clause name to in…

…clude file name (#360) * Populated filename in the output * Structured combine * Changed FileData into DataFile and handle error differently * Resolved lifetime issue with FileReport combine method * Updated status and method * Refactored to use existing DataFile struct * Changed FileData into DataFile and handle error differently * Refactored to use existing DataFile struct * Merged file report * Interim commit for structured * Resolved unit tests * Temporary commit for default rule names * Working prototype for formatting issue --------- Co-authored-by: Akshay Rane <raneaks@amazon.com>
aws-cloudformation · Jun 8, 2023 · dcd2b02 · dcd2b02
1 parent 661dbf4
commit dcd2b02
Show file tree

Hide file tree

Showing 16 changed files with 164 additions and 136 deletions.
diff --git a/guard/resources/validate/output-dir/payload_flag_fail_verbose_prev_engine.out b/guard/resources/validate/output-dir/payload_flag_fail_verbose_prev_engine.out
@@ -1,10 +1,10 @@
 Evaluation Tree
-Rule(default, PASS)
+Rule(RULES_STDIN[1]/default, PASS)
     |  Message: DEFAULT MESSAGE(PASS)
     Clause( Parameters.InstanceName EQUALS  "TestInstance", PASS)
         |  Message: DEFAULT MESSAGE(PASS)
 Evaluation Tree
-Rule(default, PASS)
+Rule(RULES_STDIN[1]/default, PASS)
     |  Message: DEFAULT MESSAGE(PASS)
     Clause( Parameters.InstanceName EQUALS  "TestInstance", PASS)
         |  Message: DEFAULT MESSAGE(PASS)
@@ -13,7 +13,7 @@ FAILED rules
 RULES_STDIN[2]/default    FAIL
 ---
 Evaluation Tree
-Rule(default, FAIL)
+Rule(RULES_STDIN[2]/default, FAIL)
     |  Message: DEFAULT MESSAGE(FAIL)
     Clause( Parameters.InstanceName EQUALS  "SomeRandomString", FAIL)
         |  From: String((Path("/Parameters/InstanceName", Location { line: 0, col: 276 }), "TestInstance"))
@@ -24,7 +24,7 @@ FAILED rules
 RULES_STDIN[2]/default    FAIL
 ---
 Evaluation Tree
-Rule(default, FAIL)
+Rule(RULES_STDIN[2]/default, FAIL)
     |  Message: DEFAULT MESSAGE(FAIL)
     Clause( Parameters.InstanceName EQUALS  "SomeRandomString", FAIL)
         |  From: String((Path("/Parameters/InstanceName", Location { line: 0, col: 276 }), "TestInstance"))

diff --git a/guard/resources/validate/output-dir/structured-payload.json b/guard/resources/validate/output-dir/structured-payload.json
@@ -6,7 +6,8 @@
     "not_compliant": [],
     "not_applicable": [],
     "compliant": [
-      "default"
+      "RULES_STDIN[1]/default",
+      "RULES_STDIN[2]/default"
     ]
   },
   {
@@ -16,27 +17,8 @@
     "not_compliant": [],
     "not_applicable": [],
     "compliant": [
-      "default"
-    ]
-  },
-  {
-    "name": "DATA_STDIN[1]",
-    "metadata": {},
-    "status": "PASS",
-    "not_compliant": [],
-    "not_applicable": [],
-    "compliant": [
-      "default"
-    ]
-  },
-  {
-    "name": "DATA_STDIN[2]",
-    "metadata": {},
-    "status": "PASS",
-    "not_compliant": [],
-    "not_applicable": [],
-    "compliant": [
-      "default"
+      "RULES_STDIN[1]/default",
+      "RULES_STDIN[2]/default"
     ]
   }
 ]
diff --git a/guard/resources/validate/output-dir/structured.json b/guard/resources/validate/output-dir/structured.json
@@ -6,7 +6,7 @@
     "not_compliant": [
       {
         "Rule": {
-          "name": "default",
+          "name": "advanced_regex_negative_lookbehind_rule.guard/default",
           "metadata": {},
           "messages": {
             "custom_message": null,
@@ -109,16 +109,7 @@
             }
           ]
         }
-      }
-    ],
-    "not_applicable": [],
-    "compliant": []
-  },
-  {
-    "name": "s3-public-read-prohibited-template-non-compliant.yaml",
-    "metadata": {},
-    "status": "FAIL",
-    "not_compliant": [
+      },
       {
         "Rule": {
           "name": "S3_BUCKET_LOGGING_ENABLED",
@@ -170,16 +161,7 @@
             }
           ]
         }
-      }
-    ],
-    "not_applicable": [],
-    "compliant": []
-  },
-  {
-    "name": "s3-public-read-prohibited-template-non-compliant.yaml",
-    "metadata": {},
-    "status": "FAIL",
-    "not_compliant": [
+      },
       {
         "Rule": {
           "name": "S3_BUCKET_PUBLIC_READ_PROHIBITED",
@@ -394,14 +376,6 @@
       }
     ],
     "not_applicable": [],
-    "compliant": []
-  },
-  {
-    "name": "s3-public-read-prohibited-template-non-compliant.yaml",
-    "metadata": {},
-    "status": "PASS",
-    "not_compliant": [],
-    "not_applicable": [],
     "compliant": [
       "S3_BUCKET_SERVER_SIDE_ENCRYPTION_ENABLED"
     ]

diff --git a/guard/resources/validate/output-dir/structured.yaml b/guard/resources/validate/output-dir/structured.yaml
@@ -3,7 +3,7 @@
   status: FAIL
   not_compliant:
   - Rule:
-      name: default
+      name: advanced_regex_negative_lookbehind_rule.guard/default
       metadata: {}
       messages:
         custom_message: null
@@ -63,12 +63,6 @@
                 comparison:
                 - Eq
                 - true
-  not_applicable: []
-  compliant: []
-- name: s3-public-read-prohibited-template-non-compliant.yaml
-  metadata: {}
-  status: FAIL
-  not_compliant:
   - Rule:
       name: S3_BUCKET_LOGGING_ENABLED
       metadata: {}
@@ -99,12 +93,6 @@
                 comparison:
                 - Exists
                 - false
-  not_applicable: []
-  compliant: []
-- name: s3-public-read-prohibited-template-non-compliant.yaml
-  metadata: {}
-  status: FAIL
-  not_compliant:
   - Rule:
       name: S3_BUCKET_PUBLIC_READ_PROHIBITED
       metadata: {}
@@ -228,11 +216,5 @@
                 - Eq
                 - false
   not_applicable: []
-  compliant: []
-- name: s3-public-read-prohibited-template-non-compliant.yaml
-  metadata: {}
-  status: PASS
-  not_compliant: []
-  not_applicable: []
   compliant:
   - S3_BUCKET_SERVER_SIDE_ENCRYPTION_ENABLED
diff --git a/guard/src/commands/validate.rs b/guard/src/commands/validate.rs
@@ -30,6 +30,7 @@ use crate::rules::eval::eval_rules_file;
 use crate::rules::eval_context::{root_scope, EventRecord};
 use crate::rules::evaluate::RootScope;
 use crate::rules::exprs::RulesFile;
+use crate::rules::parser::get_rule_name;
 use crate::rules::path_value::traversal::Traversal;
 use crate::rules::path_value::PathAwareValue;
 use crate::rules::values::CmpOperator;
@@ -713,7 +714,7 @@ fn print_failing_clause(
         "{file}/{rule:<0$}",
         longest + 4,
         file = rules_file_name,
-        rule = rule.context
+        rule = get_rule_name(rules_file_name, &rule.context)
     )
     .expect("Unable to write to the output");
     let longest = rules_file_name.len() + longest;
@@ -826,7 +827,7 @@ impl<'r> ConsoleReporter<'r> {
             let serialized_user = serde_json::to_string_pretty(&top.children).unwrap();
             writeln!(self.writer, "{serialized_user}").expect("Unable to write to the output");
         } else {
-            let longest = get_longest(top);
+            let longest = get_longest(top, self.rules_file_name);
 
             let (failed, rest): (Vec<&StatusContext>, Vec<&StatusContext>) =
                 partition_failed_and_rest(top);
@@ -1026,11 +1027,15 @@ fn partition_failed_and_rest(top: &StatusContext) -> (Vec<&StatusContext>, Vec<&
         .partition(|ctx| matches!(ctx.status, Some(Status::FAIL)))
 }
 
-fn get_longest(top: &StatusContext) -> usize {
+fn get_longest(top: &StatusContext, rule_file_name: &str) -> usize {
     top.children
         .iter()
-        .max_by(|f, s| f.context.len().cmp(&s.context.len()))
-        .map(|elem| elem.context.len())
+        .max_by(|f, s| {
+            get_rule_name(rule_file_name, &f.context)
+                .len()
+                .cmp(&get_rule_name(rule_file_name, &s.context).len())
+        })
+        .map(|elem| get_rule_name(rule_file_name, &elem.context).len())
         .unwrap_or(20)
 }
 

diff --git a/guard/src/commands/validate/cfn_reporter.rs b/guard/src/commands/validate/cfn_reporter.rs
@@ -205,36 +205,33 @@ impl super::common::GenericReporter for SingleLineReporter {
                 rules_file_name,
                 data_file_name,
                 |_, _, info| {
-                    Ok(format!("Resource [{}] traversed until [{}] for template [{}] wasn't compliant with [{}/{}] due to retrieval error. Error Message [{}]",
+                    Ok(format!("Resource [{}] traversed until [{}] for template [{}] wasn't compliant with [{}] due to retrieval error. Error Message [{}]",
                                resource,
                                info.path,
                                data_file_name,
-                               rules_file_name,
                                info.rule,
                                info.message.replace("\n", ";")
                     ))
                 },
                 |_, _, op_msg, info| {
-                    Ok(format!("Resource [{resource}] property [{property}] in template [{template}] is not compliant with [{rules}/{rule}] because needed value at [{provided}] {op_msg}. Error message [{msg}]",
+                    Ok(format!("Resource [{resource}] property [{property}] in template [{template}] is not compliant with [{rule}] because needed value at [{provided}] {op_msg}. Error message [{msg}]",
                                resource=resource,
                                property=info.path,
                                provided=info.provided.as_ref().map_or(&serde_json::Value::Null, std::convert::identity),
                                op_msg=op_msg,
                                template=data_file_name,
-                               rules=rules_file_name,
-                               rule=info.rule,
+                               rule= info.rule,
                                msg=info.message.replace("\n", ";")
                     ))
                 },
                 |_, _, msg, info| {
-                    Ok(format!("Resource [{resource}] property [{property}] in template [{template}] is not compliant with [{rules}/{rule}] because provided value [{provided}] {op_msg} match with expected value [{expected}]. Error message [{msg}]",
+                    Ok(format!("Resource [{resource}] property [{property}] in template [{template}] is not compliant with [{rule}] because provided value [{provided}] {op_msg} match with expected value [{expected}]. Error message [{msg}]",
                                resource=resource,
                                property=info.path,
                                provided=info.provided.as_ref().map_or(&serde_json::Value::Null, std::convert::identity),
                                op_msg=msg,
                                expected=info.expected.as_ref().map_or(&serde_json::Value::Null, std::convert::identity),
                                template=data_file_name,
-                               rules=rules_file_name,
                                rule=info.rule,
                                msg=info.message.replace("\n", ";")
                     ))

diff --git a/guard/src/commands/validate/common.rs b/guard/src/commands/validate/common.rs
@@ -8,6 +8,7 @@ use crate::rules::eval_context::{
     GuardClauseReport, InComparison, UnaryCheck, UnaryComparison, ValueComparisons,
     ValueUnResolved,
 };
+
 use crate::rules::values::CmpOperator;
 use crate::rules::{
     ClauseCheck, EvaluationType, NamedStatus, QueryResult, RecordType, Status, UnResolved,
@@ -384,7 +385,7 @@ pub(super) fn report_from_events(
     rules_file_name: &str,
     renderer: &dyn GenericReporter,
 ) -> crate::rules::Result<()> {
-    let mut longest_rule_name = 0;
+    let mut longest_rule_length = 0;
     let mut failed = HashMap::new();
     let mut skipped = HashSet::new();
     let mut success = HashSet::new();
@@ -395,8 +396,8 @@ pub(super) fn report_from_events(
             message,
         })) = &each_rule.container
         {
-            if name.len() > longest_rule_name {
-                longest_rule_name = name.len();
+            if name.len() > longest_rule_length {
+                longest_rule_length = name.len();
             }
             match status {
                 Status::FAIL => {
@@ -425,7 +426,7 @@ pub(super) fn report_from_events(
         failed,
         success,
         skipped,
-        longest_rule_name,
+        longest_rule_length,
     )?;
     Ok(())
 }
@@ -546,8 +547,8 @@ pub(super) fn print_compliant_skipped_info(
     for pass in passed {
         writeln!(
             writer,
-            "Rule [{}/{}] is compliant for template [{}]",
-            rules_file_name, pass, data_file_name
+            "Rule [{}] is compliant for template [{}]",
+            pass, data_file_name
         )?;
     }
     if !skipped.is_empty() {
@@ -556,8 +557,8 @@ pub(super) fn print_compliant_skipped_info(
     for skip in skipped {
         writeln!(
             writer,
-            "Rule [{}/{}] is not applicable for template [{}]",
-            rules_file_name, skip, data_file_name
+            "Rule [{}] is not applicable for template [{}]",
+            skip, data_file_name
         )?;
     }
     Ok(())
@@ -600,11 +601,12 @@ where
                 let (cmp, not) = match &each.comparison {
                     Some(cmp) => (cmp.operator, cmp.not_operator_exists),
                     None => {
-                        writeln!(writer, "Parameterized Rule {rules}/{rule_name} failed for {data}. Reason {msg}",
-                                 rules=rules_file_name,
-                                 data=data_file_name,
-                                 rule_name=each.rule,
-                                 msg=each.message.replace('\n', "; ")
+                        writeln!(
+                            writer,
+                            "Parameterized Rule {rule_name} failed for {data}. Reason {msg}",
+                            data = data_file_name,
+                            rule_name = each.rule,
+                            msg = each.message.replace('\n', "; ")
                         )?;
                         continue;
                     }