🌱 OPRUN-4573: tests: add unit and e2e tests for HTTPS_PROXY support#2654
🌱 OPRUN-4573: tests: add unit and e2e tests for HTTPS_PROXY support#2654tmshort wants to merge 1 commit intooperator-framework:mainfrom
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Adds unit + e2e test coverage around HTTPS proxying for catalog traffic, including new godog steps to reconfigure controller deployments and a lightweight “recording proxy” used to assert CONNECT tunneling.
Changes:
- Register new proxy-related godog steps and add scenario cleanup to revert modified deployments / stop the proxy.
- Add new e2e feature scenarios for “dead proxy blocks fetch” and “recording proxy observes CONNECT to catalogd”.
- Add unit tests intended to validate HTTPS CONNECT proxy tunneling behavior for
BuildHTTPClient.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/steps/steps.go | Registers proxy step definitions in the e2e step registry. |
| test/e2e/steps/proxy_steps.go | Implements deployment env patching helpers and an in-process CONNECT recording proxy. |
| test/e2e/steps/hooks.go | Adds scenario context fields and cleanup to stop proxy + restore deployments. |
| test/e2e/features/proxy.feature | Adds two e2e scenarios covering dead proxy and recording proxy routing assertions. |
| internal/shared/util/http/httputil_test.go | Adds unit tests around proxy CONNECT tunneling behavior for BuildHTTPClient. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tmshort
force-pushed
the
proxy-tests
branch
2 times, most recently
from
43f7355 to
699c428
Compare
There was a problem hiding this comment.
Pull request overview
Adds new unit and end-to-end coverage intended to validate HTTPS proxy (CONNECT tunneling) behavior for outbound catalog fetches, including scenarios where the proxy is unreachable or where traffic must be observed flowing through a proxy.
Changes:
- Registers new Godog step definitions for proxy-related E2E scenarios and adds per-scenario cleanup to restore modified deployments.
- Adds a new E2E feature file covering “dead proxy” and “recording proxy” scenarios for operator-controller catalog requests.
- Adds unit tests around
BuildHTTPClientproxy CONNECT tunneling behavior using an in-test CONNECT proxy.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/steps/steps.go | Registers the new proxy-related step set. |
| test/e2e/steps/proxy_steps.go | Implements an in-process CONNECT proxy + deployment env patching helpers and steps. |
| test/e2e/steps/hooks.go | Tracks/restores deployment env changes and stops the in-process proxy during scenario cleanup. |
| test/e2e/features/proxy.feature | Adds E2E scenarios validating failure via dead proxy and success/verification via recording proxy. |
| internal/shared/util/http/httputil_test.go | Adds unit tests for HTTPS CONNECT tunneling through a proxy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds automated coverage for outbound HTTPS proxying of operator-controller’s catalog fetches, using both unit-level CONNECT proxy tunneling tests and end-to-end scenarios that configure HTTPS_PROXY on the running deployment.
Changes:
- Register new e2e proxy-related Godog steps and add feature scenarios validating proxy failure/success cases.
- Add e2e infrastructure: an in-process recording CONNECT proxy plus deployment env patch/restore helpers.
- Add unit tests that exercise HTTPS-over-CONNECT tunneling behavior for the HTTP client used for catalog traffic.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/steps/steps.go | Registers proxy steps with the existing Godog scenario context. |
| test/e2e/steps/proxy_steps.go | Implements recording CONNECT proxy + deployment patch/restore helpers + step definitions. |
| test/e2e/steps/hooks.go | Extends scenario context and cleanup to restore patched deployments and stop proxy. |
| test/e2e/features/proxy.feature | Adds two e2e scenarios for dead-proxy failure and recording-proxy success. |
| internal/shared/util/http/httputil_test.go | Adds unit tests for HTTPS CONNECT tunneling and proxy rejection behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed) | ||
| return | ||
| } | ||
| proxied <- r.Host |
| // Point the transport directly at our test proxy, bypassing the loopback | ||
| // exclusion and env-var caching of http.ProxyFromEnvironment. | ||
| transport, ok := client.Transport.(*http.Transport) | ||
| require.True(t, ok) | ||
| transport.Proxy = http.ProxyURL(proxyURL) |
There was a problem hiding this comment.
Pull request overview
This PR adds coverage for HTTPS proxy behavior used by the operator-controller when fetching catalog content from catalogd, and ensures the shared HTTP client respects proxy environment variables.
Changes:
- Update
BuildHTTPClientto honorHTTPS_PROXY/NO_PROXYviahttp.ProxyFromEnvironment. - Add unit tests validating CONNECT tunneling through a proxy and failure behavior when CONNECT is rejected.
- Add new e2e feature + step implementations to exercise proxy behavior scenarios (dead proxy + recording proxy).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/shared/util/http/httputil.go |
Sets Transport.Proxy so runtime proxy env vars are honored. |
internal/shared/util/http/httputil_test.go |
Adds unit tests for CONNECT proxy tunneling and rejection behavior. |
test/e2e/features/proxy.feature |
Introduces e2e scenarios for HTTPS proxy behavior. |
test/e2e/steps/proxy_steps.go |
Implements e2e steps including a recording CONNECT proxy and deployment env patching helpers. |
test/e2e/steps/hooks.go |
Adds scenario cleanup for stopping the proxy and restoring patched deployments. |
test/e2e/steps/steps.go |
Registers the new proxy-related steps. |
Makefile |
Increases E2E timeout to accommodate added scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2654 +/- ##
==========================================
+ Coverage 68.89% 69.01% +0.11%
==========================================
Files 141 141
Lines 10009 10010 +1
==========================================
+ Hits 6896 6908 +12
+ Misses 2596 2587 -9
+ Partials 517 515 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds HTTPS proxy support validation across unit and end-to-end tests, and updates the shared HTTP client builder to honor HTTPS_PROXY/NO_PROXY at runtime.
Changes:
- Update
BuildHTTPClientto setTransport.Proxy = http.ProxyFromEnvironment. - Add unit tests that validate HTTPS CONNECT proxy tunneling behavior and failure modes.
- Add e2e proxy scenarios plus step implementations (including an in-process recording CONNECT proxy) and Makefile support for passing Godog args.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/steps/steps.go | Registers the new proxy-related Godog steps. |
| test/e2e/steps/proxy_steps.go | Implements proxy configuration steps, an in-process recording CONNECT proxy, and deployment patch/restore helpers. |
| test/e2e/steps/hooks.go | Adds scenario cleanup for stopping the recording proxy and restoring patched deployments. |
| test/e2e/features/proxy.feature | Adds e2e scenarios that assert proxy behavior (dead proxy + recording proxy). |
| internal/shared/util/http/httputil_test.go | Adds unit tests for proxy configuration, tunneling through CONNECT, and rejection handling. |
| internal/shared/util/http/httputil.go | Ensures the built transport honors proxy env vars. |
| Makefile | Adds GODOG_ARGS passthrough for e2e runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tmshort
changed the title
Unit tests verify that BuildHTTPClient correctly tunnels HTTPS connections through a proxy and fails when the proxy rejects the CONNECT request. E2e tests cover two scenarios: a dead proxy that blocks catalog fetches (asserted via "proxyconnect" in the Retrying condition), and a live recording proxy that asserts catalog traffic actually routes through it via CONNECT. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
Unit tests verify that BuildHTTPClient correctly tunnels HTTPS connections through a proxy and fails when the proxy rejects the CONNECT request.
E2e tests cover two scenarios: a dead proxy that blocks catalog fetches (asserted via "proxyconnect" in the Retrying condition), and a live recording proxy that asserts catalog traffic actually routes through it via CONNECT.
Description
Reviewer Checklist