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

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

Closed
wants to merge 2 commits into from
Closed

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

wants to merge 2 commits into from

Conversation

jwbensley
Copy link

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
Copy link
Author

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

@mxsasha
Copy link
Collaborator

mxsasha commented Mar 11, 2024

@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
Copy link
Author

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:        [email protected] 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
Copy link
Author

@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
Copy link
Collaborator

mxsasha commented Mar 13, 2024

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
Copy link
Author

@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
Copy link
Collaborator

mxsasha commented Mar 22, 2024

@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.

@jwbensley jwbensley closed this by deleting the head repository Mar 27, 2024
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