chore(scanner): Add retries to Scanner V4 CSAF enricher#21885
Draft
dcaravel wants to merge 1 commit into
Draft
Conversation
dcaravel
added
the
pr-update-scanner-vulns
|
Skipping CI for Draft Pull Request. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughCSAF enrichment adds retried advisory fetching with exponential backoff. ChangesCSAF advisory fetching
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant processChanges
participant fetchAdvisory
participant AdvisoryEndpoint as Advisory HTTP endpoint
participant Output as Advisory output writer
processChanges->>fetchAdvisory: Fetch advisory
fetchAdvisory->>AdvisoryEndpoint: GET advisory
AdvisoryEndpoint-->>fetchAdvisory: Success or error response
fetchAdvisory->>AdvisoryEndpoint: Retry failed request with backoff
AdvisoryEndpoint-->>fetchAdvisory: Advisory JSON
fetchAdvisory->>Output: Write compacted advisory
fetchAdvisory-->>processChanges: Success or fetch error
processChanges->>processChanges: Mark successful advisory changed
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scanner/enricher/csaf/fetcher.go`:
- Around line 386-395: Handle and propagate the error returned by w.Write in the
fetch response-body flow instead of discarding it. Update the surrounding
function to return a descriptive wrapped error when writing bc.Bytes() fails,
ensuring fetchAdvisory/processChanges do not treat an unsuccessful write as a
successful fetch.
- Around line 397-407: Update the retry flow around BetweenAttempts and
checkResponse so advisory retries stop promptly when ctx is canceled and do not
retry permanent 4xx responses. Replace the blocking backoff sleep with
context-aware waiting using ctx.Done(), and classify non-retryable 4xx errors so
retry.WithRetry exits immediately while preserving retries for transient
failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: adef1e7f-2914-4913-a5df-48eb87585779
📒 Files selected for processing (2)
scanner/enricher/csaf/fetcher.goscanner/enricher/csaf/fetcher_test.go
Comment on lines
+397
to
+407
| return retry.WithRetry(fetch, | ||
| retry.WithContext(ctx), | ||
| retry.Tries(advisoryFetchRetries), | ||
| retry.BetweenAttempts(func(attempt int) { | ||
| time.Sleep(advisoryFetchRetryBaseDelay * time.Duration(1<<attempt)) | ||
| }), | ||
| retry.OnFailedAttempts(func(err error) { | ||
| slog.WarnContext(ctx, "advisory fetch attempt failed", | ||
| "advisory", advisoryURI, "reason", err) | ||
| }), | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect retry package internals for ctx-cancellation handling and early-termination support.
fd retry.go --exec cat -n {}Repository: stackrox/stackrox
Length of output: 16230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate fetcher.go =="
fd '^fetcher\.go$' scanner/enricher/csaf || true
echo "== targeted fetcher section =="
cat -n scanner/enricher/csaf/fetcher.go | sed -n '360,425p'
echo "== search checkResponse and related symbols =="
rg -n "func checkResponse|checkResponse\\(|advisoryFetchRetries|advisoryFetchRetryBaseDelay|compressedFileTimeout|processChanges|context\\.With" scanner/enricher/csaf -S
echo "== deterministic retry behavior model =="
python3 - <<'PY'
from dataclasses import dataclass
from typing import Callable, Optional
`@dataclass`
class Event:
t: float
kind: str
def simulate(ctx_canceled_at: Optional[float], errors: list, base=0.5, cap=0):
attempts = max(1, 5) # Tries(5)
sleeps = []
events = []
canceled = False
t = 0.0
for attempt in range(attempts):
if not canceled and ctx_canceled_at is not None and t >= ctx_canceled_at:
canceled = True
if canceled:
return events + [Event(t, "ctx_err")]
err = errors[attempt] if attempt < len(errors) else None
if err is None:
return events + [Event(t, "success")] if attempt == len(errors) else events + [Event(t, "success")]
if err == "4xx":
return events + [Event(t, "4xx_no_retry")]
delta = cap if cap else base * (1 << attempt)
attempts_remaining = attempts - attempt
# retry loop increments i and only does attempts_remaining sleeps for a Tries(5) error-only case.
sleeps_delta = base * ((1 << (attempts - 1)) - (1 << attempt))
while delta > t or delta >= t:
delta += 0.01
# exact backoff not precise
sleep_amt = base * ((1 << attempts_remaining) - (1 << attempt)) # approximate count from source does (2+4+8+16=30) as comment.
# use actual formula per fixed.
PYRepository: stackrox/stackrox
Length of output: 4954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== processChanges context/deadline and advisory fetch call =="
cat -n scanner/enricher/csaf/fetcher.go | sed -n '100,345p'
echo "== retry package exact behavior probe =="
python3 - <<'PY'
# Mirrors pkg/retry/retry.go: Tries(5), base sleeps in BetweenAttempts, no explicit Backoff option.
def sleep_total_before_failure(errors):
total_sleep = 0.0
attempt_sleeps = {}
for i, err in enumerate(errors):
if err is None:
return False, total_sleep, i, attempt_sleeps
if err == "4xx":
return False, total_sleep, i, attempt_sleeps
# between(i) runs before backoff only on retryable/error path; same retry library without backoff option
# only executes BetweenAttempts callback, so current implementation sleeps.
sleep = 0.5 * (1<<i)
attempt_sleeps[i] = sleep
total_sleep += sleep
return True, total_sleep, len(errors), attempt_sleeps
# non-retryable (4xx) errors
for n in range(5):
print("4xx after attempts", n, "total_sleep", sleep_total_before_failure([4]*n + [404])[1])
print()
# transient-only errors returning 5 failures
ok, total_sleep, final_attempt, sleeps = sleep_total_before_failure([503]*5)
print("retries exhausted", ok, "total_sleep", total_sleep, "attempts", final_attempt, "sleeps", sleeps)
PYRepository: stackrox/stackrox
Length of output: 9107
Avoid blocking advisory retries on context cancellation and on permanent 4xx responses.
BetweenAttempts runs time.Sleep(...) synchronously, so each retry backoff can block until it returns even after ctx is canceled; use a select with time.After and ctx.Done() so cancellation takes effect during backoff. Also, checkResponse returns plain errors for non-OK states, including 4xx, so permanently-missing advisories still consume the full retry/backoff budget in processChanges, which has no deadline of its own while later compressed fetching uses compressedFileTimeout. Short-circuit non-retryable 4xx responses, or add an explicit timeout/deadline around the changes/advisory retry phase.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scanner/enricher/csaf/fetcher.go` around lines 397 - 407, Update the retry
flow around BetweenAttempts and checkResponse so advisory retries stop promptly
when ctx is canceled and do not retry permanent 4xx responses. Replace the
blocking backoff sleep with context-aware waiting using ctx.Done(), and classify
non-retryable 4xx errors so retry.WithRetry exits immediately while preserving
retries for transient failures.
Contributor
🚀 Build Images ReadyImages are ready for commit 5252ce2. To use with deploy scripts: export MAIN_IMAGE_TAG=4.12.x-565-g5252ce2df6 |
dcaravel
force-pushed
the
dc/csaf-enricher-resil
branch
from
July 22, 2026 19:49
dd58bb7 to
5252ce2
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21885 +/- ##
==========================================
- Coverage 51.38% 51.35% -0.03%
==========================================
Files 2856 2857 +1
Lines 178637 178806 +169
==========================================
+ Hits 91794 91828 +34
- Misses 78832 78947 +115
- Partials 8011 8031 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds retries when processing entries from
changes.csvin the CSAF enricher. Recent vuln bundle generation has been failing (example)Also logs the URI of the failing request to ease troubleshooting.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
change me!