Skip to content

Commit b333001

Browse files
tmshortclaude
andcommitted
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 b333001

File tree

6 files changed

+666
-2
lines changed

6 files changed

+666
-2
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ $(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
258258
.PHONY: e2e
259259
e2e: #EXHELP Run the e2e tests.
260260
go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT)
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
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+
15+
"github.com/stretchr/testify/require"
16+
"sigs.k8s.io/controller-runtime/pkg/log"
17+
18+
httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
19+
)
20+
21+
// startRecordingProxy starts a plain-HTTP CONNECT proxy that tunnels HTTPS
22+
// connections and records the target host of each CONNECT request.
23+
func startRecordingProxy(proxied chan<- string) *httptest.Server {
24+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
25+
if r.Method != http.MethodConnect {
26+
http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed)
27+
return
28+
}
29+
proxied <- r.Host
30+
31+
dst, err := net.Dial("tcp", r.Host)
32+
if err != nil {
33+
http.Error(w, err.Error(), http.StatusBadGateway)
34+
return
35+
}
36+
defer dst.Close()
37+
38+
hj, ok := w.(http.Hijacker)
39+
if !ok {
40+
http.Error(w, "hijacking not supported", http.StatusInternalServerError)
41+
return
42+
}
43+
conn, _, err := hj.Hijack()
44+
if err != nil {
45+
http.Error(w, err.Error(), http.StatusInternalServerError)
46+
return
47+
}
48+
defer conn.Close()
49+
50+
if _, err = conn.Write([]byte("HTTP/1.1 200 Connection established\r\n\r\n")); err != nil {
51+
return
52+
}
53+
54+
done := make(chan struct{}, 2)
55+
tunnel := func(dst, src net.Conn) {
56+
defer func() { done <- struct{}{} }()
57+
_, _ = io.Copy(dst, src)
58+
}
59+
go tunnel(dst, conn)
60+
go tunnel(conn, dst)
61+
<-done
62+
}))
63+
}
64+
65+
// certPoolWatcherForTLSServer creates a CertPoolWatcher that trusts the given
66+
// TLS test server's certificate.
67+
func certPoolWatcherForTLSServer(t *testing.T, server *httptest.Server) *httputil.CertPoolWatcher {
68+
t.Helper()
69+
70+
dir := t.TempDir()
71+
certPath := filepath.Join(dir, "server.pem")
72+
73+
certDER := server.TLS.Certificates[0].Certificate[0]
74+
f, err := os.Create(certPath)
75+
require.NoError(t, err)
76+
require.NoError(t, pem.Encode(f, &pem.Block{Type: "CERTIFICATE", Bytes: certDER}))
77+
require.NoError(t, f.Close())
78+
79+
cpw, err := httputil.NewCertPoolWatcher(dir, log.FromContext(context.Background()))
80+
require.NoError(t, err)
81+
require.NotNil(t, cpw)
82+
t.Cleanup(cpw.Done)
83+
require.NoError(t, cpw.Start(context.Background()))
84+
return cpw
85+
}
86+
87+
// TestBuildHTTPClientProxyTunnelsConnections verifies end-to-end that the
88+
// HTTP client produced by BuildHTTPClient correctly tunnels HTTPS connections
89+
// through an HTTP CONNECT proxy.
90+
//
91+
// The test sets transport.Proxy directly to a fixed URL rather than relying on
92+
// HTTPS_PROXY env-var inspection: httptest servers bind to 127.0.0.1, which
93+
// http.ProxyFromEnvironment silently excludes from proxying, and env-var changes
94+
// within the same process are unreliable due to sync.Once caching. Setting
95+
// transport.Proxy explicitly exercises the same tunnelling code path that
96+
// HTTPS_PROXY triggers in production.
97+
func TestBuildHTTPClientProxyTunnelsConnections(t *testing.T) {
98+
targetServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
99+
w.WriteHeader(http.StatusOK)
100+
}))
101+
defer targetServer.Close()
102+
103+
proxied := make(chan string, 1)
104+
proxyServer := startRecordingProxy(proxied)
105+
defer proxyServer.Close()
106+
107+
proxyURL, err := url.Parse(proxyServer.URL)
108+
require.NoError(t, err)
109+
110+
cpw := certPoolWatcherForTLSServer(t, targetServer)
111+
client, err := httputil.BuildHTTPClient(cpw)
112+
require.NoError(t, err)
113+
114+
// Point the transport directly at our test proxy, bypassing the loopback
115+
// exclusion and env-var caching of http.ProxyFromEnvironment.
116+
transport, ok := client.Transport.(*http.Transport)
117+
require.True(t, ok)
118+
transport.Proxy = http.ProxyURL(proxyURL)
119+
120+
resp, err := client.Get(targetServer.URL)
121+
require.NoError(t, err)
122+
resp.Body.Close()
123+
124+
select {
125+
case host := <-proxied:
126+
require.Equal(t, targetServer.Listener.Addr().String(), host,
127+
"proxy must have received a CONNECT request for the target server address")
128+
default:
129+
t.Fatal("HTTPS connection to target server did not go through the proxy")
130+
}
131+
}
132+
133+
// TestBuildHTTPClientProxyBlocksWhenRejected verifies that when the proxy
134+
// rejects the CONNECT tunnel, the client request fails rather than silently
135+
// falling back to a direct connection.
136+
func TestBuildHTTPClientProxyBlocksWhenRejected(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+
// A proxy that returns 403 Forbidden for every CONNECT request.
143+
rejectingProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
144+
if r.Method == http.MethodConnect {
145+
http.Error(w, "proxy access denied", http.StatusForbidden)
146+
return
147+
}
148+
http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed)
149+
}))
150+
defer rejectingProxy.Close()
151+
152+
proxyURL, err := url.Parse(rejectingProxy.URL)
153+
require.NoError(t, err)
154+
155+
cpw := certPoolWatcherForTLSServer(t, targetServer)
156+
client, err := httputil.BuildHTTPClient(cpw)
157+
require.NoError(t, err)
158+
159+
transport, ok := client.Transport.(*http.Transport)
160+
require.True(t, ok)
161+
transport.Proxy = http.ProxyURL(proxyURL)
162+
163+
resp, err := client.Get(targetServer.URL)
164+
if resp != nil {
165+
resp.Body.Close()
166+
}
167+
require.Error(t, err, "request should fail when the proxy rejects the CONNECT tunnel")
168+
}

test/e2e/features/proxy.feature

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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 routes catalog traffic through a configured HTTPS proxy
40+
Given the "operator-controller" deployment is configured with HTTPS_PROXY pointing to a recording proxy
41+
When ClusterExtension is applied
42+
"""
43+
apiVersion: olm.operatorframework.io/v1
44+
kind: ClusterExtension
45+
metadata:
46+
name: ${NAME}
47+
spec:
48+
namespace: ${TEST_NAMESPACE}
49+
serviceAccount:
50+
name: olm-sa
51+
source:
52+
sourceType: Catalog
53+
catalog:
54+
packageName: test
55+
selector:
56+
matchLabels:
57+
"olm.operatorframework.io/metadata.name": test-catalog
58+
"""
59+
Then ClusterExtension is rolled out
60+
And ClusterExtension is available
61+
And the recording proxy received a CONNECT request for the catalogd service

test/e2e/steps/hooks.go

Lines changed: 30 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,24 @@ 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. Restore in reverse (LIFO) order
205+
// so that multiple patches to the same deployment unwind back to the true
206+
// original state.
207+
for i := len(sc.deploymentRestores) - 1; i >= 0; i-- {
208+
restore := sc.deploymentRestores[i]
209+
if err := restoreDeployment(restore); err != nil {
210+
logger.Info("Error restoring deployment", "deployment", restore.name, "namespace", restore.namespace, "error", err)
211+
}
212+
}
213+
185214
if err != nil {
186215
return ctx, err
187216
}

0 commit comments

Comments
 (0)