feat(netprobe): HTTP probe v3 — response assertions (json/body/size/header/version) + per-target resolve/ip_version - #794
Conversation
…http-date, http-version
…sion; ip-resolve + CURLOPT_RESOLVE
…ersion parse + validate + scrub
…SOLVE) for http/doh
…ND, truncation-safe)
…r/version/redirect); docs
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e4e10497c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
LCOV of commit
|
…_response_size_bytes:0 requires an empty body
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5e5d0e0c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…VE leaked across same-host targets on the shared multi)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ea9c1d8c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…alformed header-matcher config Response-header capture is bounded by a 64KB byte cap. When a header line is dropped at that cap, a "pass" from the header matchers can no longer be trusted (a forbidden header could be among the dropped tail), so mark the result and fail the header assertion conservatively with a warning. Also redact the whole fail_if_header_matches/fail_if_header_not_matches key in the config echo when its value arrives as a non-object (scalar/list) shape — a malformed value can still carry a secret and must not be echoed raw.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79e9f8e5a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… IPv6 in CONNECT_TO Two Round-4 review fixes: - header_capture(): the per-response reset (fired on each "HTTP/" status line so only the final response's headers survive a redirect) cleared the header list and byte count but not headers_truncated. An oversized intermediate redirect hop therefore left the flag set, making the probe fail a header assertion that should have passed on a small final response. Reset the flag alongside the rest. - CONNECT_TO construction: an unbracketed IPv6 resolve address collided with the PORT2 field of curl's HOST1:PORT1:HOST2:PORT2 --connect-to format, so IPv6 per-target resolve never connected. Extract a pure build_connect_to_entry() that brackets IPv6 literals (accepting bracketed or unbracketed input) and rebuilds the entry; malformed entries return "" and are skipped.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9102618c59
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…der rules exist A fail_if_header_not_matches (required-header) PASS is a positive presence proof — the required header was found in the captured headers — so headers dropped at the 64KB capture cap cannot invalidate it. Only a fail_if_header_matches (forbidden-header) PASS is truncation-sensitive: a forbidden header could be hiding in the dropped tail. The previous code failed conservatively on truncation for any configured matcher, so a required-only config wrongly reported content_failures on healthy endpoints with large header blocks. Add HeaderMatchers::has_forbidden_rules() and gate the probe's truncation fail-safe on it. Tests: required-only config passes despite truncation; a combined forbidden+required config still fails; has_forbidden_rules() unit cases. The two truncation e2e tests use a few ~40KB pad headers (any two exceed the cap, so at most one is admitted) to guarantee the required header is captured regardless of httplib's unordered header iteration order.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08bc80a4a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nfigured HttpClient only set CURLOPT_PROXY when req.proxy was non-empty, so with no proxy configured libcurl fell back to ambient http_proxy/https_proxy/all_proxy env vars. For a monitoring probe with an explicit `proxy` config this made results depend on the deployment/CI environment and defeated the per-target resolve/CONNECT_TO pin (an env proxy + CONNECT_TO switches curl to tunnel mode, asking the proxy to reach the pinned address instead of connecting locally). Set CURLOPT_PROXY to "" for no-proxy requests, which disables proxy use even when an env var is set. Applies to http and doh probes alike; the explicit-proxy path (and its error_msg credential redaction) is unchanged. Test: with a bogus http_proxy set, a no-proxy request to a local server still connects directly (transport_ok, 200). The test is hermetic — an RAII guard clears every proxy env var (http_proxy/https_proxy/all_proxy/no_proxy, both cases) first, so an ambient no_proxy listing loopback can't defeat it — and restores them on scope exit.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Summary
v3 of the netprobe HTTP probe (follow-up to #793): full response-assertion parity with the mainstream HTTP probers (cloudprober's HTTP validator, the OTel
httpcheckreceiver, and the Prometheusblackbox_exporterHTTP module), plus per-target resolution controls. Every new assertion failure reuses v2'scontent_failurescounter — no new metrics or signals.New response assertions (http test type, stream-level):
json_path+json_equals— evaluate an RFC 6901 JSON Pointer (e.g./data/status) against the JSON body;json_equalscompares the value's string form (so"200"matches numeric200).json_pathalone asserts presence.not_contains/body_not_matches_regex— the inverses of v2'sexpected_body/expected_body_regex.min_response_size_bytes/max_response_size_bytes— bound the response size (true downloaded size, exact even if the captured body was truncated).fail_if_header_matches/fail_if_header_not_matches— maps ofheader-name: value_regexover the response headers.max_last_modified_diff_secs— fail ifLast-Modifiedis older than N seconds (or absent).valid_http_versions— assert the negotiated HTTP version ∈ {1.0,1.1,2,3}.Per-target resolution controls (http + doh):
targets.<name>.ip_version(4/6 →CURLOPT_IPRESOLVE) andtargets.<name>.resolve(host:port:address→CURLOPT_RESOLVE).Design notes
HttpSample.content_checktri-state → no signal/POD/metric changes. Any assertion failure →content_failures(status failures stayhttp_status_failures).expected_body/regex,not_contains/regex,json_path) are skipped (with a warning) when the body exceededbody_check_max_bytes; size/header/version assertions are unaffected (size uses the true downloaded size).HTTP/status line and setsCURLOPT_SUPPRESS_CONNECT_HEADERS, so a redirect chain's intermediate-hop headers never leak into the match set (matches blackbox/cloudprober semantics).not_contains,body_not_matches_regex,json_equals, and the headervalue_regexs) join v2's redaction — scrubbed from bothinfo_jsonand the tap-config echo.nlohmann/json(no new grammar/dep — JSON Pointer is built in); dates viacurl_getdate; case-insensitive header matching via a portable helper — MSVC-portable, no POSIX-only APIs.Out of scope (v4+)
Per-target overrides of stream-level checks; CEL / full JSONPath; OAuth2;
basic_authsugar; responsecompression/enable_http3; DoH EDNS(0)/DNSSEC/per-target qname.Testing
unit-tests-visor-http-check(64) — JSON-pointer eval across value types, inverse body checks, header matchers (present/absent/multi-value, case-insensitive), http-date parsing, version mapping.unit-tests-visor-http-client(76) — response-header capture with per-response reset (redirect test),http_version, plus all v2 cases.unit-tests-input-netprobe(219, 2× stable) — config grammar/scoping/validation errors (secret-free), the v3 secret-scrub redaction test, and an e2e matrix: json_path pass/fail, not_contains, size bound,fail_if_header_matches/not_matches,valid_http_versions,max_last_modified_diff, per-targetresolve(pins a bogus host to the test server, provingCURLOPT_RESOLVE), and a 301→200 test proving header assertions see only the final response.unit-tests-handler-netprobe(100) — unchanged (no metric/signal changes).All suites pass locally (Release).
🤖 Generated with Claude Code