[Enhancement]: JUnit reporter for validate command (#446)

* init commit for junit reporter WIP / MVP example * misc tweeks * adding junit arg for test command * updating install script to check for architecture and download the correct artifactj * [misc]: temp * cleanup * cleanup * cleanup * temp * cleanup * cleaning up structured test + adding fix for failing junit case * cleanup * cleanup * cleanup * updating error codes for ffi * updating error message for quick_xml errors * adding test case for when structured flag is not present and output is set to junit * created structuredreporter trait, and implemented the trait for both current reporters * adding explicit checks on enums for output type on the structured reporter before reporting "
aws-cloudformation · Jan 18, 2024 · 1619c0a · 1619c0a
1 parent 1396965
commit 1619c0a
Show file tree

Hide file tree

Showing 17 changed files with 599 additions and 57 deletions.
diff --git a/ATTRIBUTION b/ATTRIBUTION
@@ -1687,4 +1687,31 @@ SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
 CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
 OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR
 IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
-DEALINGS IN THE SOFTWARE.
+DEALINGS IN THE SOFTWARE.
+
+--
+tafia/quick-xml
+
+The MIT License (MIT)
+
+Copyright (c) 2016 Johann Tuffe
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+
+The above copyright notice and this permission notice shall be included in
+all copies or substantial portions of the Software.
+
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+THE SOFTWARE.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/guard-ffi/src/errors.rs b/guard-ffi/src/errors.rs
@@ -29,6 +29,9 @@ fn get_code(e: &Error) -> ErrorCode {
         Error::MissingValue(_err) => 16,
         Error::FileNotFoundError(_) => 17,
         Error::IllegalArguments(_) => 18,
+        //NOTE: skipping 19 since we already use that for something and dont want to confuse users
+        //that use both the regular cli, and the ffi
+        Error::XMLError(_) => 20,
         Error::InternalError(_) => unreachable!(),
     };
     ErrorCode::new(code)

diff --git a/guard/Cargo.toml b/guard/Cargo.toml
@@ -43,6 +43,7 @@ rstest = "0.15.0"
 fancy-regex = "0.13.0"
 indoc = "1.0.8"
 thiserror = "1.0.38"
+quick-xml = "0.30.0"
 
 [dependencies.serde_json]
 version = "1.0.85"

diff --git a/guard/resources/validate/output-dir/structured.junit b/guard/resources/validate/output-dir/structured.junit
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuites name="cfn-guard validate report" tests="4" failures="3" errors="0" time="0">
+    <testsuite name="s3-public-read-prohibited-template-non-compliant.yaml" errors="0" failures="3" time="0">
+        <testcase name="advanced_regex_negative_lookbehind_rule.guard" time="0">
+            <failure message="default">Check was not compliant as property [NotAwsAccessKey] to compare from is missing. Value traversed to [Path=[L:4,C:0] Value={&quot;Resources&quot;:{&quot;MyBucket&quot;:{&quot;Type&quot;:&quot;AWS::S3::Bucket&quot;,&quot;Properties&quot;:{&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}}}}].Check was not compliant as property [NotSecretAccessKey] to compare from is missing. Value traversed to [Path=[L:4,C:0] Value={&quot;Resources&quot;:{&quot;MyBucket&quot;:{&quot;Type&quot;:&quot;AWS::S3::Bucket&quot;,&quot;Properties&quot;:{&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}}}}].</failure>
+        </testcase>
+        <testcase name="s3_bucket_logging_enabled.guard" time="0">
+            <failure message="S3_BUCKET_LOGGING_ENABLED">
+    Violation: S3 Bucket Logging needs to be configured to enable logging.
+    Fix: Set the S3 Bucket property LoggingConfiguration to start logging into S3 bucket.
+  Check was not compliant as property [LoggingConfiguration] is missing. Value traversed to [Path=/Resources/MyBucket/Properties[L:13,C:6] Value={&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}].</failure>
+        </testcase>
+        <testcase name="s3_bucket_public_read_prohibited.guard" time="0">
+            <failure message="S3_BUCKET_PUBLIC_READ_PROHIBITED">Check was not compliant as property [PublicAccessBlockConfiguration] is missing. Value traversed to [Path=/Resources/MyBucket/Properties[L:13,C:6] Value={&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}].Check was not compliant as property [PublicAccessBlockConfiguration.BlockPublicAcls] to compare from is missing. Value traversed to [Path=/Resources/MyBucket/Properties[L:13,C:6] Value={&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}].Check was not compliant as property [PublicAccessBlockConfiguration.BlockPublicPolicy] to compare from is missing. Value traversed to [Path=/Resources/MyBucket/Properties[L:13,C:6] Value={&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}].Check was not compliant as property [PublicAccessBlockConfiguration.IgnorePublicAcls] to compare from is missing. Value traversed to [Path=/Resources/MyBucket/Properties[L:13,C:6] Value={&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}].
+    Violation: S3 Bucket Public Write Access controls need to be restricted.
+    Fix: Set S3 Bucket PublicAccessBlockConfiguration properties for BlockPublicAcls, BlockPublicPolicy, IgnorePublicAcls, RestrictPublicBuckets parameters to true.
+  Check was not compliant as property [PublicAccessBlockConfiguration.RestrictPublicBuckets] to compare from is missing. Value traversed to [Path=/Resources/MyBucket/Properties[L:13,C:6] Value={&quot;BucketEncryption&quot;:{&quot;ServerSideEncryptionConfiguration&quot;:[{&quot;ServerSideEncryptionByDefault&quot;:{&quot;SSEAlgorithm&quot;:&quot;AES256&quot;}}]},&quot;VersioningConfiguration&quot;:{&quot;Status&quot;:&quot;Enabled&quot;}}].</failure>
+        </testcase>
+        <testcase name="s3_bucket_server_side_encryption_enabled.guard" time="0" status="pass"/>
+    </testsuite>
+</testsuites>
diff --git a/guard/src/commands/mod.rs b/guard/src/commands/mod.rs
@@ -55,3 +55,7 @@ pub const STRUCTURED: (&str, char) = ("structured", 'z');
 pub(crate) const DATA_FILE_SUPPORTED_EXTENSIONS: [&str; 5] =
     [".yaml", ".yml", ".json", ".jsn", ".template"];
 pub(crate) const RULE_FILE_SUPPORTED_EXTENSIONS: [&str; 2] = [".guard", ".ruleset"];
+
+pub const FAILURE_STATUS_CODE: i32 = 19;
+pub const SUCCESS_STATUS_CODE: i32 = 0;
+pub const ERROR_STATUS_CODE: i32 = 5;
diff --git a/guard/src/commands/test.rs b/guard/src/commands/test.rs
@@ -166,17 +166,20 @@ or failure testing.
                     }
                 }
             }
+
             for file in non_guard {
                 let name = file
                     .file_name()
                     .to_str()
                     .map_or("".to_string(), |s| s.to_string());
+
                 if name.ends_with(".yaml")
                     || name.ends_with(".yml")
                     || name.ends_with(".json")
                     || name.ends_with(".jsn")
                 {
                     let parent = file.path().parent();
+
                     if parent.map_or(false, |p| p.ends_with("tests")) {
                         if let Some(candidates) = parent.unwrap().parent().and_then(|grand| {
                             let grand = format!("{}", grand.display());
@@ -193,7 +196,7 @@ or failure testing.
                 }
             }
 
-            for (_dir, guard_files) in ordered_guard_files {
+            for (_, guard_files) in ordered_guard_files {
                 for each_rule_file in guard_files {
                     if each_rule_file.test_files.is_empty() {
                         writeln!(

diff --git a/guard/src/commands/validate.rs b/guard/src/commands/validate.rs
@@ -21,7 +21,7 @@ use crate::commands::validate::tf::TfAware;
 use crate::commands::{
     ALPHABETICAL, DATA, DATA_FILE_SUPPORTED_EXTENSIONS, INPUT_PARAMETERS, LAST_MODIFIED,
     OUTPUT_FORMAT, PAYLOAD, PRINT_JSON, REQUIRED_FLAGS, RULES, RULE_FILE_SUPPORTED_EXTENSIONS,
-    SHOW_SUMMARY, STRUCTURED, TYPE, VALIDATE, VERBOSE,
+    SHOW_SUMMARY, STRUCTURED, SUCCESS_STATUS_CODE, TYPE, VALIDATE, VERBOSE,
 };
 use crate::rules::errors::{Error, InternalError};
 use crate::rules::eval::eval_rules_file;
@@ -41,6 +41,7 @@ pub(crate) mod generic_summary;
 mod structured;
 mod summary_table;
 mod tf;
+pub mod xml;
 
 #[derive(Eq, Clone, Debug, PartialEq)]
 pub(crate) struct DataFile {
@@ -70,13 +71,15 @@ pub(crate) enum OutputFormatType {
     SingleLineSummary,
     JSON,
     YAML,
+    Junit,
 }
 
 impl From<&str> for OutputFormatType {
     fn from(value: &str) -> Self {
         match value {
             "single-line-summary" => OutputFormatType::SingleLineSummary,
             "json" => OutputFormatType::JSON,
+            "junit" => OutputFormatType::Junit,
             _ => OutputFormatType::YAML,
         }
     }
@@ -130,7 +133,7 @@ impl Validate {
     }
 }
 
-const OUTPUT_FORMAT_VALUE_TYPE: [&str; 3] = ["json", "yaml", "single-line-summary"];
+const OUTPUT_FORMAT_VALUE_TYPE: [&str; 4] = ["json", "yaml", "single-line-summary", "junit"];
 const SHOW_SUMMARY_VALUE_TYPE: [&str; 5] = ["none", "all", "pass", "fail", "skip"];
 const TEMPLATE_TYPE: [&str; 1] = ["CFNTemplate"];
 
@@ -285,6 +288,12 @@ or rules files.
             )));
         }
 
+        if matches!(output_type, OutputFormatType::Junit) && !structured {
+            return Err(Error::IllegalArguments(String::from(
+                "the structured flag must be set when output is set to junit",
+            )));
+        }
+
         let data_files = match app.get_many::<String>(DATA.0) {
             Some(list_of_file_or_dir) => {
                 let mut streams = Vec::new();
@@ -370,7 +379,7 @@ or rules files.
 
         let print_json = app.get_flag(PRINT_JSON.0);
 
-        let mut exit_code = 0;
+        let mut exit_code = SUCCESS_STATUS_CODE;
 
         if app.contains_id(RULES.0) {
             let list_of_file_or_dir = app.get_many::<String>(RULES.0).unwrap();

diff --git a/guard/src/commands/validate/cfn.rs b/guard/src/commands/validate/cfn.rs
@@ -115,6 +115,7 @@ impl<'reporter> Reporter for CfnAware<'reporter> {
                         Err(e) => return Err(e),
                     }
                 }
+                OutputFormatType::Junit => unreachable!(),
             };
 
             Ok(())

diff --git a/guard/src/commands/validate/cfn_reporter.rs b/guard/src/commands/validate/cfn_reporter.rs
@@ -48,6 +48,7 @@ impl Reporter for CfnReporter {
                     as Box<dyn GenericReporter>,
                 OutputFormatType::YAML => Box::new(StructuredSummary::new(StructureType::YAML))
                     as Box<dyn GenericReporter>,
+                OutputFormatType::Junit => unreachable!(),
             };
         let failed = if !failed_rules.is_empty() {
             let mut by_resource_name = HashMap::new();
@@ -149,6 +150,7 @@ impl Reporter for CfnReporter {
                     as Box<dyn GenericReporter>,
                 OutputFormatType::YAML => Box::new(StructuredSummary::new(StructureType::YAML))
                     as Box<dyn GenericReporter>,
+                OutputFormatType::Junit => unreachable!(),
             };
         super::common::report_from_events(
             _root_record,

diff --git a/guard/src/commands/validate/generic_summary.rs b/guard/src/commands/validate/generic_summary.rs
@@ -43,6 +43,7 @@ impl Reporter for GenericSummary {
                     as Box<dyn GenericReporter>,
                 OutputFormatType::YAML => Box::new(StructuredSummary::new(StructureType::YAML))
                     as Box<dyn GenericReporter>,
+                OutputFormatType::Junit => unreachable!(),
             };
         let failed = if !failed_rules.is_empty() {
             let mut by_rule = HashMap::with_capacity(failed_rules.len());
@@ -131,6 +132,7 @@ impl Reporter for GenericSummary {
                 rules_file,
                 &(SingleLineSummary {}),
             )?,
+            OutputFormatType::Junit => unreachable!(),
         };
 
         Ok(())

diff --git a/guard/src/commands/validate/structured.rs b/guard/src/commands/validate/structured.rs
@@ -1,6 +1,8 @@
 use std::rc::Rc;
 
+use crate::commands::validate::xml::JunitReporter;
 use crate::commands::validate::{parse_rules, DataFile, OutputFormatType, RuleFileInfo};
+use crate::commands::{ERROR_STATUS_CODE, FAILURE_STATUS_CODE};
 use crate::rules;
 use crate::rules::eval::eval_rules_file;
 use crate::rules::eval_context::{root_scope, simplifed_json_from_root, FileReport};
@@ -10,6 +12,10 @@ use crate::rules::Status;
 use crate::utils::writer::Writer;
 use colored::Colorize;
 
+pub trait StructuredReporter {
+    fn report(&mut self) -> rules::Result<i32>;
+}
+
 pub struct StructuredEvaluator<'eval> {
     pub(crate) rule_info: &'eval [RuleFileInfo],
     pub(crate) input_params: Option<PathAwareValue>,
@@ -20,8 +26,28 @@ pub struct StructuredEvaluator<'eval> {
 }
 
 impl<'eval> StructuredEvaluator<'eval> {
-    fn merge_input_params_with_data(&mut self) -> Vec<DataFile> {
-        self.data.iter().fold(vec![], |mut res, file| {
+    pub(crate) fn evaluate(&mut self) -> rules::Result<i32> {
+        let rules = self.rule_info.iter().try_fold(
+            vec![],
+            |mut rules,
+             RuleFileInfo { file_name, content }|
+             -> rules::Result<Vec<(RulesFile, &str)>> {
+                match parse_rules(content, file_name) {
+                    Err(e) => {
+                        self.writer.write_err(format!(
+                            "Parsing error handling rule file = {}, Error = {e}\n---",
+                            file_name.underline()
+                        ))?;
+                        self.exit_code = ERROR_STATUS_CODE;
+                    }
+                    Ok(Some(rule)) => rules.push((rule, file_name)),
+                    Ok(None) => {}
+                }
+                Ok(rules)
+            },
+        )?;
+
+        let merged_data = self.data.iter().fold(vec![], |mut res, file| {
             let each = match &self.input_params {
                 Some(data) => data.clone().merge(file.path_value.clone()).unwrap(),
                 None => file.path_value.clone(),
@@ -30,51 +56,57 @@ impl<'eval> StructuredEvaluator<'eval> {
             let merged_file_data = DataFile {
                 path_value: each,
                 name: file.name.to_owned(),
-                content: "".to_string(), // not used later on
+                content: String::default(),
             };
 
             res.push(merged_file_data);
             res
-        })
-    }
+        });
 
-    fn get_rules(&mut self) -> rules::Result<Vec<RulesFile<'eval>>> {
-        self.rule_info.iter().try_fold(
-            vec![],
-            |mut rules, RuleFileInfo { file_name, content }| -> rules::Result<Vec<RulesFile>> {
-                match parse_rules(content, file_name) {
-                    Err(e) => {
-                        self.writer.write_err(format!(
-                            "Parsing error handling rule file = {}, Error = {e}\n---",
-                            file_name.underline()
-                        ))?;
-                        self.exit_code = 5;
-                    }
-                    Ok(Some(rule)) => rules.push(rule),
-                    Ok(None) => {}
-                }
-                Ok(rules)
-            },
-        )
+        let mut reporter = match self.output {
+            OutputFormatType::Junit => Box::new(JunitReporter {
+                data: merged_data,
+                rules,
+                writer: self.writer,
+                exit_code: self.exit_code,
+            }) as Box<dyn StructuredReporter>,
+            OutputFormatType::JSON | OutputFormatType::YAML => Box::new(CommonStructuredReporter {
+                rules,
+                data: merged_data,
+                writer: self.writer,
+                exit_code: self.exit_code,
+                output: self.output,
+            })
+                as Box<dyn StructuredReporter>,
+            OutputFormatType::SingleLineSummary => unreachable!(),
+        };
+
+        reporter.report()
     }
+}
 
-    pub(crate) fn evaluate(&mut self) -> rules::Result<i32> {
-        let rules = self.get_rules()?;
-        let merged_data = self.merge_input_params_with_data();
+pub struct CommonStructuredReporter<'reporter> {
+    pub(crate) rules: Vec<(RulesFile<'reporter>, &'reporter str)>,
+    pub(crate) data: Vec<DataFile>,
+    pub writer: &'reporter mut crate::utils::writer::Writer,
+    pub exit_code: i32,
+    pub output: OutputFormatType,
+}
 
+impl<'reporter> StructuredReporter for CommonStructuredReporter<'reporter> {
+    fn report(&mut self) -> rules::Result<i32> {
         let mut records = vec![];
-
-        for each in &merged_data {
+        for each in &self.data {
             let mut file_report: FileReport = FileReport {
                 name: &each.name,
                 ..Default::default()
             };
 
-            for rule in &rules {
+            for (rule, _) in &self.rules {
                 let mut root_scope = root_scope(rule, Rc::new(each.path_value.clone()));
 
                 if let Status::FAIL = eval_rules_file(rule, &mut root_scope, Some(&each.name))? {
-                    self.exit_code = 19;
+                    self.exit_code = FAILURE_STATUS_CODE;
                 }
 
                 let root_record = root_scope.reset_recorder().extract();

diff --git a/guard/src/commands/validate/tf.rs b/guard/src/commands/validate/tf.rs
@@ -59,6 +59,7 @@ impl<'reporter> Reporter for TfAware<'reporter> {
                 OutputFormatType::SingleLineSummary => {
                     single_line(write, data_file, rules_file, data, root, failure_report)?
                 }
+                OutputFormatType::Junit => unreachable!(),
             };
 
             Ok(())