Fix #524 - Fix incorrect journal state when combining scope filter an…

…d RPKI (#529)

irrd/rpki/validators.py

@@ -45,6 +45,7 @@ class BulkRouteROAValidator:
     11000000000000000000001000, and therefore will (and should)
     not be included in the validation process.
     """
+
     def __init__(self, dh: DatabaseHandler, roas: Optional[List[ROA]] = None):
         """
         Create a validator object. Can use either a list of ROA objects,
@@ -85,7 +86,7 @@ def validate_all_routes(self, sources: List[str]=None) -> \
         validation result, are not included in the return value.
         """
         columns = ['pk', 'rpsl_pk', 'ip_first', 'prefix_length', 'asn_first', 'source',
-                   'rpki_status']
+                   'rpki_status', 'scopefilter_status']
         q = RPSLDatabaseQuery(column_names=columns, enable_ordering=False)
         q = q.object_classes(['route', 'route6'])
         if sources:

irrd/scopefilter/validators.py

@@ -113,7 +113,7 @@ def validate_all_rpsl_objects(self, database_handler: DatabaseHandler) -> \
         validation result, are not included in the return value.
         """
         columns = ['rpsl_pk', 'ip_first', 'prefix_length', 'asn_first', 'source', 'object_class',
-                   'object_text', 'scopefilter_status']
+                   'object_text', 'scopefilter_status', 'rpki_status']
 
         objs_changed: Dict[ScopeFilterStatus, List[Dict[str, str]]] = defaultdict(list)
 

irrd/storage/database_handler.py

@@ -275,7 +275,10 @@ def update_rpki_status(self, rpsl_objs_now_valid: List[Dict[str, str]]=[],
             self.execute_statement(stmt)
 
         for rpsl_obj in rpsl_objs_now_valid + rpsl_objs_now_not_found:
-            if rpsl_obj['old_status'] == RPKIStatus.invalid:
+            if all([
+                rpsl_obj['old_status'] == RPKIStatus.invalid,
+                rpsl_obj['scopefilter_status'] == ScopeFilterStatus.in_scope
+            ]):
                 self.status_tracker.record_operation(
                     operation=DatabaseOperation.add_or_update,
                     rpsl_pk=rpsl_obj['rpsl_pk'],
@@ -324,16 +327,17 @@ def update_scopefilter_status(self, rpsl_objs_now_in_scope: List[Dict[str, str]]
             self.execute_statement(stmt)
 
         for rpsl_obj in rpsl_objs_now_in_scope:
-            self.status_tracker.record_operation(
-                operation=DatabaseOperation.add_or_update,
-                rpsl_pk=rpsl_obj['rpsl_pk'],
-                source=rpsl_obj['source'],
-                object_class=rpsl_obj['object_class'],
-                object_text=rpsl_obj['object_text'],
-                origin=JournalEntryOrigin.scope_filter,
-                source_serial=None,
-            )
-            self._object_classes_modified.add(rpsl_obj['object_class'])
+            if rpsl_obj['rpki_status'] != RPKIStatus.invalid:
+                self.status_tracker.record_operation(
+                    operation=DatabaseOperation.add_or_update,
+                    rpsl_pk=rpsl_obj['rpsl_pk'],
+                    source=rpsl_obj['source'],
+                    object_class=rpsl_obj['object_class'],
+                    object_text=rpsl_obj['object_text'],
+                    origin=JournalEntryOrigin.scope_filter,
+                    source_serial=None,
+                )
+                self._object_classes_modified.add(rpsl_obj['object_class'])
 
         for rpsl_obj in rpsl_objs_now_out_scope_as + rpsl_objs_now_out_scope_prefix:
             if rpsl_obj['old_status'] == ScopeFilterStatus.in_scope:

irrd/storage/tests/test_database.py

@@ -83,6 +83,7 @@ class TestDatabaseHandlerLive:
     """
     This test covers mainly DatabaseHandler and DatabaseStatusTracker.
     """
+
     def test_object_writing_and_status_checking(self, monkeypatch, irrd_database, config_override):
         config_override({
             'sources': {
@@ -405,6 +406,7 @@ def test_rpki_status_storage(self, monkeypatch, irrd_database, database_handler_
             'object_class': 'route',
             'object_text': 'object-text',
             'old_status': RPKIStatus.invalid,  # This always triggers a journal entry
+            'scopefilter_status': ScopeFilterStatus.in_scope,
         }]
 
         assert len(list(dh.execute_query(RPSLDatabaseQuery().rpki_status([RPKIStatus.invalid])))) == 1
@@ -444,6 +446,16 @@ def test_rpki_status_storage(self, monkeypatch, irrd_database, database_handler_
         dh.update_rpki_status(rpsl_objs_now_valid=route_rpsl_objs)
         assert not list(dh.execute_query(RPSLDatabaseJournalQuery().serial_range(6, 6)))
 
+        # State change from invalid to valid should not create journal entry
+        # if scope filter is still out of scope
+        route_rpsl_objs[0].update({
+            'old_status': RPKIStatus.invalid,
+            'scopefilter_status': ScopeFilterStatus.out_scope_as,
+        })
+        dh.update_rpki_status(rpsl_objs_now_valid=route_rpsl_objs)
+        assert len(list(dh.execute_query(RPSLDatabaseQuery().rpki_status([RPKIStatus.valid])))) == 1
+        assert not list(dh.execute_query(RPSLDatabaseJournalQuery().serial_range(6, 6)))
+
     def test_scopefilter_status_storage(self, monkeypatch, irrd_database, database_handler_with_route):
         monkeypatch.setenv('IRRD_SOURCES_TEST_KEEP_JOURNAL', '1')
         dh = database_handler_with_route
@@ -452,7 +464,8 @@ def test_scopefilter_status_storage(self, monkeypatch, irrd_database, database_h
             'source': 'TEST',
             'object_class': 'route',
             'object_text': 'object-text',
-            'old_status': ScopeFilterStatus.in_scope,  # This always triggers a journal entry
+            'old_status': ScopeFilterStatus.in_scope,
+            'rpki_status': RPKIStatus.valid,
         }]
 
         assert len(list(dh.execute_query(RPSLDatabaseQuery().scopefilter_status([ScopeFilterStatus.in_scope])))) == 1
@@ -476,6 +489,16 @@ def test_scopefilter_status_storage(self, monkeypatch, irrd_database, database_h
         journal_entry = list(dh.execute_query(RPSLDatabaseJournalQuery().serial_range(2, 2)))
         assert journal_entry[0]['operation'] == DatabaseOperation.add_or_update
 
+        # Special case: updating the status from out to in scope while RPKI invalid,
+        # should change the status but not create a journal entry - see #524
+        route_rpsl_objs[0].update({
+            'old_status': ScopeFilterStatus.out_scope_as,
+            'rpki_status': RPKIStatus.invalid,
+        })
+        dh.update_scopefilter_status(rpsl_objs_now_in_scope=route_rpsl_objs)
+        assert len(list(dh.execute_query(RPSLDatabaseQuery().scopefilter_status([ScopeFilterStatus.in_scope])))) == 1
+        assert len(list(dh.execute_query(RPSLDatabaseJournalQuery()))) == 2  # no new entry since last test
+
     def _clean_result(self, results):
         variable_fields = ['pk', 'timestamp', 'created', 'updated', 'last_error_timestamp']
         return [{k: v for k, v in result.items() if k not in variable_fields} for result in list(results)]