irrdnet / irrd

IRRd version 4
http://irrd.readthedocs.io/en/stable/
Other
157 stars 52 forks source link

Issue #917 - Add support for :: notation in GraphQL interface #918

Closed jwbensley closed 8 months ago

jwbensley commented 8 months ago

Adding support for "::" notation in AS-SET names so that a source database can be specified for the root AS-SET object only. This is to resolve the issue described in #917.

Please let me know what I can do to help.

jwbensley commented 8 months ago

@mxsasha I don't think there is anything I can do about that CI error because it relates to a Google API auth failure? :shrug:

mxsasha commented 8 months ago

@mxsasha I don't think there is anything I can do about that CI error because it relates to a Google API auth failure? 🤷

Yes indeed, this is not you. I will look into that and your feature some time this week :)

jwbensley commented 8 months ago

Here are some test results from my pull request...

I am running irrd locally and have import data from RIPE and RADB:

Bildschirmfoto vom 2024-03-11 17-30-29

I have found an AS-SET which exists in both IRR DBs. Here I query for the AS-SET with no source specified:

Bildschirmfoto vom 2024-03-11 17-30-00

Here I query with only RIPE specified, we can see the same prefixes are returned:

Bildschirmfoto vom 2024-03-11 17-30-02

Next I query with only RADB specified, now we can see that some prefixes are missing:

Bildschirmfoto vom 2024-03-11 17-30-04

Taking one of the missing prefixes, we can check that it is missing because that route object came from the RIPE DB and doesn't exist in RADB (only a less specific):

$ whois -h localhost:8043 -T route6 -s RIPE 2a02:2719:4120::/44
Warnung: RIPE-Flags wurden mit einem »traditionellen« Server verwendet.
route6:         2a02:2719::/32
descr:          AS30873 annoucement for YemenNet/PTC
origin:         AS30873
mnt-by:         YEMEN-NET-MNT
created:        2017-09-17T17:56:41Z
last-modified:  2017-09-17T17:56:41Z
source:         RIPE
remarks:        ****************************
remarks:        * THIS OBJECT IS MODIFIED
remarks:        * Please note that all data that is generally regarded as personal
remarks:        * data has been removed from this object.
remarks:        * To view the original object, please query the RIPE Database at:
remarks:        * http://www.ripe.net/whois
remarks:        ****************************

route6:         2a02:2719:4120::/44
origin:         AS30873
mnt-by:         YEMEN-NET-MNT
created:        2020-12-11T17:33:57Z
last-modified:  2020-12-11T17:33:57Z
source:         RIPE
remarks:        ****************************
remarks:        * THIS OBJECT IS MODIFIED
remarks:        * Please note that all data that is generally regarded as personal
remarks:        * data has been removed from this object.
remarks:        * To view the original object, please query the RIPE Database at:
remarks:        * http://www.ripe.net/whois
remarks:        ****************************

$ whois -h localhost:8043 -T route6 -s RADB 2a02:2719:4120::/44
Warnung: RIPE-Flags wurden mit einem »traditionellen« Server verwendet.
route6:         2a02:2719::/32
descr:          Yemen Telecommunication ISP (YemenNet)
origin:         AS30873
admin-c:        YTNT1-RIPE
tech-c:         YTNT1-RIPE
mnt-by:         MAINT-AS30873
changed:        yasser@yemen.net.ye 20170918  #16:23:36Z
source:         RADB
last-modified:  2023-11-13T15:53:46Z

Finally if I query and set the source for the root object only (the AS-SET) it is pulled from RADB but all prefixes are returned because the recursive resolution uses all specified sources (which in this case was none, meaning all sources):

Bildschirmfoto vom 2024-03-11 17-30-06

jwbensley commented 8 months ago

@mxsasha I don't think there is anything I can do about that CI error because it relates to a Google API auth failure? 🤷

Yes indeed, this is not you. I will look into that and your feature some time this week :)

Awesome, thanks!

mxsasha commented 8 months ago

I had been reading your ideas on RIPE routing-wg too, I agree this is a problem with resolving route/as-sets. I am wondering if this is the best design. One of the ideas of structured queries and data in GraphQL was to get rid of magic strings, separators and have clear structured queries instead of !r192.0.2.0/24,L. So I don't entirely like introducing a special meaning for a :: in a GraphQL query.

The recursiveSetMembers already returns each root object, with a different rootSource. So you can already pick which one you want, though there is some overhead in returning all of them. For asSetPrefixes, there is no such option. Perhaps the cleanest option is to define a new input type for RPSL pk + source, and have the parameter to asSetPrefixes (and maybe recursiveSetMembers) as a union type of that and String. That has the feature you want, backwards compatibility, and fits with GraphQL style. I have never tried that, but can play with it.

(doc build fixed btw)

jwbensley commented 8 months ago

@mxsasha Firstly, sorry or the delay in my response, that was not intentional, just been very busy with the day job.

Secondly, thanks for reviewing the PR and providing this feedback. What you have said totally makes sense. I will see if I can raise a new PR in the next few weeks (giving a wide window there because Easter is coming up!).

Just on a side note, do you have any interest in a PR to add a Dockerfile + docker-compose file? I spent way more time getting IRRd running than actually writing code. Any testing requires IRRd + Redis + Postgress + some IRR data. I ended up creating a Dockerfile for IRRd and docker-compose file which runs IRRd + Redis + Postgress and imports some IRR data from RIPE. I can raise another PR for that if you're interested.

mxsasha commented 8 months ago

@mxsasha Firstly, sorry or the delay in my response, that was not intentional, just been very busy with the day job.

Secondly, thanks for reviewing the PR and providing this feedback. What you have said totally makes sense. I will see if I can raise a new PR in the next few weeks (giving a wide window there because Easter is coming up!).

Sounds good, no rush, I am not waiting for this or anything 👍🏻

Just on a side note, do you have any interest in a PR to add a Dockerfile + docker-compose file? I spent way more time getting IRRd running than actually writing code. Any testing requires IRRd + Redis + Postgress + some IRR data. I ended up creating a Dockerfile for IRRd and docker-compose file which runs IRRd + Redis + Postgress and imports some IRR data from RIPE. I can raise another PR for that if you're interested.

Yes, I think that would be great. Could help for CI and for general test deployments. Recently did this for another project and now I'm a fan.