Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the Erik synchronization protocol (draft-spaghetti-sidrops-rpki-erik-protocol) for RPKI, introducing a new mechanism for fetching and synchronizing RPKI objects. The implementation includes parsing Erik index and partition files, fetching objects via HTTP, and storing them in the database.
Key changes:
- Added Erik protocol parsing support with new ASN.1 parsers for index and partition objects
- Refactored HTTP and directory traversal functionality to support both RRDP and Erik protocols
- Introduced new configuration options and error types for Erik protocol handling
Reviewed changes
Copilot reviewed 31 out of 36 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/RPKI/Parse/Internal/Erik.hs | New parser implementation for Erik index and partition objects |
| src/RPKI/Fetch/ErikRelay.hs | Core Erik protocol fetch logic including index/partition/manifest retrieval |
| src/RPKI/Fetch/DirectoryTraverse.hs | Refactored directory traversal code extracted from Rsync module for reusability |
| src/RPKI/Fetch/Http.hs | HTTP download functions refactored to support multiple protocols, module renamed |
| src/RPKI/Domain.hs | New domain types for Erik protocol (ErikIndex, ErikPartition, ManifestListEntry, etc.) |
| src/RPKI/Store/Database.hs | Database operations for storing and retrieving Erik indexes and partitions |
| src/RPKI/Config.hs | New ErikConf configuration type with maxSize and parallelism settings |
| src/RPKI/Reporting.hs | New ErikError type for protocol-specific errors |
| src/RPKI/Rsync.hs | Refactored to use shared directory traversal code |
| src/RPKI/Util.hs | Added utility functions for hash encoding (hashHex, hashAsBase64, firstByteStr) |
| test/src/RPKI/Parse/ObjectParseSpec.hs | Test cases for Erik index parsing and ASPA edge cases |
| Multiple files | Module rename from RPKI.RRDP.Http to RPKI.Fetch.Http and RPKI.Fetch to RPKI.Fetch.Fetch |
Comments suppressed due to low confidence (2)
src/RPKI/Fetch/Http.hs:77
- This commented-out code should be removed since the functionality has been refactored to pass
tmpDiras a parameter instead.
src/RPKI/Fetch/Http.hs:102 - This commented-out code should be removed since the functionality has been refactored to pass
tmpDiras a parameter instead.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| shouldParseErikPartition :: TestTree | ||
| shouldParseErikPartition = HU.testCase "Should parse an Erik partition" $ do | ||
| bs <- BS.readFile "test/data/erik/7I-e8mmhb4Hx5EcgYvZhRMHBkqsp-fHo1hUvReXcKe4" | ||
| let (Right p, _) = | ||
| runPureValidator (newScopes "parse") $ parseMft bs |
There was a problem hiding this comment.
This test case is parsing the file using parseMft (manifest parser) instead of parseErikPartition. It should call parseErikPartition bs instead of parseMft bs.
| let (Right p, _) = | ||
| runPureValidator (newScopes "parse") $ parseMft bs | ||
|
|
||
| HU.assertBool "Wrong index" True |
There was a problem hiding this comment.
This test assertion always passes (assertBool with True). The test should verify actual properties of the parsed Erik partition, such as checking the partition time, hash algorithm, or manifest list, similar to how shouldParseErikIndex validates the parsed index fields.
src/RPKI/Fetch/ErikRelay.hs
Outdated
| import qualified Data.List.NonEmpty as NonEmpty | ||
|
|
||
| import qualified Data.ByteString as BS | ||
| import Data.Text (Text, index) |
There was a problem hiding this comment.
The index function is imported from Data.Text but appears to be unused in this module. Consider removing it from the import list or use qualified imports to avoid potential naming conflicts.
| import Data.Text (Text, index) | |
| import Data.Text (Text) |
src/RPKI/Fetch/ErikRelay.hs
Outdated
| import Control.Monad.IO.Class | ||
| import Data.Generics.Product.Typed | ||
|
|
||
| import qualified Data.List.NonEmpty as NonEmpty |
There was a problem hiding this comment.
Data.List.NonEmpty is imported as NonEmpty but doesn't appear to be used in this module. Consider removing this unused import.
| import qualified Data.List.NonEmpty as NonEmpty |
src/RPKI/Fetch/ErikRelay.hs
Outdated
|
|
||
| import qualified Data.ByteString as BS | ||
| import Data.Text (Text, index) | ||
| import Data.Data |
There was a problem hiding this comment.
Data.Data is imported but doesn't appear to be used in this module. Consider removing this unused import.
| import Data.Data |
src/RPKI/Fetch/ErikRelay.hs
Outdated
| import Data.Maybe | ||
| import Data.Map.Strict (Map) | ||
| import qualified Data.Map.Strict as Map | ||
| import qualified Data.Map.Monoidal.Strict as MonoidalMap |
There was a problem hiding this comment.
Data.Map.Monoidal.Strict is imported as MonoidalMap but doesn't appear to be used in this module. Consider removing this unused import.
| import qualified Data.Map.Monoidal.Strict as MonoidalMap |
src/RPKI/Fetch/ErikRelay.hs
Outdated
|
|
||
| import System.Directory | ||
| import System.FilePath | ||
| import UnliftIO (pooledForConcurrentlyN, pooledForConcurrently, bracket) |
There was a problem hiding this comment.
The pooledForConcurrently and bracket functions are imported from UnliftIO but don't appear to be used in this module. Consider removing them from the import list, keeping only pooledForConcurrentlyN.
| import UnliftIO (pooledForConcurrentlyN, pooledForConcurrently, bracket) | |
| import UnliftIO (pooledForConcurrentlyN) |
src/RPKI/Fetch/ErikRelay.hs
Outdated
| import qualified RPKI.Util as U | ||
| import RPKI.Fetch.Http | ||
| import qualified RPKI.Store.Database as DB | ||
| import Data.ByteArray (create) |
There was a problem hiding this comment.
Data.ByteArray's create function is imported but doesn't appear to be used in this module. Consider removing this unused import.
| import Data.ByteArray (create) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
https://datatracker.ietf.org/doc/html/draft-spaghetti-sidrops-rpki-erik-protocol