feat(http): default User-Agent + treat 403+Retry-After as rate-limit (#1037)#1155
Open
ChrisJr404 wants to merge 1 commit into
Open
feat(http): default User-Agent + treat 403+Retry-After as rate-limit (#1037)#1155ChrisJr404 wants to merge 1 commit into
ChrisJr404 wants to merge 1 commit into
Conversation
…nchore#1037) Two of the four checkboxes from anchore#1037 ("enhance http_wrapper to be a better http citizen") were still open: - always set a User-Agent header so we identify ourselves - respect Retry-After on 403s (GitHub returns 403 + Retry-After for secondary rate limits) This patch closes both. http_wrapper.default_user_agent() Centralised helper that returns 'anchore/vunnel-<version>' (and 'anchore/vunnel-unknown' when the package metadata is unavailable, e.g. an editable install in CI). The convention matches the per-provider helpers in chainguard_libraries, secureos, and fedora. http_wrapper.get() Previously a None / empty user_agent argument both meant 'do not set a header'. Now: user_agent=None -> default_user_agent() is set automatically user_agent='' -> no User-Agent header (explicit opt-out) user_agent='x' -> 'x' is sent verbatim Callers that already set headers={'User-Agent': '...'} keep their value untouched (setdefault). Callers that pass user_agent='string' keep their existing behaviour. http_wrapper._is_rate_limited() Now also returns True for 403 responses that carry a Retry-After header. 403 without Retry-After is still treated as a permission error (not rate-limited), so 'genuine 403' callers are unaffected. Tests - test_default_user_agent_identifies_vunnel: None defaults work. - test_caller_supplied_headers_override_default_user_agent: explicit header wins over the wrapper default. - test_403_with_retry_after_is_rate_limited: full integration test through get() with retry handling. - test_403_without_retry_after_is_not_rate_limited: 403 without Retry-After still raises HTTPError without rate-limit logging. - test_403_with_header_is_rate_limited / _without_header_is_not: direct unit tests on _is_rate_limited. - Existing test_succeeds_if_retries_succeed and test_timeout_is_passed_in updated to expect the new default UA in the outgoing headers (test_empty_user_agent_sets_no_header is unchanged and still passes — the empty-string opt-out path). Full unit suite: 826 passed (47 in test_http_wrapper.py, up from 41). Signed-off-by: Chris (ChrisJr404) <11917633+ChrisJr404@users.noreply.github.com>
ChrisJr404
force-pushed
the
http-wrapper-default-ua-and-403-retry-after
branch
from
a3eab0a to
56b197a
Compare
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.
Closes the two remaining unchecked items in #1037:
The 429 and 503-with-Retry-After items were already shipped.
Default User-Agent
http_wrapper.default_user_agent()returnsanchore/vunnel-<version>, falling back toanchore/vunnel-unknownwhen package metadata is unavailable (e.g. some editable / CI installs). It matches the convention already used by the per-provider helpers inchainguard_libraries,secureos, andfedora— so a future cleanup PR could deduplicate those onto the central helper if desired.http.get()now sets the default automatically when the caller does not specify one:user_agentargumentNone(default)User-Agent: anchore/vunnel-<version>""User-Agentheader (explicit opt-out, used by tests)A caller that pre-populates
headers={"User-Agent": "..."}still wins —setdefaultonly fills the slot when it's empty.403 + Retry-After
_is_rate_limited()now treats 403 + Retry-After the same way it already treats 503 + Retry-After. 403 withoutRetry-Afteris still a normal permission error (not rate-limit handling), so existing callers that legitimately receive 403s are unaffected.Tests
tests/unit/utils/test_http_wrapper.py: 47 passed (up from 41).New tests:
test_default_user_agent_identifies_vunnel— covers the newNonedefault.test_caller_supplied_headers_override_default_user_agent— explicit header still wins.test_403_with_retry_after_is_rate_limited— full path throughget()with retry handling.test_403_without_retry_after_is_not_rate_limited— plain 403 still raises HTTPError without rate-limit logging.test_403_with_header_is_rate_limited/_without_header_is_not_rate_limited— direct unit tests on_is_rate_limited.Updated existing tests
test_succeeds_if_retries_succeedandtest_timeout_is_passed_into expect the new default UA in outgoing headers.test_empty_user_agent_sets_no_headeris unchanged and still passes — the""opt-out path is preserved exactly.Full unit suite: 826 passed, no regressions.