Skip to content

Commit ccddfe3

Browse files
committed
Add shared test helpers and improve test coverage
Extract shared test helpers into pkg/internal/transporttest: UnwrapTransport, MakeSelfSignedCA, MustParseURL. Replace local copies in transport, proxy, and observe_idps test files. Add TestBuildIDPTransport to verify that with the feature gate enabled and a component proxy with trusted CA configured, the IDP transport builder wires the proxy function and loads the CA. Add Test_validateIdPConnectivity_hashDedup to verify hash-based dedup: successful validation saves the hash and skips redundant checks; failed validation does not save the hash, allowing retry. Enable the AuthenticationComponentProxy feature gate in TestObserveIdentityProviders to exercise the gate-enabled path with no proxy configured (no-op fallback to environment).
1 parent 6e856a3 commit ccddfe3

5 files changed

Lines changed: 205 additions & 78 deletions

File tree

pkg/controllers/common/proxy_test.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
operatorv1 "github.com/openshift/api/operator/v1"
1919
operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1"
2020
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
21+
22+
"github.com/openshift/cluster-authentication-operator/pkg/internal/transporttest"
2123
)
2224

2325
var (
@@ -236,8 +238,8 @@ func TestResolvedProxy_IsProxyConfigured(t *testing.T) {
236238
}
237239

238240
func TestResolvedProxy_ProxyFunc(t *testing.T) {
239-
httpsURL := mustParseURL(t, "https://idp.example.com/.well-known/openid-configuration")
240-
httpURL := mustParseURL(t, "http://idp.example.com/callback")
241+
httpsURL := transporttest.MustParseURL(t, "https://idp.example.com/.well-known/openid-configuration")
242+
httpURL := transporttest.MustParseURL(t, "http://idp.example.com/callback")
241243

242244
tests := []struct {
243245
name string
@@ -337,15 +339,6 @@ func TestResolvedProxy_ProxyFunc(t *testing.T) {
337339
}
338340
}
339341

340-
func mustParseURL(t *testing.T, raw string) *url.URL {
341-
t.Helper()
342-
u, err := url.Parse(raw)
343-
if err != nil {
344-
t.Fatalf("failed to parse URL %q: %v", raw, err)
345-
}
346-
return u
347-
}
348-
349342
func TestResolveProxy_FeatureGateError(t *testing.T) {
350343
ch := make(chan struct{})
351344
fga := featuregates.NewHardcodedFeatureGateAccessForTesting(nil, nil, ch, errors.New("not yet observed"))

pkg/controllers/configobservation/oauth/observe_idps_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@ package oauth
22

33
import (
44
"fmt"
5+
"net/http"
56
"testing"
67
"time"
78

89
"github.com/google/go-cmp/cmp"
10+
"github.com/stretchr/testify/require"
911

1012
corev1 "k8s.io/api/core/v1"
1113
"k8s.io/apimachinery/pkg/api/equality"
@@ -16,13 +18,15 @@ import (
1618

1719
configv1 "github.com/openshift/api/config/v1"
1820
"github.com/openshift/api/features"
21+
operatorv1 "github.com/openshift/api/operator/v1"
1922
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
2023
operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1"
2124
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
2225
"github.com/openshift/library-go/pkg/operator/events"
2326
"github.com/openshift/library-go/pkg/operator/resourcesynccontroller"
2427

2528
"github.com/openshift/cluster-authentication-operator/pkg/controllers/configobservation"
29+
"github.com/openshift/cluster-authentication-operator/pkg/internal/transporttest"
2630
)
2731

2832
type mockResourceSyncer struct {
@@ -232,6 +236,57 @@ func TestObserveIdentityProviders(t *testing.T) {
232236
}
233237
}
234238

239+
// TestBuildIDPTransport verifies that when the feature gate is enabled and the
240+
// Authentication CR has a component proxy with a trusted CA, the transport
241+
// builder wires the proxy function and loads the CA into the transport.
242+
func TestBuildIDPTransport(t *testing.T) {
243+
_, caPEM := transporttest.MakeSelfSignedCA(t)
244+
proxyCA := &corev1.ConfigMap{
245+
ObjectMeta: metav1.ObjectMeta{Name: "my-proxy-ca", Namespace: "openshift-config"},
246+
Data: map[string]string{"ca-bundle.crt": string(caPEM)},
247+
}
248+
authCR := &operatorv1.Authentication{
249+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
250+
Spec: operatorv1.AuthenticationSpec{
251+
Proxy: operatorv1.AuthenticationProxyConfig{
252+
HTTPSProxy: "http://proxy:3128",
253+
TrustedCA: operatorv1.AuthenticationConfigMapReference{Name: "my-proxy-ca"},
254+
},
255+
},
256+
}
257+
258+
cmIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
259+
require.NoError(t, cmIndexer.Add(proxyCA))
260+
261+
authIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
262+
require.NoError(t, authIndexer.Add(authCR))
263+
264+
listers := configobservation.Listers{
265+
ConfigMapLister: corelistersv1.NewConfigMapLister(cmIndexer),
266+
OperatorAuthLister: operatorv1listers.NewAuthenticationLister(authIndexer),
267+
FeatureGateAccessor: featuregates.NewHardcodedFeatureGateAccess(
268+
[]configv1.FeatureGateName{features.FeatureGateAuthenticationComponentProxy},
269+
nil,
270+
),
271+
}
272+
273+
buildTransport, err := buildIDPTransport(listers)
274+
require.NoError(t, err)
275+
276+
// Call without an IdP CA — should still succeed because the proxy CA is loaded.
277+
rt, err := buildTransport("", "")
278+
require.NoError(t, err)
279+
280+
tr := transporttest.UnwrapTransport(t, rt)
281+
require.NotNil(t, tr.Proxy, "transport should have a proxy function set")
282+
283+
// Verify proxy is wired: an HTTPS request should be proxied.
284+
proxyURL, err := tr.Proxy(&http.Request{URL: transporttest.MustParseURL(t, "https://idp.example.com")})
285+
require.NoError(t, err)
286+
require.NotNil(t, proxyURL)
287+
require.Equal(t, "http://proxy:3128", proxyURL.String())
288+
}
289+
235290
func eventsReasonMessage(e []*corev1.Event) []string {
236291
reasonMessages := make([]string, 0, len(e))
237292
for _, ev := range e {

pkg/controllers/proxyconfig/proxyconfig_controller_test.go

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,6 @@ func Test_checkProxyConfig(t *testing.T) {
9090
}
9191
}
9292

93-
type workingHTTPRoundTripper struct{}
94-
type faultyHTTPRoundTripper struct{}
95-
96-
func (s *workingHTTPRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
97-
return &http.Response{StatusCode: 200}, nil
98-
}
99-
100-
func (s *faultyHTTPRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
101-
return &http.Response{StatusCode: 404}, nil
102-
}
103-
10493
func Test_extractIdPURLs(t *testing.T) {
10594
tests := []struct {
10695
name string
@@ -324,3 +313,71 @@ func Test_validateIdPConnectivity(t *testing.T) {
324313
})
325314
}
326315
}
316+
317+
func Test_validateIdPConnectivity_hashDedup(t *testing.T) {
318+
idps := []configv1.IdentityProvider{{
319+
IdentityProviderConfig: configv1.IdentityProviderConfig{
320+
Type: configv1.IdentityProviderTypeGitLab,
321+
GitLab: &configv1.GitLabIdentityProvider{URL: "https://gitlab.example.com"},
322+
},
323+
}}
324+
oauthConfig := &configv1.OAuth{
325+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
326+
Spec: configv1.OAuthSpec{IdentityProviders: idps},
327+
}
328+
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
329+
if err := indexer.Add(oauthConfig); err != nil {
330+
t.Fatal(err)
331+
}
332+
333+
t.Run("second call with same config skips validation", func(t *testing.T) {
334+
recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now()))
335+
p := &proxyConfigChecker{
336+
oauthLister: configv1listers.NewOAuthLister(indexer),
337+
}
338+
339+
reachable := &http.Client{Transport: &workingHTTPRoundTripper{}}
340+
p.validateIdPConnectivity(context.Background(), recorder, reachable, "http://proxy:3128", "", "")
341+
if len(recorder.Events()) != 0 {
342+
t.Fatalf("first call should emit no events for reachable endpoints, got %d", len(recorder.Events()))
343+
}
344+
345+
followupRecorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now()))
346+
unreachable := &http.Client{Transport: &faultyHTTPRoundTripper{}}
347+
p.validateIdPConnectivity(context.Background(), followupRecorder, unreachable, "http://proxy:3128", "", "")
348+
if len(followupRecorder.Events()) != 0 {
349+
t.Fatalf("second call with same config should skip validation, got %d events", len(followupRecorder.Events()))
350+
}
351+
})
352+
353+
t.Run("hash not saved on failure allows retry", func(t *testing.T) {
354+
recorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now()))
355+
p := &proxyConfigChecker{
356+
oauthLister: configv1listers.NewOAuthLister(indexer),
357+
}
358+
359+
unreachable := &http.Client{Transport: &faultyHTTPRoundTripper{}}
360+
p.validateIdPConnectivity(context.Background(), recorder, unreachable, "http://proxy:3128", "", "")
361+
if len(recorder.Events()) != 1 {
362+
t.Fatalf("expected 1 event on failure, got %d", len(recorder.Events()))
363+
}
364+
365+
followupRecorder := events.NewInMemoryRecorder(t.Name(), clocktesting.NewFakePassiveClock(time.Now()))
366+
p.validateIdPConnectivity(context.Background(), followupRecorder, unreachable, "http://proxy:3128", "", "")
367+
if len(followupRecorder.Events()) != 1 {
368+
t.Fatalf("expected 1 event on retry after failure, got %d", len(followupRecorder.Events()))
369+
}
370+
})
371+
}
372+
373+
type workingHTTPRoundTripper struct{}
374+
375+
func (s *workingHTTPRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
376+
return &http.Response{StatusCode: 200}, nil
377+
}
378+
379+
type faultyHTTPRoundTripper struct{}
380+
381+
func (s *faultyHTTPRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) {
382+
return &http.Response{StatusCode: 404}, nil
383+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package transporttest
2+
3+
import (
4+
"net/http"
5+
"net/url"
6+
"path"
7+
"testing"
8+
"time"
9+
10+
"github.com/openshift/library-go/pkg/crypto"
11+
)
12+
13+
// UnwrapTransport recursively unwraps a RoundTripper (e.g. from
14+
// DebugWrappers) until it reaches the underlying *http.Transport.
15+
func UnwrapTransport(t *testing.T, rt http.RoundTripper) *http.Transport {
16+
t.Helper()
17+
type unwrapper interface {
18+
WrappedRoundTripper() http.RoundTripper
19+
}
20+
for {
21+
if tr, ok := rt.(*http.Transport); ok {
22+
return tr
23+
}
24+
u, ok := rt.(unwrapper)
25+
if !ok {
26+
t.Fatalf("cannot unwrap %T to *http.Transport", rt)
27+
}
28+
rt = u.WrappedRoundTripper()
29+
}
30+
}
31+
32+
// MakeSelfSignedCA generates a self-signed CA certificate and returns
33+
// the CA object and its PEM-encoded certificate bytes.
34+
func MakeSelfSignedCA(t *testing.T) (*crypto.CA, []byte) {
35+
t.Helper()
36+
tmpDir := t.TempDir()
37+
ca, err := crypto.MakeSelfSignedCA(
38+
path.Join(tmpDir, "ca.crt"),
39+
path.Join(tmpDir, "ca.key"),
40+
"", "testCA", time.Hour*24,
41+
)
42+
if err != nil {
43+
t.Fatalf("failed to create self-signed CA: %v", err)
44+
}
45+
certPEM, _, err := ca.Config.GetPEMBytes()
46+
if err != nil {
47+
t.Fatalf("failed to get CA PEM bytes: %v", err)
48+
}
49+
return ca, certPEM
50+
}
51+
52+
// MustParseURL parses a raw URL string and fails the test if it is invalid.
53+
func MustParseURL(t *testing.T, rawURL string) *url.URL {
54+
t.Helper()
55+
u, err := url.Parse(rawURL)
56+
if err != nil {
57+
t.Fatalf("failed to parse URL %q: %v", rawURL, err)
58+
}
59+
return u
60+
}

0 commit comments

Comments
 (0)