Skip to content

Commit 1a84235

Browse files
tmshortclaude
tmshort
and
claude
committed
tests: add unit and e2e tests for HTTPS_PROXY support
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>
1 parent 29debc7 commit 1a84235

File tree

7 files changed

+722
-2
lines changed

7 files changed

+722
-2
lines changed

‎Makefile‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,10 @@ $(eval $(call install-sh,standard,operator-controller-standard.yaml))
255255
test: manifests generate fmt lint test-unit test-e2e test-regression #HELP Run all tests.
256256

257257
E2E_TIMEOUT ?= 10m
258+
GODOG_ARGS ?=
258259
.PHONY: e2e
259260
e2e: #EXHELP Run the e2e tests.
260-
go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT)
261+
go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT) $(if $(GODOG_ARGS),-args $(GODOG_ARGS))
261262

262263
E2E_REGISTRY_NAME := docker-registry
263264
E2E_REGISTRY_NAMESPACE := operator-controller-e2e

‎internal/shared/util/http/httputil.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ func BuildHTTPClient(cpw *CertPoolWatcher) (*http.Client, error) {
2020
}
2121
tlsTransport := &http.Transport{
2222
TLSClientConfig: tlsConfig,
23+
// Proxy must be set explicitly; a nil Proxy field means "no proxy" and
24+
// ignores HTTPS_PROXY/NO_PROXY env vars. Only http.DefaultTransport sets
25+
// this by default; custom transports must opt in.
26+
Proxy: http.ProxyFromEnvironment,
2327
}
2428
httpClient.Transport = tlsTransport
2529

‎internal/shared/util/http/httputil_test.go‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
package http_test
2+
3+
import (
4+
"context"
5+
"encoding/pem"
6+
"io"
7+
"net"
8+
"net/http"
9+
"net/http/httptest"
10+
"net/url"
11+
"os"
12+
"path/filepath"
13+
"testing"
14+
"time"
15+
16+
"github.com/stretchr/testify/require"
17+
"sigs.k8s.io/controller-runtime/pkg/log"
18+
19+
httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
20+
)
21+
22+
// startRecordingProxy starts a plain-HTTP CONNECT proxy that tunnels HTTPS
23+
// connections and records the target host of each CONNECT request.
24+
func startRecordingProxy(proxied chan<- string) *httptest.Server {
25+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
26+
if r.Method != http.MethodConnect {
27+
http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed)
28+
return
29+
}
30+
// Non-blocking: if there are unexpected extra CONNECT requests (retries,
31+
// parallel connections) we record the first one and drop the rest rather
32+
// than blocking the proxy handler goroutine.
33+
select {
34+
case proxied <- r.Host:
35+
default:
36+
}
37+
38+
dst, err := net.Dial("tcp", r.Host)
39+
if err != nil {
40+
http.Error(w, err.Error(), http.StatusBadGateway)
41+
return
42+
}
43+
defer dst.Close()
44+
45+
hj, ok := w.(http.Hijacker)
46+
if !ok {
47+
http.Error(w, "hijacking not supported", http.StatusInternalServerError)
48+
return
49+
}
50+
conn, _, err := hj.Hijack()
51+
if err != nil {
52+
http.Error(w, err.Error(), http.StatusInternalServerError)
53+
return
54+
}
55+
defer conn.Close()
56+
57+
if _, err = conn.Write([]byte("HTTP/1.1 200 Connection established\r\n\r\n")); err != nil {
58+
return
59+
}
60+
61+
done := make(chan struct{}, 2)
62+
tunnel := func(dst, src net.Conn) {
63+
defer func() { done <- struct{}{} }()
64+
_, _ = io.Copy(dst, src)
65+
}
66+
go tunnel(dst, conn)
67+
go tunnel(conn, dst)
68+
<-done
69+
}))
70+
}
71+
72+
// certPoolWatcherForTLSServer creates a CertPoolWatcher that trusts the given
73+
// TLS test server's certificate.
74+
func certPoolWatcherForTLSServer(t *testing.T, server *httptest.Server) *httputil.CertPoolWatcher {
75+
t.Helper()
76+
77+
dir := t.TempDir()
78+
certPath := filepath.Join(dir, "server.pem")
79+
80+
certDER := server.TLS.Certificates[0].Certificate[0]
81+
f, err := os.Create(certPath)
82+
require.NoError(t, err)
83+
require.NoError(t, pem.Encode(f, &pem.Block{Type: "CERTIFICATE", Bytes: certDER}))
84+
require.NoError(t, f.Close())
85+
86+
cpw, err := httputil.NewCertPoolWatcher(dir, log.FromContext(context.Background()))
87+
require.NoError(t, err)
88+
require.NotNil(t, cpw)
89+
t.Cleanup(cpw.Done)
90+
require.NoError(t, cpw.Start(context.Background()))
91+
return cpw
92+
}
93+
94+
// TestBuildHTTPClientTransportUsesProxyFromEnvironment verifies that the
95+
// transport returned by BuildHTTPClient has Proxy set to http.ProxyFromEnvironment
96+
// so that HTTPS_PROXY and NO_PROXY env vars are honoured at runtime.
97+
func TestBuildHTTPClientTransportUsesProxyFromEnvironment(t *testing.T) {
98+
// Use system certs (empty dir) — we only need a valid CertPoolWatcher.
99+
cpw, err := httputil.NewCertPoolWatcher("", log.FromContext(context.Background()))
100+
require.NoError(t, err)
101+
t.Cleanup(cpw.Done)
102+
require.NoError(t, cpw.Start(context.Background()))
103+
104+
client, err := httputil.BuildHTTPClient(cpw)
105+
require.NoError(t, err)
106+
107+
transport, ok := client.Transport.(*http.Transport)
108+
require.True(t, ok)
109+
require.NotNil(t, transport.Proxy,
110+
"BuildHTTPClient must set transport.Proxy so that HTTPS_PROXY env vars are respected; "+
111+
"a nil Proxy field means no proxy regardless of environment")
112+
}
113+
114+
// TestBuildHTTPClientProxyTunnelsConnections verifies end-to-end that the
115+
// HTTP client produced by BuildHTTPClient correctly tunnels HTTPS connections
116+
// through an HTTP CONNECT proxy.
117+
//
118+
// The test overrides transport.Proxy with http.ProxyURL rather than relying on
119+
// HTTPS_PROXY: httptest servers bind to 127.0.0.1, which http.ProxyFromEnvironment
120+
// silently excludes from proxying, and env-var changes within the same process
121+
// are unreliable due to sync.Once caching. Using http.ProxyURL directly exercises
122+
// the same tunnelling code path that HTTPS_PROXY triggers in production.
123+
func TestBuildHTTPClientProxyTunnelsConnections(t *testing.T) {
124+
targetServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
125+
w.WriteHeader(http.StatusOK)
126+
}))
127+
defer targetServer.Close()
128+
129+
proxied := make(chan string, 1)
130+
proxyServer := startRecordingProxy(proxied)
131+
defer proxyServer.Close()
132+
133+
proxyURL, err := url.Parse(proxyServer.URL)
134+
require.NoError(t, err)
135+
136+
cpw := certPoolWatcherForTLSServer(t, targetServer)
137+
client, err := httputil.BuildHTTPClient(cpw)
138+
require.NoError(t, err)
139+
140+
// Point the transport directly at our test proxy, bypassing the loopback
141+
// exclusion and env-var caching of http.ProxyFromEnvironment.
142+
transport, ok := client.Transport.(*http.Transport)
143+
require.True(t, ok)
144+
transport.Proxy = http.ProxyURL(proxyURL)
145+
146+
resp, err := client.Get(targetServer.URL)
147+
require.NoError(t, err)
148+
resp.Body.Close()
149+
150+
select {
151+
case host := <-proxied:
152+
require.Equal(t, targetServer.Listener.Addr().String(), host,
153+
"proxy must have received a CONNECT request for the target server address")
154+
case <-time.After(5 * time.Second):
155+
t.Fatal("HTTPS connection to target server did not go through the proxy")
156+
}
157+
}
158+
159+
// TestBuildHTTPClientProxyBlocksWhenRejected verifies that when the proxy
160+
// rejects the CONNECT tunnel, the client request fails rather than silently
161+
// falling back to a direct connection.
162+
func TestBuildHTTPClientProxyBlocksWhenRejected(t *testing.T) {
163+
targetServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
164+
w.WriteHeader(http.StatusOK)
165+
}))
166+
defer targetServer.Close()
167+
168+
// A proxy that returns 403 Forbidden for every CONNECT request.
169+
rejectingProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
170+
if r.Method == http.MethodConnect {
171+
http.Error(w, "proxy access denied", http.StatusForbidden)
172+
return
173+
}
174+
http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed)
175+
}))
176+
defer rejectingProxy.Close()
177+
178+
proxyURL, err := url.Parse(rejectingProxy.URL)
179+
require.NoError(t, err)
180+
181+
cpw := certPoolWatcherForTLSServer(t, targetServer)
182+
client, err := httputil.BuildHTTPClient(cpw)
183+
require.NoError(t, err)
184+
185+
transport, ok := client.Transport.(*http.Transport)
186+
require.True(t, ok)
187+
transport.Proxy = http.ProxyURL(proxyURL)
188+
189+
resp, err := client.Get(targetServer.URL)
190+
if resp != nil {
191+
resp.Body.Close()
192+
}
193+
require.Error(t, err, "request should fail when the proxy rejects the CONNECT tunnel")
194+
}

‎test/e2e/features/proxy.feature‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
Feature: HTTPS proxy support for outbound catalog requests
2+
3+
OLM's operator-controller fetches catalog data from catalogd over HTTPS.
4+
When HTTPS_PROXY is set in the operator-controller's environment, all
5+
outbound HTTPS requests must be routed through the configured proxy.
6+
7+
Background:
8+
Given OLM is available
9+
And ClusterCatalog "test" serves bundles
10+
And ServiceAccount "olm-sa" with needed permissions is available in test namespace
11+
12+
@HTTPProxy
13+
Scenario: operator-controller respects HTTPS_PROXY when fetching catalog data
14+
Given the "operator-controller" deployment is configured with HTTPS_PROXY "http://127.0.0.1:39999"
15+
When ClusterExtension is applied
16+
"""
17+
apiVersion: olm.operatorframework.io/v1
18+
kind: ClusterExtension
19+
metadata:
20+
name: ${NAME}
21+
spec:
22+
namespace: ${TEST_NAMESPACE}
23+
serviceAccount:
24+
name: olm-sa
25+
source:
26+
sourceType: Catalog
27+
catalog:
28+
packageName: test
29+
selector:
30+
matchLabels:
31+
"olm.operatorframework.io/metadata.name": test-catalog
32+
"""
33+
Then ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
34+
"""
35+
proxyconnect
36+
"""
37+
38+
@HTTPProxy
39+
Scenario: operator-controller sends catalog requests through a configured HTTPS proxy
40+
# The recording proxy runs on the host and cannot route to in-cluster service
41+
# addresses, so it responds 502 after recording the CONNECT. This is
42+
# intentional: the scenario only verifies that operator-controller respects
43+
# HTTPS_PROXY and sends catalog fetches through the proxy, not that the full
44+
# end-to-end request succeeds.
45+
Given the "operator-controller" deployment is configured with HTTPS_PROXY pointing to a recording proxy
46+
When ClusterExtension is applied
47+
"""
48+
apiVersion: olm.operatorframework.io/v1
49+
kind: ClusterExtension
50+
metadata:
51+
name: ${NAME}
52+
spec:
53+
namespace: ${TEST_NAMESPACE}
54+
serviceAccount:
55+
name: olm-sa
56+
source:
57+
sourceType: Catalog
58+
catalog:
59+
packageName: test
60+
selector:
61+
matchLabels:
62+
"olm.operatorframework.io/metadata.name": test-catalog
63+
"""
64+
Then the recording proxy received a CONNECT request for the catalogd service

‎test/e2e/steps/hooks.go‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ type resource struct {
2727
namespace string
2828
}
2929

30+
// deploymentRestore records the original state of a deployment container so
31+
// it can be rolled back after a test that modifies deployment configuration.
32+
type deploymentRestore struct {
33+
name string // deployment name
34+
namespace string
35+
containerName string
36+
originalEnv []string // "NAME=VALUE" pairs
37+
}
38+
3039
type scenarioContext struct {
3140
id string
3241
namespace string
@@ -39,7 +48,9 @@ type scenarioContext struct {
3948
metricsResponse map[string]string
4049
leaderPods map[string]string // component name -> leader pod name
4150

42-
extensionObjects []client.Object
51+
extensionObjects []client.Object
52+
deploymentRestores []deploymentRestore
53+
proxy *recordingProxy
4354
}
4455

4556
// GatherClusterExtensionObjects collects all resources related to the ClusterExtension container in
@@ -182,6 +193,23 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context
182193
_ = p.Kill()
183194
}
184195
}
196+
197+
// Stop the in-process recording proxy if one was started.
198+
if sc.proxy != nil {
199+
sc.proxy.stop()
200+
}
201+
202+
// Restore any deployments that were modified during the scenario. This runs
203+
// unconditionally (even on test failure) so that a misconfigured deployment
204+
// does not bleed into subsequent scenarios. configureDeploymentProxy
205+
// deduplicates entries, so each deployment appears at most once.
206+
for i := len(sc.deploymentRestores) - 1; i >= 0; i-- {
207+
restore := sc.deploymentRestores[i]
208+
if err := restoreDeployment(restore); err != nil {
209+
logger.Info("Error restoring deployment", "deployment", restore.name, "namespace", restore.namespace, "error", err)
210+
}
211+
}
212+
185213
if err != nil {
186214
return ctx, err
187215
}

0 commit comments

Comments
 (0)