Repository navigation
feat(importer): support OSV HTTP feeds alongside Git sources - #2783
Conversation
Reviewer's GuideThe PR extends OSV importing with automatically detected or explicitly configured REST feeds, using shared ingestion behavior plus bounded concurrent downloads, cancellation, and content-addressed incremental checkpoints, while retaining Git support and linking upstream CVEs from distribution advisories. Sequence diagram for incremental OSV HTTP feed ingestionsequenceDiagram
participant Runner as ImportRunner
participant Walker as HttpWalker
participant Fetcher as HttpFetcher
participant Storage as ContentAddressedStorage
participant Ingestor as OSVIngestor
Runner->>Walker: new(..., continuation)
Walker->>Storage: load_checkpoint()
Walker->>Fetcher: index()
Fetcher-->>Walker: all.json entries
Walker->>Fetcher: record(id) concurrently
Fetcher-->>Walker: bounded OSV record bytes
Walker->>Ingestor: parse and store(file, data)
Ingestor-->>Walker: ingestion result
Walker->>Storage: store(updated checkpoint)
Walker-->>Runner: continuation and report
State diagram for OSV HTTP record checkpointsstateDiagram-v2
[*] --> LoadCheckpoint
LoadCheckpoint --> FullScan: missing or invalid checkpoint
LoadCheckpoint --> CompareIndex: matching checkpoint
FullScan --> FetchChanged
CompareIndex --> FetchChanged: records changed or new
FetchChanged --> IngestSuccessful: download and validation succeed
FetchChanged --> RecordFailure: download or validation fails
RecordFailure --> SaveCheckpoint
IngestSuccessful --> SaveCheckpoint
SaveCheckpoint --> [*]: continuation returned
Flow diagram for OSV feed transport selectionflowchart TD
Source[OSV source configuration] --> Explicit{transport configured?}
Explicit -->|HTTP| ValidateHTTP[Validate HTTP index and base URLs]
Explicit -->|Git| Git[Run GitWalker]
Explicit -->|Not configured| Detect{"HTTP(S) URL ending in all.json?"}
Detect -->|Yes| ValidateHTTP
Detect -->|No| Git
ValidateHTTP --> HTTP[Run HttpWalker]
HTTP --> Shared[Shared filtering, labels, transactions, and ingestion]
Git --> Shared
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/importer/src/runner/osv/http.rs" line_range="134-137" />
<code_context>
+ let bytes = stream
+ .try_fold(Vec::new(), |mut bytes, chunk| async move {
+ ensure!(
+ chunk.len() <= INDEX_LIMIT.saturating_sub(bytes.len()),
+ "OSV checkpoint exceeds size limit"
+ );
+ bytes.extend_from_slice(&chunk);
+ Ok::<_, anyhow::Error>(bytes)
+ })
</code_context>
<issue_to_address>
**Large checkpoints become unusable**
When the serialized checkpoint grows beyond 256 MiB, the read path rejects checkpoint blobs over 256 MiB, but the write path stores the serialized record map without the same limit. Once a feed produces a larger checkpoint, the returned continuation fails to load on the next run, so imports repeatedly fall back to full scans and report errors.
Enforce the limit before storing a checkpoint, and handle oversized state with a usable fallback such as paged checkpoint storage or a full-rescan continuation.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and a malformed or incorrect feed record, or an unintended upstream CVE link, could be ingested and persist in advisory data after this change is reverted. The impact is bounded and can generally be corrected by re-importing corrected data.
Blocking findings: modules/importer/src/runner/osv/http.rs:137
9e26356 to
9e4c3f1
Compare
18d218b to
f15f4c0
Compare
| .context | ||
| .check_canceled(|| anyhow!("OSV import canceled")) | ||
| .await?; | ||
| let canceled = async { |
There was a problem hiding this comment.
is there a scenario where this might hang forever on cancel checks ?
There was a problem hiding this comment.
Yes, there was a possible deadlock here. The production cancellation check holds a mutex while awaiting a database read. If a download completed while the retained cancellation future held that mutex, the next direct cancellation check could wait on the same mutex indefinitely, because the retained future was no longer being polled.
Fixed in 09547b3: each select now creates its own cancellation future, so a pending check is dropped and releases its guard before any direct check. Added regression tests with a mutex held across a pending check, covering both empty and nonempty feeds. The delayed-download cancellation/retry test also still passes.
Validation: all 34 OSV tests, formatting, and strict importer Clippy passed.
| document.id == entry.id, | ||
| "OSV record ID does not match index" | ||
| ); | ||
| ensure!( |
There was a problem hiding this comment.
not sure what happens when document.modified != entry.modified eg. the first full scan of hundreds of thousands of records could take 'a very long time' during which some records might change mid scan. Maybe a frist run never gets watermark and every future run is another full scan ?
There was a problem hiding this comment.
Yes, this could prevent watermark advancement indefinitely if every full scan encountered a record updated after the index was fetched.
Fixed in 09547b3: accept a document with a matching ID when its modified timestamp is equal to or newer than the index entry. Older documents and mismatched IDs still fail validation. The watermark remains the scan start time, captured before fetching the index, so records changed during the scan remain eligible on the next run.
Added a regression test that ingests a newer document, establishes a watermark, fetches the record again after the index catches up, and then skips it on the following run. Additional tests verify that older documents and mismatched IDs preserve the previous continuation in both full and incremental scans.
Validation: all 34 OSV tests, formatting, and strict importer Clippy passed.
|
@ctron @rh-jfuller Can you review this as a priority please. As the PR includes an update to the tokio dependency that is causing other PRs to fail? Thanks, |
Need: no. Could: yes. It's closing a gap we have right now with OSV sources. Ran across it when trying to get more 3rd party data. |
09547b3 to
c4ff1b5
Compare
c4ff1b5 to
5e109bf
Compare
Summary
transport: "http"; Git remains the default.baseUrlonly for HTTP andbranch/pathonly for Git; legacy configurations withouttransportstill parse as Git. There is no filename-based auto-detection.Validation
cargo test -p trustify-module-importer runner::osv --lib— 8 tests passed after the explicit-transport change.cargo clippy -p trustify-module-importer -p trustify-server --tests -- -D warnings— passed.git diff --check— passed.Limitations
Summary by Sourcery
Extend OSV importing to support explicitly selected HTTP REST feeds alongside the existing Git workflow.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Tracking
Closes #1171
Jira: https://redhat.atlassian.net/browse/TC-7106