Skip to content

Commit

Permalink
Merge pull request #241 from irrdnet/weak-refs-240
Browse files Browse the repository at this point in the history
  • Loading branch information
mxsasha committed Jun 29, 2019
2 parents 13717a3 + 1a10ef5 commit 4a949b7
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 37 deletions.
26 changes: 26 additions & 0 deletions 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
1 change: 1 addition & 0 deletions docs/releases/index.rst
Expand Up @@ -6,3 +6,4 @@ Release notes

4.0.1
4.0.2
4.0.3
24 changes: 14 additions & 10 deletions docs/users/database-changes.rst
Expand Up @@ -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
Expand Down Expand Up @@ -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] []

Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion irrd/rpsl/fields.py
Expand Up @@ -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)
Expand Down
15 changes: 9 additions & 6 deletions irrd/rpsl/parser.py
Expand Up @@ -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):
Expand Down Expand Up @@ -108,27 +108,29 @@ 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')
- RPSL object class names of objects referred (e.g. ['role', 'person']
- 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
Expand All @@ -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

Expand Down Expand Up @@ -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'
Expand Down
20 changes: 10 additions & 10 deletions irrd/rpsl/rpsl_objects.py
Expand Up @@ -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)),
Expand All @@ -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)),
Expand Down Expand Up @@ -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'])),
Expand Down Expand Up @@ -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)),
Expand All @@ -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'])),
Expand All @@ -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)),
Expand All @@ -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)),
Expand Down
18 changes: 16 additions & 2 deletions irrd/rpsl/tests/test_rpsl_objects.py
Expand Up @@ -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']
Expand All @@ -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')

Expand Down Expand Up @@ -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')

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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()
Expand All @@ -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:
Expand All @@ -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')

Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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
6 changes: 3 additions & 3 deletions irrd/updates/validators.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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'],
Expand Down

0 comments on commit 4a949b7

Please sign in to comment.