fix(opeartor): != not working as expected (#506)

* fix(opeartor): != not working as expected * updating check * updating check
aws-cloudformation · May 8, 2024 · b756f49 · b756f49
1 parent 1e380cc
commit b756f49
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 7 deletions.
diff --git a/guard/src/rules/eval/operators.rs b/guard/src/rules/eval/operators.rs
@@ -634,6 +634,17 @@ impl Comparator for crate::rules::CmpOperator {
     }
 }
 
+fn reverse_diff(
+    diff: Vec<Rc<PathAwareValue>>,
+    other: &[Rc<PathAwareValue>],
+) -> Vec<Rc<PathAwareValue>> {
+    other
+        .iter()
+        .filter(|e| !diff.contains(e))
+        .map(Rc::clone)
+        .collect()
+}
+
 impl Comparator for (crate::rules::CmpOperator, bool) {
     fn compare<'value>(
         &self,
@@ -651,13 +662,14 @@ impl Comparator for (crate::rules::CmpOperator, bool) {
                                 ValueEvalResult::ComparisonResult(ComparisonResult::Fail(c)) => {
                                     match c {
                                         Compare::QueryIn(qin) => {
-                                            let mut reverse_diff =
-                                                Vec::with_capacity(qin.lhs.len());
-                                            for each in &qin.lhs {
-                                                if !qin.diff.contains(each) {
-                                                    reverse_diff.push(Rc::clone(each))
-                                                }
-                                            }
+                                            let reverse_diff = if rhs.len() >= lhs.len()
+                                                && matches!(self.0, crate::rules::CmpOperator::Eq)
+                                            {
+                                                reverse_diff(qin.diff, &qin.rhs)
+                                            } else {
+                                                reverse_diff(qin.diff, &qin.lhs)
+                                            };
+
                                             if reverse_diff.is_empty() {
                                                 ValueEvalResult::ComparisonResult(
                                                     ComparisonResult::Success(Compare::QueryIn(

diff --git a/guard/src/rules/eval/operators_tests.rs b/guard/src/rules/eval/operators_tests.rs
@@ -52,6 +52,11 @@ Resources:
     Properties:
       ge: [10, 20, 30]
       le: 10
+  OutboundRule:
+    Type: AWS::EC2::SecurityGroupEgress
+    Properties:
+      FromPort: 46
+      ToPort: 56
 "###;
 
 // const RULES_EQ: &str = r###"
@@ -1089,3 +1094,39 @@ fn test_operator_eq_vs_in_from_queries() -> crate::rules::Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn test_operator_not_eq() -> crate::rules::Result<()> {
+    let to_port = AccessQuery::try_from(
+        r#"Resources[ Type == "AWS::EC2::SecurityGroupEgress" ].Properties.ToPort"#,
+    )?;
+
+    let from_port = AccessQuery::try_from(
+        r#"Resources[ Type == "AWS::EC2::SecurityGroupEgress" ].Properties.FromPort"#,
+    )?;
+
+    let value = PathAwareValue::try_from(crate::rules::values::read_from(RESOURCES)?)?;
+    let mut evaluator = BasicQueryTesting {
+        root: Rc::new(value),
+        recorder: None,
+    };
+
+    let resolved_to = evaluator.query(&to_port.query)?;
+    assert_eq!(resolved_to.len(), 1);
+
+    let resolved_from = evaluator.query(&from_port.query)?;
+    assert_eq!(resolved_from.len(), 1);
+
+    let result = match (CmpOperator::Eq, true).compare(&resolved_to, &resolved_from)? {
+        EvalResult::Result(v) => v,
+        _ => unreachable!(),
+    };
+
+    assert_eq!(result.len(), 1);
+    assert!(matches!(
+        result[0],
+        ValueEvalResult::ComparisonResult(ComparisonResult::Success(_))
+    ));
+
+    Ok(())
+}