djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #702 from vmware-tanzu/supervisor-https-proxy-fix-timeout

Browse Source 
Fix broken upstream OIDC discovery timeout added in previous commit
This commit is contained in:
Ryan Richard 2021-07-08 12:54:39 -07:00 committed by GitHub
commit d403c8b44b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go
View File

@ -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) discoveredProvider, err = oidc.NewProvider(oidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer)
defer cancelFunc()
discoveredProvider, err = oidc.NewProvider(timeoutCtx, upstream.Spec.Issuer)
if err != nil { if err != nil {
const klogLevelTrace = 6 const klogLevelTrace = 6
c.log.V(klogLevelTrace).WithValues( c.log.V(klogLevelTrace).WithValues(

2
internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
View File

@ -807,6 +807,8 @@ oidc: issuer did not match the issuer returned by provider, expected "` + testIs
actualTransportProxyFunction := reflect.ValueOf(actualTransport.Proxy).Pointer() actualTransportProxyFunction := reflect.ValueOf(actualTransport.Proxy).Pointer()
require.Equal(t, httpProxyFromEnvFunction, actualTransportProxyFunction, require.Equal(t, httpProxyFromEnvFunction, actualTransportProxyFunction,
"Transport should have used http.ProxyFromEnvironment as its Proxy func") "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{}) actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().OIDCIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{})

6
test/integration/concierge_api_serving_certs_test.go
View File

@ -111,7 +111,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
var err error var err error
secret, err = kubeClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, defaultServingCertResourceName, metav1.GetOptions{}) secret, err = kubeClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, defaultServingCertResourceName, metav1.GetOptions{})
requireEventually.NoError(err) requireEventually.NoError(err)
}, 10*time.Second, 250*time.Millisecond) }, time.Minute, 250*time.Millisecond)
regeneratedCACert := secret.Data["caCertificate"] regeneratedCACert := secret.Data["caCertificate"]
regeneratedPrivateKey := secret.Data["tlsPrivateKey"] regeneratedPrivateKey := secret.Data["tlsPrivateKey"]
regeneratedCertChain := secret.Data["tlsCertificateChain"] regeneratedCertChain := secret.Data["tlsCertificateChain"]
@ -131,7 +131,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
apiService, err := aggregatedClient.ApiregistrationV1().APIServices().Get(ctx, apiServiceName, metav1.GetOptions{}) apiService, err := aggregatedClient.ApiregistrationV1().APIServices().Get(ctx, apiServiceName, metav1.GetOptions{})
requireEventually.NoErrorf(err, "get for APIService %q returned error", apiServiceName) 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) 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, // 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, // 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{}) }, metav1.CreateOptions{})
requireEventually.NoError(err, "dynamiccertificates.Notifier broken?") requireEventually.NoError(err, "dynamiccertificates.Notifier broken?")
} }
}, 30*time.Second, 250*time.Millisecond) }, time.Minute, 250*time.Millisecond)
}) })
} }
} }