Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ASPA support #132

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Implement ASPA support #132

merged 2 commits into from
Feb 3, 2025

Conversation

devsnek
Copy link
Contributor

@devsnek devsnek commented Jan 15, 2025

The new version of rpki-rs sets rtr max protocol version to 2, so we can do aspas here now. This provides aspa data in rtr and json. The rtr is very straightforward as it's all handled in rpki-rs. The json is complex due to the streaming model, but the state machine is maintained. Some tests are also included since it's difficult to reason about.

@partim
Copy link
Member

partim commented Jan 20, 2025

Thank you for the PR!

This is getting very unwieldy and confusing. I wonder if it might be worth redesigning the whole thing – possibly also move it to rpki-rs so the code can be shared between RTRTR and Routinator.

@devsnek
Copy link
Contributor Author

devsnek commented Jan 20, 2025

@partim you mean the json serialization part? I could leave those changes out of this PR and then someone could come by with a better impl that fits routinator and rtrtr.

@partim
Copy link
Member

partim commented Jan 23, 2025

I think we can probably patch up the existing state machine for now – especially since you’ve kindly done all the hard work already – and then look into improving the code later.

@devsnek
Copy link
Contributor Author

devsnek commented Jan 23, 2025

error: package rpki v0.18.5 cannot be built because it requires rustc 1.73 or newer, while the currently active rustc version is 1.71.1

Should the CI be updated to use a later version?

@partim
Copy link
Member

partim commented Jan 23, 2025

Yes, please – both the CI and Cargo.toml.

I updated Routinator to 1.74 because of clap, so perhaps you can go that far here as well while we are at it.

@devsnek
Copy link
Contributor Author

devsnek commented Feb 2, 2025

@partim is this ok to be merged now?

@partim
Copy link
Member

partim commented Feb 3, 2025

Apologies for the delay! I had it on my list but then got caught up in other stuff.

Looks great now. Thank you again for the PR!

@partim partim merged commit c5756ff into NLnetLabs:main Feb 3, 2025
9 checks passed
@devsnek devsnek deleted the aspa branch February 3, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants