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

fix: ASPA JSON deserialization failing on ASNs #134

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ember-ana
Copy link
Contributor

As far as I could tell, there is no consistent spec for how ASPA should be represented as JSON. Here are some examples i found in the wild:

type JsonAspaRpkiClient {
	customer_asid: number,
	providers: number[],
}

type JsonAspaRoutinator {
	customer: string, // "AS64500"
	providers: string[],
}

type JsonAspaKrill {
	customer: number,
	providers: number[]
}

#132 implements serialization to rpki-client notation, as well as serialization from rpki-client notation.

Unfortunately, this means that e.g. using RTRTR with Routinator leads to errors such as Failed parsing source: invalid type: string "AS64500", expected u32 or Failed parsing source: missing field 'customer_asid'.

This PR implements support for deserializing all 3 notations, and changes serialization to krill notation, as it's the one with the most common ground between all, and therefore has the highest likeliness of a client being able to understand it.

@partim
Copy link
Member

partim commented Feb 4, 2025

Thank you for the PR – being flexible here certainly makes sense. The PR looks good – apart from whatever Clippy is complaining about.

@devsnek – the PR changes the output formatting of ASPAs you defined in #132. I think the new format makes more sense (for one, underscore names in JSON feel wrong to me). I hope this is okay for you?

@devsnek
Copy link
Contributor

devsnek commented Feb 4, 2025

this is fine, I was trying to copy other implementations but I guess I wasn't broad enough.

@ember-ana
Copy link
Contributor Author

resolved clippy issues :)

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.

3 participants