Fixes handling of Custom IOA Rule Group Versioning (#216)

* Fixes handling of Custom IOA Rule Group Versioning

* Fix incorrect logic in rule group version tracking

* Fix missing side effect in tests & fix tracking of rules

* Fix linting complaints

* fix: linting

---------

Co-authored-by: Atte Niemi <[email protected]>

caracara/modules/custom_ioa/custom_ioa.py

@@ -1,6 +1,7 @@
 """Caracara Indicator of Attack (IOA) API module."""
 
 from functools import partial
+from itertools import chain
 from time import monotonic
 from typing import Dict, List, Union
 
@@ -96,7 +97,7 @@ def create_rule_group(
             rule.group = new_group
 
         # Update the rules
-        new_group = self._create_update_delete_rules(new_group, comment=comment)
+        new_group = self._update_create_delete_rules(new_group, comment=comment)
 
         return new_group
 
@@ -138,7 +139,7 @@ def update_rule_group(
         new_group.rules_to_delete = group.rules_to_delete
 
         # Update the rules
-        new_group = self._create_update_delete_rules(new_group, comment=comment)
+        new_group = self._update_create_delete_rules(new_group, comment=comment)
 
         return new_group
 
@@ -167,7 +168,7 @@ def delete_rule_groups(
                 ids_to_delete.append(rule_group)
         instr(self.custom_ioa_api.delete_rule_groups)(ids=ids_to_delete, comment=comment)
 
-    def _create_update_delete_rules(self, group: IoaRuleGroup, comment: str) -> IoaRuleGroup:
+    def _update_create_delete_rules(self, group: IoaRuleGroup, comment: str) -> IoaRuleGroup:
         existing_rules = []
         to_be_created = []
         for rule in group.rules:
@@ -176,25 +177,13 @@ def _create_update_delete_rules(self, group: IoaRuleGroup, comment: str) -> IoaR
             else:
                 to_be_created.append(rule)
 
-        # Create the new rules
-        new_rules = []
-        for rule in to_be_created:
-            resp = instr(self.custom_ioa_api.create_rule)(body=rule.dump_create(comment=comment))
-            raw_rule = resp["body"]["resources"][0]
-            new_rule = CustomIoaRule.from_data_dict(
-                raw_rule,
-                rule_type=self._get_rule_types_cached()[raw_rule["ruletype_id"]],
-            )
-            new_rule.rulegroup_id = group.id_
-            new_rules.append(new_rule)
-
         # Update the existing rules, if there are any
         if len(existing_rules) > 0:
             response = instr(self.custom_ioa_api.update_rules)(
                 body={
                     "comment": comment,
                     "rule_updates": [rule.dump_update() for rule in existing_rules],
-                    "rulegroup_version": group.version + 1,
+                    "rulegroup_version": group.version,
                     "rulegroup_id": group.id_,
                 }
             )
@@ -204,9 +193,28 @@ def _create_update_delete_rules(self, group: IoaRuleGroup, comment: str) -> IoaR
             rule_types = self._get_rule_types_cached()
             new_group = IoaRuleGroup.from_data_dict(data_dict=raw_group, rule_type_map=rule_types)
         else:
-            group.rules = new_rules
             new_group = group
 
+        # Create the new rules
+        new_rules = []
+        for rule in to_be_created:
+            resp = instr(self.custom_ioa_api.create_rule)(body=rule.dump_create(comment=comment))
+            raw_rule = resp["body"]["resources"][0]
+            new_rule = CustomIoaRule.from_data_dict(
+                raw_rule,
+                rule_type=self._get_rule_types_cached()[raw_rule["ruletype_id"]],
+            )
+            new_rule.rulegroup_id = group.id_
+            new_rules.append(new_rule)
+            new_group.version += 1
+
+        new_group.rules = list(
+            chain(
+                (rule for rule in group.rules if rule.exists_in_cloud()),
+                (new_rule for rule in new_rules),
+            )
+        )
+
         # Delete rules queued for deletion, if any
         if len(group.rules_to_delete) > 0:
             ids_to_delete = [rule.instance_id for rule in group.rules_to_delete]
@@ -215,6 +223,8 @@ def _create_update_delete_rules(self, group: IoaRuleGroup, comment: str) -> IoaR
             )
             # If successful (i.e. no exceptions raised), clear the deletion queue
             group.rules_to_delete = []
+            new_group.version += 1
+
         return new_group
 
     @filter_string

caracara/modules/custom_ioa/rules.py

@@ -262,7 +262,7 @@ def dump_rules_update(self, comment: str) -> dict:
         return {
             "comment": comment,
             "rule_updates": [rule.dump_update(group=self) for rule in self.rules],
-            "rulegroup_version": self.version + 1,
+            "rulegroup_version": self.version,
             "rulegroup_id": self.id_,
         }
 

tests/unit_tests/test_custom_ioas.py

@@ -575,14 +575,15 @@ def mock_update_rule_group(body):
         raw_group["description"] = body["description"]
         raw_group["enabled"] = body["enabled"]
         raw_group["comment"] = body["comment"]
+        raw_group["version"] += 1
         return {"body": {"resources": [raw_group]}}
 
     custom_ioa_api.update_rule_group.side_effect = mock_update_rule_group
 
     def mock_update_rules(body):
         assert body["rulegroup_id"] == raw_group["id"]
-        assert body["rulegroup_version"] == raw_group["version"] + 1
-        raw_group["version"] = body["rulegroup_version"]
+        assert body["rulegroup_version"] == raw_group["version"]
+        raw_group["version"] += 1
         for raw_rule_update in body["rule_updates"]:
             matching_rules = [
                 i
@@ -604,6 +605,7 @@ def mock_update_rules(body):
 
     def mock_create_rule(body):
         assert raw_group["id"] == body["rulegroup_id"]
+        raw_group["version"] += 1
         new_rule = {
             "customer_id": "test_customer",
             "instance_id": "test_rule_03",
@@ -632,6 +634,15 @@ def mock_create_rule(body):
         raw_group["rules"].append(new_rule)
         return {"body": {"resources": [new_rule]}}
 
+    def mock_delete_rule(rule_group_id, ids, comment):
+        assert raw_group["id"] == rule_group_id
+        assert ids
+        assert comment
+        raw_group["version"] += 1
+        return {"body": {}}
+
+    custom_ioa_api.delete_rules.side_effect = mock_delete_rule
+
     custom_ioa_api.create_rule.side_effect = mock_create_rule
 
     custom_ioa_api.query_rule_types.side_effect = create_mock_query_resources(
@@ -647,8 +658,8 @@ def mock_create_rule(body):
     # Assert falconpy called correctly
     # This consists of
     # - A rule group update
-    # - A rule deletion
     # - A rule update
+    # - A rule deletion
     # - A rule creation
     custom_ioa_api.update_rule_group.assert_called_once_with(
         body={
@@ -660,11 +671,6 @@ def mock_create_rule(body):
             "comment": "test update comment",
         }
     )
-    custom_ioa_api.delete_rules.assert_called_once_with(
-        rule_group_id="test_group_01",
-        ids=["test_rule_01"],
-        comment="test update comment",
-    )
     custom_ioa_api.update_rules.assert_called_once_with(
         body={
             "rulegroup_id": "test_group_01",
@@ -695,5 +701,10 @@ def mock_create_rule(body):
             "comment": "test update comment",
         }
     )
+    custom_ioa_api.delete_rules.assert_called_once_with(
+        rule_group_id="test_group_01",
+        ids=["test_rule_01"],
+        comment="test update comment",
+    )
     # Assert new group is as expected
-    assert new_group.version == group.version + 1
+    assert new_group.version == group.version + 4