RPKI parse tests #49

jurraca · 2025-01-18T18:30:26Z

Add initial tests for RPKI parsing: check that duplicates, incompletes, ROA type, and invalids are parsed correctly. Mostly, setting up a test Context and adjusting some setup.

Includes a RPKI fixture CSV and handles casting it to a rpki_raw.json file as expected by the parsing code.

rebased on #47

fjahr

Took a first look but will revisit once #47 is merged.

fjahr · 2025-01-19T13:36:42Z

kartograf/context.py

@@ -39,6 +37,12 @@ def __init__(self, args):
            self.epoch = self.args.epoch
            self.epoch_dir = "r" + self.args.epoch
            self.epoch_datetime = repro_epoch
+        # This is only for testing, to set an epoch in the Test context.
+        # the CLI parser prevents this argument from existing without the 'reproduce' arg
+        elif self.args.epoch:


Hm, couldn't we do this from outside of the context by mocking the epoch? Unless it's much more complex I would prefer not to add test specific code in the normal code.

yep, had to do a small refactor to make this work here cfb044d.

fjahr · 2025-01-19T13:39:19Z

tests/context.py

+    TEST_ARGS.epoch = epoch
+    context = Context(TEST_ARGS)


Adding to what I mentioned above, didn't test it though.

Suggested change

TEST_ARGS.epoch = epoch

context = Context(TEST_ARGS)

context = Context(TEST_ARGS)

context.epoch = epoch

context.epoch_dir = epoch

context.epoch_datetime = datetime.utcfromtimestamp(int(epoch))

kartograf/context.py

fjahr · 2025-01-19T18:31:44Z

Needs rebase now that #47 is merged

jurraca · 2025-01-21T17:26:25Z

I was intending to introduce just RPKI test fixtures here, but now each commit has some fairly independent changes in it:

changing f-strings to os.path.join should be harmless, just prevents us from having to manually set / path delimiters when creating paths.
cfb044d includes the RPKI fixtures and tests that check the parsing for those fixtures. In order to do so, it introduces a test Context in tests/context.py. It also modifies the epoch handling in the parsing code, in order to be able to mock it properly in this test context. (let me know if it makes sense to split this up).
b2a23ff fixes the path in the test context to properly create test data in the tmp_path.

fjahr

Looks pretty good, leaving a few comments that are mostly non-essential but the util file should be moved to the folder and renamed.

Don't forget to take it out of draft when you think it could be ready to merge.

fjahr · 2025-01-26T17:44:27Z

tests/util.py

@@ -0,0 +1,9 @@
+'''
+Utilities for testing purposes only.


we now have a tests/util folder, this file location is probably influenced by an older state of the codebase.

fjahr · 2025-01-26T17:52:05Z

kartograf/context.py

+        If doing a reproduction run, we will prepend the directory name with a "r"
+        to separate it from the original run directory.
+        '''
+        if self.reproduce and self.args.epoch:


nit: sometimes using self.epoch and sometimes self.args.epoch in this function

fjahr · 2025-01-26T17:58:40Z

tests/data/rpki_raw.csv

+rsync://rpki.arin.net/repository/arin-rpki-ta/5e4a23ea-e80a-403e-b08c-2171da2157d3/f60c9f32-a87c-4339-a2f3-6299a3b02e29/1146938c-c605-4779-bf60-820a16fa701c/8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b.cer,86:A9:90:76:61:0E:7C:08:AE:DD:CE:87:67:AA:5D:C4:52:8C:49:08,/CN=8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b,775EA3FF97551B8DAB1870EE20B90C6D38AFE3F3,unable to get local issuer certificate,/home/kartograf/data/1730210400/rpki/cache/rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/32332e3135342e38312e302f32342d3234203d3e203533333536.roa,7pPxkgwX/p8qZcZ4U4B6r43qRmLaDzJ9eipWTWYXVgQ=,rsync://rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/32332e3135342e38312e302f32342d3234203d3e203533333536.roa,1702699880,98:23:C0:CD:73:C0:73:4B:88:4C:8E:A2:FE:FC:94:59:27:54:FC:CF,roa,1702699580,1734149480,OK,23.154.81.0/24,53356,24,valid
+rsync://rpki.arin.net/repository/arin-rpki-ta/5e4a23ea-e80a-403e-b08c-2171da2157d3/f60c9f32-a87c-4339-a2f3-6299a3b02e29/1146938c-c605-4779-bf60-820a16fa701c/8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b.cer,86:A9:90:76:61:0E:7C:08:AE:DD:CE:87:67:AA:5D:C4:52:8C:49:08,/CN=8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b,66D4203E6D324EDF7DB13F151CE102BE0DBE1200,unable to get local issuer certificate,/home/kartograf/data/1730210400/rpki/cache/rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/32332e3135342e38312e302f32342d3234203d3e20333936353033.roa,Wfz/lTUfLoaYSWzIWHHqfxFY68pwclupNoh7qMD/dy4=,rsync://rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/32332e3135342e38312e302f32342d3234203d3e20333936353033.roa,1721069363,EF:A6:FD:C7:47:52:EA:52:97:81:CB:E6:BC:08:E1:89:74:01:CE:54,roa,1721069063,1752518963,OK,23.154.81.0/24,396503,24,duplicate of row above
+rsync://rpki.arin.net/repository/arin-rpki-ta/5e4a23ea-e80a-403e-b08c-2171da2157d3/f60c9f32-a87c-4339-a2f3-6299a3b02e29/1146938c-c605-4779-bf60-820a16fa701c/8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b.cer,86:A9:90:76:61:0E:7C:08:AE:DD:CE:87:67:AA:5D:C4:52:8C:49:08,/CN=8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b,35802871A8C539AD5A498BEDDDEF0D3F70E27D85,unable to get local issuer certificate,/home/kartograf/data/1730210400/rpki/cache/rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/32332e3135342e38302e302f32342d3234203d3e20323133313836.roa,5DOfCcd4V1zI0oWAJNiXnZVa/zwdfo+24eD0OBlrFy4=,rsync://rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/32332e3135342e38302e302f32342d3234203d3e20323133313836.roa,1721921309,BA:47:CE:B1:56:FB:2F:78:B5:37:B4:63:10:F4:17:1F:BB:2C:6A:FB,err,1721921009,1753370909,OK,23.154.80.0/24,213186,24,type is not “roa”
+rsync://rpki.arin.net/repository/arin-rpki-ta/5e4a23ea-e80a-403e-b08c-2171da2157d3/f60c9f32-a87c-4339-a2f3-6299a3b02e29/1146938c-c605-4779-bf60-820a16fa701c/8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b.cer,86:A9:90:76:61:0E:7C:08:AE:DD:CE:87:67:AA:5D:C4:52:8C:49:08,/CN=8f6916d463bfc5c35e4659c12889a337f3cc6f6b7fe978372b,77C98DB3245AF48F037FF77D087EB06EC63AAC8D,unable to get local issuer certificate,/home/kartograf/data/1730210400/rpki/cache/rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/3135382e35312e3131322e302f32342d3234203d3e20333936353033.roa,/pHsoAr4Krm+kx64aCsYjFRuk+JnfbiC6JA9UWV27TQ=,rsync://rpki.tools.westconnect.ca/repo/WestConnect-Pub/0/3135382e35312e3131322e302f32342d3234203d3e20333936353033.roa,1721069362,C5:65:B7:B3:AA:F9:59:32:0B:F5:50:24:70:01:09:71:4D:FC:20:24,roa,1721069062,1752518962,Failed,158.51.112.0/24,396503,24,failed validation


Hm, I am usually in favor of keeping realistic data in the fixtures but since the rsync and file paths are pretty long and make this file much harder to parse, I would maybe replace them with dummy values. But up to you!

yea i'll fix this in a follow up. I used existing examples for authenticity's sake.

fjahr · 2025-01-26T18:02:08Z

kartograf/context.py

@@ -9,14 +9,14 @@ class Context:
    def __init__(self, args):
        self.args = args

-        # The epoch is used to keep artifacts seperated for each run. This
+        # The epoch is used to keep artifacts separated for each run. This
        # makes cleanup and debugging easier.
        if args.wait:


nit: Could update this to self.args.wait

fjahr · 2025-01-26T18:02:29Z

kartograf/context.py


        if os.path.exists(self.data_dir) and not self.reproduce:
            print("Not so fast, a folder with that epoch already exists.")
            sys.exit()

+        self.data_dir_irr = os.path.join(self.data_dir, "irr")


Instead of os.path.join could use Pathlib here too

jurraca · 2025-01-30T16:15:37Z

I moved the pathlib / os.path changes to #54 for clarity, this PR is rebase on that, so let's merge that first. still many follow ups to be had with this, mainly in terms of fixtures and parsing scenarios...

jurraca · 2025-02-03T17:46:41Z

do not merge yet -- found a bug #56

jurraca · 2025-02-04T11:59:36Z

rebased on #56 , which should go first.

set up a test context to use with pytest's tmp_path refactor epoch dir setup in order to handle mocking it. Add a rpki_raw.json fixture, and a tests/util.py module.

fjahr reviewed Jan 19, 2025

View reviewed changes

jurraca force-pushed the rpki-parse-tests branch 2 times, most recently from 8eaa02c to b2a23ff Compare January 21, 2025 17:14

fjahr reviewed Jan 26, 2025

View reviewed changes

jurraca force-pushed the rpki-parse-tests branch 2 times, most recently from 2140739 to 79396c0 Compare January 30, 2025 15:19

jurraca mentioned this pull request Jan 30, 2025

context: use pathlib instead of os.path #54

Merged

jurraca force-pushed the rpki-parse-tests branch 2 times, most recently from 8f15280 to 261b317 Compare January 30, 2025 16:14

jurraca marked this pull request as ready for review January 30, 2025 16:15

jurraca force-pushed the rpki-parse-tests branch from 261b317 to b2f63c5 Compare February 2, 2025 18:51

jurraca mentioned this pull request Feb 3, 2025

Add tests for RPKI ASN tiebreaks #55

Open

jurraca force-pushed the rpki-parse-tests branch from b2f63c5 to 3297b58 Compare February 4, 2025 11:57

jurraca added 4 commits February 7, 2025 12:27

Add tests/context.py, a context for testing

ba9b24e

rpki parsing tests: test validations, set up test context

ffa208a

set up a test context to use with pytest's tmp_path refactor epoch dir setup in order to handle mocking it. Add a rpki_raw.json fixture, and a tests/util.py module.

rename files: test_rpki_parser, move helper module to util folder

fd5a978

test fixtures: simplify roa fixtures for legibility

e91bd62

jurraca force-pushed the rpki-parse-tests branch from 3297b58 to e91bd62 Compare February 7, 2025 12:27

jurraca mentioned this pull request Feb 8, 2025

IRR parsing tests: add fixtures and basic test, update context #58

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RPKI parse tests #49

RPKI parse tests #49

jurraca commented Jan 18, 2025

fjahr left a comment

fjahr Jan 19, 2025

jurraca Jan 21, 2025

fjahr Jan 19, 2025

fjahr commented Jan 19, 2025

jurraca commented Jan 21, 2025

fjahr left a comment

fjahr Jan 26, 2025

fjahr Jan 26, 2025

fjahr Jan 26, 2025

jurraca Jan 30, 2025

fjahr Jan 26, 2025

fjahr Jan 26, 2025

jurraca commented Jan 30, 2025

jurraca commented Feb 3, 2025 •

edited

Loading

jurraca commented Feb 4, 2025

RPKI parse tests #49

Are you sure you want to change the base?

RPKI parse tests #49

Conversation

jurraca commented Jan 18, 2025

fjahr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjahr commented Jan 19, 2025

jurraca commented Jan 21, 2025

fjahr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jurraca commented Jan 30, 2025

jurraca commented Feb 3, 2025 • edited Loading

jurraca commented Feb 4, 2025

jurraca commented Feb 3, 2025 •

edited

Loading