From e97798a7fadeda48367306c1a78da030437a577d Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Sat, 29 Jun 2019 17:54:35 +0200 Subject: [PATCH 1/2] Fix #240 - Add support for weak references for some object refs. This adds weak reference support, which means a reference between RPSL objects is checked for syntax, but not whether the object exists. This commit enables weak refs for all existing strong references of members, member-of, mp-members and mbrs-by-ref. --- docs/users/database-changes.rst | 24 ++++++++++++++---------- irrd/rpsl/fields.py | 7 ++++++- irrd/rpsl/parser.py | 15 +++++++++------ irrd/rpsl/rpsl_objects.py | 20 ++++++++++---------- irrd/rpsl/tests/test_rpsl_objects.py | 18 ++++++++++++++++-- irrd/updates/validators.py | 6 +++--- irrd/utils/rpsl_samples.py | 10 +++++----- 7 files changed, 63 insertions(+), 37 deletions(-) diff --git a/docs/users/database-changes.rst b/docs/users/database-changes.rst index 8ed7cc8b7..b91f6c28f 100644 --- a/docs/users/database-changes.rst +++ b/docs/users/database-changes.rst @@ -117,7 +117,9 @@ Referential integrity IRRd enforces referential integrity between objects. This means it is not permitted to delete an object that is still referenced by other objects. When an object is created or updated, all references to other -objects, such as a `mntner`, must be valid. +objects, such as a `mntner`, must be valid. This only applies to strong +references, as indicated in the object template. For weak references, +only the syntax is validated. When creating or deleting multiple objects, these are considered together, which means that an attempt to delete A and B in one submission, while B depends @@ -159,19 +161,19 @@ retrieved with ``-t route``, looks like this:: descr: [optional] [multiple] [] origin: [mandatory] [single] [primary key] holes: [optional] [multiple] [] - member-of: [optional] [multiple] [look-up key, references route-set] + member-of: [optional] [multiple] [look-up key, weak references route-set] inject: [optional] [multiple] [] aggr-bndry: [optional] [single] [] aggr-mtd: [optional] [single] [] export-comps: [optional] [single] [] components: [optional] [single] [] - admin-c: [optional] [multiple] [look-up key, references role/person] - tech-c: [optional] [multiple] [look-up key, references role/person] + admin-c: [optional] [multiple] [look-up key, strong references role/person] + tech-c: [optional] [multiple] [look-up key, strong references role/person] geoidx: [optional] [multiple] [] roa-uri: [optional] [single] [] remarks: [optional] [multiple] [] notify: [optional] [multiple] [] - mnt-by: [mandatory] [multiple] [look-up key, references mntner] + mnt-by: [mandatory] [multiple] [look-up key, strong references mntner] changed: [mandatory] [multiple] [] source: [mandatory] [single] [] @@ -183,12 +185,14 @@ This template shows: attempt to update the current object. * The `member-of` attribute is a look-up key, meaning it can be used with ``-i`` queries. -* The `member-of` attribute references to the `route-set` class. Its value - must be a reference to a valid, existing `route-set`. This `route-set` - can be created as part of the same submission. The attribute is also - optional, so it can be left out entirely. +* The `member-of` attribute references to the `route-set` class. It is a + weak references, meaning the referred `route-set` does not have to exist, + but is required to meet the syntax of a `route-set` name. The attribute + is also optional, so it can be left out entirely. * The `admin-c` and `tech-c` attributes reference a `role` or `person`. - This means they may refer to either object class. + This means they may refer to either object class, but must be a + reference to a valid, existing `person` or `role. This `person` or + `role` can be created as part of the same submission. Notifications diff --git a/irrd/rpsl/fields.py b/irrd/rpsl/fields.py index 142e6032e..aa422c64e 100644 --- a/irrd/rpsl/fields.py +++ b/irrd/rpsl/fields.py @@ -396,12 +396,17 @@ class RPSLReferenceField(RPSLTextField): The list of referred objects can contain multiple entries, which means that the value must refer to one of these objects (e.g. tech-c can refer to role or person). + + If the references are strong, this reference is included in reference checks + on updates, i.e. adding an object with a strong reference to another object + that does not exist, is a validation failure. """ keep_case = False - def __init__(self, referring: List[str], *args, **kwargs) -> None: + def __init__(self, referring: List[str], strong=True, *args, **kwargs) -> None: from .parser import RPSLObject self.referring = referring + self.strong = strong self.referring_object_classes: List[Type[RPSLObject]] = [] self.referring_identifier_fields: List[RPSLTextField] = [] super().__init__(*args, **kwargs) diff --git a/irrd/rpsl/parser.py b/irrd/rpsl/parser.py index 040d39031..e43a42231 100644 --- a/irrd/rpsl/parser.py +++ b/irrd/rpsl/parser.py @@ -31,7 +31,7 @@ def __init__(cls, name, bases, clsdict): # noqa: N805 cls.attrs_allowed = [field[0] for field in fields.items()] cls.attrs_required = [field[0] for field in fields.items() if not field[1].optional] cls.attrs_multiple = [field[0] for field in fields.items() if field[1].multiple] - cls.referring_fields = [(field[0], field[1].referring) for field in fields.items() if hasattr(field[1], 'referring')] + cls.referring_strong_fields = [(field[0], field[1].referring) for field in fields.items() if hasattr(field[1], 'referring') and getattr(field[1], 'strong')] class RPSLObject(metaclass=RPSLObjectMeta): @@ -108,12 +108,14 @@ def ip_version(self) -> Optional[int]: return self.ip_first.version() return None - def referred_objects(self) -> List[Tuple[str, List, List]]: + def referred_strong_objects(self) -> List[Tuple[str, List, List]]: """ Get all objects that this object refers to (e.g. an admin-c attribute on this object, that refers to person/role) along with the data this object has for that reference. This information can be used to check whether all references from an object are valid. + Only references which have strong=True are returned, weak references + are not returned as they should not be included in reference validation. Returns a list of tuples, which each tuple having: - field name on this object (e.g. 'admin-c') @@ -121,14 +123,14 @@ def referred_objects(self) -> List[Tuple[str, List, List]]: - Values this object has for that field (e.g. ['A-RIPE', 'B-RIPE'] """ result = [] - for field_name, referred_objects in self.referring_fields: # type: ignore + for field_name, referred_objects in self.referring_strong_fields: # type: ignore data = self.parsed_data.get(field_name) if not data: continue result.append((field_name, referred_objects, data)) return result - def references_inbound(self) -> Set[str]: + def references_strong_inbound(self) -> Set[str]: """ Get a set of field names under which other objects refer to this object. E.g. for a person object, this would typically @@ -138,7 +140,7 @@ def references_inbound(self) -> Set[str]: from irrd.rpsl.rpsl_objects import OBJECT_CLASS_MAPPING for rpsl_object in OBJECT_CLASS_MAPPING.values(): for field_name, field in rpsl_object.fields.items(): - if self.rpsl_object_class in getattr(field, 'referring', []): + if self.rpsl_object_class in getattr(field, 'referring', []) and getattr(field, 'strong'): result.add(field_name) return result @@ -173,7 +175,8 @@ def generate_template(self): elif field.lookup_key: metadata.append('look-up key') if getattr(field, 'referring', []): - metadata.append('references ' + '/'.join(field.referring)) + reference_type = 'strong' if getattr(field, 'strong') else 'weak' + metadata.append(f'{reference_type} references ' + '/'.join(field.referring)) metadata_str = ', '.join(metadata) name_padding = (max_name_width - len(name)) * ' ' template += f'{name}: {name_padding} {mandatory} {single} [{metadata_str}]\n' diff --git a/irrd/rpsl/rpsl_objects.py b/irrd/rpsl/rpsl_objects.py index fe4451a16..39f279ab5 100644 --- a/irrd/rpsl/rpsl_objects.py +++ b/irrd/rpsl/rpsl_objects.py @@ -40,8 +40,8 @@ class RPSLAsSet(RPSLObject): fields = OrderedDict([ ('as-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='AS')), ('descr', RPSLTextField(multiple=True, optional=True)), - ('members', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['aut-num', 'as-set'])), - ('mbrs-by-ref', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'], allow_kw_any=True)), + ('members', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['aut-num', 'as-set'], strong=False)), + ('mbrs-by-ref', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'], allow_kw_any=True, strong=False)), ('admin-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), ('tech-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), ('remarks', RPSLTextField(optional=True, multiple=True)), @@ -57,7 +57,7 @@ class RPSLAutNum(RPSLObject): ('aut-num', RPSLASNumberField(primary_key=True, lookup_key=True)), ('as-name', RPSLGenericNameField(allowed_prefixes=['AS'])), ('descr', RPSLTextField(multiple=True, optional=True)), - ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['as-set'])), + ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['as-set'], strong=False)), ('import', RPSLTextField(optional=True, multiple=True)), ('mp-import', RPSLTextField(optional=True, multiple=True)), ('import-via', RPSLTextField(optional=True, multiple=True)), @@ -121,7 +121,7 @@ class RPSLInetRtr(RPSLObject): ('interface', RPSLTextField(optional=True, multiple=True)), ('peer', RPSLTextField(optional=True, multiple=True)), ('mp-peer', RPSLTextField(optional=True, multiple=True)), - ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['rtr-set'])), + ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['rtr-set'], strong=False)), ('rs-in', RPSLTextField(optional=True)), ('rs-out', RPSLTextField(optional=True)), ('admin-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), @@ -388,7 +388,7 @@ class RPSLRoute(RPSLObject): ('descr', RPSLTextField(multiple=True, optional=True)), ('origin', RPSLASNumberField(primary_key=True)), ('holes', RPSLIPv4PrefixesField(optional=True, multiple=True)), - ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['route-set'])), + ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['route-set'], strong=False)), ('inject', RPSLTextField(optional=True, multiple=True)), ('aggr-bndry', RPSLTextField(optional=True)), ('aggr-mtd', RPSLTextField(optional=True)), @@ -411,7 +411,7 @@ class RPSLRouteSet(RPSLObject): ('route-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='RS')), ('members', RPSLRouteSetMembersField(ip_version=4, lookup_key=True, optional=True, multiple=True)), ('mp-members', RPSLRouteSetMembersField(ip_version=None, lookup_key=True, optional=True, multiple=True)), - ('mbrs-by-ref', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'], allow_kw_any=True)), + ('mbrs-by-ref', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'], allow_kw_any=True, strong=False)), ('descr', RPSLTextField(multiple=True, optional=True)), ('admin-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), ('tech-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), @@ -429,7 +429,7 @@ class RPSLRoute6(RPSLObject): ('descr', RPSLTextField(multiple=True, optional=True)), ('origin', RPSLASNumberField(primary_key=True)), ('holes', RPSLIPv6PrefixesField(optional=True, multiple=True)), - ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['route-set'])), + ('member-of', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['route-set'], strong=False)), ('inject', RPSLTextField(optional=True, multiple=True)), ('aggr-bndry', RPSLTextField(optional=True)), ('aggr-mtd', RPSLTextField(optional=True)), @@ -451,9 +451,9 @@ class RPSLRtrSet(RPSLObject): fields = OrderedDict([ ('rtr-set', RPSLSetNameField(primary_key=True, lookup_key=True, prefix='RTRS')), ('descr', RPSLTextField(multiple=True, optional=True)), - ('members', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['inet-rtr', 'rtr-set'])), - ('mp-members', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['inet-rtr', 'rtr-set'])), - ('mbrs-by-ref', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'], allow_kw_any=True)), + ('members', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['inet-rtr', 'rtr-set'], strong=False)), + ('mp-members', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['inet-rtr', 'rtr-set'], strong=False)), + ('mbrs-by-ref', RPSLReferenceListField(lookup_key=True, optional=True, multiple=True, referring=['mntner'], allow_kw_any=True, strong=False)), ('admin-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), ('tech-c', RPSLReferenceField(lookup_key=True, optional=True, multiple=True, referring=['role', 'person'])), ('remarks', RPSLTextField(optional=True, multiple=True)), diff --git a/irrd/rpsl/tests/test_rpsl_objects.py b/irrd/rpsl/tests/test_rpsl_objects.py index 4969c9962..600cc4807 100644 --- a/irrd/rpsl/tests/test_rpsl_objects.py +++ b/irrd/rpsl/tests/test_rpsl_objects.py @@ -122,12 +122,12 @@ def test_parse(self): assert obj.__class__ == RPSLAsSet assert not obj.messages.errors() assert obj.pk() == 'AS-SETTEST' - assert obj.referred_objects() == [ - ('members', ['aut-num', 'as-set'], ['AS65538', 'AS65539', 'AS65537']), + assert obj.referred_strong_objects() == [ ('admin-c', ['role', 'person'], ['PERSON-TEST']), ('tech-c', ['role', 'person'], ['PERSON-TEST']), ('mnt-by', ['mntner'], ['TEST-MNT']) ] + assert obj.references_strong_inbound() == set() assert obj.source() == 'TEST' assert obj.parsed_data['members'] == ['AS65538', 'AS65539', 'AS65537'] @@ -149,6 +149,7 @@ def test_parse(self): assert obj.asn_first == 65537 assert obj.asn_last == 65537 assert obj.ip_version() is None + assert obj.references_strong_inbound() == set() # Field parsing will cause our object to look slightly different than the original, hence the replace() assert obj.render_rpsl_text() == rpsl_text.replace('as065537', 'AS65537') @@ -228,6 +229,7 @@ def test_parse(self): assert obj.ip_first == IP('192.0.2.0') assert obj.ip_last == IP('192.0.2.255') assert obj.ip_version() == 4 + assert obj.references_strong_inbound() == set() # Field parsing will cause our object to look slightly different than the original, hence the replace() assert obj.render_rpsl_text() == rpsl_text.replace('192.0.02.255', '192.0.2.255') @@ -308,6 +310,7 @@ def test_parse(self): assert obj.pk() == 'TEST-MNT' assert obj.parsed_data['mnt-by'] == ['TEST-MNT', 'OTHER1-MNT', 'OTHER2-MNT'] assert obj.render_rpsl_text() == rpsl_text + assert obj.references_strong_inbound() == {'mnt-by'} def test_parse_invalid_partial_dummy_hash(self): rpsl_text = object_sample_mapping[RPSLMntner().rpsl_object_class] @@ -360,6 +363,7 @@ def test_parse(self): assert obj.pk() == 'PERSON-TEST' assert obj.parsed_data['nic-hdl'] == 'PERSON-TEST' assert obj.render_rpsl_text() == rpsl_text + assert obj.references_strong_inbound() == {'admin-c', 'tech-c', 'zone-c'} def test_generate_template(self): template = RPSLPerson().generate_template() @@ -378,6 +382,7 @@ def test_parse(self): assert not obj.messages.errors() assert obj.pk() == 'ROLE-TEST' assert obj.render_rpsl_text() == rpsl_text + assert obj.references_strong_inbound() == {'admin-c', 'tech-c', 'zone-c'} class TestRPSLRoute: @@ -396,6 +401,7 @@ def test_parse(self): assert obj.asn_first == 65537 assert obj.asn_last == 65537 assert obj.ip_version() == 4 + assert obj.references_strong_inbound() == set() # Field parsing will cause our object to look slightly different than the original, hence the replace() assert obj.render_rpsl_text() == rpsl_text.replace(' 192.0.02.0/24', ' 192.0.2.0/24') @@ -429,6 +435,7 @@ def test_parse(self): assert obj.pk() == 'RS-TEST' assert obj.parsed_data['mp-members'] == ['2001:db8::/48'] assert obj.render_rpsl_text() == rpsl_text.replace('2001:0dB8::/48', '2001:db8::/48') + assert obj.references_strong_inbound() == set() class TestRPSLRoute6: @@ -449,6 +456,7 @@ def test_parse(self): assert obj.ip_version() == 6 assert obj.parsed_data['mnt-by'] == ['TEST-MNT'] assert obj.render_rpsl_text() == rpsl_text + assert obj.references_strong_inbound() == set() class TestRPSLRtrSet: @@ -463,4 +471,10 @@ def test_parse(self): assert not obj.messages.errors() assert obj.pk() == 'RTRS-SETTEST' assert obj.parsed_data['rtr-set'] == 'RTRS-SETTEST' + assert obj.referred_strong_objects() == [ + ('admin-c', ['role', 'person'], ['PERSON-TEST']), + ('tech-c', ['role', 'person'], ['PERSON-TEST']), + ('mnt-by', ['mntner'], ['TEST-MNT']) + ] + assert obj.references_strong_inbound() == set() assert obj.render_rpsl_text() == rpsl_text diff --git a/irrd/updates/validators.py b/irrd/updates/validators.py index 457d88bba..50d6812bf 100644 --- a/irrd/updates/validators.py +++ b/irrd/updates/validators.py @@ -65,7 +65,7 @@ def check_references_to_others(self, rpsl_obj: RPSLObject) -> ValidatorResult: all references to other objects actually exist in the database. """ result = ValidatorResult() - references = rpsl_obj.referred_objects() + references = rpsl_obj.referred_strong_objects() source = rpsl_obj.source() for field_name, objects_referred, object_pks in references: @@ -113,11 +113,11 @@ def check_references_from_others(self, rpsl_obj: RPSLObject) -> ValidatorResult: that is also about to be deleted, is acceptable. """ result = ValidatorResult() - if not rpsl_obj.references_inbound(): + if not rpsl_obj.references_strong_inbound(): return result query = RPSLDatabaseQuery().sources([rpsl_obj.source()]) - query = query.lookup_attrs_in(rpsl_obj.references_inbound(), [rpsl_obj.pk()]) + query = query.lookup_attrs_in(rpsl_obj.references_strong_inbound(), [rpsl_obj.pk()]) query_results = self.database_handler.execute_query(query) for query_result in query_results: reference_to_be_deleted = (query_result['object_class'], query_result['rpsl_pk'], diff --git a/irrd/utils/rpsl_samples.py b/irrd/utils/rpsl_samples.py index a3637d518..68c98848f 100644 --- a/irrd/utils/rpsl_samples.py +++ b/irrd/utils/rpsl_samples.py @@ -810,19 +810,19 @@ descr: [optional] [multiple] [] origin: [mandatory] [single] [primary key] holes: [optional] [multiple] [] -member-of: [optional] [multiple] [look-up key, references route-set] +member-of: [optional] [multiple] [look-up key, weak references route-set] inject: [optional] [multiple] [] aggr-bndry: [optional] [single] [] aggr-mtd: [optional] [single] [] export-comps: [optional] [single] [] components: [optional] [single] [] -admin-c: [optional] [multiple] [look-up key, references role/person] -tech-c: [optional] [multiple] [look-up key, references role/person] +admin-c: [optional] [multiple] [look-up key, strong references role/person] +tech-c: [optional] [multiple] [look-up key, strong references role/person] geoidx: [optional] [multiple] [] roa-uri: [optional] [single] [] remarks: [optional] [multiple] [] notify: [optional] [multiple] [] -mnt-by: [mandatory] [multiple] [look-up key, references mntner] +mnt-by: [mandatory] [multiple] [look-up key, strong references mntner] changed: [mandatory] [multiple] [] source: [mandatory] [single] [] """ @@ -835,7 +835,7 @@ nic-hdl: [mandatory] [single] [primary/look-up key] remarks: [optional] [multiple] [] notify: [optional] [multiple] [] -mnt-by: [mandatory] [multiple] [look-up key, references mntner] +mnt-by: [mandatory] [multiple] [look-up key, strong references mntner] changed: [mandatory] [multiple] [] source: [mandatory] [single] [] """ From 1a10ef57ff6cdef6bd17a69ab9ba9545f2444728 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Sat, 29 Jun 2019 18:04:44 +0200 Subject: [PATCH 2/2] Add IRRd 4.0.3 release notes. --- docs/releases/4.0.3.rst | 26 ++++++++++++++++++++++++++ docs/releases/index.rst | 1 + 2 files changed, 27 insertions(+) create mode 100644 docs/releases/4.0.3.rst diff --git a/docs/releases/4.0.3.rst b/docs/releases/4.0.3.rst new file mode 100644 index 000000000..8d3e076af --- /dev/null +++ b/docs/releases/4.0.3.rst @@ -0,0 +1,26 @@ +============================ +Release notes for IRRd 4.0.3 +============================ + +IRRd 4.0.3 was released on June 29th, 2019, and contains +one change relating to referential integrity validation +between RPSL objects. + +In earlier versions of IRRd, the `members` attribute of +an `as-set`, along with several others, +`were strongly validated`_. This meant it was not possible +to add or update an `as-set`, if some of the members were +not valid objects in the same source. The same applied +to fields like `member-of` in a `route-set`. + +In IRRd 4.0.3, all references from `members`, `mp-member`, +`mbrs-by-ref` and `member-of` are weak references. Their +syntax is validated, e.g. for `as-set` members, values must +be a valid `as-set` name or a valid AS number. However, there +is no validation on whether the objects actually exist. +It is also possible to delete e.g. a `route-set`, even when +the object is still listed in the `member-of` in a `route` +object, or to reference a maintainer that does not exist +in a `mbrs-by-ref`. + +.. _were strongly validated: https://github.com/irrdnet/irrd4/pull/240 diff --git a/docs/releases/index.rst b/docs/releases/index.rst index 52f3d6d92..82a908a55 100644 --- a/docs/releases/index.rst +++ b/docs/releases/index.rst @@ -6,3 +6,4 @@ Release notes 4.0.1 4.0.2 + 4.0.3