LOG-9383: Document NO_PROXY domain matching behavior difference between Vector and Go#3281
LOG-9383: Document NO_PROXY domain matching behavior difference between Vector and Go#3281vparfonov wants to merge 1 commit into
Conversation
…en Vector and Go Add a "Known Limitation" section documenting that Vector's no-proxy crate handles plain domain entries (e.g., example.com) differently than Go's httpproxy package. Plain domains in Vector match exactly, not as a prefix for subdomains. Include a comparison table and workaround: use both the domain and leading-dot form (e.g., apps.example.com,.apps.example.com) to match both the domain and its subdomains. This is tracked upstream at https://github.com/jdrouet/no-proxy Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@vparfonov: This pull request references LOG-9383 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.8.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis PR adds documentation explaining that Vector's ChangesHTTP Proxy Support Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/features/logforwarding/outputs/http-proxy-support.adoc (1)
221-222: ⚡ Quick winConsider adding the upstream tracking link.
The PR description mentions upstream tracking at https://github.com/jdrouet/no-proxy, but the NOTE doesn't include this link. Adding it would help users track progress on the upstream fix.
📝 Suggested enhancement
-NOTE: This is a known upstream issue in the `no-proxy` Rust crate used by Vector. Once the upstream fix is released and adopted by Vector, plain domain entries will match subdomains as expected. +NOTE: This is a known upstream issue in the `no-proxy` Rust crate used by Vector (tracked at https://github.com/jdrouet/no-proxy). Once the upstream fix is released and adopted by Vector, plain domain entries will match subdomains as expected.🤖 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 `@docs/features/logforwarding/outputs/http-proxy-support.adoc` around lines 221 - 222, Update the NOTE paragraph that mentions the upstream issue in the `no-proxy` Rust crate so it includes the upstream tracking link (https://github.com/jdrouet/no-proxy) — locate the NOTE text "This is a known upstream issue in the `no-proxy` Rust crate used by Vector..." and append or embed the GitHub URL so users can directly follow the upstream PR/issue.
🤖 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 `@docs/features/logforwarding/outputs/http-proxy-support.adoc`:
- Around line 172-199: The "Known Limitation: NO_PROXY Domain Matching Behavior"
section incorrectly claims the Rust no-proxy crate treats plain domains (e.g.,
example.com) as exact-match-only; verify the no-proxy crate behavior against its
docs/source and then update or remove this section accordingly: if the crate
treats example.com and .example.com equivalently (matching domain and subdomains
like Go's httpproxy), delete or rewrite the table rows and the workaround text
(the paragraph and lines referenced as the workaround) to reflect parity with
Go's behavior, and remove any admonitions or workaround instructions that assume
differing behavior; keep references to the no-proxy crate and Go's httpproxy in
the explanation so readers know what was verified.
---
Nitpick comments:
In `@docs/features/logforwarding/outputs/http-proxy-support.adoc`:
- Around line 221-222: Update the NOTE paragraph that mentions the upstream
issue in the `no-proxy` Rust crate so it includes the upstream tracking link
(https://github.com/jdrouet/no-proxy) — locate the NOTE text "This is a known
upstream issue in the `no-proxy` Rust crate used by Vector..." and append or
embed the GitHub URL so users can directly follow the upstream PR/issue.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fef79483-d692-49fa-a84d-2a7b19e6bf95
📒 Files selected for processing (1)
docs/features/logforwarding/outputs/http-proxy-support.adoc
| == Known Limitation: NO_PROXY Domain Matching Behavior | ||
|
|
||
| The Vector log collector uses a Rust library (`no-proxy` crate) to evaluate `NO_PROXY` entries. This library handles plain domain entries differently than Go's `httpproxy` package used by OpenShift and most Kubernetes components: | ||
|
|
||
| [cols="1,2,2",options="header"] | ||
| |=== | ||
| | NO_PROXY Entry | Go (OpenShift behavior) | Vector (collector behavior) | ||
|
|
||
| | `example.com` | ||
| | Matches `example.com` **and all subdomains** (e.g., `sub.example.com`) | ||
| | Matches `example.com` **only** (exact match) | ||
|
|
||
| | `.example.com` | ||
| | Matches subdomains only (e.g., `sub.example.com`), **not** `example.com` itself | ||
| | Matches subdomains only (e.g., `sub.example.com`), **not** `example.com` itself | ||
|
|
||
| | `10.0.0.1` | ||
| | Exact IP match | ||
| | Exact IP match | ||
|
|
||
| | `10.0.0.0/24` | ||
| | CIDR range match | ||
| | CIDR range match | ||
|
|
||
| | `*` | ||
| | Matches all hosts | ||
| | Matches all hosts | ||
| |=== |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Rust no-proxy crate NO_PROXY domain matching behavior for plain domains
💡 Result:
In the context of Rust crates that implement NO_PROXY parsing (such as the popular no-proxy crate [1] or the NoProxy implementation found in reqwest [2]), the matching behavior for plain domains is designed to be inclusive of subdomains. Specifically, for these implementations: 1. Domain Equivalence: A plain domain (e.g., example.com) and its variant with a leading dot (e.g., .example.com) are treated as equivalent [3][4][2]. 2. Subdomain Matching: Both example.com and .example.com will match the domain itself as well as all of its subdomains (e.g., sub.example.com, a.b.example.com) [3][4][2]. This behavior is consistent with common expectations for NO_PROXY handling in many modern libraries, where the goal is to simplify configuration by ensuring that a single entry covers a domain and its entire hierarchy [3][2]. If you are using a specific crate, you can verify this behavior by checking its documentation for NoProxy::from_string or NoProxy::from_env, which typically outlines these rules explicitly [3][4][2]. For example, the no-proxy crate documentation confirms that it parses and evaluates these configurations to match both the specified domain and its subdomains [1][5].
Citations:
- 1: https://crates.io/crates/no-proxy
- 2: https://docs.rs/reqwest/latest/reqwest/struct.NoProxy.html
- 3: https://docs.rs/rquest/latest/rquest/struct.NoProxy.html
- 4: https://docs.rs/reqwest-patched/latest/reqwest_patched/struct.NoProxy.html
- 5: https://docs.rs/no-proxy/latest/no_proxy/struct.NoProxy.html
🌐 Web query:
Go httpproxy package NO_PROXY domain matching behavior for plain domains
💡 Result:
In Go's net/http (via the golang.org/x/net/http/httpproxy package), the NO_PROXY environment variable defines a comma-separated list of hosts that should be excluded from proxying [1][2]. The matching behavior for plain domain names is as follows: 1. Domain Matching: A plain domain name (e.g., example.com) matches that specific domain and all of its subdomains [1][3]. For example, example.com will match both example.com and sub.example.com [1][3]. 2. Leading Dot: If a domain name is specified with a leading dot (e.g., .example.com), it matches subdomains only, not the domain itself [1][3]. For example, .example.com matches sub.example.com but does not match example.com [1][3]. 3. Implementation Detail: Internally, the package processes these rules by checking if the request host ends with the specified domain string [1][3]. If a domain is provided without a leading dot, the implementation effectively treats it as a match for the host itself or any host that has the specified domain as a suffix [1][3]. 4. Special Case: A single asterisk (*) in the NO_PROXY list indicates that no proxying should be performed for any request [1][3]. It is important to note that NO_PROXY matching is performed against the host portion of the request URL [1][3]. The package does not perform DNS lookups to resolve hostnames to IP addresses when evaluating these rules [4]. If you need to exclude IP addresses, you should provide them directly or use CIDR notation, which the package also supports [1][5].
Citations:
- 1: https://github.com/golang/net/blob/master/http/httpproxy/proxy.go
- 2: https://smartbft-go.github.io/godoc/pkg/golang.org/x/net/http/httpproxy/index.html
- 3: https://go.dev/src/vendor/golang.org/x/net/http/httpproxy/proxy.go?m=text
- 4: https://stackoverflow.com/questions/75370020/is-there-any-way-to-use-regular-expression-in-no-proxy-environment-variable
- 5: net/http: considering supporting CIDR notation in no_proxy env variable golang/go#16704
CRITICAL: The Rust no-proxy crate behavior claim is incorrect.
The comparison table states that Vector (using Rust no-proxy crate) treats plain domains like example.com as exact-match-only, differing from Go's behavior. However, according to the no-proxy crate documentation, example.com and .example.com are treated as equivalent and both match the domain and all subdomains—identical to Go's httpproxy behavior. This means the documented "Known Limitation" does not actually exist.
The incorrect claim undermines the entire section and the workaround guidance (lines 203-220). If Vector and Go handle plain domains identically, the workaround may be unnecessary. Remove or correct this section based on actual library behavior verification.
🤖 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 `@docs/features/logforwarding/outputs/http-proxy-support.adoc` around lines 172
- 199, The "Known Limitation: NO_PROXY Domain Matching Behavior" section
incorrectly claims the Rust no-proxy crate treats plain domains (e.g.,
example.com) as exact-match-only; verify the no-proxy crate behavior against its
docs/source and then update or remove this section accordingly: if the crate
treats example.com and .example.com equivalently (matching domain and subdomains
like Go's httpproxy), delete or rewrite the table rows and the workaround text
(the paragraph and lines referenced as the workaround) to reflect parity with
Go's behavior, and remove any admonitions or workaround instructions that assume
differing behavior; keep references to the no-proxy crate and Go's httpproxy in
the explanation so readers know what was verified.
There was a problem hiding this comment.
@vparfonov if this is correct then that means there is either a documentation error in the crate or an implementation error. Let's figure out which is correct
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Upstream no-proxy crate v0.3.6
File: src/lib.rs, line 111 in jdrouet/no-proxy
Self::Plain(source) => source == value
This is exact match only, no subdomain matching.
So "Known Limitation" is correct at the moment
|
@vparfonov: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcantrill, vparfonov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
3 participants
Description
Add a "Known Limitation" section documenting that Vector's
no-proxycrate handles plain domain entries (e.g., example.com) differently than Go'shttpproxypackage. Plain domains in Vector match exactly, not as a prefix for subdomains. Include a comparison table and workaround: use both the domain and leading-dot form (e.g.,apps.example.com,.apps.example.com) to match both the domain and its subdomains.This is tracked upstream at https://github.com/jdrouet/no-proxy
/cc @Clee2691
/assign @jcantrill @r2d2rnd
Links
Summary by CodeRabbit