diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index d5a8d302..889d66fc 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -264,12 +264,15 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *v1 } } - httpClient = &http.Client{Transport: &http.Transport{Proxy: http.ProxyFromEnvironment, TLSClientConfig: tlsConfig}} + httpClient = &http.Client{ + Timeout: time.Minute, + Transport: &http.Transport{ + Proxy: http.ProxyFromEnvironment, + TLSClientConfig: tlsConfig, + }, + } - timeoutCtx, cancelFunc := context.WithTimeout(oidc.ClientContext(ctx, httpClient), time.Minute) - defer cancelFunc() - - discoveredProvider, err = oidc.NewProvider(timeoutCtx, upstream.Spec.Issuer) + discoveredProvider, err = oidc.NewProvider(oidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer) if err != nil { const klogLevelTrace = 6 c.log.V(klogLevelTrace).WithValues( diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index 24cb9b14..8712af26 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -807,6 +807,8 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs actualTransportProxyFunction := reflect.ValueOf(actualTransport.Proxy).Pointer() require.Equal(t, httpProxyFromEnvFunction, actualTransportProxyFunction, "Transport should have used http.ProxyFromEnvironment as its Proxy func") + // We also want a reasonable timeout on each request/response cycle for OIDC discovery and JWKS. + require.Equal(t, time.Minute, actualIDP.Client.Timeout) } actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().OIDCIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{}) diff --git a/test/integration/concierge_api_serving_certs_test.go b/test/integration/concierge_api_serving_certs_test.go index 3689ed7a..2bb02f26 100644 --- a/test/integration/concierge_api_serving_certs_test.go +++ b/test/integration/concierge_api_serving_certs_test.go @@ -111,7 +111,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) { var err error secret, err = kubeClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, defaultServingCertResourceName, metav1.GetOptions{}) requireEventually.NoError(err) - }, 10*time.Second, 250*time.Millisecond) + }, time.Minute, 250*time.Millisecond) regeneratedCACert := secret.Data["caCertificate"] regeneratedPrivateKey := secret.Data["tlsPrivateKey"] regeneratedCertChain := secret.Data["tlsCertificateChain"] @@ -131,7 +131,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) { apiService, err := aggregatedClient.ApiregistrationV1().APIServices().Get(ctx, apiServiceName, metav1.GetOptions{}) requireEventually.NoErrorf(err, "get for APIService %q returned error", apiServiceName) requireEventually.Equalf(regeneratedCACert, apiService.Spec.CABundle, "CA bundle in APIService %q does not yet have the expected value", apiServiceName) - }, 10*time.Second, 250*time.Millisecond, "never saw CA certificate rotate to expected value") + }, time.Minute, 250*time.Millisecond, "never saw CA certificate rotate to expected value") // Check that we can still make requests to the aggregated API through the kube API server, // because the kube API server uses these certs when proxying requests to the aggregated API server, @@ -150,7 +150,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) { }, metav1.CreateOptions{}) requireEventually.NoError(err, "dynamiccertificates.Notifier broken?") } - }, 30*time.Second, 250*time.Millisecond) + }, time.Minute, 250*time.Millisecond) }) } }