diff --git a/deploy/supervisor/deployment.yaml b/deploy/supervisor/deployment.yaml index 1e0c75c0a..effba2fb7 100644 --- a/deploy/supervisor/deployment.yaml +++ b/deploy/supervisor/deployment.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") @@ -101,6 +101,15 @@ spec: protocol: TCP - containerPort: 8443 protocol: TCP + env: + #@ if data.values.https_proxy: + - name: HTTPS_PROXY + value: #@ data.values.https_proxy + #@ end + #@ if data.values.no_proxy: + - name: NO_PROXY + value: #@ data.values.no_proxy + #@ end livenessProbe: httpGet: path: /healthz diff --git a/deploy/supervisor/values.yaml b/deploy/supervisor/values.yaml index a4575c3c6..62b632f7d 100644 --- a/deploy/supervisor/values.yaml +++ b/deploy/supervisor/values.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@data/values @@ -56,3 +56,11 @@ service_loadbalancer_ip: #! e.g. 1.2.3.4 #! Specify the verbosity of logging: info ("nice to know" information), debug (developer #! information), trace (timing information), all (kitchen sink). log_level: #! By default, when this value is left unset, only warnings and errors are printed. There is no way to suppress warning and error logs. + +#! Set the standard golang HTTPS_PROXY and NO_PROXY environment variables on the Supervisor containers. +#! These will be used when the Supervisor makes backend-to-backend calls to upstream identity providers using HTTPS, +#! e.g. when the Supervisor fetches discovery documents, JWKS keys, and tokens from an upstream OIDC Provider. +#! The Supervisor never makes insecure HTTP calls, so there is no reason to set HTTP_PROXY. +#! Optional. +https_proxy: #! e.g. http://proxy.example.com +no_proxy: #! e.g. 127.0.0.1 diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go index b6b553283..edf9ba387 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go @@ -265,9 +265,13 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.OIDC Message: err.Error(), } } - httpClient = &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} - discoveredProvider, err = oidc.NewProvider(oidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer) + httpClient = &http.Client{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) if err != nil { return &v1alpha1.Condition{ Type: typeOIDCDiscoverySucceeded, diff --git a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go index 02e96e8ee..4b97a07be 100644 --- a/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go +++ b/internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go @@ -9,6 +9,7 @@ import ( "encoding/json" "net/http" "net/url" + "reflect" "strings" "testing" "time" @@ -650,6 +651,15 @@ func TestController(t *testing.T) { require.Equal(t, tt.wantResultingCache[i].GetUsernameClaim(), actualIDP.GetUsernameClaim()) require.Equal(t, tt.wantResultingCache[i].GetGroupsClaim(), actualIDP.GetGroupsClaim()) require.ElementsMatch(t, tt.wantResultingCache[i].GetScopes(), actualIDP.GetScopes()) + + // We always want to use the proxy from env on these clients, so although the following assertions + // are a little hacky, this is a cheap way to test that we are using it. + actualTransport, ok := actualIDP.Client.Transport.(*http.Transport) + require.True(t, ok, "expected cached provider to have client with Transport of type *http.Transport") + httpProxyFromEnvFunction := reflect.ValueOf(http.ProxyFromEnvironment).Pointer() + actualTransportProxyFunction := reflect.ValueOf(actualTransport.Proxy).Pointer() + require.Equal(t, httpProxyFromEnvFunction, actualTransportProxyFunction, + "Transport should have used http.ProxyFromEnvironment as its Proxy func") } actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().OIDCIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{})