Skip to content

feat(feed): Retry logic + Stale check - #61

Open
Kyl-Bempah wants to merge 4 commits into
traust-security:developmentfrom
Kyl-Bempah:feat/feeds-retry
Open

Kyl-Bempah wants to merge 4 commits into
traust-security:developmentfrom
Kyl-Bempah:feat/feeds-retry

Conversation

@Kyl-Bempah

Copy link
Copy Markdown
Contributor

Addressing XWING-2223

Hardens fetch_feeds so a rate limit or transient error no longer becomes an error, and a run built on a stale fallback is no longer indistinguishable from a fresh one. Not a rewrite — adds seams around the existing urllib flow. XWING-2223

What changed

  • Retry (RetryPolicy + fetch_with_retry) around all three download sites (non-paginated fetch, the paginated page loop, load_vex): 429 honours Retry-After (seconds or HTTP-date, capped by max_wait_s); 5xx and timeouts use capped exponential backoff + jitter; a non-429 4xx fails fast; exhausted attempts raise FeedError and the caller falls back to the cached copy.
  • Stale visibility: feeds-meta.json now records last_error, last_attempt_at, stale per feed (on failure and success), and feed_status() surfaces them.
  • Validate-before-replace: a downloaded payload is checked against its declared type (gzip / JSON / UTF-8, dispatched on the cache file extension — no feed name hardcoded) before it overwrites the cache. A corrupt 200 keeps the old copy instead of clobbering it (also fixes a pre-existing write-before-validate bug).
  • Config, not code: a retry: block in config/feeds.yaml (defaults) with per-feed override, loaded via feeds_config.retry_defaults().

Acceptance criteria

  • AC#1 (429 Retry-After; 5xx/timeout backoff) ✅
  • AC#2 (last_error/last_attempt_at/stale in meta + feed_status) ✅
  • AC#4 (fake-HTTP tests: 429 ±Retry-After, 503→200, timeout, corrupt-keeps-old-copy, give-up) ✅
  • AC#5 (retry limits in feeds.yaml, defaults + per-feed override) ✅
  • AC#3 (degraded exit code / --result-json) — deferred to XWING-2220 (_outcome.py + the exit-code value, Girish). main() is left untouched so it bolts on cleanly once 2220 lands.

Open question (does not block review)

AC#4 says "sha256 mismatch keeps the old copy," but EPSS/KEV/rh-cve publish no checksum — only the VEX archive does (XWING-2224). This implements "corrupt" as "fails to decode as its declared type," isolated in _validate_payload; if a checksum source exists, only that one function changes. Confirming with Shaun on the ticket.

Tests

test_fetch_feeds.py 50 passed (7 new), test_feeds_config.py green, ruff clean. All new tests are offline (fake urlopen).

Out of scope

VEX deletions/mirror (2224); shared cache location (2225); the degraded exit code (2220).

🤖 Generated with Claude Code

Addressing XWING-2223

Signed-off-by: Kyl-Bempah <24903083+Kyl-Bempah@users.noreply.github.com>
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2026

@chubirka chubirka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't a feed-service with these retries go into traust-core @smith-xyz ?

@Kyl-Bempah

Copy link
Copy Markdown
Contributor Author

So the logic behind this is in part due to the XWING-2225 work:

  • traust-core is a no-HTTP, in-process library by design, according to it's AGENT logic and README and the fetch_feeds call makes outbound calls to EPSS/KEV/VEX, so I assumed the service didn't belong in core. Core had the the FeedProvider & ObjectStore interfaces, so I funciton belonged here.
  • XWING-2223's ticket also scopes it to traust ("harden fetch_feeds, not a rewrite") as the "feed router".

Moving feeds onto core's FeedProvider + ObjectStore — the actual "feed-service" — is XWING-2225, which is gated on core shipping the S3 ObjectStore. I can move the RetryPolicy up as part of that if you'd rather it live in core. @smith-xyz do you have thoughts on the implementation between 2223 vs 2225?

@smith-xyz
smith-xyz changed the base branch from main to development October 15, 2026 20:13
Signed-off-by: Kyl-Bempah <24903083+Kyl-Bempah@users.noreply.github.com>
@openshift-ci

openshift-ci Bot commented Oct 16, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Kyl-Bempah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2026
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2026
@Kyl-Bempah

Kyl-Bempah commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor Author

Hey Michele updated this pull, you were right, and my earlier reply was based on a wrong premise. I misinterpreted the http statement as traust-core doing no outbound work instead of it just not having landed yet. So the retry logic now lives in core, and this PR is cut down to more of an framework layer, summary of changes below.

Moved to traust-core (traust-security/traust-core#8, feat/http-provider):

  • RetryPolicy + fetch_with_retry → traust_core.v1.providers.http.HttpFetcher(user_agent, policy).get(url). Same behaviour: 429 honours Retry-After (capped), 5xx/timeouts get capped exponential backoff, other 4xx fail fast.
  • FeedError → core's ServiceError (carries .status).
  • _validate_payload → traust_core.v1.providers.feeds.validate_payload(raw, filename).
  • The retry/backoff tests moved with it (tests/v1/providers/test_http.py, test_feeds.py).

What's left in this PR:

  • Config: the retry: block in config/feeds.yaml (defaults plus per-feed override). Keys renamed to core's RetryPolicy fields (timeout_seconds, backoff_base_seconds, max_wait_seconds, jitter_seconds). A new test fails if the shipped config doesn't build a valid policy.
  • Staleness: last_error / last_attempt_at / stale in feeds-meta.json, surfaced by feed_status().
  • Validate before overwriting: the order in fetch() is unchanged (download → validate → write); it just calls core's validator.
  • Call sites: all three download sites (fetch, the paginated loop, load_vex) go through HttpFetcher. fetch_feeds.py no longer imports urllib.
  • Tests: the stale-fallback and corrupt-download tests stay here and now stub HttpFetcher.get.

Dependency: The PR also now adds traust-core as a dependency. [tool.uv.sources] currently has a placeholder with a TODO to update to the revision when the core PR is merged and run uv lock before this merges.

The degraded exit code (AC#3) is still deferred to XWING-2220.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants