-
Notifications
You must be signed in to change notification settings - Fork 27
fix(cli): forward harness args after --, honor saved provider, bypass proxies on loopback probes #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(cli): forward harness args after --, honor saved provider, bypass proxies on loopback probes #107
Changes from all commits
dbe1e93
05388e7
d4f6316
649f812
dca3cb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Loopback readiness probes must ignore env proxies (HTTP_PROXY/NO_PROXY).""" | ||
|
|
||
| import threading | ||
| from http.server import BaseHTTPRequestHandler, HTTPServer | ||
|
|
||
| from switchyard.cli.launchers.launcher_runtime import wait_for_proxy_ready | ||
|
|
||
|
|
||
| class _HealthHandler(BaseHTTPRequestHandler): | ||
| def do_GET(self) -> None: | ||
| self.send_response(200) | ||
| self.end_headers() | ||
|
|
||
| def log_message(self, format: str, *args: object) -> None: # noqa: A002 | ||
| pass | ||
|
|
||
|
|
||
| def test_wait_for_proxy_ready_bypasses_env_proxy(monkeypatch): | ||
| """A configured HTTP_PROXY must not intercept the 127.0.0.1 health probe.""" | ||
| server = HTTPServer(("127.0.0.1", 0), _HealthHandler) | ||
| port = server.server_address[1] | ||
| thread = threading.Thread(target=server.serve_forever, daemon=True) | ||
| thread.start() | ||
|
|
||
| # Point env proxy at a dead port and remove any NO_PROXY exemption; before | ||
| # the fix the loopback probe would route here and fail. | ||
| monkeypatch.setenv("HTTP_PROXY", "http://127.0.0.1:9") | ||
| monkeypatch.setenv("http_proxy", "http://127.0.0.1:9") | ||
| monkeypatch.delenv("NO_PROXY", raising=False) | ||
| monkeypatch.delenv("no_proxy", raising=False) | ||
|
Comment on lines
+30
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Use a test-owned or dynamically allocated proxy endpoint. The test assumes 🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test asserts wait_for_proxy_ready returns True, which only happens if the loopback probe ignores the proxy and reaches the health server. Port 9 (discard) won't answer as a working HTTP forward proxy, so if the bypass ever regressed the probe would fail through :9 and the test would go red. What the test checks doesn't depend on :9 being unbound, so I'll keep the fixed port rather than spin up a stub proxy thread. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| try: | ||
| assert wait_for_proxy_ready(port, timeout_s=2) is True | ||
| finally: | ||
| server.shutdown() | ||
| server.server_close() | ||
Uh oh!
There was an error while loading. Please reload this page.