NLnetLabs / krill

RPKI Certificate Authority and Publication Server written in Rust
https://nlnetlabs.nl/projects/routing/krill/
Mozilla Public License 2.0
280 stars 37 forks source link

Remove PreSaveEventListener #1182

Open timbru opened 6 months ago

timbru commented 6 months ago

We currently use PreSaveEventListener to trigger certain actions before saving an update to an aggregate (a CertAuth in particular). The issue with this is that, in principle, it's possible for this triggered event to succeed and the saving of the updated CA to fail - e.g. because of a disk full or reboot happening right at the wrong moment.

While the chances of this happening are very low, it would still be better to use the PostSaveEventListener instead. I.e. these listeners are used to trigger that follow-up actions are scheduled on the task queue.

The code currently uses PreSaveEventListener to trigger a new manifest and CRL to be generated (in CaObjects) whenever a new RPKI object (such as a ROA) is created in a CA. The other path for updating the manifest and CRL is through a background job that triggers the re-issuance of the manifest and CRL before they expire. See: Task::RepublishIfNeeded.

This could lead to a corner case where a new CRL and Manifest are created, pre-save, and then saving the CertAuth fails. So, it would be better to have an idempotent task for this instead.

It would be better to make the CaObjects code for Task::RepublishIfNeeded idempotent and re-schedule it "PostSave" when a relevant change occurs. The CaObjects code would then no longer get an event with changed RPKI objects but would simply need to query the CA for the complete current object set so that it can decide whether a content change occured since the last manifest/CRL was issued, in which case it could re-issue.

This also allows one to re-implement CaObjects and use the WalSupport trait instead of the way things are done now. Reducing the different ways of handling state changes would make the code easier to understand and more consistent.