Skip to content

Commit 967be14

Browse files
tmshortclaude
andauthored
tests: add unit and e2e tests for HTTPS_PROXY support (#2654)
- Set Proxy: http.ProxyFromEnvironment on the custom http.Transport in BuildHTTPClient so HTTPS_PROXY/NO_PROXY env vars are honoured - Add unit tests verifying the transport uses env-based proxy, tunnels connections through an HTTP CONNECT proxy, and fails when the proxy rejects the tunnel - Add an in-process recording proxy and deployment patch helpers to the e2e step library - Add two @httpproxy e2e scenarios: one verifying operator-controller blocks catalog fetches when the proxy is unreachable, one verifying CONNECT requests are routed through a configured proxy Signed-off-by: Todd Short <tshort@redhat.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c8585e9 commit 967be14

8 files changed

Lines changed: 720 additions & 24 deletions

File tree

Makefile

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

257-
E2E_TIMEOUT ?= 10m
257+
E2E_TIMEOUT ?= 15m
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
export CLUSTER_REGISTRY_HOST := docker-registry.operator-controller-e2e.svc:5000
263264
.PHONY: extension-developer-e2e

internal/shared/util/http/httputil.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@ func BuildHTTPClient(cpw *CertPoolWatcher) (*http.Client, error) {
1818
RootCAs: pool,
1919
MinVersion: tls.VersionTLS12,
2020
}
21-
tlsTransport := &http.Transport{
21+
httpClient.Transport = &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
}
24-
httpClient.Transport = tlsTransport
2528

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

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" component 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" component 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: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@ type resource struct {
2828
namespace string
2929
}
3030

31-
// deploymentRestore records the original container args of a deployment so that
32-
// it can be patched back to its pre-test state during scenario cleanup.
31+
// deploymentRestore records the original state of a deployment so it can be
32+
// rolled back after a test that modifies deployment configuration.
3333
type deploymentRestore struct {
34-
namespace string
35-
deploymentName string
36-
originalArgs []string
34+
name string // deployment name
35+
namespace string
36+
containerName string // container to patch (for env var restores)
37+
patchedArgs bool // true when container args were modified (for TLS profile patches)
38+
originalArgs []string // original container args; may be nil if args were unset
39+
originalEnv []string // original env vars as "NAME=VALUE" (for proxy patches)
3740
}
3841

3942
type scenarioContext struct {
@@ -48,8 +51,8 @@ type scenarioContext struct {
4851
metricsResponse map[string]string
4952
leaderPods map[string]string // component name -> leader pod name
5053
deploymentRestores []deploymentRestore
51-
52-
extensionObjects []client.Object
54+
extensionObjects []client.Object
55+
proxy *recordingProxy
5356
}
5457

5558
// GatherClusterExtensionObjects collects all resources related to the ClusterExtension container in
@@ -190,19 +193,27 @@ func stderrOutput(err error) string {
190193

191194
func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context.Context, error) {
192195
sc := scenarioCtx(ctx)
193-
// Always restore deployments whose args were modified during the scenario,
194-
// even when the scenario failed, so that a misconfigured TLS profile does
195-
// not leak into subsequent scenarios. Restore in reverse order so that
196-
// multiple patches to the same deployment unwind back to the true original.
196+
// Stop the in-process recording proxy if one was started.
197+
if sc.proxy != nil {
198+
sc.proxy.stop()
199+
}
200+
201+
// Restore any deployments that were modified during the scenario. Runs
202+
// unconditionally (even on failure) to prevent a misconfigured deployment
203+
// from bleeding into subsequent scenarios. Restored in LIFO order so that
204+
// multiple patches to the same deployment unwind to the true original.
197205
for i := len(sc.deploymentRestores) - 1; i >= 0; i-- {
198206
dr := sc.deploymentRestores[i]
199-
if err2 := patchDeploymentArgs(dr.namespace, dr.deploymentName, dr.originalArgs); err2 != nil {
200-
logger.Info("Error restoring deployment args", "name", dr.deploymentName, "error", err2)
201-
continue
207+
if dr.patchedArgs {
208+
if err2 := patchDeploymentArgs(dr.namespace, dr.name, dr.originalArgs); err2 != nil {
209+
logger.Info("Error restoring deployment args", "name", dr.name, "error", err2)
210+
} else if _, err2 := k8sClient("rollout", "status", "-n", dr.namespace,
211+
fmt.Sprintf("deployment/%s", dr.name), "--timeout=2m"); err2 != nil {
212+
logger.Info("Timeout waiting for deployment rollout after restore", "name", dr.name)
213+
}
202214
}
203-
if _, err2 := k8sClient("rollout", "status", "-n", dr.namespace,
204-
fmt.Sprintf("deployment/%s", dr.deploymentName), "--timeout=2m"); err2 != nil {
205-
logger.Info("Timeout waiting for deployment rollout after restore", "name", dr.deploymentName)
215+
if err2 := restoreDeployment(dr); err2 != nil {
216+
logger.Info("Error restoring deployment env", "deployment", dr.name, "namespace", dr.namespace, "error", err2)
206217
}
207218
}
208219

0 commit comments

Comments
 (0)