mxsasha / nrtmv4

Ideas and work on the NRTM v4 protocol and implementations.
5 stars 7 forks source link

Consider deletion based on RPSL PK #22

Closed mxsasha closed 1 year ago

mxsasha commented 2 years ago

v00 of the draft has the existing_object key for action=delete in deltas. In our last meeting we said to change that to "object", but maybe we don't actually need the object. In IRRD we already do a relaxed object parsing for these delete objects, as we only need the RPSL class, primary key, and the source, to process the deletion (or reject it). We don't look at other fields and don't care if they are present or valid. IRRD actually needs this flexibility because some servers will send very incomplete objects for deletions.

So instead of:

{
    ...,
    "changes": [
        {
            "action": "delete",
            "object": "as-set: AS-DEMO\nmntner: foo\nchanged: blablabla\nsource: EXAMPLE"
        }
    ]
}

We could do:

{
    ...,
    "changes": [
        {
            "action": "delete",
            "rpsl_pk": "AS-DEMO",
            "object_class": "as-set",
            "source": "EXAMPLE"
        }
    ]
}

We might skip source there - I always check in IRRD as an extra consistency check for weird data.

One catch: we need a consistent view on what the RPSL pk of an object is. We might not have that now. For IRRD, "as-set: as-DeMo" has "AS-DEMO" as PK, and route objects with their composite keys are even a bit more complex. Perhaps this requirement makes it not worth it.

eshryane commented 1 year ago

I think this is a good idea @mxsasha as the object content doesn't change on delete, we don't need to include it, saving some space. However a "delete" element now has a different structure from an "add_modify" (but I think this is OK).

I think the "source" element is redundant.

Can I suggest "primary_key" as the element name instead of "rpsl_pk" (if "rpsl" is redundant and "pk" is an abbreviation which may not be very clear).

mxsasha commented 1 year ago

I think this is a good idea @mxsasha as the object content doesn't change on delete, we don't need to include it, saving some space. However a "delete" element now has a different structure from an "add_modify" (but I think this is OK).

The only catch is defining what the key is unambiguously. We can refer to:

  1. For objects that are listed in RFC2622 and RFC4012: which field is defined as "class key".
  2. For object classes that define a pair of attributes as class key (route, route6), append them all together - both IRRDv4 and RIPE db already do this, i.e. route 192.0.2.0/24 origin AS65530 has 192.0.2.0/24AS65530 as key.
  3. For any other objects (e.g. organisation), take the attribute with the same name as the object class name as key.

Alternatively, we could define the key not as a string but a more complex structure to support pairs. But to me that seems to dive too deep into RPSL content.

On deeper thought, perhaps this whole concept dives too deep into RPSL content: the important part is that a deletion can be linked to an object that was added in the past. If we include full object text in deletes, NRTMv4 doesn't have to mention class keys at all and stays a bit further from RPSL details.

I think the "source" element is redundant.

Fine with me to drop it.

Can I suggest "primary_key" as the element name instead of "rpsl_pk" (if "rpsl" is redundant and "pk" is an abbreviation which may not be very clear).

Yes, that is better.

eshryane commented 1 year ago

Hi @mxsasha I am happy with your suggested approach (1...3) and not any alternative. We need the combination of the primary_key and object_class to ensure we are referencing a single object (i.e. different object_class can have an identical primary_key).